docs: correct CI facts and add the missing test tiers to the testing strategy

[dichovsky]

Closes #33

docs/design/testing-strategy.md is the doc that answers "how is behavior verified", so a wrong operational assumption in it is load-bearing. Three claims no longer held; this fixes all three, docs-only. Two commits: the original drift fix, plus a review follow-up that tightened three claims (see "Review follow-up" below).

What changed and why

1. Self-hosted runner → GitHub-hosted + apt-installed tmux

• The environment matrix said "Self-hosted GitHub Actions runner, Node 24.18.0" and "CI requires the self-hosted runner to provide tmux". It now says GitHub-hosted runners (ubuntu-latest) at Node 24.18.0, and that the tmux-dependent workflows install tmux themselves via apt-get (then tmux -V to prove it).
  Evidence: every job is runs-on: ubuntu-latest — .github/workflows/ci.yml:15, ci.yml:53, stress.yml:26, launch-e2e.yml:27, ui-e2e.yml:24, pages.yml:32, publish.yml:34. No self-hosted label exists anywhere in the repo. tmux is installed in-workflow at ci.yml:25-26, stress.yml:38-39, launch-e2e.yml:39-40 (and publish.yml:64-65). The wording matches CLAUDE.md's "GitHub-hosted runners" and avoids quoting a partial command — the workflows actually run sudo apt-get update && sudo apt-get install -y tmux, so a half-quoted apt-get install would fail on a cold package index.
• The CREW_REQUIRE_TMUX=1 hard-failure guarantee (ci.yml:42), the local graceful-skip sentence, and the "recording-adapter tests remain mandatory everywhere" sentence are kept unchanged in meaning — only the runner attribution was wrong.

2. Stress tier: nightly + manual dispatch, not "at release"

The stress paragraph said the full 500-iteration tier runs "nightly and at release on the self-hosted runner". Both halves were wrong. stress.yml:10-19 has only schedule, workflow_dispatch, and pull_request: types:[labeled] — there is no release: trigger — and the actual release path publish.yml:70-73 runs npm run test:coverage with CREW_STRESS_ITERS: '25', i.e. the fast tier. The doc now says nightly and on manual dispatch during release prep, at Node 24.18.0 — matching stress.yml:8, which calls that trigger "release prep" itself.

3. Package-verification bullet no longer claims import.meta.url

The bullet claimed pack-smoke verifies "packaged Role/Team resolution through import.meta.url". src/templates.ts:1-9 documents the actual design: templates are "embedded as string constants so they compile into dist/** and ship with the package without any runtime path resolution".

The bullet sits under "Install the tarball into a clean temporary prefix and verify:", so it has to name something the suite actually asserts. It now states the compiled-in-string-constant design plus the packed-file allowlist (tests/integration/package/pack-smoke.test.ts:60-96), which proves no template ships as a separate asset by rejecting anything outside dist/**/*.js, the bundled Console page, README.md, LICENSE, and package.json. It deliberately does not claim init verifies template content: the init case (pack-smoke.test.ts:132-134) only asserts the {type:'init',schema_version:1} envelope, and the following join --role worker proves nothing either, because resolveRole (src/roles.ts:143-150) falls back to PACKAGED_ROLES when no .crew/roles/worker.md exists.

Note for reviewers: import.meta.url is still live elsewhere in src/ (src/ui/server.ts, src/version.ts); the bullet was wrong because it attributed Role/Team template resolution to that mechanism, not because the mechanism is gone.

4. Layer table gains the three missing tiers

