docs: sync setup-integration with shipped workflow text and registry revision - #71
Conversation
dichovsky
left a comment
There was a problem hiding this comment.
Review
Two reviewers per .github/ISSUE_TRIAGE.md's roster: the general code reviewer, and — the diff being docs-only — a documentation reviewer verifying the new prose against the code it describes and against the authority order, in place of the TypeScript reviewer. 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.
CRITICAL: none. HIGH: none.
MEDIUM
M1 — the §2 note is wrong for copilot-cli, the one matrix row it most needs to explain. docs/design/setup-integration.md:38-41
Both reviewers found this independently. The note attributes the resume suffix to invocation(), but for copilot-cli the Launcher never calls invocation() — paneLaunch takes the launchArgs branch and returns early:
// src/launcher/session.ts:130-134
const args = target.launchArgs?.(role, agentId, options);
if (args !== undefined) {
const command = [executable, ...args];
return { command, invocation: command.join(' '), pasteInvocation: false };
}src/platforms/copilot.ts:36-42 defines launchArgs, so a resumed Copilot pane starts as copilot --agent=crew --prompt "worker w1 --resume" (verified by execution). The /agent (select crew), then: … string is never used on the launch path. §7:506 — edited by this very PR — already documents launch_args?(role,id,options?), so the note contradicts the same diff by omission, directly under the matrix row whose Invocation cell is the odd one out.
M2 — the note's first sentence over-claims across the two Model Backend rows. docs/design/setup-integration.md:38-39
"…that is the invocation setup prints for a human to type" holds for 7 of 10 rows. ollama and lmstudio are BackendTargets with no invocation() at all (src/platforms/shared.ts:97-109); runSetupBackend prints checks and recipe lines and never emits an Invoke: line (src/setup/index.ts:279-284), so their cells are recipe summaries. copilot-cli's cell is a paraphrase of what setup prints, not the printed string. Generalizing an invocation claim over the backend rows also brushes CONTEXT.md's rule to avoid treating Ollama and LM Studio as Participants.
LOW
L1 — "only the Launcher passes it" is imprecise. docs/design/setup-integration.md:40. src/launcher/session.ts:129 builds { resume } and passes it on every launch, with resume: false on a normal one; only the value is resume-specific. "sets it" would be exact. (POST /api/team/resume is a second entrypoint but routes through the same runTeamResume, so the cited path stays correct.)
L2 — §3 step 3 now joins the resume clause and the id-suffix note in one sentence. docs/design/setup-integration.md:85-87. docs/design/cli-contract.md:119 — rank 3, higher authority — is explicit that "--resume targets that exact archived id", so a resumed join never returns a suffix. The canonical §4.0 text keeps these as two separate sentences and does not invite the reading; §3's "and"-joined form does.
L3 — src/platforms/shared.ts:8-9 now visibly conflicts with this PR's deliberate non-bump. That comment says the doc is mirrored "verbatim — a change there is a registry-revision bump here". Taken literally it demands a REGISTRY_REVISION bump for this PR. The authoritative rule is §7 (:534-536), "whenever any generated file's bytes change", by which the non-bump is correct — but the source comment is a trap for the next doc-only sync. Out of scope here; filed separately.
L4 — nothing guards these facts against drifting again. tests/unit/platforms.test.ts:469-489 is the only test that reads this file, and it compares only officialSources. Neither the §4.0 block nor the revision literals are asserted anywhere, which is how revisions 2 and 5 came to coexist in one file on main. Filed separately.
L5 — the branch is behind origin/main. main moved to 236c45c (PR #49) after this was pushed. No conflict — #49 touched only docs/design/srs.md and tests/unit/format.test.ts — and its new FR-U47 actively corroborates this PR's prose ("each resumed pane's Participant CLI is invited to crew join <id> --resume"). Rebasing before ready-for-review.
Verified clean
- §4.0 is byte-identical to
SHARED_WORKFLOW. Both reviewers checked mechanically rather than by eye — extract both, compare — and both got 1446 bytes, strict equality, emptydiff -u. Whitespace, indentation, wrapping, and the absent trailing newline all match. - All six revision literals updated; no stale value remains anywhere. Repo-wide grep confirms the only non-5 values left are deliberate drift-state fixtures (
tests/unit/platforms.test.ts:270,277,285,tests/integration/commands/setup.test.ts:423,tests/integration/commands/doctor.test.ts:595).docs-site/generated/facts.jsonalready carriesregistry.revision: 5. - Not bumping
REGISTRY_REVISIONis correct. Zero files undersrc/change, so everyrender()output is byte-identical tomainand no installed artifact can becomemanaged-outdated. Corroborated by the per-platform content-hash snapshots intests/unit/platforms.test.ts:230-244. - §7's signature edit is faithful to
src/platforms/shared.ts:81and:86, in the block's existing snake_case convention. - No test breaks. No
officialSourcesURL moved or changed;tests/unit/platforms.test.ts:471is the only test reading this file; it passes, as doestests/unit/docs-facts.test.ts. CI is green on this PR (ubuntu / node 24.18.0✅,publish dry-run✅; the two e2e jobs are label-gated and skipped). - No higher-authority contradiction. Checked against every ADR, the SRS (FR-G07, FR-C05/C06, the new FR-U39–FR-U49), the CLI contract, and the data model. Terminology is clean against
CONTEXT.md. - Scope is clean. Every changed line traces to #32; no adjacent-improvement creep.
Disposition
Nothing CRITICAL or HIGH survived, so under .github/ISSUE_TRIAGE.md this PR is eligible to be marked ready as-is. It is taking the one allowed fix pass anyway: M1 and M2 are factual defects in the single net-new sentence this PR introduces, in a change whose entire purpose is documentation accuracy, and two reviewers found M1 independently. The fix covers M1, M2, L1, and L2, rebases onto 236c45c, and re-runs the gate. L3 and L4 are distinct problems and are being filed as their own issues rather than folded in.
Generated by Claude Code
47f2ebe to
00770f4
Compare
dichovsky
left a comment
There was a problem hiding this comment.
Re-check after the fix pass — 00770f4
Per .github/ISSUE_TRIAGE.md, the reviewer re-checks the changed areas only. Scope was confirmed before anything else: diffing the old three-dot diff against the new one, only the two flagged hunks differ — every other hunk is character-for-character identical with shifted @@ offsets. The extra files in a two-dot 47f2ebe..00770f4 comparison are exactly the seven origin/main gained between d311767 and 8cd6d99, i.e. rebase inheritance, not amend content.
M1, M2, L1 and L2 are fully resolved. Nothing at CRITICAL, HIGH, or MEDIUM.
Each rewritten claim re-verified against source rather than taken on trust:
- The
launch_argsearly return is real —src/launcher/session.ts:129-134returns ahead of thetarget.invocation(...)at:137. copilot-cliis exhaustively the only target supplyinglaunch_args—grep launchArgs src/platforms/*.tsacross all 13 modules yields exactly two hits (the definition atcopilot.ts:36, the optional interface member atshared.ts:86), and at runtimePARTICIPANT_TARGETS.filter(t => typeof t.launchArgs === 'function')→['copilot-cli'].- The quoted resumed-Copilot argv is exact — executing
copilot.launchArgs('<role>','<id>',{resume:true})gives["--agent=crew","--prompt","<role> <id> --resume"], and the executable reachingpaneLaunchis unconditionallycopilot: it comes fromplan.executable(src/launcher/plan.ts:118-124, FR-H07) andsrc/launcher/config.ts:40-45refuses any repo-configexecutableoverride. BackendTargetreally has noinvocation(src/platforms/shared.ts:97-109;typeof b.invocation === 'undefined'for both backends), andrunSetupBackend(src/setup/index.ts:266-285) prints checks then recipe lines — the onlyInvoke:write in the codebase issrc/setup/index.ts:244, on the participant path.- "the Launcher sets" is now precise, and matches the verb the SRS itself uses in FR-U47 ("the resume flag set in
src/launcher/resume.tsreachingpaneLaunch"). - §4.0 is still byte-identical — re-extracted from the amended file (block now at
:113-134) and compared mechanically: both 1446 bytes, cleandiff -u. Noregistry-revisionliteral changed (eight: 5, zero stale: 2), the URL set is identical between the two commits, and nosrc/orbin/file is touched.
On L2, the misreading is gone rather than moved: the suffix claim now terminates its own sentence and --resume is a separate one, dissolving the and-coupling that put §3 in tension with docs/design/cli-contract.md:119. The new order mirrors §4.0 step 2 — the shipped canonical text, which cannot change — so §3 matching it is the right resting place, and no behavioral claim was added to a document that sits outside the authority order.
Worth recording: the authority-order gap raised at LOW on the first pass was closed by main, not by this PR. #49 landed FR-U39–FR-U48, and FR-U47 is directly on point — "crew never runs the join itself — each resumed pane's Participant CLI is invited to crew join <id> --resume" — with a verification clause naming the same resume.ts → session.ts wiring this note cites. Both hunks are consistent with it.
Remaining LOW (not blocking)
Neither hunk states how a Participant learns it is recovering a clean stop — the only signal is the --resume token the Launcher appends, arriving inside {{ROLE_ARGS}}. The rewrite mitigates it materially (the append mechanism is now stated for both paths, and the concrete --prompt "<role> <id> --resume" example makes the token visible on the page), and the root cause is in SHARED_WORKFLOW itself, which §4.0 must mirror verbatim and therefore cannot fix here. Filed as #75.
CI is green on 00770f4 (ubuntu / node 24.18.0 ✅, publish dry-run ✅; the two e2e jobs are label-gated and skipped). Nothing CRITICAL or HIGH survived the fix pass, so the PR is marked ready for review. Per .github/ISSUE_TRIAGE.md the routine does not approve its own work — a human merges.
Generated by Claude Code
…revision The §4.0 canonical block claims every Participant template embeds it word for word, but SHARED_WORKFLOW in src/platforms/shared.ts gained a resume sentence in step 2 that the doc never picked up, so the doc's normative text no longer matched the bytes `crew setup` writes. Four example artifacts and both marker-grammar samples still showed `registry-revision: 2` while §7 and REGISTRY_REVISION say 5, making the doc internally inconsistent and giving readers a spurious mismatch when comparing a freshly generated file. Syncs §4.0 byte for byte with SHARED_WORKFLOW, adds the same resume clause to the §3 prose summary, records the optional `options` parameter in §7's registry record signature, and moves the six stale examples to revision 5. The §2 Invocation cells are left alone on purpose: they state the interactive invocation setup prints (FR-G07), and the ` --resume` suffix is produced only on the Launcher path — a note after the matrix explains that split. No generated bytes change, so REGISTRY_REVISION is deliberately not bumped. Closes #32 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yGqn3pZ7CNmAqBtSGTBYq
00770f4 to
4763e17
Compare
Closes #32
What changed and why
docs/design/setup-integration.mdhad drifted from the code it documents, in two ways. §4.0 promises that every Participant template embeds its block "word for word", butSHARED_WORKFLOW(src/platforms/shared.ts:121-142) gained a resume sentence in step 2 that the doc never picked up — so the doc's normative artifact text no longer matched the bytescrew setupactually writes (grep -ni resumereturned zero hits across the whole 587-line file). Separately, four example artifacts and both marker-grammar samples still readregistry-revision: 2while §7 andREGISTRY_REVISION(src/platforms/shared.ts:19) both say 5 — leaving the doc internally inconsistent, since the pi and opencode examples already showed 5, and giving readers a spurious mismatch when they compare a freshly generated file against the examples.Five edits, all in that one file:
--resumeto the join command.", copied from the source including its line wrapping and continuation indent. The block is byte-for-byte identical toSHARED_WORKFLOW(verified mechanically, see below), restoring the "word for word" guarantee.setupprints (src/setup/index.ts:244) and what FR-G07 (docs/design/srs.md) requires, whereas the--resumesuffix appears only on the Launcher path. The note documents that split precisely — it scopes the "what setup prints" claim to the Participant CLI rows (the two Model Backend rows areBackendTargets with noinvocationat all, and their cells summarize the printed recipe), and it covers both resume-carrying paths:invocationfor the seven targets whose panes paste an invocation, andlaunch_argsforcopilot-cli, whose resumed pane instead starts ascopilot --agent=crew --prompt "<role> <id> --resume"(src/launcher/session.ts:130-134,src/platforms/copilot.ts:36-42).invocation(role,id)/launch_args?(role,id)now show the optional options parameter, matchingsrc/platforms/shared.ts:81and:86. Nothing else in §7 restructured.docs/design/cli-contract.md:119governs that).registry-revision: 2examples →5(claude / codex / gemini / copilot artifacts, plus both marker-grammar samples), using the literal5to match how §4.5–§4.8 already read.REGISTRY_REVISIONis deliberately not bumped. §7 requires a bump only when a generated file's bytes change; here no rendered byte changes — the doc is catching up to already-shipped output. Bumping it would wrongly reclassify every installed artifact asmanaged-outdated.No URL in the document was added, removed, or altered, so the
officialSourcesassertions intests/unit/platforms.test.ts:469-489are unaffected.Verification
Docs-only change — claiming the docs-only test exemption per
CONTRIBUTING.md("tests are not applicable for docs-only changes") and.github/ISSUE_TRIAGE.md. No file undersrc/,bin/,tests/,web/, ordocs-site/is touched; there is no changed logic to cover.Run on Node 24.18.0 (CI's pinned version) after rebasing onto
origin/main@8cd6d99:npm run typechecknpm run lintnpm run format:checknpm run buildnpm run test:coverageThe 2 failures are in
tests/integration/commands/doctor.test.ts— "degrades a raw filesystem read failure in project roles instead of aborting doctor" and "degrades unreadable roles and teams directories to whole-listing warnings". They are environmental and pre-date this branch, proven rather than assumed: with the change stashed andgit status --porcelainreturning zero lines, the same file still fails the same two tests (exit 1,2 failed | 31 passed). Bothchmod 000a directory and expect the read to fail; that sandbox runs as uid 0, which bypasses the permission bits, so the expectedINVALID_CONFIGwarnings never fire. CI does not run as root, and this PR's CI run is authoritative for the gate. Vitest suppresses the coverage table on a failing run, so the 95% thresholds were not evaluated locally — nosrc/**orbin/**file changed, so coverage is unchanged frommain.§4.0's agreement with the source was verified mechanically rather than by eye, both before and after the review fix pass: a script extracts the
SHARED_WORKFLOWtemplate literal fromsrc/platforms/shared.tsand the §4.0 fenced block from the doc and compares them —equal: true, both 1446 bytes, with a cleandiff -u. Both sides carry the{{ROLE_ARGS}}token identically, so no allowance was needed.Review
Two reviewers ran per
.github/ISSUE_TRIAGE.md: the general code reviewer, and — the diff being docs-only — a documentation reviewer verifying the new prose against the code it describes, in place of the TypeScript reviewer. Findings are posted in full on this PR. Nothing CRITICAL or HIGH. The one allowed fix pass was taken anyway, for two MEDIUM accuracy defects in the §2 note (thelaunch_argspath forcopilot-cli, and an over-claim across the two Model Backend rows) plus two LOW wording items — all four are folded into the current commit.Related open PRs
None. Checked at file level rather than from PR prose: no open PR's changed-file list includes
docs/design/setup-integration.md— #69 (docs/design/security.md), #64 (docs/design/srs.md), #63 (docs/design/configuration.md), #61 (docs/design/cli-contract.md,docs/design/data-model.md), #57 (README.md,docs-site/sections/modules.tsx,docs/design/architecture.md,docs/design/product-spec.md). (#49 and #56 were open when this branch was cut and have since merged; this branch is rebased onto8cd6d99, which contains both. Neither touched this file.)Out of scope
Deliberately untouched. The first three were found during implementation and review and are filed as their own issues rather than folded in, per
.github/ISSUE_TRIAGE.md's scope rule:src/platforms/shared.ts:7-9says "a change there is a registry-revision bump here", contradicting §7's actual bytes-changed rule and, read literally, demanding a bump for this very PR — Setup: shared.ts header says any setup-integration.md change bumps the registry revision, contradicting §7 #73.registry-revisionliterals against re-drifting;tests/unit/platforms.test.tsreads this file but compares onlyofficialSources, which is how revisions 2 and 5 came to coexist here onmain— Tests: nothing guards setup-integration.md §4.0 or its registry-revision examples against drift #74.SHARED_WORKFLOWtells a pane to detect a clean-stop resume while its own step-1 parse rule admits only<role> [id], hiding the appended--resumetoken that is the only available signal — Setup: the shared workflow asks a pane to detect a clean-stop resume its own parse rule hides #75.CLAUDE.md/AGENTS.mdcommand-annotation drift — CLAUDE.md/AGENTS.md command annotations have drifted from the actual scripts and gates #41; docs-site prose drift — docs-site prose drift: lifecycle omits abandoned, deferred-seam roster, glossary count #42;architecture.md§4.1 staleness — architecture.md §4.1 source tree is substantially stale #35.little-codertarget may be stale now thatopencode-clialso exists; §7's revision number itself is correct at 5, and setup-integration.md drift: canonical workflow text and registry-revision examples out of date #32 says nothing about the attribution, so it was left alone rather than changed on a guess.