From a3749bfa4b0fcb33aebf875134d9f21252f23c46 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 27 Aug 2026 08:46:41 -0700 Subject: [PATCH] v1.70.1.0 fix: ship names the /document-release subagent at every decision point (tripwire + gate E2E) (#2700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ship): name the /document-release subagent at every Step 18 decision point The v1.54.0.0 carve moved Step 18 (documentation sync) into ship/sections/pr-body.md and the Claude-host skeleton stopped saying "document-release" anywhere in the workflow body — the dispatch became invisible at exactly the moments an agent decides whether to open the section. Restore visibility at three touchpoints, all subagent-framed (never bare-slash-framed, which would invite an inline Skill invocation that bypasses the fresh-context subagent + JSON contract): - manifest trigger (renders into the section-index row AND the STOP pointer): "dispatching the /document-release subagent to sync docs (Step 18) and then creating or updating the PR/MR (Step 19)" - Step 17 handoff line names Step 18's dispatch explicitly - new hoisted doc-sync invariant beside the PR-title invariant: the dispatch itself is never skipped; only a failed subagent is non-blocking Pin it in carve-guards: 'the /document-release subagent' (all three touchpoints) + 'dispatches the /document-release subagent' (invariant) must stay in the skeleton; the carved imperative 'Dispatch /document-release as a subagent' must stay carved. Skeleton cap 91,600 → 92,300 (measured 91,764; trigger renders twice). Goldens regenerated for all three hosts. Co-Authored-By: Claude Fable 5 * test: pin the ship→document-release Step 18 wiring with a free tripwire Five substring/structure asserts across the carved section, the Claude skeleton's three touchpoints, the manifest trigger, and the codex/factory goldens (inlined Step 18 ordered before Step 19). Claude-golden asserts deliberately omitted: host-config.test.ts already enforces golden == generated byte-for-byte. Co-Authored-By: Claude Fable 5 * test: gate-tier E2E proving /ship dispatches the document-release subagent New skill-e2e-ship-docsync: a live agent gets the sliced Step 17→19 tail of the generated ship skeleton in a bare-remote git fixture (Steps 0-16 "done"), under a fake HOME so the STOP pointer and the Step 18 subagent prompt resolve to planted copies, with a stub document-release skill that returns the empty-result JSON contract. Hard assert: an Agent/Task tool-call matching /document-release/i exists in result.toolCalls and precedes any `gh pr create`. Neutral prompt (no STOP-Read priming, no document-release mention — the prompt echoes into the transcript, so asserts read toolCalls only). Hardening from review: throw-on-marker-drift fixture slice; per-test GSTACK_HOME + .redact-prepush-prompted marker (routes Step 17's credential guard to its silent branch — the hermetic GSTACK_HOME pin defeats a HOME-only override); 480s/540s timeouts (nested subagent adds wall clock the 300s sibling never carried); 'timeout' accepted in exitReason only because the dispatch assert is independently hard; whole-file describeE2ETier('gate') composed with diff selection (keeps the file out of the periodic shard census, which sits at its ceiling, and under the hard tier-alignment invariant). Registered as 'ship-docsync' in E2E_TOUCHFILES + E2E_TIERS (gate) in the same commit — touchfiles.test.ts rejects either half landing first. Co-Authored-By: Claude Fable 5 * docs: fix stale document-release TODOS entry + three review-deferred items The SHIPPED entry still described the deleted Step 8.5 post-PR cat-delegation design from v0.8.4; replace with the current Step 18 subagent design and its test pins. Add the three P3 items deferred from the v1.69 plan review: dispatch receipt enforcement, land-and-deploy→canary dispatch-pin pattern, and the periodic shard-census boundary. Co-Authored-By: Claude Fable 5 * fix: pre-landing review fixes Testing-specialist findings, all mechanical: (1) pin the E2E fixture's git branch (-b main / init.defaultBranch=main) and assert every setup command's exit status so operator git config can't silently corrupt a paid run; (2) tighten the dispatch matcher to Step 18-prompt-specific markers (document-release/SKILL.md | executing the /document-release workflow) so a subagent merely quoting section text can't false-pass the regression assert (verified against recorded burn-in transcripts); (3) replace the subsumed carve-guards anchor with three non-overlapping per-touchpoint anchors (gerund/imperative/3rd-person) so each touchpoint is independently enforced. Co-Authored-By: Claude Fable 5 * fix: red-team review fixes Five informational findings: TODOS shard-census arithmetic corrected (census is 67 with one free ungated slot; the SECOND ungated file trips the floor) and version pointer fixed (v0.18.2.0, not v0.18.1.0); the free tripwire now pins the two dispatch-matcher marker strings so a pr-body prompt reword fails the free suite instead of surfacing as a paid-tier mystery; the E2E matcher gains a section-paste exclusion (scaffold strings disqualify) — verified against all recorded runs; the E2E header documents the tierless test:evals invisibility tradeoff. Co-Authored-By: Claude Fable 5 * fix: adversarial review fixes Pin the E2E matcher's two EXCLUSION markers in the free tripwire (an unpinned 'Parent processing:' reword would silently deaden the section-paste guard while every test stayed green); add an ordering pin (the hoisted doc-sync invariant must sit above the pr-body STOP pointer — presence-only anchors can't catch drift below it); plant a third cwd-relative pr-body copy inside the fixture repo, gitignored so the agent never tries to commit test scaffolding. Co-Authored-By: Claude Fable 5 * chore: bump version and changelog (v1.70.1.0) Co-Authored-By: Claude Fable 5 * docs: CHANGELOG accuracy fixes from the doc-release review Three factual corrections the Step 18 doc subagent caught in the fresh v1.70.1.0 entry: 5 tripwire tests (not 6), cost floor $0.63 per the cited eval store (not $0.59), and the visibility claim scoped to decision points (the re-run checklist mention survived the carve). Plus the E2E header's stale pending-burn-in note replaced with the observed numbers. Co-Authored-By: Claude Fable 5 * fix: raise bun-polyfill subprocess budget to 60s for degraded Windows runners The 50ms-sleep test blew the 20s budget on BOTH bun retry attempts on PR #2700's windows-latest runner (run 32989821401) — sustained AV/runner pressure, not just the documented cold-start. Same flake passed-on-rerun on the prompt-token-load-reduction branch yesterday. Budget only; every assertion still checks exact output. Co-Authored-By: Claude Fable 5 * fix(ci): run the ship-docsync gate E2E in the evals matrix + silent-skip tripwire The evals.yml matrix is hand-enumerated and the Run step never exported EVALS_TIER, so the new whole-file-gated ship-docsync E2E would have self-skipped even with a row — a hollow green one layer deeper than the documented rehomed-monolith incident. Add the e2e-ship-docsync row with a row-level `tier: gate` property, exported as EVALS_TIER by the Run step (empty = unset for every existing row: all readers are `=== ''` or truthiness). New free tripwire test/evals-workflow-matrix.test.ts ratchets the class: matrix files must exist; gate-hosting files must have a row; whole-file-gated matrix files must carry a matching row tier; and the burn-down lists enforce their own cleanup. It enumerates the PRE-EXISTING holes found while wiring this (8 gate-hosting files with no row; codex/gemini rows running zero tests; the pty-plan-smoke row hollow since its files adopted describeE2ETier) — tracked in TODOS as the CI gate-lane hollow-coverage burn-down. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- .github/workflows/evals.yml | 17 ++ CHANGELOG.md | 41 +++ TODOS.md | 100 ++++++- VERSION | 2 +- browse/test/bun-polyfill.test.ts | 9 +- package.json | 2 +- ship/SKILL.md | 8 +- ship/SKILL.md.tmpl | 4 +- ship/sections/manifest.json | 2 +- test/evals-workflow-matrix.test.ts | 196 ++++++++++++++ test/fixtures/golden/claude-ship-SKILL.md | 8 +- test/fixtures/golden/codex-ship-SKILL.md | 4 +- test/fixtures/golden/factory-ship-SKILL.md | 4 +- test/helpers/carve-guards.ts | 32 ++- test/helpers/touchfiles-data.ts | 2 + test/ship-document-release-dispatch.test.ts | 135 ++++++++++ test/skill-e2e-ship-docsync.test.ts | 283 ++++++++++++++++++++ 17 files changed, 830 insertions(+), 19 deletions(-) create mode 100644 test/evals-workflow-matrix.test.ts create mode 100644 test/ship-document-release-dispatch.test.ts create mode 100644 test/skill-e2e-ship-docsync.test.ts diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index 2a5916dbe..cfe7cbc0b 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -135,6 +135,17 @@ jobs: file: test/skill-e2e-coverage-audit.test.ts - name: e2e-triage file: test/skill-e2e-triage.test.ts + # ship-docsync is whole-file tier-gated (describeE2ETier('gate') keeps + # it out of the periodic shard census), so its row MUST set tier: gate + # — without it the self-gate skips every test and the job reports a + # hollow green (the same silent-skip class as the rehomed monolith + # above, one layer deeper). The Run step exports EVALS_TIER from this + # property; rows without it keep EVALS_TIER empty (= unset: every + # reader is `=== ''` or truthiness). Enforced by + # test/evals-workflow-matrix.test.ts. + - name: e2e-ship-docsync + file: test/skill-e2e-ship-docsync.test.ts + tier: gate - name: e2e-routing file: test/skill-routing-e2e.test.ts - name: e2e-codex @@ -337,6 +348,12 @@ jobs: GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} EVALS_CONCURRENCY: "40" PLAYWRIGHT_BROWSERS_PATH: /opt/playwright-browsers + # Per-row tier activation for whole-file-gated suites. Empty when the + # row declares no tier — every EVALS_TIER reader treats empty as unset + # (`=== ''` comparisons and the truthiness check in + # test/helpers/e2e-helpers.ts:70), so untiered rows are byte-for-byte + # unaffected. + EVALS_TIER: ${{ matrix.suite.tier || '' }} run: EVALS=1 bun test --retry ${{ matrix.suite.retries || 1 }} --concurrent --max-concurrency 40 ${{ matrix.suite.file }} - name: Upload eval results diff --git a/CHANGELOG.md b/CHANGELOG.md index 8afc088a5..29ad00d1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,46 @@ # Changelog +## [1.70.1.0] - 2026-08-26 + +**Ship names its documentation subagent at every decision point.** +**The handoff is now pinned by tests that fail loud if it ever goes quiet.** + +`/ship` has dispatched `/document-release` as Step 18 since v0.18.2.0, but the v1.54.0.0 carve moved that step into an on-demand section and the always-loaded skeleton stopped saying "document-release" at any decision point (one mention survived, buried in the re-run checklist). The wiring was intact. The visibility was gone, and nothing tested the handoff. This release restores the visibility and locks it in: the section index, the STOP pointer, the Step 17 handoff line, and a new hoisted doc-sync invariant all name "the /document-release subagent" (subagent-framed on purpose, so an agent dispatches the isolated worker instead of running a weaker inline copy). A free tripwire pins the wording, carve-guard anchors pin each touchpoint independently, and a new gate-tier E2E proves a live agent actually fires the dispatch before creating the PR. + +### The numbers that matter + +Source: this branch's eval store (`~/.gstack/projects//evals/`, runs of `test/skill-e2e-ship-docsync.test.ts`) and `wc -c ship/SKILL.md`. + +| Property | Before | After | +|--------|--------|-------| +| Doc-sync subagent named in the Claude-host ship workflow body | 0 mentions at any decision point | 4 (section index, STOP pointer, Step 17 handoff, hoisted invariant) | +| Tests pinning the ship→document-release handoff | none | 5 free tripwire tests + 3 per-touchpoint carve anchors + 1 gate E2E | +| Live dispatch proof | never measured | 9/9 runs fire the dispatch before PR creation ($0.63-1.04, 234-319s each, sonnet-4-6) | +| Always-loaded skeleton cost | 91,267 B | 91,764 B (+497 B, cap raised to 92,300) | + +Nine out of nine live runs is the line that matters. The E2E asserts on the actual tool-call stream, with a dispatch-specific matcher that a subagent merely quoting section text cannot satisfy, and a timeout-tolerant exit check that never softens the dispatch assert itself. + +### What this means for gstack users + +When you run `/ship`, the docs sync step is no longer an invisible line in a file the agent may summarize past. It is named at the exact moments the agent decides what to do next, and a merge-blocking test fails if any future edit makes it invisible again. A failed docs subagent still never blocks your ship. Nothing to configure. Upgrade and ship. + +### Itemized changes + +#### Fixed + +- `/ship`'s Claude-host skeleton names "the /document-release subagent" at all three Step 18 decision points (manifest trigger rendering into the section index and STOP pointer, the Step 17 handoff line, and a hoisted doc-sync invariant beside the PR-title invariant). The invariant states the contract plainly: the dispatch itself is never skipped; only a failed subagent is non-blocking. + +#### Added + +- `test/ship-document-release-dispatch.test.ts`: free tripwire pinning the carved Step 18 contract (imperative, `subagent_type`, JSON return keys, non-blocking clause), the three skeleton touchpoints, the invariant-above-STOP ordering, the E2E matcher's four marker strings, and the inlined Step 18 → Step 19 ordering in the codex/factory goldens. +- `test/skill-e2e-ship-docsync.test.ts` (`ship-docsync`, gate tier): a live agent runs the sliced Step 17→19 ship tail in a hermetic git fixture; hard assert that an Agent dispatch matching the Step 18 prompt markers appears in the tool-call stream before any `gh pr create`. Fixture fails loud on step-marker drift, pins its git branch against operator config, asserts every setup command, and neutralizes the credential pre-push guard's question branch. + +#### For contributors + +- Carve-guards ship entry: three non-overlapping per-touchpoint anchors (gerund, imperative, third-person: no anchor subsumes another, so each is independently enforced), the carved imperative pinned to stay carved, skeleton byte cap 91,600 → 92,300 with the measured value recorded. +- `ship-docsync` registered in `E2E_TOUCHFILES` and `E2E_TIERS` (gate), with a whole-file `describeE2ETier('gate')` self-gate composed with diff selection so the file stays out of the periodic shard census; the tierless `test:evals` invisibility tradeoff is documented in the file header. +- Internal backlog notes corrected to describe the current Step 18 design (the pre-v1.54 "Step 8.5" prose was still documented as current), plus three deferred follow-ups recorded: a machine-checkable dispatch receipt, the same dispatch-pin treatment for land-and-deploy→canary, and the periodic shard-census ceiling arithmetic. + ## [1.69.0.0] - 2026-08-22 **The silent-failure wave: tools that reported success while doing nothing —** diff --git a/TODOS.md b/TODOS.md index 8a0488d56..d8f0d0936 100644 --- a/TODOS.md +++ b/TODOS.md @@ -2329,7 +2329,105 @@ Shipped as v0.5.0 on main. Includes `/plan-design-review` (report-only design au ### Auto-invoke /document-release from /ship — SHIPPED -Shipped in v0.8.3. Step 8.5 added to `/ship` — after creating the PR, `/ship` automatically reads `document-release/SKILL.md` and executes the doc update workflow. Zero-friction doc updates. +Shipped in v0.8.4; redesigned twice since. Current design (v0.18.2.0+, carved in +v1.54.0.0): `/ship` Step 18 (`ship/sections/pr-body.md`) dispatches +`/document-release` as a general-purpose subagent AFTER Step 17 (push) and +BEFORE Step 19 (PR creation); the subagent's JSON contract (`files_updated`, +`commit_sha`, `pushed`, `documentation_section`) is baked into the initial PR +body. Subagent failure is non-blocking. The skeleton names "the +/document-release subagent" at three touchpoints (section-index trigger + STOP +pointer, Step 17 handoff, hoisted doc-sync invariant). Pinned by +`test/ship-document-release-dispatch.test.ts` + carve-guards anchors; behavior +proven by the `ship-docsync` gate E2E (`test/skill-e2e-ship-docsync.test.ts`). + +### Machine-checkable Step 18 dispatch receipt in /ship's Section self-check + +**What:** Make ship's "Section self-check" verify a document-release dispatch +actually occurred (a machine-checkable marker/receipt), instead of relying on +prompt-level invariants alone. + +**Why:** Prompt wording deters skipping but can't prove the dispatch happened. +Two residual gaps from the v1.69 review are folded into this scope: (1) an +agent invoking `/document-release` inline via the Skill tool bypasses the +fresh-context subagent + JSON contract and no test can see it; (2) the ship +RE-RUN path names document-release in the re-run list but no test asserts +doc-sync on re-run. + +**Context:** The `ship-docsync` E2E asserts the dispatch tool-call on the +primary path; this TODO is the enforcement layer beyond wording. Start from +ship's Section self-check (ship/SKILL.md.tmpl) and the Step 18 parent +processing in ship/sections/pr-body.md.tmpl. + +**Effort:** M (human) → S (CC+gstack) +**Priority:** P3 +**Depends on:** ship-docsync E2E landed + +### Apply the dispatch-pin + E2E pattern to /land-and-deploy → /canary + +**What:** Same treatment ship→document-release got: name the handoff at the +skeleton decision points, pin with carve-guards anchors + a free tripwire, +prove with a toolCalls-assert E2E. + +**Why:** Identical failure class — a carve or reword can silently strand the +canary handoff out of the always-loaded skeleton, and nothing tests it today. + +**Context:** Model files: `test/ship-document-release-dispatch.test.ts` (free +pin) and `test/skill-e2e-ship-docsync.test.ts` (dispatch E2E, gate tier). + +**Effort:** M (human) → S (CC+gstack) +**Priority:** P3 +**Depends on:** None + +### CI gate-lane hollow-coverage burn-down (evals.yml matrix) + +**What:** `test/evals-workflow-matrix.test.ts` (added v1.70.1.0) ratchets two +pre-existing CI coverage holes; burn them down. (1) Eight gate-hosting test +files have no `evals.yml` matrix row, so CI never runs them +(`KNOWN_MATRIX_GAPS` in the test enumerates them — notably the plan-mode and +finding-floor smokes and the AUQ format-compliance gate). (2) Four matrix rows +point at whole-file tier-gated files but set no row `tier:` property, so with +`EVALS_TIER` unexported those suites self-skip: `codex-e2e`/`gemini-e2e` run +ZERO tests and report green on every PR (vestigial rows; the periodic cron +lane owns them — consider deleting the rows), and `e2e-pty-plan-smoke` spends +~7 min on setup then skips every describe (hollow-green since the files +adopted `describeE2ETier('gate')` — set `tier: gate` on the row to reactivate, +after confirming the smokes still pass). + +**Why:** "Gate tier blocks merge" is silently false for these files. Each fix +is a deliberate cost/flake decision (activating paid suites on every PR), so +they're enumerated instead of drive-by-fixed. The mechanism already exists: +per-row `tier:` property, exported as `EVALS_TIER` by the Run step. + +**Context:** Found 2026-08-26 on PR #2700 while adding the `ship-docsync` row. +Fix = add/adjust the matrix row, then DELETE the corresponding burn-down entry +(the tripwire fails on stale entries, so cleanup is enforced). + +**Effort:** S per file (mechanical) + one burn-in run each to confirm green +**Priority:** P2 +**Depends on:** None + +### Periodic paid-test shard census is one ungated file from the detach-timeout floor + +**What:** The periodic tier's shard census is 67 files — one ungated slot below +the 68-file (17×4) ceiling. The next paid `skill-e2e-*` file WITHOUT a +whole-file `describeE2ETier` self-gate lands at 68 (still 17 waves, floor +32,130s ≤ 32,400s — passes); the SECOND ungated file trips 18 waves → 34,020s +floor > the 32,400s configured detach timeout, and +`test/eval-detach-timeout-floor.test.ts` fails with a confusing message. + +**Why:** Whoever adds the second ungated periodic E2E gets a floor failure +unrelated to their change. Fix options: raise the periodic detach timeout, or +enforce whole-file tier self-gates on all paid files (upgrades them from the +tier-alignment warn-only bucket to the hard invariant, and — bonus — restores +tierless `bun run test:evals` coverage decisions to diff selection alone). + +**Context:** `scripts/test-paid-shards.ts` `classifyPaidTestFile` counts +ungated files in both tiers; `ship-docsync` composed `describeE2ETier('gate')` +with diff selection specifically to avoid consuming the last free slot. + +**Effort:** S +**Priority:** P3 +**Depends on:** None ### `{{DOC_VOICE}}` shared resolver diff --git a/VERSION b/VERSION index 478f55a2a..d0207acbe 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.69.0.0 +1.70.1.0 diff --git a/browse/test/bun-polyfill.test.ts b/browse/test/bun-polyfill.test.ts index 960c934b1..21ab985cb 100644 --- a/browse/test/bun-polyfill.test.ts +++ b/browse/test/bun-polyfill.test.ts @@ -3,8 +3,13 @@ import * as path from 'path'; // Every test here spawnSync's a `node` child; Windows CI cold-start (AV scan, // first-touch of node.exe) alone can blow bun's 5s default — observed 5,007ms -// on a 50ms sleep test. Subprocess budget, not assertion looseness. -setDefaultTimeout(20_000); +// on a 50ms sleep test. 20s was still not enough: on 2026-08-26 (PR #2700, +// run 32989821401) the 50ms sleep test blew 20s on BOTH bun retry attempts on +// a degraded windows-latest runner, so cold-start alone doesn't explain it — +// sustained AV/runner pressure does. Subprocess budget, not assertion +// looseness: every assertion still checks exact output, only the slowness +// allowance grows. +setDefaultTimeout(60_000); // Load the polyfill into a fresh object (don't clobber globalThis.Bun) const polyfillPath = path.resolve(import.meta.dir, '../src/bun-polyfill.cjs'); diff --git a/package.json b/package.json index 36ce5a63b..70505e5e4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.69.0", + "version": "1.70.1", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/ship/SKILL.md b/ship/SKILL.md index 6c61f37ed..584d2bc53 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -937,7 +937,7 @@ sections. Read a section in full before doing its step; do not work from memory. | addressing Greptile review comments when a PR exists (Step 10) | `sections/greptile.md` | | the adversarial review and learnings capture (Step 11) | `sections/adversarial.md` | | writing the CHANGELOG entry (Step 13) | `sections/changelog.md` | -| syncing docs and creating or updating the PR/MR (Steps 18-19) | `sections/pr-body.md` | +| dispatching the /document-release subagent to sync docs (Step 18) and then creating or updating the PR/MR (Step 19) | `sections/pr-body.md` | --- @@ -1417,13 +1417,15 @@ If `ALREADY_PUSHED`, skip the push but continue to Step 18. Otherwise push with git push -u origin ``` -**You are NOT done.** The code is pushed but documentation sync and PR creation are mandatory final steps. Continue to Step 18. +**You are NOT done.** The code is pushed but Step 18 (dispatch the /document-release subagent to sync docs) and Step 19 (create the PR/MR) are mandatory final steps. Continue to Step 18. --- **PR/MR title invariant (always applies — do not skip even if you don't open the section below):** Any PR or MR you create OR update in the next step MUST have a title that starts with `v$NEW_VERSION` (the version bumped in Step 12), in the format `v : `. Never create or edit a PR/MR title without this prefix. Compute the correct title with the single source of truth helper: `~/.claude/skills/gstack/bin/gstack-pr-title-rewrite.sh "$NEW_VERSION" ""`. The full create/update procedure (idempotency, redaction scan, self-check) is in the section below. -> **STOP.** Before syncing docs and creating or updating the PR/MR (Steps 18-19), Read `~/.claude/skills/gstack/ship/sections/pr-body.md` and execute it +**Doc-sync invariant (always applies — do not skip even if you don't open the section below):** Step 18 dispatches the /document-release subagent BEFORE the PR/MR is created or updated in Step 19. Never skip the dispatch itself; only a failed subagent is non-blocking (proceed to Step 19 without a `## Documentation` section). + +> **STOP.** Before dispatching the /document-release subagent to sync docs (Step 18) and then creating or updating the PR/MR (Step 19), Read `~/.claude/skills/gstack/ship/sections/pr-body.md` and execute it > in full. Do not work from memory — that section is the source of truth for this step. ## Step 20: Persist ship metrics diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index 19c2a0876..ac41e2c5a 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -496,12 +496,14 @@ If `ALREADY_PUSHED`, skip the push but continue to Step 18. Otherwise push with git push -u origin ``` -**You are NOT done.** The code is pushed but documentation sync and PR creation are mandatory final steps. Continue to Step 18. +**You are NOT done.** The code is pushed but Step 18 (dispatch the /document-release subagent to sync docs) and Step 19 (create the PR/MR) are mandatory final steps. Continue to Step 18. --- **PR/MR title invariant (always applies — do not skip even if you don't open the section below):** Any PR or MR you create OR update in the next step MUST have a title that starts with `v$NEW_VERSION` (the version bumped in Step 12), in the format `v : `. Never create or edit a PR/MR title without this prefix. Compute the correct title with the single source of truth helper: `~/.claude/skills/gstack/bin/gstack-pr-title-rewrite.sh "$NEW_VERSION" ""`. The full create/update procedure (idempotency, redaction scan, self-check) is in the section below. +**Doc-sync invariant (always applies — do not skip even if you don't open the section below):** Step 18 dispatches the /document-release subagent BEFORE the PR/MR is created or updated in Step 19. Never skip the dispatch itself; only a failed subagent is non-blocking (proceed to Step 19 without a `## Documentation` section). + {{SECTION:pr-body}} ## Step 20: Persist ship metrics diff --git a/ship/sections/manifest.json b/ship/sections/manifest.json index 4ffc25023..e4394e562 100644 --- a/ship/sections/manifest.json +++ b/ship/sections/manifest.json @@ -56,7 +56,7 @@ "id": "pr-body", "file": "pr-body.md", "title": "Documentation sync + PR/MR creation", - "trigger": "syncing docs and creating or updating the PR/MR (Steps 18-19)" + "trigger": "dispatching the /document-release subagent to sync docs (Step 18) and then creating or updating the PR/MR (Step 19)" } ] } diff --git a/test/evals-workflow-matrix.test.ts b/test/evals-workflow-matrix.test.ts new file mode 100644 index 000000000..5ae3f8958 --- /dev/null +++ b/test/evals-workflow-matrix.test.ts @@ -0,0 +1,196 @@ +/** + * CI eval-matrix completeness tripwire — kills the silent-skip class where a + * gate-tier test exists in the repo but the hand-enumerated matrix in + * .github/workflows/evals.yml never runs it, so "gate tier blocks merge" is + * quietly false in CI. This has happened before (see the "rehomed from the + * deleted pre-split monolith" comment in evals.yml) and was found again on + * PR #2700: nine gate-hosting files absent from the matrix, plus matrix rows + * whose whole-file tier guards can never fire because the Run step exported + * no EVALS_TIER. + * + * Ratchet, not amnesty: the KNOWN_* lists below enumerate the PRE-EXISTING + * gaps with reasons, so no NEW gap can land while the backlog burns down + * (same pattern as SCANNER_EXEMPT in egress-receipt-wiring). If you fix a + * listed gap (add its matrix row / tier property), this test FAILS until you + * remove the entry — stale exemptions are enforced, not decorative. + * + * Wiring pinned: + * - every matrix `file:` path exists on disk (no stale rows), + * - every gate-hosting paid file (whole-file gate self-gate, or named in the + * dep list of a gate-tier E2E_TOUCHFILES key) appears in the matrix or in + * KNOWN_MATRIX_GAPS, + * - every matrix file with a whole-file tier guard has a matching row-level + * `tier:` property (else the suite self-skips and the job is hollow-green) + * or sits in KNOWN_TIER_UNSET. + */ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; +import { E2E_TOUCHFILES, E2E_TIERS } from './helpers/touchfiles-data'; +import { isPaidTestFile } from './helpers/paid-test-set'; + +const ROOT = path.join(import.meta.dir, '..'); +const WORKFLOW = path.join(ROOT, '.github', 'workflows', 'evals.yml'); + +/** + * Pre-existing gate-hosting files with no matrix row (found 2026-08-26, + * PR #2700). Adding a row activates real paid runs on every PR — a cost and + * flake-surface decision per file, tracked in TODOS.md ("CI gate-lane + * hollow-coverage burn-down"). Fix = add a matrix row (plus `tier: gate` when + * the file is whole-file gated), then DELETE the entry here. + */ +const KNOWN_MATRIX_GAPS = new Set([ + 'test/skill-e2e-ask-user-question-format-compliance.test.ts', + 'test/skill-e2e-hermetic-canary.test.ts', + 'test/skill-e2e-ios.test.ts', + 'test/skill-e2e-plan-ceo-finding-floor.test.ts', + 'test/skill-e2e-plan-ceo-plan-mode.test.ts', + 'test/skill-e2e-plan-design-with-ui.test.ts', + 'test/skill-e2e-plan-devex-finding-floor.test.ts', + 'test/skill-e2e-plan-devex-plan-mode.test.ts', +]); + +/** + * Matrix files whose whole-file tier guard has no matching row `tier:` + * property (pre-existing, found 2026-08-26). Consequences today: + * - codex-e2e / gemini-e2e declare 'periodic' → both jobs run ZERO tests and + * report green on every PR (vestigial rows; the periodic cron lane owns + * these suites). + * - the two PTY plan-mode smokes declare 'gate' → the e2e-pty-plan-smoke job + * spends ~7 min on container setup and skill registration, then bun test + * skips every describe — hollow-green since the files adopted + * describeE2ETier. + * Fixing either means deliberately (re)activating paid suites on every PR — + * tracked in the same TODOS burn-down. Fix = add `tier:` to the row (or + * delete the vestigial row), then DELETE the entry here. + */ +const KNOWN_TIER_UNSET = new Map([ + ['test/codex-e2e.test.ts', 'periodic'], + ['test/gemini-e2e.test.ts', 'periodic'], + ['test/skill-e2e-office-hours-auto-mode.test.ts', 'gate'], + ['test/skill-e2e-plan-mode-no-op.test.ts', 'gate'], +]); + +interface MatrixRow { + name: string; + files: string[]; + tier?: string; +} + +/** Parse the `matrix: suite:` rows (name / file / optional tier) from evals.yml. */ +function parseMatrixRows(source: string): MatrixRow[] { + const rows: MatrixRow[] = []; + let current: MatrixRow | null = null; + for (const line of source.split('\n')) { + const name = line.match(/^\s+- name: (\S+)\s*$/); + if (name) { + if (current) rows.push(current); + current = { name: name[1], files: [] }; + continue; + } + if (!current) continue; + const file = line.match(/^\s+file: (.+?)\s*$/); + if (file) current.files.push(...file[1].trim().split(/\s+/)); + const tier = line.match(/^\s+tier: (\S+)\s*$/); + if (tier) current.tier = tier[1]; + // `steps:` ends the strategy block — stop before step-level keys leak in. + if (/^\s{4}steps:\s*$/.test(line)) break; + } + if (current) rows.push(current); + return rows.filter((r) => r.files.length > 0); +} + +const wholeFileTier = (source: string): string | null => { + const m = + /\b(?:describeE2ETier|e2eTierEnabled)\(\s*['"`](gate|periodic)['"`]/.exec(source) || + /EVALS_TIER\s*===\s*['"`](gate|periodic)['"`]/.exec(source); + return m ? m[1] : null; +}; + +const workflowSource = fs.readFileSync(WORKFLOW, 'utf-8'); +const rows = parseMatrixRows(workflowSource); +const matrixFiles = new Map(); +for (const row of rows) for (const f of row.files) matrixFiles.set(f, row); + +const paidFiles = fs + .readdirSync(path.join(ROOT, 'test')) + .filter((f) => f.endsWith('.test.ts')) + .map((f) => `test/${f}`) + .filter(isPaidTestFile); + +describe('evals.yml matrix completeness (gate-lane silent-skip tripwire)', () => { + test('matrix parse sanity: rows and known suites present', () => { + expect(rows.length).toBeGreaterThanOrEqual(15); + expect(matrixFiles.has('test/skill-e2e-workflow.test.ts')).toBe(true); + expect(matrixFiles.has('test/skill-e2e-ship-docsync.test.ts')).toBe(true); + }); + + test('every matrix file exists on disk', () => { + const missing = [...matrixFiles.keys()].filter( + (f) => !fs.existsSync(path.join(ROOT, f)) + ); + expect(missing).toEqual([]); + }); + + test('every gate-hosting paid file is in the matrix (or the documented backlog)', () => { + const gaps: string[] = []; + for (const file of paidFiles) { + const source = fs.readFileSync(path.join(ROOT, file), 'utf-8'); + const declaresGate = wholeFileTier(source) === 'gate'; + const inGateDeps = Object.entries(E2E_TOUCHFILES).some( + ([key, deps]) => + (E2E_TIERS as Record)[key] === 'gate' && + (deps as string[]).includes(file) + ); + if (!declaresGate && !inGateDeps) continue; + if (matrixFiles.has(file) || KNOWN_MATRIX_GAPS.has(file)) continue; + gaps.push(file); + } + expect( + gaps, + `Gate-hosting test file(s) missing from the evals.yml matrix — CI will ` + + `never run them and "gate tier blocks merge" becomes silently false. ` + + `Add a matrix row (with tier: gate when the file is whole-file gated). ` + + `Do NOT extend KNOWN_MATRIX_GAPS for new files.` + ).toEqual([]); + }); + + test('matrix rows for whole-file-gated files carry a matching tier property', () => { + const mismatches: string[] = []; + for (const [file, row] of matrixFiles) { + if (!fs.existsSync(path.join(ROOT, file))) continue; + const declared = wholeFileTier(fs.readFileSync(path.join(ROOT, file), 'utf-8')); + if (!declared) continue; + if (row.tier === declared) continue; + if (KNOWN_TIER_UNSET.get(file) === declared && row.tier === undefined) continue; + mismatches.push(`${file} declares '${declared}' but row '${row.name}' has tier: ${row.tier ?? 'unset'}`); + } + expect( + mismatches, + `A whole-file tier guard with no matching row tier means the suite ` + + `self-skips and the CI job reports a hollow green. Set tier: ` + + `on the row (the Run step exports it as EVALS_TIER).` + ).toEqual([]); + }); + + test('burn-down lists hold only live gaps (ratchet cleanup enforcement)', () => { + const staleGaps = [...KNOWN_MATRIX_GAPS].filter( + (f) => matrixFiles.has(f) || !fs.existsSync(path.join(ROOT, f)) + ); + expect( + staleGaps, + 'Entry fixed or file removed — delete it from KNOWN_MATRIX_GAPS.' + ).toEqual([]); + const staleTiers = [...KNOWN_TIER_UNSET.entries()].filter(([f, declared]) => { + const row = matrixFiles.get(f); + if (!row) return true; // row deleted — entry no longer applies + if (row.tier === declared) return true; // fixed — entry must go + if (!fs.existsSync(path.join(ROOT, f))) return true; + return wholeFileTier(fs.readFileSync(path.join(ROOT, f), 'utf-8')) !== declared; + }); + expect( + staleTiers.map(([f]) => f), + 'Entry fixed, row removed, or guard changed — delete it from KNOWN_TIER_UNSET.' + ).toEqual([]); + }); +}); diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 6c61f37ed..584d2bc53 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -937,7 +937,7 @@ sections. Read a section in full before doing its step; do not work from memory. | addressing Greptile review comments when a PR exists (Step 10) | `sections/greptile.md` | | the adversarial review and learnings capture (Step 11) | `sections/adversarial.md` | | writing the CHANGELOG entry (Step 13) | `sections/changelog.md` | -| syncing docs and creating or updating the PR/MR (Steps 18-19) | `sections/pr-body.md` | +| dispatching the /document-release subagent to sync docs (Step 18) and then creating or updating the PR/MR (Step 19) | `sections/pr-body.md` | --- @@ -1417,13 +1417,15 @@ If `ALREADY_PUSHED`, skip the push but continue to Step 18. Otherwise push with git push -u origin ``` -**You are NOT done.** The code is pushed but documentation sync and PR creation are mandatory final steps. Continue to Step 18. +**You are NOT done.** The code is pushed but Step 18 (dispatch the /document-release subagent to sync docs) and Step 19 (create the PR/MR) are mandatory final steps. Continue to Step 18. --- **PR/MR title invariant (always applies — do not skip even if you don't open the section below):** Any PR or MR you create OR update in the next step MUST have a title that starts with `v$NEW_VERSION` (the version bumped in Step 12), in the format `v : `. Never create or edit a PR/MR title without this prefix. Compute the correct title with the single source of truth helper: `~/.claude/skills/gstack/bin/gstack-pr-title-rewrite.sh "$NEW_VERSION" ""`. The full create/update procedure (idempotency, redaction scan, self-check) is in the section below. -> **STOP.** Before syncing docs and creating or updating the PR/MR (Steps 18-19), Read `~/.claude/skills/gstack/ship/sections/pr-body.md` and execute it +**Doc-sync invariant (always applies — do not skip even if you don't open the section below):** Step 18 dispatches the /document-release subagent BEFORE the PR/MR is created or updated in Step 19. Never skip the dispatch itself; only a failed subagent is non-blocking (proceed to Step 19 without a `## Documentation` section). + +> **STOP.** Before dispatching the /document-release subagent to sync docs (Step 18) and then creating or updating the PR/MR (Step 19), Read `~/.claude/skills/gstack/ship/sections/pr-body.md` and execute it > in full. Do not work from memory — that section is the source of truth for this step. ## Step 20: Persist ship metrics diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index ba77c8d78..7466c4b07 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -2672,12 +2672,14 @@ If `ALREADY_PUSHED`, skip the push but continue to Step 18. Otherwise push with git push -u origin ``` -**You are NOT done.** The code is pushed but documentation sync and PR creation are mandatory final steps. Continue to Step 18. +**You are NOT done.** The code is pushed but Step 18 (dispatch the /document-release subagent to sync docs) and Step 19 (create the PR/MR) are mandatory final steps. Continue to Step 18. --- **PR/MR title invariant (always applies — do not skip even if you don't open the section below):** Any PR or MR you create OR update in the next step MUST have a title that starts with `v$NEW_VERSION` (the version bumped in Step 12), in the format `v : `. Never create or edit a PR/MR title without this prefix. Compute the correct title with the single source of truth helper: `$GSTACK_ROOT/bin/gstack-pr-title-rewrite.sh "$NEW_VERSION" ""`. The full create/update procedure (idempotency, redaction scan, self-check) is in the section below. +**Doc-sync invariant (always applies — do not skip even if you don't open the section below):** Step 18 dispatches the /document-release subagent BEFORE the PR/MR is created or updated in Step 19. Never skip the dispatch itself; only a failed subagent is non-blocking (proceed to Step 19 without a `## Documentation` section). + ## Step 18: Documentation sync (via subagent, before PR creation) **Dispatch /document-release as a subagent** using the Agent tool with `subagent_type: "general-purpose"`. The subagent gets a fresh context window — zero rot from the preceding 17 steps. It also runs the **full** `/document-release` workflow (with CHANGELOG clobber protection, doc exclusions, risky-change gates, named staging, race-safe PR body editing) rather than a weaker reimplementation. diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index 6f0a3d0d4..2fca22277 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -3078,12 +3078,14 @@ If `ALREADY_PUSHED`, skip the push but continue to Step 18. Otherwise push with git push -u origin ``` -**You are NOT done.** The code is pushed but documentation sync and PR creation are mandatory final steps. Continue to Step 18. +**You are NOT done.** The code is pushed but Step 18 (dispatch the /document-release subagent to sync docs) and Step 19 (create the PR/MR) are mandatory final steps. Continue to Step 18. --- **PR/MR title invariant (always applies — do not skip even if you don't open the section below):** Any PR or MR you create OR update in the next step MUST have a title that starts with `v$NEW_VERSION` (the version bumped in Step 12), in the format `v : `. Never create or edit a PR/MR title without this prefix. Compute the correct title with the single source of truth helper: `$GSTACK_ROOT/bin/gstack-pr-title-rewrite.sh "$NEW_VERSION" ""`. The full create/update procedure (idempotency, redaction scan, self-check) is in the section below. +**Doc-sync invariant (always applies — do not skip even if you don't open the section below):** Step 18 dispatches the /document-release subagent BEFORE the PR/MR is created or updated in Step 19. Never skip the dispatch itself; only a failed subagent is non-blocking (proceed to Step 19 without a `## Documentation` section). + ## Step 18: Documentation sync (via subagent, before PR creation) **Dispatch /document-release as a subagent** using the Agent tool with `subagent_type: "general-purpose"`. The subagent gets a fresh context window — zero rot from the preceding 17 steps. It also runs the **full** `/document-release` workflow (with CHANGELOG clobber protection, doc exclusions, risky-change gates, named staging, race-safe PR body editing) rather than a weaker reimplementation. diff --git a/test/helpers/carve-guards.ts b/test/helpers/carve-guards.ts index f665248c2..6caa90872 100644 --- a/test/helpers/carve-guards.ts +++ b/test/helpers/carve-guards.ts @@ -116,17 +116,41 @@ export const CARVE_GUARDS: Record = { // The PR-title-version invariant MUST stay always-loaded: the v1.54.0.0 // carve stranded it in pr-body.md and PRs started landing with bare titles // (CI backstop: test/pr-title-sync-workflow-safety.test.ts). - mustStayInSkeleton: ['v$NEW_VERSION', 'gstack-pr-title-rewrite'], + // Same carve also stranded the Step 18 /document-release dispatch out of + // sight — the skeleton never named it and the handoff "got lost" (#2666 + // follow-up). Three NON-OVERLAPPING anchors pin the restored visibility, + // one per touchpoint (no anchor is a substring of another, so each is + // independently enforced — a subsumed anchor adds zero enforcement): + // gerund form → manifest trigger (renders 2x: section index + STOP) + // imperative → Step 17 handoff line + // 3rd person → hoisted doc-sync invariant + // Matching is case-sensitive String.includes — "dispatching the" does NOT + // contain "dispatch the" — so update anchors in lockstep with any + // touchpoint rewording. + mustStayInSkeleton: [ + 'v$NEW_VERSION', + 'gstack-pr-title-rewrite', + 'dispatching the /document-release subagent to sync docs', + 'dispatch the /document-release subagent to sync docs', + 'dispatches the /document-release subagent', + ], // ...while the full create/update procedure stays carved into pr-body.md // (out of the skeleton, present in the union). Asserts BOTH PR paths - // survive: the create path and the idempotent update path. - mustMoveToSection: ['gh pr create --base', 'gh pr edit --title'], + // survive: the create path and the idempotent update path. The Step 18 + // dispatch imperative stays carved too — pasting that literal into the + // skeleton (correctly) fails this guard; the skeleton speaks of "the + // /document-release subagent", never the carved imperative. + mustMoveToSection: [ + 'gh pr create --base', + 'gh pr edit --title', + 'Dispatch /document-release as a subagent', + ], // ship is operational (multi-STOP, not a plan review); no single post-STOP gate. gateAfterStop: undefined, }, behavioral: 'external', externalTest: 'test/skill-e2e-ship-section-loading.test.ts', - maxSkeletonBytes: 91_600, // v1.68 fix wave: unconditional learnings capture (#2402, ~450B/skill); measured 91,061 + maxSkeletonBytes: 92_300, // document-release visibility restore: named trigger (renders twice) + Step 17 handoff + hoisted doc-sync invariant; measured 91,764 minUnionBytes: 120_000, mustContain: ['VERSION', 'CHANGELOG', 'review', 'merge', 'PR'], // v1.58.5.0: pre-push-guard install (#2077) stacks on the shared first-run-guidance preamble. diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index 667495ba9..b65520847 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -281,6 +281,7 @@ export const E2E_TOUCHFILES: Record = { 'review-coverage-audit': ['review/**', 'test/fixtures/coverage-audit-fixture.ts', 'test/skill-e2e-coverage-audit.test.ts'], 'plan-eng-coverage-audit': ['plan-eng-review/**', 'test/fixtures/coverage-audit-fixture.ts', 'test/skill-e2e-coverage-audit.test.ts'], 'ship-triage': ['ship/**', 'bin/gstack-repo-mode', 'test/skill-e2e-triage.test.ts'], + 'ship-docsync': ['ship/**', 'document-release/**', 'scripts/gen-skill-docs.ts', 'scripts/resolvers/sections.ts', 'test/skill-e2e-ship-docsync.test.ts'], // Plan completion audit + verification 'ship-plan-completion': ['ship/**', 'scripts/gen-skill-docs.ts'], @@ -642,6 +643,7 @@ export const E2E_TIERS: Record = { 'ship-local-workflow': 'gate', 'ship-coverage-audit': 'gate', 'ship-triage': 'gate', + 'ship-docsync': 'gate', 'ship-plan-completion': 'gate', 'ship-plan-verification': 'gate', diff --git a/test/ship-document-release-dispatch.test.ts b/test/ship-document-release-dispatch.test.ts new file mode 100644 index 000000000..638fd0124 --- /dev/null +++ b/test/ship-document-release-dispatch.test.ts @@ -0,0 +1,135 @@ +/** + * /ship → /document-release Step 18 dispatch stays wired AND visible. + * + * The v1.54.0.0 carve (46c1fae7) moved Step 18 (documentation sync) out of + * the ship skeleton into sections/pr-body.md. The wiring survived, but on + * the Claude host the skeleton stopped saying "document-release" anywhere + * in the workflow body — the dispatch became invisible at the decision + * points and "the idea of running document-release as part of ship got + * lost". This tripwire pins both halves so neither can silently regress: + * + * - the carved section keeps the full dispatch contract (imperative, + * subagent_type, JSON return keys, non-blocking failure), and + * - the Claude skeleton names "the /document-release subagent" at its + * three touchpoints (manifest trigger via section-index + STOP pointer, + * Step 17 handoff, hoisted doc-sync invariant) while the imperative + * itself stays carved. + * + * Structural backstop lives in test/helpers/carve-guards.ts (ship entry); + * this named test documents the regression story. Behavioral proof is the + * ship-docsync E2E (test/skill-e2e-ship-docsync.test.ts). + */ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.join(import.meta.dir, '..'); + +const SECTION_SITES = [ + path.join(ROOT, 'ship', 'sections', 'pr-body.md'), + path.join(ROOT, 'ship', 'sections', 'pr-body.md.tmpl'), +]; + +// ship/SKILL.md only — NOT the claude golden: test/host-config.test.ts +// already enforces golden == generated byte-for-byte, so substring asserts +// on the golden would be pure duplication. The codex/factory goldens ARE +// asserted below because for them the check is content (the inlined +// section survived generation), which byte-equality alone doesn't prove. +const CLAUDE_SKELETON = path.join(ROOT, 'ship', 'SKILL.md'); +const INLINED_GOLDENS = [ + path.join(ROOT, 'test', 'fixtures', 'golden', 'codex-ship-SKILL.md'), + path.join(ROOT, 'test', 'fixtures', 'golden', 'factory-ship-SKILL.md'), +]; + +const CARVED_IMPERATIVE = 'Dispatch /document-release as a subagent'; + +describe('/ship Step 18 dispatches /document-release (carve visibility)', () => { + test('carved section carries the full dispatch contract', () => { + for (const file of SECTION_SITES) { + const content = fs.readFileSync(file, 'utf-8'); + expect(content).toContain( + '## Step 18: Documentation sync (via subagent, before PR creation)' + ); + expect(content).toContain(CARVED_IMPERATIVE); + expect(content).toContain('subagent_type: "general-purpose"'); + // The JSON return contract Step 19 bakes into the PR body. + expect(content).toContain('"files_updated"'); + expect(content).toContain('"commit_sha"'); + expect(content).toContain('"pushed"'); + expect(content).toContain('"documentation_section"'); + // Deliberate design: docs sync never holds a ship hostage. + expect(content).toContain('Do not block /ship on subagent failure'); + // These four strings are the ship-docsync E2E's dispatch-matcher markers + // (test/skill-e2e-ship-docsync.test.ts): the first two are INCLUSION + // markers (verbatim from the dictated Step 18 subagent prompt); the last + // two are EXCLUSION markers (section scaffolding that disqualifies a + // whole-section paste). Rewording any of them in pr-body.md.tmpl silently + // deadens the paid matcher; update all four in lockstep. + expect(content).toContain('You are executing the /document-release workflow'); + expect(content).toContain('.claude/skills/gstack/document-release/SKILL.md'); + expect(content).toContain('## Step 19: Create PR/MR'); + expect(content).toContain('Parent processing:'); + } + }); + + test('claude skeleton names the subagent at all three touchpoints', () => { + const content = fs.readFileSync(CLAUDE_SKELETON, 'utf-8'); + // Manifest trigger — renders into the section-index row AND the STOP + // pointer, so it must appear at least twice. + const trigger = + 'dispatching the /document-release subagent to sync docs (Step 18) and then creating or updating the PR/MR (Step 19)'; + expect(content.split(trigger).length - 1).toBeGreaterThanOrEqual(2); + // Step 17 handoff. + expect(content).toContain( + 'Step 18 (dispatch the /document-release subagent to sync docs)' + ); + // Hoisted doc-sync invariant (beside the PR-title invariant). + expect(content).toContain('**Doc-sync invariant'); + expect(content).toContain('dispatches the /document-release subagent'); + // The STOP-Read pointer still routes to the carved section. + expect(content).toMatch( + /> \*\*STOP\.\*\*[^\n]*Read `[^`]*ship\/sections\/pr-body\.md`/ + ); + // Ordering pin: the hoisted invariant must sit ABOVE the pr-body STOP + // pointer (mustStayInSkeleton asserts presence only — a future edit could + // drift the paragraph below the STOP with every registry check green). + const invariantIdx = content.indexOf('**Doc-sync invariant'); + const stopIdx = content.indexOf( + '> **STOP.** Before dispatching the /document-release subagent' + ); + expect(invariantIdx).toBeGreaterThan(-1); + expect(stopIdx).toBeGreaterThan(invariantIdx); + }); + + test('the dispatch imperative stays carved out of the claude skeleton', () => { + const content = fs.readFileSync(CLAUDE_SKELETON, 'utf-8'); + expect(content).not.toContain(CARVED_IMPERATIVE); + }); + + test('manifest trigger names the subagent', () => { + const manifest = JSON.parse( + fs.readFileSync( + path.join(ROOT, 'ship', 'sections', 'manifest.json'), + 'utf-8' + ) + ); + const prBody = manifest.sections.find( + (s: { id: string }) => s.id === 'pr-body' + ); + expect(prBody).toBeDefined(); + expect(prBody.trigger).toContain('the /document-release subagent'); + }); + + test('codex + factory goldens inline Step 18 before Step 19', () => { + for (const file of INLINED_GOLDENS) { + const content = fs.readFileSync(file, 'utf-8'); + const step18 = content.indexOf( + '## Step 18: Documentation sync (via subagent, before PR creation)' + ); + const step19 = content.indexOf('## Step 19: Create PR/MR'); + expect(step18).toBeGreaterThan(-1); + expect(step19).toBeGreaterThan(step18); + expect(content).toContain(CARVED_IMPERATIVE); + } + }); +}); diff --git a/test/skill-e2e-ship-docsync.test.ts b/test/skill-e2e-ship-docsync.test.ts new file mode 100644 index 000000000..58d586b2b --- /dev/null +++ b/test/skill-e2e-ship-docsync.test.ts @@ -0,0 +1,283 @@ +/** + * /ship Step 18 doc-sync dispatch E2E — proves a live agent executing the + * ship tail (Step 17 push → Step 19 PR creation) actually dispatches the + * /document-release subagent BEFORE creating the PR. This is the behavioral + * guardrail for the wiring pinned statically by + * test/ship-document-release-dispatch.test.ts: the v1.54 carve made the + * dispatch invisible once; this test makes that class of regression loud. + * + * Gating: whole-file gate-tier self-gate (describeE2ETier) COMPOSED with + * diff-based selection (describeIfSelected). The self-gate keeps this file + * out of the periodic shard census (near its ceiling) and under the hard + * tier-alignment invariant. DELIBERATE TRADEOFF: tierless runs (`bun run + * test:evals` / `test:e2e`) skip every tier-gated file, so this test does + * NOT run there even when ship/** changed — use the gate lane locally: + * EVALS_TIER=gate bun run test:evals # diff-selected gate lane + * EVALS=1 EVALS_TIER=gate EVALS_ALL=1 bun test test/skill-e2e-ship-docsync.test.ts + * + * Fixture layout (non-obvious — fake HOME + planted skill tree): + * + * / (passed as env HOME) + * ├── repo/ bare-remote git fixture, feature branch, + * │ Steps 0-16 already "done" (VERSION bumped, + * │ CHANGELOG entry, change committed, not pushed) + * ├── ship/SKILL-tail.md sliced Step 17 → Step 20 from the generated + * │ skeleton (live worktree, extract-don't-copy) + * ├── ship/sections/pr-body.md planted copy (relative-resolution + * │ insurance) + * ├── gstack-home/.redact-prepush-prompted marker + env GSTACK_HOME → + * │ Step 17's credential pre-push guard takes its + * │ silent "continue" branch instead of its + * │ AskUserQuestion branch (gstack-config is absent + * │ so REDACT_PREPUSH falls back to "false") + * └── .claude/skills/gstack/ + * ├── ship/sections/pr-body.md ← the STOP pointer's literal + * │ `~/.claude/...` path resolves HERE via the + * │ HOME override (CLAUDE_CONFIG_DIR is already + * │ hermetic, so overriding HOME is safe) + * └── document-release/SKILL.md ← stub: instructs the dispatched + * subagent to emit the empty-result JSON + * contract in 2-3 turns (the DISPATCH is what + * is under test; Step 18 is non-blocking) + * + * The prompt is deliberately neutral — it does NOT command STOP-Read + * compliance and does NOT name document-release. Priming the behavior under + * test would make the assert tautological (and the prompt echoes into the + * transcript, which is why asserts only ever read result.toolCalls). + * + * Cost: observed $0.63-1.04/run, 234-319s (9/9 burn-in + review runs passed; + * gate tier confirmed). + */ +import { expect, beforeAll, afterAll } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { spawnSync } from 'child_process'; +import { runSkillTest } from './helpers/session-runner'; +import { + ROOT, runId, + describeIfSelected, testConcurrentIfSelected, + createEvalCollector, recordE2E, finalizeEvalCollector, logCost, +} from './helpers/e2e-helpers'; +import { describeE2ETier } from './helpers/e2e-gate'; + +const describeE2E = describeE2ETier('gate'); +const evalCollector = createEvalCollector('e2e-ship-docsync'); + +const DOC_RELEASE_STUB = `--- +name: document-release +description: Post-ship documentation update (E2E stub). +--- + +# Document Release (E2E stub) + +You are running the documentation-sync workflow after a code push. +For this environment: compare the docs to the diff briefly; nothing needs +updating. Do NOT edit any files. Do NOT push. + +Output EXACTLY this JSON object on the LAST line of your response, with no +text after it: + +{"files_updated":[],"commit_sha":null,"pushed":false,"documentation_section":null} +`; + +describeE2E('Ship doc-sync dispatch E2E (gate)', () => { + describeIfSelected('Ship doc-sync dispatch E2E', ['ship-docsync'], () => { + let workDir: string; + let repoDir: string; + let remoteDir: string; + + beforeAll(() => { + workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-docsync-home-')); + remoteDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-docsync-remote-')); + repoDir = path.join(workDir, 'repo'); + + // Bare remote + clone; Steps 0-16 "already done": feature branch with a + // committed change, VERSION bumped, CHANGELOG entry written. Not pushed — + // Step 17 (the slice's first step) does that. + // Branch pinned with -b main / -c init.defaultBranch=main so operator git + // config never leaks into the fixture (default-config machines would + // otherwise create master and the later `push -u origin main` would fail). + // Every setup command asserts status — a broken fixture must fail loud + // and free here, never burn a paid run downstream. + const assertOk = (r: ReturnType, what: string) => { + if (r.status !== 0) { + throw new Error( + `ship-docsync fixture setup failed: ${what} → exit ${r.status}\n${r.stderr?.toString() ?? ''}` + ); + } + return r; + }; + assertOk( + spawnSync('git', ['init', '--bare', '-b', 'main'], { cwd: remoteDir, stdio: 'pipe', timeout: 15000 }), + 'git init --bare -b main' + ); + assertOk( + spawnSync('git', ['-c', 'init.defaultBranch=main', 'clone', remoteDir, repoDir], { stdio: 'pipe', timeout: 15000 }), + 'git clone' + ); + const run = (cmd: string, args: string[]) => + assertOk( + spawnSync(cmd, args, { cwd: repoDir, stdio: 'pipe', timeout: 10000 }), + `${cmd} ${args.join(' ')}` + ); + run('git', ['config', 'user.email', 'test@test.com']); + run('git', ['config', 'user.name', 'Test']); + run('git', ['config', 'commit.gpgsign', 'false']); + fs.writeFileSync(path.join(repoDir, 'app.ts'), 'console.log("v1");\n'); + fs.writeFileSync(path.join(repoDir, 'VERSION'), '0.1.0.0\n'); + fs.writeFileSync( + path.join(repoDir, 'CHANGELOG.md'), + '# Changelog\n\n## [0.1.0.0] - 2026-01-01\n\n- Initial release\n' + ); + // The cwd-relative pr-body plant (below) lives inside this working tree; + // ignore it so the fixture repo stays clean and the agent never tries to + // commit test scaffolding. + fs.writeFileSync(path.join(repoDir, '.gitignore'), 'ship/\n'); + run('git', ['add', 'app.ts', 'VERSION', 'CHANGELOG.md', '.gitignore']); + run('git', ['commit', '-m', 'initial']); + run('git', ['push', '-u', 'origin', 'main']); + run('git', ['checkout', '-b', 'feature/docsync-test']); + fs.writeFileSync(path.join(repoDir, 'app.ts'), 'console.log("v2");\n'); + fs.writeFileSync(path.join(repoDir, 'VERSION'), '0.1.0.1\n'); + fs.writeFileSync( + path.join(repoDir, 'CHANGELOG.md'), + '# Changelog\n\n## [0.1.0.1] - 2026-01-02\n\n- Docsync test feature\n\n## [0.1.0.0] - 2026-01-01\n\n- Initial release\n' + ); + run('git', ['add', 'app.ts', 'VERSION', 'CHANGELOG.md']); + run('git', ['commit', '-m', 'feat: docsync test feature']); + + // Extract-don't-copy: slice the LIVE generated skeleton's Step 17→19 + // tail. Fail loudly on marker drift — a tolerant slice silently builds + // a wrong fixture (mirrors extractSkillSections's throw-on-rename). + const skeleton = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8'); + const start = skeleton.indexOf('## Step 17: Push'); + const end = skeleton.indexOf('## Step 20: Persist ship metrics'); + if (start === -1 || end === -1 || end <= start) { + throw new Error( + 'ship/SKILL.md step markers moved — update the skill-e2e-ship-docsync fixture slice' + ); + } + const tail = skeleton.slice(start, end); + fs.mkdirSync(path.join(workDir, 'ship', 'sections'), { recursive: true }); + fs.writeFileSync(path.join(workDir, 'ship', 'SKILL-tail.md'), tail); + + // Plant the real pr-body section at the STOP pointer's ~ path (HOME + // override) and at a relative path as insurance. + const prBody = fs.readFileSync( + path.join(ROOT, 'ship', 'sections', 'pr-body.md'), 'utf-8' + ); + const plantedSkills = path.join(workDir, '.claude', 'skills', 'gstack'); + fs.mkdirSync(path.join(plantedSkills, 'ship', 'sections'), { recursive: true }); + fs.mkdirSync(path.join(plantedSkills, 'document-release'), { recursive: true }); + fs.writeFileSync(path.join(plantedSkills, 'ship', 'sections', 'pr-body.md'), prBody); + fs.writeFileSync(path.join(workDir, 'ship', 'sections', 'pr-body.md'), prBody); + // Third plant: resolvable relative to the agent's cwd (repoDir), not just + // relative to SKILL-tail.md — saves a wasted turn if the agent tries a + // cwd-relative read before the ~ path. + fs.mkdirSync(path.join(repoDir, 'ship', 'sections'), { recursive: true }); + fs.writeFileSync(path.join(repoDir, 'ship', 'sections', 'pr-body.md'), prBody); + fs.writeFileSync( + path.join(plantedSkills, 'document-release', 'SKILL.md'), + DOC_RELEASE_STUB + ); + + // Route Step 17's credential pre-push guard to its silent branch. + fs.mkdirSync(path.join(workDir, 'gstack-home'), { recursive: true }); + fs.writeFileSync( + path.join(workDir, 'gstack-home', '.redact-prepush-prompted'), '' + ); + }); + + afterAll(() => { + try { fs.rmSync(workDir, { recursive: true, force: true }); } catch {} + try { fs.rmSync(remoteDir, { recursive: true, force: true }); } catch {} + }); + + testConcurrentIfSelected('ship-docsync', async () => { + const result = await runSkillTest({ + prompt: `You are executing the /ship workflow; your working directory is the git repo. Steps 0-16 are complete: tests passed, review done, VERSION bumped to 0.1.0.1, CHANGELOG updated, changes committed on branch feature/docsync-test. The remaining workflow is in ${path.join(workDir, 'ship', 'SKILL-tail.md')} — Read it and continue the workflow from Step 17 to completion. Base branch: main. There is no GitHub/GitLab service in this environment: if gh or glab commands fail, print the would-be PR title and body and stop. gstack helper binaries (gstack-*) are unavailable in this environment — treat their failures as no-ops and continue. Do NOT ask questions.`, + workingDirectory: repoDir, + maxTurns: 30, + allowedTools: ['Bash', 'Read', 'Grep', 'Glob', 'Write', 'Agent', 'Task'], + timeout: 480_000, + env: { + HOME: workDir, + GSTACK_HOME: path.join(workDir, 'gstack-home'), + }, + testName: 'ship-docsync', + runId, + }); + + logCost('/ship doc-sync dispatch', result); + + // Assert ONLY on result.toolCalls — the prompt and skill text echo into + // the transcript and would false-positive any transcript-wide match + // (trap documented in skill-e2e-autoplan-dual-voice.test.ts). + const calls = Array.isArray(result.toolCalls) ? result.toolCalls : []; + // Matcher is dispatch-SPECIFIC, not mention-specific: both markers come + // verbatim from the Step 18 subagent prompt dictated by pr-body.md. A + // subagent that merely quotes section text mentioning "document-release" + // (e.g. a PR-body drafter) must NOT count — that false-pass would mask + // the exact regression this test exists to catch. Verified against + // recorded burn-in transcripts: real dispatch inputs carry both markers. + // Section-paste exclusion: a subagent handed the WHOLE pr-body.md as + // context carries the markers too. The dictated Step 18 prompt never + // contains the section's scaffolding, so its presence disqualifies. + // Verified across all recorded runs: real dispatches match markers, + // zero contain scaffold strings. + const dispatchIdx = calls.findIndex((tc) => { + if (tc.tool !== 'Agent' && tc.tool !== 'Task') return false; + const input = JSON.stringify(tc.input ?? {}); + return ( + /document-release\/SKILL\.md|executing the \/document-release workflow/i.test(input) && + !/## Step 19: Create PR\/MR|Parent processing:/.test(input) + ); + }); + const prCreateIdx = calls.findIndex( + (tc) => + tc.tool === 'Bash' && + /gh pr create|glab mr create/.test(String((tc.input as any)?.command ?? '')) + ); + const readPrBody = calls.some( + (tc) => + (tc.tool === 'Read' && + /sections\/pr-body\.md/.test(String((tc.input as any)?.file_path ?? ''))) || + (tc.tool === 'Bash' && + /sections\/pr-body\.md/.test(String((tc.input as any)?.command ?? ''))) + ); + + if (!readPrBody) { + // Diagnostic only — near-tautological under any prompt; the dispatch + // below is the invariant. + console.warn('ship-docsync: pr-body.md was never opened'); + } + + recordE2E(evalCollector, '/ship doc-sync dispatch', 'Ship doc-sync dispatch E2E', result, { + passed: + dispatchIdx >= 0 && + (prCreateIdx < 0 || dispatchIdx < prCreateIdx) && + ['success', 'error_max_turns', 'timeout'].includes(result.exitReason), + }); + + // THE regression assert: the /document-release subagent was dispatched. + expect(dispatchIdx).toBeGreaterThanOrEqual(0); + // Sequencing: dispatch happens BEFORE PR creation (when a create was attempted). + if (prCreateIdx >= 0) expect(dispatchIdx).toBeLessThan(prCreateIdx); + // 'timeout' is acceptable ONLY because the dispatch assert above is + // independently hard — a run that times out AFTER a clean dispatch + // proves the invariant; one that times out before it already failed on + // dispatchIdx. Never soften dispatchIdx to compensate. + expect(['success', 'error_max_turns', 'timeout']).toContain(result.exitReason); + + console.log( + `dispatchIdx=${dispatchIdx} prCreateIdx=${prCreateIdx} readPrBody=${readPrBody} exit=${result.exitReason}` + ); + }, 540_000); + }); +}); + +// Module-level afterAll — finalize eval collector after all tests complete +afterAll(async () => { + await finalizeEvalCollector(evalCollector); +});