Skip to content

docs: reconcile architecture.md and data-model.md with the shipped code - #111

Open
dichovsky wants to merge 2 commits into
mainfrom
docs/architecture-data-model-drift
Open

docs: reconcile architecture.md and data-model.md with the shipped code#111
dichovsky wants to merge 2 commits into
mainfrom
docs/architecture-data-model-drift

Conversation

@dichovsky

@dichovsky dichovsky commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Closes #35
Closes #58
Closes #62
Closes #65
Closes #67
Closes #89
Closes #99

Seven pieces of drift in the two documents a contributor reads to orient themselves, plus the two user-facing files that restate one of the same facts. Each one misleads exactly where it is consulted: §4.1 is the source map opened first and it named a directory (src/commands/) that does not exist while omitting four platform modules and five launcher modules; the transaction catalogue is what an auditor reads to learn which operations are all-or-nothing and it was missing both terminal Task operations; the Tasks invariants are what a reader trusts to know what survives a crash and the land bullet claimed an atomicity boundary wider than the code provides; and README.md/EXAMPLES.md are where understating a destructive command's reach costs the most.

Per docs/README.md:5-12, architecture.md is authority rank 4 — the lowest — so where it disagreed with ADR-0015 (rank 1), the SRS (rank 2), or the registry, it is the bug. data-model.md is rank 3, above architecture, which is why #62/#67/#99's data-model half matters more, not less; each edit there conforms to a rank-1 or rank-2 source cited inline.

Verified before editing, not taken from the issue bodies

Deltas against the issue bodies — noted rather than silently absorbed

  1. Docs: architecture.md Console route list omits GET /api/events, and its actions.ts row omits two handlers #89's /api/events half does not reproduce. architecture.md:155 on current main already reads …GET /api/snapshot,/api/events,/api/health,/api/sessions,/api/resumable-sessions,/api/peek… — PR docs: correct CLI contract drift on the --json surface #85 (93433fc, the commit that introduced the string) fixed it after the issue was filed. That line is left byte-identical. Only the actions.ts half of Docs: architecture.md Console route list omits GET /api/events, and its actions.ts row omits two handlers #89 was still true, and it is fixed here. Also worth recording: GET_PATHS (src/ui/server.ts:251-259) actually has seven entries, not six — the issue counts the six /api/* routes and omits /, which the same line already covers as "static assets".
  2. Docs: architecture.md Console route list omits GET /api/events, and its actions.ts row omits two handlers #89 understates the actions.ts gap. It names four missing handlers (listResumableTeamSessions, peekPane, archiveAgent, restoreAgent); resumeTeam (src/ui/actions.ts:377) is a fifth. The gloss now enumerates all fourteen exported handlers, so it is exhaustive as the Acceptance's first option asks.
  3. architecture.md §4.1 source tree is substantially stale #35 is wider than its list, in both directions. It says now-view.tsx is "unlisted"; it is in fact already covered collectively by "the six views" (PR docs: correct the Console view count to six #57 corrected that count). Conversely it does not name web/api.ts, web/types.ts, or web/styles.css, which genuinely were absent, nor src/ui/index.ts, nor twenty-one src/ root modules (io.ts, fs-safe.ts, init.ts, agents.ts, messages.ts, tasks.ts, maintenance.ts, retention.ts, duration.ts, agent-id.ts, task-id.ts, templates.ts, config.ts, yaml-load.ts, version.ts, delay.ts, and the five it does name). The tree was regenerated from the actual layout rather than patched against the issue's list, and the view-model.ts gloss picks up the nowWorklist / pillBg / rolePillBg omission raised in architecture.md §4.1 source tree is substantially stale #35's comment.
  4. Data model: the land invariant puts the on-disk worktree removal inside the atomic step, but it happens before the transaction #67's Acceptance is met but its implied remedy is not documentable. The invariant now states the crash case; it deliberately does not say a later land repairs it. Verified it cannot: hasUnlandedChanges (src/worktree.ts:291-306) runs git -C <worktreePath> status --porcelain and throws NOT_FOUND when that directory is gone, so a rerun refuses before reaching landTask. Stating otherwise would have replaced one false invariant with another.
  5. Docs: architecture.md and the release-smoke recorder still understate the Participant roster #58's SRS premise is now true. The triage comment corrected the body, noting FR-G07 still listed seven at the time. PR docs: align every roster introduction on the eight Participant CLIs #56 has since merged: srs.md:513-515 now enumerates all eight, so the rank-2-versus-rank-4 contradiction the Impact paragraph describes is the present state, and architecture.md is conformed to FR-G07's exact enumeration.
  6. Line numbers drifted. Docs: prune's abandoned-Task deletions are still missing from architecture.md, README.md, and EXAMPLES.md #65 cites architecture.md:497 (correct at main), Docs: architecture.md §4 and data-model.md describe Workspace discovery without FR-W04's pointer redirect #99 cites :204-207 (correct), Docs: architecture.md Console route list omits GET /api/events, and its actions.ts row omits two handlers #89 cites :155/:157 (correct); Docs: data-model transaction catalogue omits task abandon and task land #62's data-model.md:399-411 is :401-413 and Data model: the land invariant puts the on-disk worktree removal inside the atomic step, but it happens before the transaction #67's land bullet is :367-373. Cited here so the mapping is checkable.

