docs: correct EXAMPLES.md worktree, reap, and confirm claims - #81
Conversation
The walkthrough steered users away from a shipped feature and set wrong
expectations about cleanup and the Console's confirmation UX:
- The live `--worktree` launch is supported (ADR-0008, ADR-0011): the
worktree is resolved as the last preparation step, before anything
touches tmux, and the launched Crew runs inside it. The bullet still
claimed a `USAGE` refusal.
- The token-scoped reap runs only from the failed-launch teardown path,
never on a normal session end. The bullet promised the opposite, so
readers expected Agent ids to free themselves; `team stop`, `leave`,
and `clean` are what actually wind a Team down, and archiving keeps an
id rather than freeing it.
- FR-U25 is a one-click `{ "confirm": true }` flag since ADR-0013, not a
typed confirmation phrase — in the prose, in the image's alt text
(the screenshot shows the Tasks board, never a confirm dialog), and in
the `src/ui/server.ts` module docstring.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFed9ERdsRHX63NiYWndro
dichovsky
left a comment
There was a problem hiding this comment.
Routine review. Three reviewers ran per .github/ISSUE_TRIAGE.md: the general code reviewer (always), the TypeScript reviewer (the diff touches src/), and — the substance being prose — a documentation reviewer verifying the new text against the code it describes and against the authority order. No security reviewer: the diff touches none of src/store/, src/process.ts, src/which.ts, src/fs-safe.ts, src/setup/, or the Launcher.
Findings are consolidated below, most severe first, deduplicated across the three.
CRITICAL
None.
HIGH
1. README.md:153 still asserts the typed-phrase gate this PR exists to remove. "…and run prune/clean after you type a confirmation phrase." Contradicted by ADR-0013 (docs/adr/0013-console-redesign.md:30, :61), FR-U25 (docs/design/srs.md:837-847), web/components/confirm-dialog.tsx:1-5 ("relaxed from the former typed-phrase gate"), and src/ui/actions.ts:279-290. grep -rn "confirmation phrase" over the repo returns exactly one live hit — this line. The README links to this very walkthrough two lines later, so after this PR the two user-facing documents disagree in adjacent reading order.
Deliberately not fixed here, and the PR therefore stays a draft. README.md is not named in #34 — neither in its body nor its Acceptance, which enumerate three EXAMPLES.md passages and the src/ui/server.ts comment. .github/ISSUE_TRIAGE.md's scope rule is explicit ("every changed line must trace to the issue … If you find a distinct problem while implementing, file it as a new issue — do not fold it into this change"), so the routine filed it rather than widening the diff on its own authority. A human should decide whether the one-line README correction rides along in this PR or lands separately — it is a ~10-word edit either way, and the routine has no mandate to make that call.
MEDIUM
2. EXAMPLES.md:55 — "Only a launch that fails partway deletes anything" is an over-broad absolute this PR introduced. crew prune deletes Messages and Tasks (cli-contract.md:431-434) and crew clean --force deletes the State Store files (:436-437) — both documented in this same file at EXAMPLES.md:189-201, three lines after the sentence that denies it. The true, narrower claim is about automatic Agent-row deletion. Fixed in the fix pass.
3. EXAMPLES.md:51-52 — "crew team resume later reactivates exactly those rows" reads as unconditional. runTeamResume (src/launcher/resume.ts:174-221) requires tmux, a clean-stop marker and a stored launch-plan.json, no live session of that name, a stored plan still equal to a freshly built one, and every planned Agent still present as its archived exact-id match (assertArchivedResumable, :101-121) — otherwise TEAM_DRIFT/ALREADY_EXISTS/NOT_FOUND (cli-contract.md:342-351). It is the strictest command in the CLI, presented here as a guarantee. Fixed in the fix pass.
4. EXAMPLES.md:52 — crew clean is offered as a wind-down step in precisely the state where bare crew clean refuses. It refuses with ACTIVE_AGENTS while any Agent is active (cli-contract.md:436-437, :468), and this document already states the precise behavior at EXAMPLES.md:197-201 ("deletes only the State Store files … never touches the tracked .crew/roles//.crew/teams/"). Fixed in the fix pass.
5. EXAMPLES.md:45-52 — the two new bullets contradict each other for the worktree case. The new worktree bullet establishes that the launched Crew's State Store and generated files live in the worktree; the very next bullet then says "Wind the Team down with crew team stop <session>" with no note that it must run from inside the worktree. Run from the main checkout it fails — the pane map lives under the worktree's own .crew/generated/ (src/launcher/stop.ts:38-40, :191-195), which is why cli-contract.md:324-328 spells the cwd requirement out. The gap is created by this PR's own juxtaposition: the old text could not hit it, because it claimed live worktree launches were refused. Fixed in the fix pass.
6. EXAMPLES.md:240 and src/ui/server.ts:18 — "the three destructive actions" / "the destructive three" now undercount FR-U25. Archiving an Agent is a fourth gated action: srs.md:837-838 and :926, cli-contract.md:396, ADR-0017, and src/ui/actions.ts:646-648 (archiveAgent → assertConfirmed); restoring is deliberately ungated. Not fixed here — the enumeration was left untouched on purpose and is tracked as #83, together with the same file's omission of archive/restore from its Console action list. Two reviewers noted the count sits on a line this PR rewrote; the routine's scope rule still puts it outside #34, and #83 states the corrected wording.
LOW
7. EXAMPLES.md:43-45 — "as its last preparation step … before anything touches tmux" is loose. tmux is touched before it, read-only (adapter.isPresent/hasSession/listOwnedSessions, src/launcher/session.ts:330-348), and buildLaunchArtifacts writes to disk after it. Not fixed: cli-contract.md:319-321 and ADR-0011:36-40 use the same phrasing, so EXAMPLES.md is faithfully echoing its own contract. Correcting it here would put the walkthrough at odds with two higher-authority documents; it belongs upstream if anywhere.
8. EXAMPLES.md:41-47 — a true claim was lost in the rewrite. The old bullet's "--print, which still shows the path the worktree would use" remains correct (src/launcher/plan.ts:142-144 derives the path and never invokes git) and is more useful now that the live path genuinely mutates. Restored in the fix pass.
9. src/ui/server.ts:12-16 — the "EXACTLY the FR-U19 Operator action POSTs" list omits the two FR-U36 agent routes that AGENT_ACTION_ROUTE (:264-265) registers and :510-517 dispatches. Pre-existing; tracked as #60. Not touched.
10. EXAMPLES.md:206 — the console-dashboard.png alt text describes panels the image does not contain. Pre-existing and tracked as #54; the companion staleness of console-action.png is filed as #82. Not touched.
11. docs/adr/0012-optional-local-ui-server.md:41 still says "behind a typed confirmation" with status: accepted and no superseded marker of its own, though docs/adr/README.md:17 and ADR-0013:61 record the supersession. ADRs are historical records; no change requested.
Verified correct — no finding
Checked against primary sources rather than the PR body, by all three reviewers independently:
- The three removed claims were all genuinely false on
main. TheUSAGErefusal for live worktree launches is gone (ADR-0011:44-45, ADR-0008:55); the reap is reachable only from thecatchinrunLiveLaunch(src/launcher/session.ts:265-275), so ending the session yourself never freed ids; the typed phrase stopped existing at ADR-0013. - The ADR-0011 vs ADR-0015 worktree trap was navigated correctly.
--no-worktreeoverridesworkspace.worktree.*in tracked.crew/launcher.yaml(src/launcher/config.ts:378-389), a different key from.crew/config.yaml'sworker_worktrees(src/config.ts:17, ADR-0015). The new prose never conflates them. - Reap mechanics are exact: confirmed-teardown-only,
launch_token-scoped, restricted to rows with no Task, Task Event, or Message (src/store/agents.ts:197-218), frees ids, best-effort, skipped when teardown is unconfirmed — matchingsession.ts:589-615andcli-contract.md:302-314. - Archive-versus-delete is handled correctly, and it is the subtle point the old text got wrong:
leaveAgentTxkeeps the row (store/agents.ts:170-181),joinAgentTx's suffix loop never hands out an archived id (:146-166), and--resumetargets that exact id. - The new image alt text is honest. All three reviewers opened
docs/images/console-action.png: it is the Tasks view — the four status columns beside a detail panel with "Approve — complete task" and a requeue form. The former alt text was wrong on all three of its claims. - The
src/ui/server.tschange is provably comment-only —git diff --numstatis26 12 EXAMPLES.mdand1 1 src/ui/server.ts, the hunk falls inside the/** … */block closing at:25, andgit diff -wshows no hidden churn. No test reads that docstring (tests/unit/docs-facts.test.tsreads onlydocs/adr/*.md,package.json, andvitest.config.ts), and a block comment produces no statement, branch, or function, so the 95% v8 gate cannot move. - Vocabulary is correct per
CONTEXT.md— notably "approve a Submission or requeue its Task", which respects the Submission/Review/Task distinction more carefully than the untouched line above it. - Scope is clean at the line level: no unrelated re-wraps, no opportunistic edits, no stray files.
CI
Green: ubuntu / node 24.18.0 ✅ (the authoritative run for the 95% coverage gate, which the sandbox could not evaluate) and publish dry-run ✅. publish dry-run first failed on a GitHub Actions service outage — "Internal Server Error occurred while resolving actions/checkout@v7 … actions/setup-node@v6", no repository code involved — and passed on re-run. launch-e2e and ui-e2e skipped as expected (label-gated).
Generated by Claude Code
84910e7 to
e2f2755
Compare
Closes #34
EXAMPLES.mdis the user-facing walkthrough, so a false claim in it is the first thing a new user believes. Three passages had drifted from the code they describe. Per the authority order indocs/README.md, the ADRs, the SRS, and the CLI contract are the correct side of all three, so this is a documentation correction with no behavior change.What changed and why
1. Live launch into a worktree is supported (
EXAMPLES.md:41-45)The bullet said "A live launch with a worktree isn't supported yet and is refused with
USAGE". That refusal is gone, recorded in two accepted ADRs:docs/adr/0008-relay-process-model-and-live-launch.md:55("theUSAGErefusal is gone") anddocs/adr/0011-worktree-store-location.md:44-45.preflightLaunchresolves or creates the worktree as its last preparation step,runLiveLaunchsetsconst launchRoot = worktree?.path ?? root(src/launcher/session.ts:195) and uses it as every pane'scwd, and--worktree <branch>is registered atsrc/cli.ts:298.grep -rn USAGE src/launcher/finds no worktree refusal.The replacement tracks
docs/design/cli-contract.md:315-323: the worktree is created or reused after every read-only check has passed and before anything touches tmux, and the whole launched Crew then runs inside it — its State Store, its generated files, and every pane's working directory. The bullet's opening verb moved from "plan … against" to "run … in", because "plan" was framing carried by the false claim (--print-only); the flag now governs a live run.2. The token-scoped reap fires only on a failed launch (
EXAMPLES.md:46-49)The bullet attributed the reap to "After you disconnect and end the tmux session".
store.reapByLaunchTokenhas exactly one call site —src/launcher/session.ts:607, insideteardownOwnedSession, whichrunLiveLaunchreaches only from thecatcharoundbuildOwnedSession(:266), gated on a confirmed teardown (:596-601). The success path emits the result and attaches; it never reaps.Split into two bullets so the two situations cannot be confused:
crew team stop <session>, which kills a session crew can prove it owns and archives the Agents named in its pane map, which is exactly whatcrew team resumelater reactivates (cli-contract.md:334-351).crew leavearchives a single Agent;crew cleanresets the Workspace.cli-contract.md:302-314).Archive and delete are kept strictly apart, because they differ in the thing the old bullet got wrong: deleting a row frees its id, archiving one does not.
cli-contract.md:119-124— the suffix logic never hands out an archived id, andcrew join <id> --resumetargets that exact archived id — is now stated rather than implied.3. One-click confirm, and an alt text that matches the picture (
EXAMPLES.md:230-233)ADR-0013 (
docs/adr/0013-console-redesign.md:33-40) re-scoped FR-U25 from a typed phrase to one click plus a{ "confirm": true }flag; FR-U25 (docs/design/srs.md:837-843) now specifies exactly that, enforced atsrc/ui/actions.ts:279-290. The prose now says so, including that an unconfirmed POST is aUSAGEfailure.The image needed more than a re-word.
docs/images/console-action.pngis not a confirmation dialog at all: it shows the Tasks view — the "reviewed work board", columns Queued / In progress / In review / Completed, and the task-detail panel with "Approve — complete task" and the requeue form. There is no dialog in the frame and nopruneaction, so the old alt text ("The Console's typed-confirmation dialog for a destructive prune action") was wrong on all three counts. Alt text has to describe the picture — claiming a confirmation dialog would misdescribe the screenshot for screen-reader users — so it now names what the capture shows, with a one-line lead-in bullet (wording tracking the app's own string atweb/app.tsx:129) since the old lead-in was the confirmation sentence's colon. The image file is untouched.4.
src/ui/server.ts:19"gated by the FR-U25 typed confirmation" → "gated by the FR-U25 one-click confirmation flag". Comment text only, inside the module docstring. No executable line in
src/changed.Verification
Run after rebasing onto
origin/main@040ee90. (node_moduleswas absent on arrival;npm ciexited 0 first.)npm run typecheckweb+docs-site)npm run lintnpm run format:checknpm run buildnpm run test:coverageCoverage thresholds were NOT evaluated locally. Vitest suppresses the coverage table on a failing run, so the 95% statements/branches/functions/lines gate is unverified here — this PR's CI run is authoritative for it. No
src/**orbin/**statement, branch, or function changed, so coverage cannot have moved.Pre-existing-failure proof.
git stash push -u, thengit status --porcelainempty andgit diff origin/main --statshowing nothing, then the four affected files re-run on the clean tree: the identical 9 failures, same names, same assertions. Restored withgit stash pop. All nine are sandbox-environmental:tests/integration/commands/doctor.test.ts×2 — the twochmod 000cases. This sandbox runs as uid 0, which ignores the mode bits, so the reads succeed and the expectedINVALID_CONFIGwarnings never fire.tests/integration/package/pack-smoke.test.ts×4 andtests/unit/bin-entry-guard.test.ts×2 — thenodeon PATH for spawned subprocesses is v22.22.2, below crew's hard>=24.15floor, so the bin shim'sassertNodeFloor()fires first and every subprocess exits 1 withcrew requires Node >=24.15.0 (found v22.22.2).tests/spawn/tmux-e2e.test.ts×1 — the SIGINT-during-roster-wait case times out waiting for the pane map, same Node-floor root cause.CI runs at Node
24.18.0on GitHub-hosted runners and is not root, so none of the nine applies there.Tests: none added — claiming the docs-only exemption (
.github/ISSUE_TRIAGE.md: "Tests are not required for docs-only, config-only, or pure-UI-only changes — say so explicitly";CONTRIBUTING.md: "tests are not applicable for docs-only changes"). Honest qualifier: the diff is not literally 100% docs — one line lands insrc/ui/server.ts. That line is inside a block comment, adds no statement, branch, or function, changes no behavior, and is invisible to coverage instrumentation.Related open PRs
None. Checked at file level against every open PR: #76 (
docs/design/testing-strategy.md,CONTRIBUTING.md) and #71 (docs/design/setup-integration.md). Neither touchesEXAMPLES.mdorsrc/ui/server.ts.Base note: this branch was rebased onto
origin/main@040ee90immediately before pushing;mainthen advanced to12fd129(PR #69 merged) while the gate was re-running. #69 touched onlydocs/design/security.md, so the bases are disjoint and the branch still merges cleanly.Out of scope
Deliberately untouched; the first two are filed as their own issues rather than folded in, per
.github/ISSUE_TRIAGE.md's scope rule:docs/images/console-action.pngpredates ADR-0017 — its navigation rail shows five entries with no Now view — and no image inEXAMPLES.mdillustrates the FR-U25 confirm dialog, because none ever did. A re-capture is a content decision, not a drift fix.prune, andclean" here, and "the destructive three" atsrc/ui/server.ts:18, both now undercount FR-U25, which also gates archiving an Agent (FR-U36) — andEXAMPLES.md's Console action list omits archive/restore entirely.src/ui/server.tsheader's route-list omission — Console: server.ts header comment omits the FR-U36 agent-archive POSTs from its "EXACTLY" route list #60.cli-contract.md:344-351) is not mentioned in the--worktreebullet; the walkthrough is deliberately short, and EXAMPLES.md drift: worktree live launch, launch-reap timing, removed typed confirmation #34's Acceptance does not reach it.EXAMPLES.md:205'sconsole-dashboard.pngalt text — Docs: README Console screenshot predates ADR-0017, and EXAMPLES.md misdescribes it #54.architecture.md§4.1 staleness — architecture.md §4.1 source tree is substantially stale #35; docs-site prose drift — docs-site prose drift: lifecycle omits abandoned, deferred-seam roster, glossary count #42.Generated by Claude Code