docs: authorize the Console team-resume action in FR-U19 - #64
Conversation
dichovsky
left a comment
There was a problem hiding this comment.
Routine review — general reviewer + docs-verification reviewer
Roster per .github/ISSUE_TRIAGE.md: general code reviewer (always) and, for a docs-only diff, prose-against-code verification instead of the TypeScript reviewer. No security reviewer — nothing under src/store/, src/process.ts, src/which.ts, src/fs-safe.ts, src/setup/, or the Launcher changed.
Both reviewers ran independently and converged on the same conclusion. This PR should not merge as-is. The factual claim it makes is true, but the repair was made at the wrong authority level, and the PR body's rationale — quoted from my own assessment on #25 — contains a claim that is simply wrong.
CRITICAL
C1 — "no accepted ADR forbids it" is false. ADR-0012 (accepted, authority level 1) closes the Console action enumeration and excludes resume.
docs/adr/0012-optional-local-ui-server.md:38-41:
The Console offers only the approved Operator actions. These are: send a Message, create a Task, approve a Submission or send it back, launch a Team without attaching to it, stop a Team, peek at a pane's text (with terminal control characters stripped out), and run
pruneorcleanbehind a typed confirmation.
Status accepted. ADR-0013 supersedes only ADR-0012's "presentation and confirmation specifics" (docs/adr/README.md:17), not the action list. ADR-0017 amended the surface for archive/restore and said so explicitly ("amends FR-U19, adds FR-U36", 0017:42-43). Nothing amends it for resume.
So this PR widens level 2 to permit an action that level 1 excludes. That inverts the authority order in docs/README.md instead of resolving it — the rule is to fix the lower-authority document.
C2 — ADR-0017 anticipated this exact change and told reviewers to reject it.
docs/adr/0017-console-now-view-theme-and-agent-archive.md:77-79:
The Console's action surface is no longer limited to Message/Task/Team/maintenance operations (FR-U19 amended); reviewers of future Console changes should treat FR-U19's enumeration, not "whatever's already wired," as the actual boundary — anything beyond it needs its own FR.
This PR takes what is already wired and edits the enumeration to match, adding no FR. That is precisely the move the ADR forbids.
Relevant provenance, established by both reviewers: git log -S"'/api/team/resume'" returns only 091976a ("Initial public release"). The route predates ADR-0017 and was never authorized at any level. So "the SRS is the incomplete side" was only part of the story — the route is unauthorized at level 1 too, and no amount of level-2 editing fixes that.
HIGH
H1 — docs/design/cli-contract.md:391-393 now directly contradicts the amended FR-U19, in the same words.
The Console's actions additionally cover launching a Team …, stopping a Team crew owns, peeking at a pane, running
pruneorclean, and archiving or restoring an Agent — and nothing else.
Another closed enumeration, still excluding resume. Before this PR the SRS and the CLI contract agreed (both wrong); after it they disagree. docs/README.md and CONTRIBUTING.md:86 both require the lower-authority document to be fixed in the same change.
This also undercuts the PR's rationale. The rationale cites cli-contract.md:267 and :343-351 — which specify the CLI command crew team resume. The CLI contract's Console section separately closes the action list without resume. The document is internally split, and this PR fixes only the half that supports its conclusion.
MEDIUM
M1 — "resuming a Team" is the only entry in FR-U19 with no governing requirement anywhere in the SRS. Every other item has one: launch → FR-U20, stop → FR-U26–U29, peek → FR-U24, prune/clean → FR-U25, archive/restore → FR-U36. The no-FR-pointer decision is stylistically consistent with the unpointered neighbours but substantively unlike all of them, because those have a governing FR elsewhere and this does not.
Correction to the PR body while we're here: PR #49 adds FR-U39–FR-U49, not FR-U39–FR-U42, and it never touches FR-U19 or adds any Console-side FR — all its requirements specify the CLI command. So #49 does not fix #25, and merging both still leaves the Console action ungoverned.
M2 — FR-U19's *Verify:* clause cites two test files, neither of which exercises the newly enumerated action. grep -rn "api/team/resume" src/ tests/ web/ e2e/ matches only src/ui/server.ts, src/ui/actions.ts, and web/app.tsx:332. TEAM_POSTS (tests/integration/ui-server-team.test.ts:266-271) lists launch/stop/prune/clean and omits resume, so the route is excluded from the 401/403 posture sweep, the malformed-body sweep, and the exhaustiveness argument. ACTION_PATHS (tests/integration/ui-server-actions.test.ts:113-120) omits it too. Defensible as written since the stated method is "inspection", but the newly added item is now the least-verified entry in a requirement whose entire job is bounding the write surface. One line — adding ['/api/team/resume', { session: SESSION }] to TEAM_POSTS — closes most of the gap.
M3 — FR-U34's Operations-view enumeration (docs/design/srs.md:895-898) has the identical drift and was left untouched. The Console demonstrably renders a resumable-session list and a Resume control (web/components/operations.tsx:121-124, tested at web/components/operations.test.tsx:98). FR-U34's list is descriptive rather than closed, so this is weaker than the FR-U19 defect, but it is one requirement away in the same file.
LOW
- L1 —
src/ui/server.ts:58says "the four FR-U19 action writes"; there are seven fixed POST paths plus two parametrized families. Pre-existing, and adjacent to the already-filed #60. - L2 — SRS §1.1/§1.2/§1.3.4 and Appendix A still describe the post-v1 additions as "the Console and … stopping a Team". Correctly left alone — PR #49 fixes exactly those lines and touching them would create a real conflict. Noted so it is not mistaken for an omission.
- L3 — Vocabulary is clean. Strictly,
CONTEXT.md:38makes a Team a roster template while what resumes is the tmux session, but that shorthand is pre-existing and used identically by both neighbouring clauses. No change warranted in isolation.
Things that are clean
Scope discipline (both changed lines trace to #25; the second is a pure Prettier re-wrap), no drift guard affected (tests/unit/docs-facts.test.ts reads docs/adr/, package.json, and vitest.config.ts — never srs.md), and no merge conflict with #49 or #56 (their srs.md hunks are at lines 49–1374, none within three lines of 802–808). CI is green on GitHub-hosted runners, including npm run test:coverage at its 95% thresholds — which confirms the two doctor.test.ts failures reported in the PR body were sandbox-only (uid 0 defeats chmod 000).
Disposition — no fix pass taken; this needs a human
.github/ISSUE_TRIAGE.md grants the implementer exactly one fix pass on CRITICAL/HIGH findings. I am deliberately not taking it, because every available repair requires amending an accepted ADR, and an ADR records a decision that is hard to reverse — that is a human's call, not something an unattended routine should do silently. The same document says the correct verdict for a large, contract-touching change is PLAN-ONLY "even when you are confident you could implement it." The review has established that this was such a change; my IMPLEMENT verdict on #25 was wrong, and I have retracted it there.
PR left as a draft. Deleting the route is not the recommended alternative — it would remove a capability shipped in 0.1.0 and strand web/components/operations.tsx:139-142 and web/app.tsx:331. The proportionate fix is additive and needs a human decision on step 1:
- An ADR (or an amendment section in ADR-0017's style) recording that Console team-resume is an approved Operator action, superseding ADR-0012:38-41 on that one point, with the "reuses an operation the CLI already exposes, invents no new authority" argument ADR-0017:44-47 makes for archive/restore.
- A companion Console-resume FR in the FR-U36 mould — detached relaunch, the strict clean-stop-marker / stored-plan /
TEAM_DRIFTpreconditions, reuse ofrunTeamResumewith no new Store authority, that it is deliberately not FR-U25-gated, and theGET /api/resumable-sessionsread. - This one-phrase FR-U19 edit, then referencing that FR as
(FR-Uxx). cli-contract.md:391-393widened in the same change./api/team/resumeadded toTEAM_POSTSandACTION_PATHS.
Generated by Claude Code
There was a problem hiding this comment.
Pull request overview
Updates the SRS (FR-U19) to explicitly authorize the Console’s existing Team resume action, aligning the closed “action surface shall be limited to …” enumeration with the already-implemented /api/team/resume route and its UI flow.
Changes:
- Add “resuming a Team” to FR-U19’s closed Console action-scope list.
- Re-wrap the FR-U19 line break without changing surrounding semantics.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
FR-U19 states the Console action surface "shall be limited to" a closed enumeration that omitted resuming a Team, yet the Console has long shipped POST /api/team/resume (src/ui/server.ts) backed by resumeTeam (src/ui/actions.ts) and driven from the browser. The action is legitimate: it is normatively specified one authority level down in the CLI contract (docs/design/cli-contract.md) and implemented in src/launcher/resume.ts, and no accepted ADR forbids it. Per the authority order in docs/README.md the enumeration was the incomplete side, not the code, so widen FR-U19 to cover it rather than remove a working, specified action. No FR id is created — the resume requirements that would be cited live in an unmerged PR, and a dangling reference is worse than the prose the neighbouring "launching a Team" and "stopping a Team" entries already use. Closes #25 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7bMA53xLfiWtbswfzEgAf
f6b68c4 to
81dc20d
Compare
Caution
The rationale below was wrong, and this PR should not merge on it. After the body was written, the routine's own reviewers found that
docs/adr/0012-optional-local-ui-server.md:38-41— an accepted ADR, authority level 1 — closes the Console action enumeration and excludes resume, and thatdocs/adr/0017-...md:77-79explicitly instructs reviewers to treat FR-U19's enumeration, not "whatever's already wired," as the boundary, with anything beyond it needing its own FR. This PR does exactly what that ADR forbids. The claim "No accepted ADR forbids it" below is false — it is struck through in place and preserved only so the review thread reads coherently. See the full review on this PR and the retraction and 5-step plan on #25.The diff itself is factually accurate and CI is green — the problem is that the repair was made at the wrong authority level. Merging this alone would put the SRS (level 2) in contradiction with both ADR-0012 (level 1) and
cli-contract.md:391-393(level 3), where levels 2 and 3 previously agreed. A human needs to decide whether Console team-resume is an approved Operator action before any of this lands; if it is, it needs an ADR amendment first and this edit becomes step 3 of five.The routine marked this PR draft and deliberately declined its one fix pass, because every viable repair amends an accepted ADR. It was subsequently taken out of draft by an action the routine did not take.
Closes #25
What changed and why
FR-U19 (
docs/design/srs.md:803-808) is a closed enumeration — "The Console action surface shall be limited to …" — and it did not list resuming a Team. The Console registers the route anyway:'/api/team/resume'inPOST_PATHS(src/ui/server.ts:272), dispatched atsrc/ui/server.ts:492-493, backed byresumeTeam(src/ui/actions.ts:377), driven from the browser atweb/app.tsx:331-332. The server's own header (src/ui/server.ts:12-15) then cited FR-U19 as the authorization — "The write surface is EXACTLY the FR-U19 Operator action POSTs: …/api/team/resume…" — claiming coverage the requirement did not grant.The action is not rogue. It is normatively specified one authority level down (cli-contract.md:267grammar,:343-351semantics) and implemented insrc/launcher/resume.ts. No accepted ADR forbids it, and group U is post-v1 additive in its entirety (srs.md:1322). Per the authority order indocs/README.md, the level-2 enumeration is the incomplete side, not the route — so the fix is to widen FR-U19, exactly as FR-U36 previously widened it for Agent archive/restore.Corrected:
cli-contract.md:267and:343-351specify the CLI commandcrew team resume. The CLI contract's Console section separately closes the action list at:391-393with "and nothing else", excluding resume — so that document is internally split, and the original rationale cited only the half supporting its conclusion.git log -S"'/api/team/resume'"returns only091976a("Initial public release"): the route predates ADR-0017 and was never authorized at any level, so "the SRS is the incomplete side" was only part of the story. Widening FR-U19 does not fix that; it inverts the authority order.One insertion:
The second changed line is pure Prettier re-wrap of the same words.
Three deliberate constraints, all of which still describe the diff accurately:
The requirements that would be cited (FR-U39–FR-U42) exist only in open PR docs: trace crew team resume with SRS requirements #49, which is unmerged.Corrected: PR docs: trace crew team resume with SRS requirements #49 adds FR-U39–FR-U49, not FR-U39–FR-U42, and it never touches FR-U19 or adds any Console-side FR — all of its requirements specify the CLI command. So docs: trace crew team resume with SRS requirements #49 does not resolve this issue, and merging both still leaves the Console action ungoverned. "resuming a Team" is the only entry in FR-U19 with no governing requirement anywhere in the SRS.*Verify:*untouched.It already points at …Corrected: the resume route has no coverage there.tests/integration/ui-server-team.test.ts, which is where the resume route's coverage lives.TEAM_POSTS(tests/integration/ui-server-team.test.ts:266-271) lists launch/stop/prune/clean and omits resume;ACTION_PATHS(tests/integration/ui-server-actions.test.ts:113-120) omits it too. Nothing intests/ore2e/posts to/api/team/resume. FR-U19's stated method is "inspection", which does hold, but the newly added item is the least-verified entry in a requirement whose whole job is bounding the write surface.FR-*/NFR-*id added or renumbered, so the group-U range statements atsrs.md:1322andsrs.md:1377are correctly left alone.src/ui/server.tsis deliberately not touched, keeping the diff docs-only.Note on
GET /api/resumable-sessionsThe issue names it alongside the POST, but it is not part of the violation. FR-U19 constrains the action surface;
/api/resumable-sessionsis a read-only session inventory inGET_PATHS(src/ui/server.ts:258), and its sibling/api/sessions(:257) sits outside the enumeration for the same reason without ever having been treated as excess. (It is, separately, undocumented atcli-contract.md:405-411whereGET /api/sessionsis described in full.)Verification
Docs-only change: no tests added, claiming the docs-only exemption in
.github/ISSUE_TRIAGE.mdandCONTRIBUTING.md.CI on GitHub-hosted runners is green — both jobs, including
npm run test:coverageat its 95% thresholds, andnpm run build:docs. That is the authoritative gate result.The local sandbox run reported
test:coveragefailing with 2 tests intests/integration/commands/doctor.test.ts. Green CI confirms those were environmental: the sandbox runs as uid 0, so the tests'chmod 000does not make the directories unreadable and the expectedINVALID_CONFIGwarnings never fire. Confirmed pre-existing before push by stashing the change and re-running on the clean tree (same 2 failed / 31 passed). Locallytypecheck,lint,format:check, andbuildall passed under Node 24.19.0.Rebased onto
origin/main@d311767immediately before pushing — the base had not moved.Related open PRs
Two other open routine PRs touch
docs/design/srs.md, both in regions disjoint from FR-U19 (line 803) — verified hunk-by-hunk, no overlap within three lines, so git merges cleanly in any order:docs: trace crew team resume with SRS requirements. Hunks at lines 50, 65, 102, 121, 731, 939, 1165, 1319, 1374. Semantically complementary (it specifies the CLI verb) but, as corrected above, it does not fix Console: team-resume routes exceed FR-U19's closed action list (companion to #3) #25.docs: align every roster introduction on the eight Participant CLIs.srs.mdhunks at lines 49, 82, 508.#57, #61, and #63 do not touch
srs.md.Out of scope
crew team resumeSRS requirements — Docs: crew team resume has no SRS requirements (documented in CLI contract but untraced) #3, with PR docs: trace crew team resume with SRS requirements #49 open.src/ui/server.tsheader's omission of the FR-U36/api/agents/:id/archive|restorePOSTs — Console: server.ts header comment omits the FR-U36 agent-archive POSTs from its "EXACTLY" route list #60.src/ui/server.ts:58's stale "four FR-U19 action writes" count.resume_resultenvelope — CLI contract drift: doctor finding codes, resume_result, ui --json opener, implies-launch flags, error-code scopes #28.