What changed and why

File / location Change Issue
architecture.md §4.1 tree Regenerated from the real src/, web/, tests/, e2e/ layout: commands/*.ts and the top-level templates/ block removed, 20 src/ root modules + store/connection.ts, review-worktrees.ts, change-signature.ts + platforms/{antigravity,pi,little-coder,opencode,agent-skills}.ts + launcher/{derive,ref,prompts,stop,resume}.ts + ui/index.ts + web/{api.ts,types.ts,styles.css} added, tests/{…} corrected, e2e/ui/ added #35
architecture.md §4.1 actions.ts gloss Now names all fourteen exported handlers (send/create/approve/requeue, launch/resume/stop, listSessions/listResumableTeamSessions, peek, prune/clean, archive/restore) #89
architecture.md §4.1 view-model.ts gloss Adds "Now worklist" and "pill backgrounds" #35 (comment)
architecture.md §4.3 Discovery gains the pointer-redirect step and its NOT_WORKSPACE failure mode, and the ADR-0011 whole-Crew worktree (no pointer, local Store) is split from the ADR-0015 Task / Review Worktrees (pointer, shared Store) #99
architecture.md §8 "Claude and Gemini use /crew" → the same eight invocations FR-G07 enumerates #58
architecture.md §10 prune names abandoned Tasks with abandoned_at as their cutoff, and the unread-notification retention rule now plainly covers both statuses #65
data-model.md "Store location and opening" Same discovery correction as §4.3, at rank 3 #99
data-model.md Tasks invariants, land bullet On-disk removal moved outside the atomic step; adds the undeletable-branch warning, the archived-assignee skip, and the crash-between-halves state #67
data-model.md transaction catalogue Adds task abandon and task land rows; land's row states that it is not a transition, appends no event, and CASes on worktree_path rather than revision #62
README.md retention bullet Names abandoned Tasks and both cutoff columns; retention rule covers both kinds #65
EXAMPLES.md prune bullet Same #65
tests/tools/release-smoke.test.ts:15 "the five Participant CLIs / backends" → "all ten targets … the eight Participant CLIs and the two Model Backends" #58

Added in review round 1

File / location Change Origin
data-model.md catalogue, both new rows Rewritten in the table's terse style; see the width note below review (#62 acceptance)
data-model.md, prose under the catalogue Carries the two facts the terse rows shed: the assignee's notification is the clear_safe Sign-off in both terminal rows, and land skips it only for an archived assignee review
architecture.md:12 and the §3 Workspace row Two surviving restatements of discovery as a pure upward walk, both now naming the pointer redirect review (#99, the #97 lesson)
architecture.md §4.1 worktree.ts gloss "(whole-Crew and per-Task)" → "(whole-Crew, per-Task, review)" review (#35)
architecture.md §4.1 tree web/styles.d.ts and web/tsconfig.json added; process.ts/which.ts/relay.ts moved into the contiguous root block review (#35)
architecture.md §4.1, note under the block States that the Console's own tests are co-located under web/, not under tests/ review (#35)
architecture.md untrusted-config bullet Drops the executable-confirmation-flag promise #112 consequence
architecture.md §11 and decisions.md:152 Hardcoded FR-X01–X08 ranges dropped #112 consequence

Why three lines changed for a requirement this PR does not touch. PR #112 retires FR-H08 and adds FR-X09. Three lines at this PR's tier go stale as a result, and the coordinator widened scope to cover them (including a narrow authorization to touch decisions.md:152 and nothing else in that file):

  • architecture.md's untrusted-configuration bullet read "A custom executable is accepted only from an explicit command-line flag, and crew prints it for you to confirm before running it." That promised a security mechanism that does not exist: grep -n executable src/cli.ts returns nothing, participantExecutable (src/launcher/plan.ts:118-124) resolves target.executable from the registry alone, and src/launcher/config.ts:45 lists executable among the keys repo config may never supply. With FR-H08 retired there is no requirement behind it either. It now states only that registry ids are selectable, matching what docs: close seven SRS drifts against the code they govern #112 does to security.md and configuration.md.
  • The two FR-X01–X08 ranges (architecture.md §11, decisions.md:152) are dropped, not bumped to –X09, per the precedent PR docs: signpost ADR amendments and fix stale ADR cross-references #91 set: bumping re-drifts the next time a requirement is deferred, and dropping is the only option that cannot. This also removes any merge-order dependency on docs: close seven SRS drifts against the code they govern #112no FR-X09 reference appears anywhere in this diff (grep -rn "FR-X09" docs/ README.md EXAMPLES.md → no match), so the id need not exist yet for this PR to be correct.

Surroundings were checked for the same claim restated (the #97 lesson). grep -n "prune\|completed Task" over all three prune files found no second copy beyond the three fixed passages; grep for the roster over architecture.md found only §8; data-model.md:519-522 already covered both cutoff columns correctly and needed no change; architecture.md:370 ("one git worktree shared by the whole Crew") is correct as written per ADR-0015 :11-16 and is deliberately untouched.

Width, including the table rows — measured on the pushed tree, nothing excluded. The first push's two catalogue rows were 254 and 245 characters against a catalogue whose widest pre-existing row is 117, and the wrapping note quoted "97" while silently leaving them out. Both are now inside the band:

Catalogue row First push Now
| task abandon | 254 116
| task land | 245 115
| launch-teardown reap | (widest pre-existing) 117 117

Two facts had to leave the rows to get there — the clear_safe identity of the assignee's notification, and land's archived-assignee skip. Neither was dropped: both moved into the paragraph directly under the table, where width is not scarce. abandon's explicit notify assignee/creator/reviewer was kept rather than shortened to a vaguer phrase, because abandonTask (src/store/tasks.ts:689) really does notify all three; the six characters came out of clear lease/review/worktree instead.

Prose wrapping: new architecture.md prose lines top out at 92 chars against an existing distribution that already carries 88/89/90/92; new data-model.md prose lines top out at 97, matching the paragraph they replace; new README.md/EXAMPLES.md lines top out at 97 against the same band. Two lines this round exceeded their band on first write and were rewrapped before pushing (architecture.md:14 at 137, decisions.md:152 at 100). The one line legitimately over band is the §3 module-map table row, at 281 — that column was already 213 characters before this PR. The two touched README.md/EXAMPLES.md bullets were rewrapped whole so they do not end ragged; no untouched paragraph was reflowed. Prettier is proseWrap: "preserve", so format:check passes either way.

Verification

Tests: none added — claiming the docs exemption, granted by CONTRIBUTING.md:95 and .github/ISSUE_TRIAGE.md. The diff is five Markdown files plus a one-line comment in an unshipped maintainer-run recorder; no executable line changes anywhere. Nothing guards the edited passages, checked rather than assumed:

  • tests/unit/docs-facts.test.ts derives docs-site/generated/facts.json from src/platforms/registry.ts, src/store/schema.ts, src/cli.ts, package.json, vitest.config.ts, and ADR H1 headings — never these files. No regeneration needed.
  • tests/store/schema.test.ts:409-421 ("data-model.md normative stamp") reads data-model.md, but asserts only on PRAGMA user_version = N; and the ## Schema version N (current) heading. Neither was touched; the test passes.
  • The remaining tests/ references to docs/design/architecture.md (tests/unit/launcher/{config,prompts}.test.ts, tests/integration/commands/team-launch.test.ts:45, tests/fixtures/launch-plan.dev.json:47) use the path as a focus.docs string; nothing reads it from disk.
Command Exit Result
npm run typecheck 0 clean (root + web/ + docs-site/)
npm run lint 0 clean
npm run format:check 0 "All matched files use Prettier code style!"
npm run build:docs 0 dist-docs/main.js 102.1kb
npx vitest run tests/tools/release-smoke.test.ts 0 1 skipped (the intended gated skip — CREW_RELEASE_SMOKE unset)
npx vitest run tests/unit/docs-facts.test.ts tests/store/schema.test.ts 0 21 passed

Re-run in full after review round 1, on the pushed tree (b406613):

Command Exit Result
npm run typecheck 0 clean (root + web/ + docs-site/)
npm run lint 0 clean
npm run format:check 0 "All matched files use Prettier code style!"
npm run build:docs 0 dist-docs/main.js 102.1kb
npx vitest run tests/tools/release-smoke.test.ts tests/unit/docs-facts.test.ts tests/store/schema.test.ts 0 21 passed, 1 skipped (the intended gated recorder skip)

Branch cut from origin/main @ 09e67cd.

Contract impact

None. Every edit conforms a lower-authority document toward what a higher one already says — ADR-0011/ADR-0015 (rank 1), FR-G07/FR-E24/FR-W04 (rank 2), or the registry and Store code. No ADR is amended, no FR-*/NFR-* is added, renumbered, or given new meaning, no CLI grammar rule, record type, error code, command, or flag moves, and CURRENT_SCHEMA_VERSION stays 7.