• New row Browser component — the jsdom vitest projects web and docs (vitest.config.ts:45-52 and :58-65).
• New row Console browser e2e — the Playwright specs under e2e/ui/ driven by npm run e2e:ui (package.json → playwright test --config e2e/ui/playwright.config.ts).
• A new paragraph after the table states that the two jsdom projects run on every PR as part of npm run test:coverage (ci.yml:39) but sit outside the 95% coverage gate (vitest.config.ts:12 includes src/** and bin/** only, per the comment at :43-44), and that the Console browser e2e is off the PR critical path: nightly (cron: '45 6 * * *'), workflow_dispatch, and on a PR only behind the ui-e2e label (.github/workflows/ui-e2e.yml:10-23). It closes by referring to the existing full-tier no-blind-retry convention rather than declaring a gate — docs/design/product-spec.md:107 reserves the canonical release-gate table to itself ("Other documents reference it and must not keep a separate copy"), and that table has no Console-browser-e2e entry.
• No release gate was invented or relocated: the "Quality gates" checklist here is untouched, and product-spec.md is untouched.

5. CONTRIBUTING.md touch-up

The "Two-tier CI" full-tier list named stress.yml and launch-e2e.yml but never ui-e2e.yml. One bullet added in the same voice and shape: what it does, its label, and how to run it locally. The local instruction names npx playwright install --with-deps chromium first, because npm ci installs @playwright/test but no browser binaries — which is exactly why ui-e2e.yml:33-34 runs that step.

Review follow-up (second commit)

Addressing the MEDIUM findings from both reviewers, all within #33's scope:

1. Pack-smoke bullet reworded to claim only what the suite asserts (see 3 above) — the first version swapped one unverified claim for another.
2. "at release" stress claim replaced with the real triggers (see 2 above).
3. Browser-e2e sentence softened from a gate declaration to a reference to the existing full-tier convention (see 4 above).
4. "GitHub-hosted GitHub Actions runners" → CLAUDE.md's "GitHub-hosted runners"; partial apt-get install -y tmux → "via apt-get" (see 1 above).
5. CONTRIBUTING.md local-run instruction now includes the Playwright browser install (see 5 above).

Verification

Run at Node 24.18.0 (the CI version) on the final rebased branch (316a6f6, rebased onto 12fd129):

Command	Exit	Result
npm run typecheck	0	pass
npm run lint	0	pass
npm run format:check	0	"All matched files use Prettier code style!"
npm run build	0	pass
npm run test:coverage	1	1395 passed, 3 skipped, 2 failed — both pre-existing, see below

Pre-existing failure proof. The two failures are 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 rely on chmod 000 making a directory unreadable, which has no effect for uid 0; the sandbox running these commands is root. Proven pre-existing rather than assumed: the change was stashed, git status --porcelain confirmed empty, and npx vitest run tests/integration/commands/doctor.test.ts on the clean tree produced the identical 2 failures (31 passed). The change was then restored. Both gate runs (first commit and after the review follow-up) failed on exactly those two tests and nothing else.

Because that run failed, vitest suppressed the coverage table — the 95% thresholds were not evaluated locally. This PR's CI run is authoritative for coverage. This diff touches no src/** or bin/** file, so coverage cannot have moved.

Tests: none added. This is a docs-only change (two Markdown files, zero code), which is explicitly exempt — CONTRIBUTING.md ("tests are not applicable for docs-only changes") and .github/ISSUE_TRIAGE.md ("Tests are not required for docs-only, config-only, or pure-UI-only changes — say so explicitly in the PR body when claiming that exemption").

Out of scope

Deliberately untouched:

• Every workflow file, vitest.config.ts, package.json, and everything under src/, tests/, web/, docs-site/, e2e/. The workflows are correct; the prose was wrong.
• docs/design/product-spec.md and the "Quality gates" checklist — inventing or relocating a release gate is outside testing-strategy.md drift: self-hosted runner claim, obsolete template resolution, missing browser test tiers #33.
• The - init + join + send + receive + reviewed Task flow; bullet, which pack-smoke also does not do (it runs init, join, agents, leave), and the macOS/Linux shebang bullet that sits oddly against the single-OS environment matrix — both being filed separately.
• CONTRIBUTING.md's "typecheck → lint → format:check → build → test" line versus ci.yml's actual test:coverage plus build:docs — filed separately.
• e2e/ui/tsconfig.json not being wired into npm run typecheck — issue e2e/ui tsconfig is neither wired into typecheck nor able to compile #24.
• CLAUDE.md/AGENTS.md command-annotation drift (CLAUDE.md/AGENTS.md command annotations have drifted from the actual scripts and gates #41) and docs-site prose drift (docs-site prose drift: lifecycle omits abandoned, deferred-seam roster, glossary count #42).

Related open PRs

None. Checked at file level: the only other open PR is #71, which touches docs/design/setup-integration.md only. (#57, #64, and #69 merged into main while this branch was in review; none touched docs/design/testing-strategy.md or CONTRIBUTING.md, and both rebases were clean.)

[dichovsky]

Routine review of 6cae444. Roster per .github/ISSUE_TRIAGE.md: general code reviewer (always) + prose-against-code verification (the docs-only substitute for the TypeScript reviewer). No security reviewer — the diff touches no src/store/, src/process.ts, src/which.ts, src/fs-safe.ts, src/setup/, or Launcher file. CI is green (ubuntu / node 24.18.0 and publish dry-run; launch-e2e and ui-e2e correctly skipped as label-gated).

No CRITICAL findings. No HIGH findings.

Scope, hygiene, and acceptance all pass: two files, +28/−11, no code, no unrelated edits. The 5-line rewrap at :126-131 is the unavoidable consequence of deleting "on the self-hosted runner" and is in scope. Single docs: commit explaining why, branched from the then-current main. All three of #33's acceptance items are done. All four must-survive guarantees are intact and near-verbatim at :38-41 (CREW_REQUIRE_TMUX=1 hard failure, "never a silent skip", local graceful skip, "recording-adapter tests remain mandatory everywhere"). Roughly 20 of the ~23 atomic claims in the new prose were independently re-derived against the workflows, vitest.config.ts, package.json, the Playwright specs, and src/templates.ts and confirmed correct — including that all seven workflow jobs are ubuntu-latest at 24.18.0, and that publish.yml is a fourth tmux-installing workflow (:64-65, CREW_REQUIRE_TMUX: '1' at :72) which the unenumerated wording correctly covers.

MEDIUM

1. docs/design/testing-strategy.md:246-247 — the rewritten bullet claims a verification that does not exist. It sits under "Install the tarball into a clean temporary prefix and verify:", but nothing in tests/integration/package/pack-smoke.test.ts asserts template seeding: the only init assertion (:132-134) checks the {type:'init',schema_version:1} envelope, and the following join --role worker proves nothing either, because resolveRole (src/roles.ts:143-150) falls back to PACKAGED_ROLES when no .crew/roles/worker.md exists. The compiled-in-constant half is correct and well-sourced (src/templates.ts:1-9; the only surviving import.meta.url uses are src/version.ts:16 and src/ui/server.ts:143, neither template-related) — but the fix swapped one unverified claim for another in the very bullet #33 asked to correct.
2. docs/design/testing-strategy.md:129 — "and at release" is not implemented. stress.yml:10-19 has only schedule, workflow_dispatch, and pull_request: types:[labeled] — no release: event — and the actual release path publish.yml:70-73 runs npm run test:coverage with CREW_STRESS_ITERS: '25', i.e. the fast tier, not 500. The clause survived the rewrite because only "on the self-hosted runner" was removed, but this PR is the change that touches the sentence. (The nightly 500 and Node 24.18.0 halves are correct: stress.yml:12, :45, :33.)
3. CONTRIBUTING.md:59-60 — the local instruction fails on a fresh clone. "locally you run it on demand with npm run e2e:ui" omits the browser install: npm ci installs @playwright/test but no binaries, and there is no postinstall, which is exactly why ui-e2e.yml:33-34 runs npx playwright install --with-deps chromium. A contributor following the bullet hits Playwright's "Executable doesn't exist". Every other clause in that bullet checks out (build chain via package.json:47-48, crew ui --json --no-open at e2e/ui/dashboard.spec.ts:36, the ui-e2e label at ui-e2e.yml:23, and the specs being *.spec.ts outside the vitest project globs).
4. docs/design/testing-strategy.md:32-33 — a rank-4 document asserting a release gate. "A flaky failure there is a release failure, not a candidate for a blind retry" mirrors ui-e2e.yml:3-9 and the established stress.yml/launch-e2e.yml/CONTRIBUTING.md phrasing, so it invents no new practice — but docs/design/product-spec.md:107 reserves the canonical release-gate list to itself ("Other documents reference it and must not keep a separate copy") and that table carries no Console-browser-e2e entry. Either phrase it as a reference to the existing full-tier convention, or add the gate upstream — the former is in scope here, the latter is not.

LOW

5. :37 — "GitHub-hosted GitHub Actions runners" repeats "GitHub" twice in four words; CLAUDE.md's "GitHub-hosted runners" is the phrasing #33 cites as the reference.
6. :38-39 — the install is quoted as apt-get install -y tmux, while the workflows run sudo apt-get update && sudo apt-get install -y tmux; a reader copying it onto a cold runner hits a stale package index.
7. :129 — the rewrap leaves a doubled "at" ("nightly and at release at Node 24.18.0"), which finding 2's fix should resolve anyway.
8. :38-40 — "run the test suite with CREW_REQUIRE_TMUX=1" is precise for ci.yml:40 and publish.yml:70 but not for stress.yml:42 (npx vitest run tests/spawn) or launch-e2e.yml:43 (a single file).
9. :29 — "the 95% coverage gate, which measures src/** and bin/** only" is true per vitest.config.ts:12 and matches NFR-MNT-01 (srs.md:1179-1181), but "only" reads as exhaustive when :13 carries an exclude: ['src/io.ts'] carve-out.
10. :18 — "real server + Chromium" is accurate in effect but Chromium is Playwright's implicit default: e2e/ui/playwright.config.ts:8-16 declares no projects and no browser selection. The row is also understated — the specs additionally cover XSS inertness (actions.spec.ts:212-256) and deleted-workspace recovery (recovery.spec.ts:82).

Pre-existing, filed separately rather than fixed here

Out of scope for #33's acceptance, so deliberately untouched: the :245 bullet "init + join + send + receive + reviewed Task flow" (pack-smoke runs init, join, agents, leave — no send, no receive, no Task flow); the :248 macOS/Linux shebang bullet, which the now-explicit single-OS environment matrix visibly contradicts since no workflow runs macOS; and CONTRIBUTING.md:36's "typecheck → lint → format:check → build → test" against ci.yml's actual test:coverage plus a separate build:docs step.

Verdict: in scope, cleanly committed, and factually correct on every central claim, with four MEDIUM items worth one small pass — three of them in lines this PR wrote. The implementer is taking its single fix pass on findings 1–5 and 7 now; findings 8–10 and the pre-existing items are recorded here rather than fixed, to keep the diff traceable to #33.

---

Generated by Claude Code

[dichovsky]

Routine review — general code reviewer plus, per .github/ISSUE_TRIAGE.md's docs-only roster, a prose-versus-code verification pass in place of the TypeScript reviewer. No security reviewer: the diff touches no src/store/, src/process.ts, src/which.ts, src/fs-safe.ts, src/setup/, or Launcher path.

No CRITICAL and no HIGH findings. CI is green on 6cae444 (ubuntu / node 24.18.0 and publish dry-run both success; ui-e2e and launch-e2e correctly skipped — neither label is present).

What was verified independently

Every factual assertion the new prose makes was re-checked against the code rather than against the PR body:

• All six workflow jobs are runs-on: ubuntu-latest at node-version: '24.18.0' — ci.yml:15/:20 and :53/:58, stress.yml:26/:33, launch-e2e.yml:27/:34, ui-e2e.yml:24/:30, pages.yml:32/:40, publish.yml:34/:42. No self-hosted label, no matrix, no divergent Node pin. The replaced sentence was genuinely false and the new one is universally true.
• The tmux conjunction does not over-claim. Exactly four workflows install tmux, and all four also set CREW_REQUIRE_TMUX: '1' — ci.yml:25-26/:42, publish.yml:64-65/:72, stress.yml:38-39/:44, launch-e2e.yml:39-40/:49. pages.yml and ui-e2e.yml install none and need none.
• Both new table rows match reality: vitest.config.ts:45-52 and :58-65 for the jsdom web/docs projects; e2e/ui/*.spec.ts build via npm run build in beforeAll, spawn dist/bin/crew.js ui --json --no-open, and src/ui/server.ts:141-150 serves dist/ui-assets — so "built dashboard bundle served by crew ui" and "real server + Chromium" are both accurate.
• Coverage-gate scoping and the ui-e2e trigger set (ui-e2e.yml:10-15 cron/dispatch/PR-labeled, :23 if:) check out exactly.
• Completeness of the fix. A repo-wide case-insensitive grep finds exactly the three self-hosted occurrences this PR removes and no others; the only prose import.meta.url template-resolution claim was :234, now replaced (remaining hits are live code at src/ui/server.ts:143 and src/version.ts:16, correctly untouched). CLAUDE.md:71, AGENTS.md:71, and CONTRIBUTING.md:36 already said GitHub-hosted, so nothing else is left inconsistent.
• Scope. git diff --word-diff on the stress paragraph confirms it is word-for-word main apart from deleting "on the self-hosted runner"; the re-wrap is a necessary consequence, not gratuitous. No adjacent-prose improvements, no reformatting of untouched content.
• Vocabulary and authority. No CONTEXT.md misuse (Console, Operator, Workspace, Launcher, Relay all in their defined senses); no conflict with any accepted ADR, the SRS, the CLI contract, or the data model.
• Docs-only exemption correctly claimed and valid — zero src/**/bin/** files touched, so coverage cannot have moved.

Findings

MEDIUM — docs/design/testing-strategy.md:243-249: the rewritten bullet asserts a verification the pack-smoke does not perform. The list is introduced by "Install the tarball into a clean temporary prefix and verify:", so each bullet is a requirement on the packaging gate. tests/integration/package/pack-smoke.test.ts:129-134 does run init --json through the installed tarball binary, but asserts only toMatchObject({ type: 'init', schema_version: 1 }) — nothing checks that a Role or Team file was seeded. A regression emitting empty template bodies would still pass. Not a regression introduced here (the import.meta.url bullet it replaces was equally unasserted), but the PR edits this exact bullet. Cheapest fix: init --json already emits seeded (src/init.ts:163), so the test could assert seeded: expect.arrayContaining([...]). That is a tests/ change, outside a docs-only diff — filed separately as #77, which also covers two neighbouring bullets in the same list that are false in the same way (:245 claims "send + receive + reviewed Task flow" and :248 claims the engine-floor check, neither of which the pack-smoke performs — its cases are init, join, agents, leave).

MEDIUM — docs/design/testing-strategy.md:32 versus the "Quality gates" list at :256-266. The new paragraph asserts a release-level consequence for the Console browser e2e ("A flaky failure there is a release failure, not a candidate for a blind retry"), but the section that enumerates release gates — and which already carries "The real tmux e2e passes before release" — gains no corresponding entry. The PR body defends the omission ("no browser-tier release gate was added, since the repo does not enforce one"), which is precisely the tension: the prose claims gate semantics the list withholds. Either add - The Console browser e2e passes before release. to Quality gates, or attribute rather than assert — "ui-e2e.yml treats a flaky failure there as a release failure".

MEDIUM — docs/design/testing-strategy.md:22-24: the "e2e" definition is now under-inclusive. "'e2e' means end-to-end: the whole path is exercised, from the built executable to a real tmux session" sits directly below a table that now has a second e2e row ending at Chromium. Suggest "…to a real tmux session or a real browser."

LOW — :38-39. "run the test suite with CREW_REQUIRE_TMUX=1" over-states two of the four: stress.yml runs npx vitest run tests/spawn, launch-e2e.yml a single spec file. Suggest "run their vitest tiers with".

LOW — :28-29. "the 95% coverage gate, which measures src/** and bin/** only" — vitest.config.ts:13 also excludes src/io.ts, so "only" slightly overstates. Drop the word.

LOW — :18. The Console browser e2e Purpose omits recovery.spec.ts's guarantee (the Console degrades to a bounded recovery state when .crew/state vanishes, keeps last-known data, disables actions, creates no DB, and resumes against a fresh Store) and actions.spec.ts:123-132's hostile-content-stays-inert assertion.

LOW — :37. "GitHub-hosted GitHub Actions runners" repeats "GitHub".

LOW — :129. Removing "on the self-hosted runner" left a doubled preposition: "nightly and at release at Node 24.18.0". Separately, publish.yml:73 runs CREW_STRESS_ITERS: '25', so "at release" really means a manual workflow_dispatch of stress.yml during release prep.

LOW — :14. The interface cell ends "…modules in jsdom" while the next column already says "in-process, jsdom".

LOW — PR body, section 3. Cites ci.yml:39 for the test:coverage run; the actual line is :40. Body only, not merged prose.

Not filed, already tracked

The reviewers flagged that docs/design/architecture.md:166-169 still shows a top-level templates/ directory (which does not exist) and tests/{unit,integration,spawn,e2e,fixtures} (there is no tests/e2e; store/, helpers/, tools/ are missing, and Playwright lives at top-level e2e/ui/). Both lines are already enumerated in #35, so nothing new was filed and this PR correctly leaves them alone.

Outcome: nothing CRITICAL or HIGH survives, so this PR is being marked ready for review. The MEDIUM and LOW items above are non-blocking; #77 tracks the one that needs a tests/ change.

---

Generated by Claude Code

[dichovsky]

Correction to the review above: the follow-up issue for the package-verification bullets was filed as #80, not #77 — concurrent runs took the intervening numbers between drafting and filing. Both references to "#77" in that review should read #80. Findings and outcome are unchanged.

---

Generated by Claude Code

[dichovsky]

Process note for whoever reviews this — two things about the branch history that the record should carry.

1. The reviewed SHA no longer exists on this branch. The review above was posted against 6cae444. During the fix pass the branch was rebased onto the moving main (which absorbed #57, #64, and #69 while this PR was open) and pushed with --force-with-lease, so the history is now 24e2335 (the original commit, re-parented) + 316a6f6 (the fix pass). 6cae444 is unreachable. The content of the reviewed commit is unchanged — only its parent — but a reviewer clicking the SHA in the review will get nothing, hence this note.

2. That force-push is outside this routine's own Action limits. .github/ISSUE_TRIAGE.md lists "Force-push, rewrite published history" under Never do, and the rule exists precisely so a review stays anchored to what it reviewed. The cause was contention inside the routine, not a judgment that the rule should be bent: the implementer's rebase and a concurrent correction to the same working copy produced divergent local history, and the branch was re-pushed over it. Flagging it rather than leaving it in the reflog for someone to find later.

Nothing about the diff is in question — the final tree is the reviewed content plus the five fix-pass edits, and CI is green on 316a6f6 (ubuntu / node 24.18.0 and publish dry-run both pass; the two label-gated e2e workflows correctly skip). An earlier publish dry-run failure reported on the superseded SHA 6a2263c was a GitHub Actions infrastructure outage — "Failed to resolve action download info: Service Unavailable", then "Bad Gateway", before any repo step ran — and not a defect in this change.

The four MEDIUM findings from the review are addressed in 316a6f6. The LOW items and the pre-existing inaccuracies are left as filed: #77 (the package-verification list asserting checks pack-smoke does not perform, plus the macOS claim against the single-OS matrix) and #78 (CONTRIBUTING.md's CI step list omitting test:coverage and build:docs).

---

Generated by Claude Code