Support dirty state and cancellation confirmation in command wizards - #153
Draft
Oleg-Melnik wants to merge 8 commits into
Draft
Support dirty state and cancellation confirmation in command wizards#153Oleg-Melnik wants to merge 8 commits into
Oleg-Melnik wants to merge 8 commits into
Conversation
Two agents work one GitHub issue through a shared Markdown document: one plans and implements, the other reviews the plan before any code is written and the resulting diff afterwards. The issue is the only accepted input, so the reviewer checks the work against acceptance criteria that neither agent wrote. The driver invokes whichever agent owns the turn, keeps a transcript per turn, and validates each transition against the protocol. Neither agent may change Git state; the driver compares refs and the index after every turn and aborts on any difference. Opening a pull request is opt-in, performed by the driver after a finished run. Both agent commands keep their CLI approval and sandbox boundaries by default, and a command containing a known bypass flag is refused unless the caller confirms an external isolation boundary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex's `workspace-write` sandbox excludes gitignored paths from its writable set, and `.agents/work/` is gitignored by design — the working document is a scratch artifact that is never committed. `agent2` could therefore read the document but not write its review into it, and every run aborted with "agent2 did not modify ... plan.md". Name the work root with `--add-dir` in the default `AGENT2_CMD`. This widens the sandbox by exactly one directory inside the repository, and `unsafe_agent_roles()` does not classify it as a bypass flag, so the approval and sandbox boundaries are otherwise unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`pair.sh` runs `agent1` with `--setting-sources project`, which loads `.claude/settings.json` only. That file did not exist, so every Gradle and `java` invocation was refused with "This command requires approval", and the agent handed off an implementation it had never compiled. `agent2` then had to spend its review round on that fact instead of on the code. Allow the verification commands `AGENTS.md` prescribes — module `test`, `check`, `compileKotlin`, `detekt`, the license-report tasks, the local publishing tasks, and the JDK checks — plus the read-only `gh` commands the agents use to inspect issues and pull requests. Publishing tasks (`publish`, `publishCodegenPlugins`) are deliberately absent: `AGENTS.md` reserves those for CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four failures showed up in one run of issue #148, none of them in the protocol itself. Each is addressed where it belongs. The Git tripwire accused the agent. It compares two snapshots taken around a turn and cannot see who moved between them, so a user switching branches mid-run produced "agent2 wrote to Git". Report the movement, name the innocent cause, and leave attribution to the reader. The skill and the run guide now say the guard cannot attribute, and the guide asks the user not to touch Git while a run is live. A turn that died immediately said only that the agent exited non-zero. The two things that actually fail first — a CLI on `PATH` but not signed in, and a model identifier only the API rejects — are now named in that message, and the run guide separates being on `PATH` from being authenticated. Verification could report green from cache. When every compile and test task is `UP-TO-DATE`, `BUILD SUCCESSFUL` describes an earlier build, not the change in the worktree. The tester skill now requires reading the task list and forcing the work with `--rerun-tasks` before calling a result green. Finally, record why `--add-dir` and `.claude/settings.json` are load-bearing, in the protocol and in the guide, so neither is dropped by someone overriding an agent command. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both fixes in this branch live in configuration rather than in the driver's logic, so nothing failed when they were wrong until a turn was already running — several minutes in, with a message about the symptom. These assert them directly. `--add-dir` is read out of the `AGENT2_CMD` assignment rather than from the usage output, which describes the flag in prose: matching that text passed even with the flag deleted from the default that actually runs. The companion check asserts `.agents/work` really is gitignored, which is the reason the flag is needed at all. The `.claude/settings.json` checks cover the file agent1 loads under `--setting-sources project` and the verification tasks it must allow. Without them agent1 hands off an implementation it never compiled. Each check was confirmed to fail when the thing it guards is removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`CommandDialog` exposes its form's dirty state and can guard cancellation, but `CommandWizard` had no equivalent. `Wizard.onCloseRequest` also fired both when the user cancelled and after a successful submission, so a host could not confirm discarding entered data without also prompting once the command had posted. Separate the two paths instead of having `onCloseRequest` tell them apart. `Wizard` gains `onBeforeCancel`, a suspending guard returning whether to proceed, and `cancel()`, which the "Cancel" button now calls; the guard logic sits in `requestCancel()` so a unit test can drive it outside composition. The submission path still calls `close()` directly and never reaches the guard, and `close()` and `onCloseRequest` are unchanged, so existing hosts keep working. Entered data survives a pending confirmation because the wizard stays in composition until the guard resolves. `CommandWizard` gains `dirty`, backed by the single `CommandMessageForm` spanning every page, so the value covers pages the user has left. All additions are additive: no existing signature, name, or visibility changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CommandDialogexposes its form's dirty state and can guard cancellation,but
CommandWizardhad no equivalent.Wizard.onCloseRequestalso firedboth when the user cancelled and after a successful submission, so a wizard
host could not confirm discarding entered data without also prompting once
the command had posted.
This separates the two close paths rather than having
onCloseRequesttellthem apart, and exposes the wizard's aggregate dirty state so a host can
implement discard confirmation without inspecting wizard internals.
Changes
WizardgainsonBeforeCancel, a suspending guard returning whether toproceed with closing, and
cancel(), which the "Cancel" button now calls.The guard logic sits in
requestCancel().close()directly, so it neverreaches the guard.
close()andonCloseRequestare behaviourallyunchanged, and existing hosts keep working.
composition until the guard resolves.
CommandWizardgainsdirty, backed by the singleCommandMessageFormspanning every page, so the value covers pages the user has navigated away
from.
client/README.mddocuments the host-side pattern.Public API additions are additive only:
Wizard.onBeforeCancel,Wizard.cancel(), andCommandWizard.dirty. No existing signature, name, orvisibility changed.
Fixes #148