Out of scope

Deliberately untouched, owned by other work:

One thing worth a maintainer's eye, raised not fixed: @slegarraga commented on #89 offering to take it. No PR was opened and the issue is unassigned, so it was implemented here; if that was the wrong call, the actions.ts line is a one-line revert.

Related open PRs

#112 (FR-H08 retirement, FR-X09 addition) is the only one that interacts with this PR, and only in one direction: three lines here are downstream consequences of it. They are written so that neither merge order matters — the FR-X01–X08 ranges are dropped rather than bumped, and no FR-X09 id is referenced, so this PR is correct whether #112 lands before it, after it, or not at all. #112 edits srs.md, security.md, and configuration.md; this PR touches none of those three, so there is no line overlap either.

The authority order in docs/README.md exists so a reader can trust the
lower-tier documents to agree with the ADRs, the SRS, and the code. Seven
places had stopped agreeing, and each one misleads at the moment it is
consulted: the source tree is the map a new contributor opens first, the
transaction catalogue is what an auditor reads to learn which operations are
all-or-nothing, the Tasks invariants are what a reader trusts to know what
survives a crash, and the prune passages in README/EXAMPLES are where
understating a destructive command's reach costs the most.

Every claim was re-checked against main before editing; where the code and the
lower-tier prose disagreed, the prose was the bug. The land invariant is the
one correction that is not merely additive: it claimed an atomicity boundary
wider than the code provides, which told a reader that a crash between the git
half and the database half was impossible when it is not.
The two new transaction rows were 2.6x the catalogue's width band, which is
the opposite of the terse style FR-62's acceptance asked for; the two facts
they had to shed to fit are now carried in the prose under the table, where
width is not scarce.

Three further corrections follow from PR #112 retiring FR-H08: the untrusted-
configuration bullet promised an executable-confirmation flag that no longer
has a requirement behind it and never had code, and the two hardcoded FR-X01-X08
ranges would re-drift the next time a requirement is deferred. Both ranges are
dropped rather than bumped, per the precedent PR #91 set, so this change carries
no merge-order dependency on #112.

The remaining edits close restatements the first pass left standing: discovery
described as a pure upward walk in two more places, and the worktree module
glossed as owning only two of the three worktree kinds it actually owns.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment