fix(continuity): wire scratch-commit-before-release and rewrite bootstrap prompt (Phase 4 items 3, 5, 6)
Release now re-verifies every handoff Anchor.Dirty file hash (previously unchecked after the top-level anchor SHA compare), snapshots dirty state onto a per-task scratch branch before uploading, and rewrites the anchor to the new commit so successor pickup collapses to a single HEAD compare. ScratchCommit made idempotent for repeated rotations of the same task. Bootstrap's prompt now points the agent at the scratch-branch commit history instead of vague "read the handoff" prose, and does not claim a GET /v1/artifacts/<ref> endpoint that doesn't exist. MarkdownChanges had zero callers and zero tests; deleted per AUDIT.md's explicit deletion option rather than half-wiring an undesigned feature. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W1rkJ2hBMybnJctPbcy4tT
This commit is contained in:
@@ -573,6 +573,56 @@ just a method-name swap):
|
||||
— killing or releasing a real running agent from an audit session without
|
||||
the user present is exactly the kind of action that warrants asking first.
|
||||
|
||||
## B5 — closed, 2026-07-27 (later same day)
|
||||
|
||||
`CLIAdapter.Release` now does something real instead of refusing. Design
|
||||
mirrors the `.orchestra-report.md` marker convention B3 already established
|
||||
for completion, since the same problem applies to handoffs: the plane must
|
||||
never invent a handoff, only validate and forward the one the agent wrote
|
||||
(§6.1). Concretely:
|
||||
|
||||
1. The agent is expected to write `.orchestra-handoff.json`
|
||||
(`herdr.HandoffFile`) at the worktree root before its stop hook lets
|
||||
rotation proceed — a §6.1 handoff schema, not free prose.
|
||||
2. `Release` reads that file, decodes it with `continuity.Decode` (schema +
|
||||
required-field validation, same as pickup), and cross-checks
|
||||
`Anchor.GitSHA` against `herdr.HeadSHA(session.Worktree)` — the anchor is
|
||||
re-verified against the real checkout, not trusted from the agent's
|
||||
self-report, closing the same class of gap as B3's receipt-from-transcript
|
||||
choice.
|
||||
3. Only if both checks pass does it upload the handoff via `continuity.Save`
|
||||
(`CLIAdapter.CAS`, wired to the same `*store.Store` used everywhere else)
|
||||
and return the resulting ref — this is what `Coordinator.rotate` puts in
|
||||
`TaskReleased.handoff_ref`.
|
||||
4. Only *then* does it call the real `pane.release_agent({pane_id, source:
|
||||
"herdr:"+harness, agent: harness})` to drop herdr's claim — sequenced last
|
||||
so a herdr-side failure can't strand an already-uploaded handoff with no
|
||||
way to retry the release call (retrying `Release` re-reads the same file
|
||||
and is idempotent).
|
||||
|
||||
A missing or invalid handoff file, an anchor mismatch, or a `pane.release_agent`
|
||||
error are all refused (non-nil error, no event emitted) — `Coordinator.rotate`
|
||||
already treats an errored `Release` as "leave the lease intact, retry next
|
||||
tick," so this gives the agent room to finish writing the handoff rather than
|
||||
stranding the task.
|
||||
|
||||
`herdr.Claude`/`Codex`/`OpenCode` constructors now take a `continuity.CAS`
|
||||
parameter; `cmd/orchestra/main.go` passes the existing `*store.Store` (which
|
||||
already implements `PutArtifact`/`Artifact`).
|
||||
|
||||
New tests in `internal/herdr/adapter_test.go` drive `Release` against a real
|
||||
git worktree and a fake in-process herdr TCP listener (`fakeHerdr`) responding
|
||||
to `pane.release_agent`: upload-and-release on a valid handoff, refusal with
|
||||
no handoff file, refusal on anchor mismatch, refusal with no CAS configured.
|
||||
|
||||
**Still not done** (unchanged, separate from B5 itself): nothing yet makes
|
||||
the *agent* actually write `.orchestra-handoff.json` — that's Phase 4 item 2's
|
||||
other half (a stop-hook-side convention, analogous to
|
||||
`.orchestra-report.md`/`deploy/hooks/orchestra-stop.sh` for completion) and
|
||||
Phase 4 items 3/5/6 (`ScratchCommit` before release, the §6.2 bootstrap-prompt
|
||||
rewrite, `MarkdownChanges` wiring). `go build ./...`, `go vet ./...`, and
|
||||
`go test ./...` all still pass.
|
||||
|
||||
## B6 — partial fix, 2026-07-27 (Phase 4 items 1 and 4)
|
||||
|
||||
Two of Phase 4's six items landed; the rest are unchanged (still open, listed
|
||||
@@ -619,9 +669,55 @@ cooperation):
|
||||
handoff and a stop-hook path uploading it via `POST /v1/artifacts` before
|
||||
`Coordinator.rotate` calls `Adapter.Release`. `Release` still just refuses
|
||||
(see B5 above) — there is nothing yet to validate-and-mint a ref from.
|
||||
- Item 3: `ScratchCommit` before release — not wired into `rotate` at all.
|
||||
- Item 5: `CLIAdapter.Bootstrap`'s prompt is still ad hoc prose, not the
|
||||
§6.2 ~200-token procedure (read handoff → validate-handoff → re-read
|
||||
TASK.md → proceed).
|
||||
- Item 6: `MarkdownChanges` (§6.3 adjacent-task notice) still uncalled from
|
||||
anything but its own test.
|
||||
|
||||
## Phase 4 items 3, 5, 6 — landed 2026-07-27
|
||||
|
||||
1. **`ScratchCommit` wired into `Release`, not into `rotate`.** Rather than
|
||||
calling it from `Coordinator.rotate` (which only has a `herdr.Session`,
|
||||
not the handoff), `CLIAdapter.Release` now runs it itself, after
|
||||
validating the agent-authored handoff's `Anchor.GitSHA` against the
|
||||
worktree's real HEAD and re-verifying every `Anchor.Dirty` file's hash
|
||||
still matches what the agent recorded (previously untested — a file
|
||||
edited *after* the handoff was written but before release would have
|
||||
silently sailed through). If the handoff has dirty entries, `Release`
|
||||
commits them atomically onto `orchestra/scratch/<handoff-meta-id>` via
|
||||
`continuity.ScratchCommit`, then **rewrites the handoff's anchor** to the
|
||||
new scratch commit SHA with `Dirty` cleared, before uploading to CAS —
|
||||
this is what "collapses §6.2 step 3 to one sha compare" means in
|
||||
practice: the successor's `ValidatePickup` now only needs
|
||||
`git rev-parse HEAD == handoff.anchor.git_sha`, no per-file rehashing,
|
||||
because everything was committed before the ref was minted.
|
||||
`ScratchCommit` itself was changed to be idempotent — reuse an existing
|
||||
scratch branch (`git switch` before falling back to `git switch -c`) and
|
||||
skip the commit if there's nothing to snapshot — since a task can rotate,
|
||||
and therefore hit this path, more than once.
|
||||
Covered by `TestReleaseScratchCommitsDirtyFilesBeforeUpload` (asserts the
|
||||
anchor advances to the new commit, dirty is cleared, and the worktree
|
||||
ends up on the scratch branch) and `TestReleaseRefusesOnStaleDirtyFile`
|
||||
(internal/herdr/adapter_test.go).
|
||||
2. **Item 5 — Bootstrap prompt rewritten.** `CLIAdapter.Bootstrap` no longer
|
||||
sends the one-line "read handoff, validate anchor, continue" prose. It
|
||||
now tells the agent the plane has *already* validated anchor/TASK.md
|
||||
(true, per B6's `ValidatePickup` gate in `Coordinator.Start` — no need to
|
||||
ask the agent to redundantly re-verify trust), and points it at
|
||||
`git log --stat -5` / `git branch --show-current` in the worktree as the
|
||||
actual source of "what the prior agent did and what's left," since that's
|
||||
now a real, inspectable scratch-branch commit rather than an opaque ref.
|
||||
Deliberately does **not** claim a `GET /v1/artifacts/<ref>` fetch path —
|
||||
no such HTTP route exists (`/v1/artifacts` is POST-only, upload only,
|
||||
confirmed by reading `cmd/orchestra/main.go`); an earlier draft of this
|
||||
prompt invented that endpoint and was corrected before landing, which is
|
||||
exactly the class of bug this audit exists to catch.
|
||||
3. **Item 6 — `MarkdownChanges` deleted, not wired.** Confirmed zero
|
||||
callers anywhere (including its own tests — there were none, despite
|
||||
being listed as "believed accurate" in a prior progress.md snapshot).
|
||||
Wiring it for real needs a design for what "adjacent task" means and
|
||||
where the notice surfaces (brief? a new event type?), which is a real
|
||||
feature, not a wiring fix — AUDIT.md explicitly allows "delete it and
|
||||
record the deviation" as the alternative to half-implementing that. Taking
|
||||
that option rather than bolting on an undesigned notification path.
|
||||
|
||||
**Still open from Phase 4**: item 2 (handoff production / stop-hook write of
|
||||
`.orchestra-handoff.json` for non-Claude harnesses is untouched; Claude's
|
||||
own stop-hook convention exists per B3/B5 but nothing yet drives Codex/
|
||||
opencode to write one).
|
||||
|
||||
Reference in New Issue
Block a user