From 8b4f5aaf5c8792d9cf9cbc48ff7aed674d4df7a0 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 28 Oct 2026 18:58:13 -0700 Subject: [PATCH] v1.91.302.0 fix: repair three flaky gate cases from the 2026-10-28 flaky pass (index flags, exploratory QA, cookie auth) (#3553) * fix(review): shared-code evidence gathering is read-only (index-flags flaky, gate 40619718717) The failing assume-unchanged trial ran git update-index --really-refresh while gathering evidence, which cleared the flag the checker reads. Step 1 now says the read is read-only and names that command; nearby prose tightened so the section stays byte-identical in size (no pin or ledger move). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(qa-callers): admit ship Step 9's review-plan helper and place its Step 11 voices (late-input 40664336141, plan-checks 40707142223) Gate edit: the caller interface admits the gstack-review-plan forms Step 9 names at the exact installed path; the ship prompt says the plan's adversarial-native and outside-voice lines are Step 11's, outside the Steps 8-9 excerpt. Free replays of both 40707142223 captures and the real plan for all four ship cases. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(browse): real-Chromium cookie-auth positive and early-return rejection cases use the product default timeout Gate edit approved by GStack Minder. The shared 400 ms came from a84b0b5b6 (#2964) as a fixture value no product path uses; --verify-auth callers run under MAX_TIMEOUT_MS 15000. Identity mismatch/missing/ambiguous cases resolve only at the deadline, so they keep the 400 ms bound; explicit timeout tests are unchanged. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(review): read-only evidence test exercises the resolver on every host (test-of-test ratchet) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: curate shared-libs-index-flags-readonly off the Windows subset Its fixture launches bin/gstack-review-log, a bash script, directly; windows-latest reports it not found in PATH (Windows Free Tests run 40727825859, shard 5). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: changelog and test portfolio for the 2026-10-28 S6 flaky repairs Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: keep the credential-rejection curation comment above its own entry Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Revert "test: keep the credential-rejection curation comment above its own entry" The swap changed an input the s6-docs-2 documentation audit hashed, and /ship allows no third audit in one invocation. The misplaced comment is listed as a review note instead. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(qa-callers): admit review-army's specialist diff at the fixture merge base (screen 40737741454 ship-exploratory-unavailable) The review plan dispatches the state-machines specialist, and review-army tells the parent to write the merge-base sha into its diff command. Both screen trials failed on that child's git diff . Also: cite the deadline loop beside the identity_* cookie-auth cases (GStack Minder accepted the 400 ms narrowing). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(qa-callers): the prompt's chaining rules name both DIFF_BASE forms Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 15 ++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- browse/test/cookie-auth-verification.test.ts | 24 +-- docs/TEST_PORTFOLIO.md | 2 +- package.json | 2 +- review/sections/shared-code-reuse.md | 21 +-- scripts/lib/windows-curation.ts | 1 + scripts/resolvers/review-scope.ts | 21 +-- ship/sections/shared-code-reuse.md | 21 +-- ...qa-caller-merge-base-diff-40737741454.json | 101 +++++++++++++ .../qa-caller-review-plan-40707142223.json | 139 ++++++++++++++++++ test/helpers/free-fixtures-data.ts | 2 + test/helpers/qa-callers-fixture.ts | 21 ++- test/qa-caller-review-plan.test.ts | 137 +++++++++++++++++ test/shared-libs-index-flags-readonly.test.ts | 41 ++++++ 16 files changed, 506 insertions(+), 46 deletions(-) create mode 100644 test/fixtures/qa-caller-merge-base-diff-40737741454.json create mode 100644 test/fixtures/qa-caller-review-plan-40707142223.json create mode 100644 test/qa-caller-review-plan.test.ts create mode 100644 test/shared-libs-index-flags-readonly.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 08a99c739..d78f0464a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## [1.91.302.0] - 2026-10-29 + +**Three flaky gate cases from the 2026-10-28 daily flaky pass are repaired: shared-libs-review-index-flags, ship-exploratory-late-input (with ship-exploratory-plan-checks, which failed the same way), and the real-Chromium cookie-auth positive case.** Each red was diagnosed from its captured run before the fix, and each repair has a free test that fails without it. + +### Fixed +- **Shared-code reuse no longer lets a review rewrite the index flags it checks.** In train gate 40619718717, the reviewer ran `git update-index --really-refresh` while gathering evidence. That cleared the assume-unchanged flag the checker reads, so the trial's prerequisites went stale. Step 1 of the /review and /ship shared-code reuse section now says evidence gathering is read-only and names that command. Nearby wording was tightened so the section keeps its exact size. +- **The /ship exploratory-QA cases follow Step 9's review plan again.** Since #3527, Step 9 runs `gstack-review-plan --base main`. The fixture refused that command (paid screen 40707142223, both ship-exploratory-plan-checks trials) and never said that the plan's adversarial-native and outside-voice lines belong to Step 11, which is outside the fixture's Steps 8–9 excerpt. In gate 40664336141, ship-exploratory-late-input reported pass while listing that Step 11 pass as remaining work. Once the plan ran, it dispatched the state-machines specialist, and review-army tells the parent to write the merge-base sha into that specialist's diff command. The fixture refused `git diff `, which failed both ship-exploratory-unavailable trials in this PR's screen 40737741454. +- **The real-browser cookie-auth checks no longer time out under load.** The positive case and the rejection cases that return early now use the product's default 15 s limit instead of a 400 ms test-only value. That value timed out twice on Windows and once on a Ubicloud VM. + +### Gate integrity +- `test/helpers/qa-callers-fixture.ts`: the caller command interface now admits the `gstack-review-plan` forms ship Step 9 names (`--base main` with its `--pass`, `--last` and finding flags, or `--fix-check-prompt` with literal file arguments), only at the fixture's installed helper path. Pipes, other arguments, other binaries and calls without the fixture's runtime are refused, and `test/qa-caller-review-plan.test.ts` checks these as negative controls. The ship prompt names the Step 11 boundary. The interface also admits the fixture's own merge base (at least 7 hex characters) as a literal diff base, bare or in review-army's `DIFF_BASE= && git diff "$DIFF_BASE"` specialist form, and the prompt declares it. Any other sha, a shorter prefix, other Git options and composition are still refused. No threshold, trial count, case kind or timeout changed. +- `browse/test/cookie-auth-verification.test.ts`: GStack Minder approved dropping the shared `timeoutMs: 400`, which came from a84b0b5b6 (#2964) as a fixture value no product path uses (`--verify-auth` callers run under `MAX_TIMEOUT_MS` 15000). The identity mismatch, missing and ambiguous cases keep 400 ms because they resolve only at the deadline, and the 15 s default would exceed bun's 5 s test timeout. The explicit 100 ms, 30 ms, 60 s-cap and invalid-timeout tests are unchanged. +- New free tests: `test/qa-caller-review-plan.test.ts` (replays both 40707142223 captures and both 40737741454 ship-exploratory-unavailable captures, and runs the real plan for all four ship cases) and `test/shared-libs-index-flags-readonly.test.ts` (replays the captured refresh against the assume-unchanged fixture, with a read-only control). +- `scripts/lib/windows-curation.ts`: `test/shared-libs-index-flags-readonly.test.ts` is curated off the Windows subset. Its fixture starts the review through `bin/gstack-review-log`, a bash script, which windows-latest cannot launch (Windows Free Tests run 40727825859, shard 5). + ## [1.91.301.0] - 2026-10-29 **Four paid gate cases that went red on 2026-10-28 without a product defect now pass on the runs that failed them.** Each repair replays the failing run as a free test that fails without it. diff --git a/VERSION b/VERSION index bd148f120..79fda26f5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.301.0 +1.91.302.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 1c74abbf3..0b685966d 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.301.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.302.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/browse/test/cookie-auth-verification.test.ts b/browse/test/cookie-auth-verification.test.ts index 742c1c2c7..4e073cfcc 100644 --- a/browse/test/cookie-auth-verification.test.ts +++ b/browse/test/cookie-auth-verification.test.ts @@ -423,6 +423,7 @@ describe('cookie auth configuration and bounded operations', () => { }); describe('cookie authentication and storage with isolated Chromium profiles', () => { + const browserOptions = { identitySelector: options.identitySelector, expectedIdentity: options.expectedIdentity }; let browser: Browser; let context: BrowserContext; let page: Page; @@ -486,13 +487,13 @@ describe('cookie authentication and storage with isolated Chromium profiles', () test('verifies a reloaded protected page only with an exact visible identity and synthetic session', async () => { await context.addCookies([{ name: 'fixture_session', value: 'approved', url: origin }]); await page.goto(`${origin}/protected`); - expect(await verifyCookieAuthentication(page, options, origin)).toEqual({ verified: true, reason: 'verified', status: 200 }); + expect(await verifyCookieAuthentication(page, browserOptions, origin)).toEqual({ verified: true, reason: 'verified', status: 200 }); }); for (const path of ['/normalized', '/one-visible', '/hidden-child', '/delayed']) { test(`accepts one normalized visible identity at ${path}`, async () => { await page.goto(`${origin}${path}`); - const result = await verifyCookieAuthentication(page, { ...options, expectedIdentity: ` \n${identity} `, timeoutMs: 1000 }, origin); + const result = await verifyCookieAuthentication(page, { ...browserOptions, expectedIdentity: ` \n${identity} `, timeoutMs: 1000 }, origin); expect(result).toEqual({ verified: true, reason: 'verified', status: 200 }); expect(JSON.stringify(result)).not.toContain(identity); }); @@ -506,7 +507,8 @@ describe('cookie authentication and storage with isolated Chromium profiles', () test(`rejects ${path} with a safe ${reason} result`, async () => { await page.goto(`${origin}${path === '/login-hop' ? '/identity' : path}`); if (path === '/login-hop') await page.evaluate(() => history.replaceState({}, '', '/login-hop')); - const result = await verifyCookieAuthentication(page, options, origin); + // identity_mismatch/missing/ambiguous keep the 400 ms options (GStack Minder, 2026-10-28): they resolve only when the deadline fires (browse/src/cookie-auth-verification.ts:105-120), so the 15 s default would exceed bun's 5 s test timeout. + const result = await verifyCookieAuthentication(page, reason.startsWith('identity_') ? options : browserOptions, origin); expect(result.verified).toBe(false); expect(result.reason).toBe(reason); expect(JSON.stringify(result)).not.toMatch(/Synthetic Account|127\.0\.0\.1/); @@ -517,13 +519,13 @@ describe('cookie authentication and storage with isolated Chromium profiles', () await context.addCookies([{ name: 'fixture_session', value: 'approved', url: origin }]); await page.goto(`${origin}/protected`); await context.clearCookies(); - expect((await verifyCookieAuthentication(page, options, origin)).reason).toBe('login_redirect'); + expect((await verifyCookieAuthentication(page, browserOptions, origin)).reason).toBe('login_redirect'); }); for (const status of [401, 403, 500, 503]) { test(`rejects HTTP ${status} even with a positive identity`, async () => { await page.goto(`${origin}/status/${status}`); - expect(await verifyCookieAuthentication(page, options, origin)).toEqual({ verified: false, reason: 'http_error', status }); + expect(await verifyCookieAuthentication(page, browserOptions, origin)).toEqual({ verified: false, reason: 'http_error', status }); }); } @@ -531,7 +533,7 @@ describe('cookie authentication and storage with isolated Chromium profiles', () for (const destination of ['/redirect-cross', '/roundtrip']) { await page.goto(`${origin}/identity`); await page.evaluate(path => history.replaceState({}, '', path), destination); - expect((await verifyCookieAuthentication(page, options, origin)).reason).toBe('target_changed'); + expect((await verifyCookieAuthentication(page, browserOptions, origin)).reason).toBe('target_changed'); } }); @@ -553,7 +555,7 @@ describe('cookie authentication and storage with isolated Chromium profiles', () return value; }); try { - expect((await verifyCookieAuthentication(page, options, origin)).reason).toBe('target_changed'); + expect((await verifyCookieAuthentication(page, browserOptions, origin)).reason).toBe('target_changed'); } finally { locator.mockRestore(); } @@ -563,7 +565,7 @@ describe('cookie authentication and storage with isolated Chromium profiles', () await page.goto(`${origin}/identity`); await page.evaluate(() => history.replaceState({}, '', '/slow')); try { - expect(await verifyCookieAuthentication(page, { ...options, timeoutMs: 100 }, origin)) + expect(await verifyCookieAuthentication(page, { ...browserOptions, timeoutMs: 100 }, origin)) .toEqual({ verified: false, reason: 'timeout' }); } finally { releaseSlowResponse?.(); @@ -572,11 +574,11 @@ describe('cookie authentication and storage with isolated Chromium profiles', () test('sanitizes invalid selector errors and handles a closed target', async () => { await page.goto(`${origin}/identity`); - const result = await verifyCookieAuthentication(page, { ...options, identitySelector: '[synthetic-token' }, origin); + const result = await verifyCookieAuthentication(page, { ...browserOptions, identitySelector: '[synthetic-token' }, origin); expect(result.reason).toBe('verification_failed'); expect(JSON.stringify(result)).not.toContain('synthetic-token'); await page.close(); - expect((await verifyCookieAuthentication(page, options, origin)).reason).toBe('target_closed'); + expect((await verifyCookieAuthentication(page, browserOptions, origin)).reason).toBe('target_closed'); expect(await clearCookieTargetStorage(page, origin).then(() => null, error => error)).toMatchObject({ code: 'target_closed' }); }); @@ -586,7 +588,7 @@ describe('cookie authentication and storage with isolated Chromium profiles', () localStorage.setItem('local', 'original'); sessionStorage.setItem('session', 'original'); }); - expect((await verifyCookieAuthentication(page, options, origin)).verified).toBe(true); + expect((await verifyCookieAuthentication(page, browserOptions, origin)).verified).toBe(true); expect(await page.evaluate(() => [localStorage.getItem('local'), sessionStorage.getItem('session')])) .toEqual(['original', 'original']); }); diff --git a/docs/TEST_PORTFOLIO.md b/docs/TEST_PORTFOLIO.md index ef393a88b..78510a41e 100644 --- a/docs/TEST_PORTFOLIO.md +++ b/docs/TEST_PORTFOLIO.md @@ -60,7 +60,7 @@ depends on measured duration or an explicit `QUICK_CORE` entry, not this table. | CLI/API/webhook QA without browser setup | `qa-functional-fixture`, `qa-functional-evidence`, `qa-lazy-sections` | `skill-e2e-qa-functional`: CLI and webhook report sessions | Linux/macOS free; selected PR gate; Windows only where curated | | Report-only preserves local and remote authority | `qa-only-capability`, `qa-functional-observer`, `qa-functional-observer-atomic`, `qa-caller-authority` | Independent report-only sessions with synthetic owned endpoints/auth | Linux kernel observation; free callback controls plus selected PR gate | | Repair reproduces the defect, adds a failing regression and rechecks adjacent behavior | `qa-fix-loop-fixture`, `qa-functional-evidence` | `skill-e2e-qa-functional-fix`: CLI and webhook repair sessions | Free controls plus selected PR gate | -| Review and Ship actually explore | `qa-exploratory-callers`, `qa-caller-report-observer`, `qa-checkpoint-evidence` | `skill-e2e-qa-callers`: actual Review/Ship callers | Free captures plus selected PR gate | +| Review and Ship actually explore | `qa-exploratory-callers`, `qa-caller-report-observer`, `qa-caller-review-plan`, `qa-checkpoint-evidence` | `skill-e2e-qa-callers`: actual Review/Ship callers | Free captures plus selected PR gate | | Smoke expiry preserves required plan checks | `qa-deadline`, `qa-deadline-selection`, `qa-browser-deadline-evidence` | `ship-exploratory-plan-checks` | Free deadline/dispatch controls plus selected PR gate | | Late changes invalidate affected results | `qa-caller-freshness-order`, `qa-deadline-publication-observer`, `shared-libs-revalidation-prompt` | `ship-exploratory-late-input` and the existing late-input documentation handoff | Free stale-input controls plus selected PR gate | | Documentation completes before publication and respects protected files | `docsync-authority`, `docsync-atomic-writes`, `docsync-report-interface`, `docsync-lifecycle-interface` | `skill-e2e-ship-docsync`, `skill-e2e-docsync-spawned` | Free state/permission controls; registered gate/periodic scenarios retain their tiers | diff --git a/package.json b/package.json index 7cd518b7a..1485ec75c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.301", + "version": "1.91.302", "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/review/sections/shared-code-reuse.md b/review/sections/shared-code-reuse.md index 4e4d6ba10..9aca55812 100644 --- a/review/sections/shared-code-reuse.md +++ b/review/sections/shared-code-reuse.md @@ -2,11 +2,12 @@ **Reuse a skipped shared-code advisory only with complete structural evidence:** -1. **Read the evidence.** Read all supporting callers and the helper destination. - Establish first-party authored provenance and whether the current extraction - is worthwhile; the checker cannot decide that. Retain `evidence_paths`/`helper_target`. +1. **Read the evidence.** Read all supporting callers and the helper target + read-only: `git update-index`, even `--really-refresh`, clears the index flags + the checker reads. Establish first-party authored provenance and whether this + extraction is worthwhile; the checker cannot. Retain `evidence_paths`/`helper_target`. 2. **Run the checker.** From the repository root, pass the current finding as - literal JSON on stdin. Replace REVIEW_START with this pass's captured token + literal JSON on stdin. Replace REVIEW_START with this pass's token and the example paths/symbol with actual evidence. Keep the quoted delimiter. ```bash @@ -15,17 +16,17 @@ GSTACK_SHARED_LIBS_REUSE_JSON ``` -3. **Act on its result.** Read the JSON. Only `reusable: true` permits suppression. +3. **Act on its result.** Only `reusable: true` permits suppression. False, command failure or unreadable output requires fresh source review and a new decision, never suppression. Do not supply your own snapshot, prior record or coverage. -4. **Persist through the logger.** The logger recomputes final coverage; never - supply proof yourself. Real defects retain normal Fix-First handling independently. +4. **Persist through the logger.** The logger recomputes coverage; never supply + proof. Real defects retain normal Fix-First handling. -**What a reusable result proves (do not reconstruct these checks yourself):** -- Identity: `sharedLibsFingerprint` plus the actual repo, raw branch and current snapshot. +**What a reusable result proves (never reconstruct these checks):** +- Identity: `sharedLibsFingerprint` plus the repo, raw branch and current snapshot. The checker reads REVIEW_START without consuming/replacing it. Sanitized branch names are not identity. - Prior decision: completed/converged review, verified binding, explicit Skip and - logger-versioned `snapshot_covered_paths`; older unversioned coverage needs a fresh decision. + logger-versioned `snapshot_covered_paths`; unversioned coverage needs a fresh decision. - Source: `canReuseSharedLibsAdvisory` requires every supporting path's raw file byte-for-byte with its blob. Exclude assume-unchanged, skip-worktree and sparse index entries; symlinks/ancestors, submodules, ignored/outside or unreadable files; diff --git a/scripts/lib/windows-curation.ts b/scripts/lib/windows-curation.ts index cf9787d22..c884f2358 100644 --- a/scripts/lib/windows-curation.ts +++ b/scripts/lib/windows-curation.ts @@ -52,6 +52,7 @@ const WINDOWS_FRAGILE_PATTERNS: Array<{ pattern: RegExp; reason: string }> = [ // the failure mode is structural rather than detectable via source-file scan. export const KNOWN_WINDOWS_INCOMPATIBLE: Array<{ file: string; reason: string }> = [ // Like test/paid-rerun.test.ts (excluded by pattern), it replays a gate capture through the paid lane's --report and --rerun-plan CLI. + { file: 'test/shared-libs-index-flags-readonly.test.ts', reason: 'its assume-unchanged replay seeds the review through test/helpers/shared-libs-path-fixture.ts, which launches bin/gstack-review-log (a bash script) directly; windows-latest refuses it as not found in $PATH (windows-free-shard 5, run 40727825859)' }, { file: 'test/credential-rejection.test.ts', reason: 'replays run 39696775546 through the --report and --rerun-plan CLI of the paid lane, which runs only on Linux CI; on windows-latest that CLI exits 1 with EINVAL in open() (windows-free-shard run 39982914852)' }, // Field list 2, W11: the review-voice install is a POSIX launcher contract, exercised end to end on Linux and macOS. { file: 'test/capy-review-voice.test.ts', reason: 'copies the checkout with tar -C into the temp root (GNU tar reads a D: path as a remote host) and launches the bash --for review-voice installer with a colon-joined GSTACK_LAUNCH PATH' }, diff --git a/scripts/resolvers/review-scope.ts b/scripts/resolvers/review-scope.ts index ff5688c59..40a94a5dd 100644 --- a/scripts/resolvers/review-scope.ts +++ b/scripts/resolvers/review-scope.ts @@ -133,11 +133,12 @@ or missing-reviewer rules.`; export function generateSharedCodeReuse(ctx: TemplateContext): string { return `**Reuse a skipped shared-code advisory only with complete structural evidence:** -1. **Read the evidence.** Read all supporting callers and the helper destination. - Establish first-party authored provenance and whether the current extraction - is worthwhile; the checker cannot decide that. Retain \`evidence_paths\`/\`helper_target\`. +1. **Read the evidence.** Read all supporting callers and the helper target + read-only: \`git update-index\`, even \`--really-refresh\`, clears the index flags + the checker reads. Establish first-party authored provenance and whether this + extraction is worthwhile; the checker cannot. Retain \`evidence_paths\`/\`helper_target\`. 2. **Run the checker.** From the repository root, pass the current finding as - literal JSON on stdin. Replace REVIEW_START with this pass's captured token + literal JSON on stdin. Replace REVIEW_START with this pass's token and the example paths/symbol with actual evidence. Keep the quoted delimiter. \`\`\`bash @@ -146,17 +147,17 @@ export function generateSharedCodeReuse(ctx: TemplateContext): string { GSTACK_SHARED_LIBS_REUSE_JSON \`\`\` -3. **Act on its result.** Read the JSON. Only \`reusable: true\` permits suppression. +3. **Act on its result.** Only \`reusable: true\` permits suppression. False, command failure or unreadable output requires fresh source review and a new decision, never suppression. Do not supply your own snapshot, prior record or coverage. -4. **Persist through the logger.** The logger recomputes final coverage; never - supply proof yourself. Real defects retain normal Fix-First handling independently. +4. **Persist through the logger.** The logger recomputes coverage; never supply + proof. Real defects retain normal Fix-First handling. -**What a reusable result proves (do not reconstruct these checks yourself):** -- Identity: \`sharedLibsFingerprint\` plus the actual repo, raw branch and current snapshot. +**What a reusable result proves (never reconstruct these checks):** +- Identity: \`sharedLibsFingerprint\` plus the repo, raw branch and current snapshot. The checker reads REVIEW_START without consuming/replacing it. Sanitized branch names are not identity. - Prior decision: completed/converged review, verified binding, explicit Skip and - logger-versioned \`snapshot_covered_paths\`; older unversioned coverage needs a fresh decision. + logger-versioned \`snapshot_covered_paths\`; unversioned coverage needs a fresh decision. - Source: \`canReuseSharedLibsAdvisory\` requires every supporting path's raw file byte-for-byte with its blob. Exclude assume-unchanged, skip-worktree and sparse index entries; symlinks/ancestors, submodules, ignored/outside or unreadable files; diff --git a/ship/sections/shared-code-reuse.md b/ship/sections/shared-code-reuse.md index 4e4d6ba10..9aca55812 100644 --- a/ship/sections/shared-code-reuse.md +++ b/ship/sections/shared-code-reuse.md @@ -2,11 +2,12 @@ **Reuse a skipped shared-code advisory only with complete structural evidence:** -1. **Read the evidence.** Read all supporting callers and the helper destination. - Establish first-party authored provenance and whether the current extraction - is worthwhile; the checker cannot decide that. Retain `evidence_paths`/`helper_target`. +1. **Read the evidence.** Read all supporting callers and the helper target + read-only: `git update-index`, even `--really-refresh`, clears the index flags + the checker reads. Establish first-party authored provenance and whether this + extraction is worthwhile; the checker cannot. Retain `evidence_paths`/`helper_target`. 2. **Run the checker.** From the repository root, pass the current finding as - literal JSON on stdin. Replace REVIEW_START with this pass's captured token + literal JSON on stdin. Replace REVIEW_START with this pass's token and the example paths/symbol with actual evidence. Keep the quoted delimiter. ```bash @@ -15,17 +16,17 @@ GSTACK_SHARED_LIBS_REUSE_JSON ``` -3. **Act on its result.** Read the JSON. Only `reusable: true` permits suppression. +3. **Act on its result.** Only `reusable: true` permits suppression. False, command failure or unreadable output requires fresh source review and a new decision, never suppression. Do not supply your own snapshot, prior record or coverage. -4. **Persist through the logger.** The logger recomputes final coverage; never - supply proof yourself. Real defects retain normal Fix-First handling independently. +4. **Persist through the logger.** The logger recomputes coverage; never supply + proof. Real defects retain normal Fix-First handling. -**What a reusable result proves (do not reconstruct these checks yourself):** -- Identity: `sharedLibsFingerprint` plus the actual repo, raw branch and current snapshot. +**What a reusable result proves (never reconstruct these checks):** +- Identity: `sharedLibsFingerprint` plus the repo, raw branch and current snapshot. The checker reads REVIEW_START without consuming/replacing it. Sanitized branch names are not identity. - Prior decision: completed/converged review, verified binding, explicit Skip and - logger-versioned `snapshot_covered_paths`; older unversioned coverage needs a fresh decision. + logger-versioned `snapshot_covered_paths`; unversioned coverage needs a fresh decision. - Source: `canReuseSharedLibsAdvisory` requires every supporting path's raw file byte-for-byte with its blob. Exclude assume-unchanged, skip-worktree and sparse index entries; symlinks/ancestors, submodules, ignored/outside or unreadable files; diff --git a/test/fixtures/qa-caller-merge-base-diff-40737741454.json b/test/fixtures/qa-caller-merge-base-diff-40737741454.json new file mode 100644 index 000000000..0115de2f8 --- /dev/null +++ b/test/fixtures/qa-caller-merge-base-diff-40737741454.json @@ -0,0 +1,101 @@ +{ + "source": "Paid screen 40737741454 (#3553 head 4cd0777) ship-exploratory-unavailable: paid-slice-11-a1 and paid-rerun-1-a1 native-events.json, every Bash command in order (parent and child). In both trials the review plan dispatched the state-machines specialist, and the parent told it to run git diff with the literal merge-base sha.", + "runs": { + "40737741454-slice-11": { + "root": "/home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs", + "mergeBase": "8c9a9c5", + "commands": [ + { + "parent": false, + "command": "date -u +%Y-%m-%dT%H:%M:%SZ" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/host/runtime/bin/gstack-qa-deadline start /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/product/reports/deadline.json 300 2026-10-28T18:00:28.179Z" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/host/runtime/bin/gstack-qa-evidence capture /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/product/reports 001 --public --deadline /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/product/reports/deadline.json -- bun scripts/probe.ts 4" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/host/runtime/bin/gstack-qa-deadline status /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/product/reports/deadline.json" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/host/runtime/bin/gstack-qa-evidence capture /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/product/reports 002 --public --deadline /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/product/reports/deadline.json --after 001 --hypothesis 'Riskiest changed edge: out-of-range 10 must exit 2 with the declared stderr; also confirms whether the missing native-engine prerequisite blocks every CLI path' -- bun scripts/probe.ts 10" + }, + { + "parent": false, + "command": "bun run test" + }, + { + "parent": false, + "command": "/home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/host/runtime/bin/gstack-review-plan --base main" + }, + { + "parent": true, + "command": "git diff 8c9a9c5" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/host/runtime/bin/gstack-qa-evidence materialize /home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/product/reports annotations.json" + }, + { + "parent": false, + "command": "git status --short" + }, + { + "parent": false, + "command": "/home/runner/.cache/gstack-paid-shard-hEauF9/tmp/qc-qBCQGs/host/runtime/bin/gstack-review-log '{\"skill\":\"review\",\"timestamp\":\"'\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\"'\",\"status\":\"issues_found\",\"issues_found\":0,\"critical\":0,\"informational\":0,\"quality_score\":10,\"specialists\":{\"state-machines\":{\"dispatched\":true,\"findings\":0,\"critical\":0,\"informational\":0},\"red-team\":{\"dispatched\":false,\"reason\":\"scope\"}},\"findings\":[],\"commit\":\"'\"$(git rev-parse --short HEAD)\"'\",\"via\":\"ship\",\"completed\":false,\"converged\":false,\"cycles\":0,\"pass\":1,\"review_scope\":\"frozen\",\"range_base\":null}' --finish bbf15eec-7962-4aee-bcfd-88233b0375fe" + } + ] + }, + "40737741454-rerun-1": { + "root": "/home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK", + "mergeBase": "0f7b81c", + "commands": [ + { + "parent": false, + "command": "date -u +%Y-%m-%dT%H:%M:%SZ" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/host/runtime/bin/gstack-qa-deadline start /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/product/reports/deadline.json 300 2026-10-28T18:18:29.666Z" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/host/runtime/bin/gstack-qa-evidence capture /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/product/reports 001 --public --deadline /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/product/reports/deadline.json -- bun scripts/probe.ts 3" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/host/runtime/bin/gstack-qa-evidence capture /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/product/reports 002 --public --deadline /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/product/reports/deadline.json --after 001 --hypothesis 'Success path blocked by missing vendor/native-engine.ts; check whether the riskiest changed edge (out-of-range 10) is equally blocked before reaching scale().' -- bun scripts/probe.ts 10" + }, + { + "parent": false, + "command": "bun run test" + }, + { + "parent": false, + "command": "/home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/host/runtime/bin/gstack-review-plan --base main" + }, + { + "parent": true, + "command": "git diff 0f7b81c" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/host/runtime/bin/gstack-qa-evidence materialize /home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/product/reports annotations.json" + }, + { + "parent": false, + "command": "git status --short" + }, + { + "parent": false, + "command": "/home/runner/.cache/gstack-paid-shard-2u70Ma/tmp/qc-lV9GKK/host/runtime/bin/gstack-review-log '{\"skill\":\"review\",\"timestamp\":\"'\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\"'\",\"status\":\"issues_found\",\"issues_found\":0,\"critical\":0,\"informational\":0,\"quality_score\":10,\"specialists\":{\"state-machines\":{\"dispatched\":true,\"findings\":0,\"critical\":0,\"informational\":0},\"testing\":{\"dispatched\":false,\"reason\":\"scope\"},\"maintainability\":{\"dispatched\":false,\"reason\":\"scope\"},\"red-team\":{\"dispatched\":false,\"reason\":\"scope\"}},\"findings\":[],\"commit\":\"'\"$(git rev-parse --short HEAD)\"'\",\"via\":\"ship\",\"completed\":false,\"converged\":false,\"cycles\":0,\"pass\":1,\"review_scope\":\"frozen\",\"range_base\":null}' --finish 4b1c9f3b-d67b-4919-8237-21634b5b4c0b" + } + ] + } + } +} \ No newline at end of file diff --git a/test/fixtures/qa-caller-review-plan-40707142223.json b/test/fixtures/qa-caller-review-plan-40707142223.json new file mode 100644 index 000000000..740f5a351 --- /dev/null +++ b/test/fixtures/qa-caller-review-plan-40707142223.json @@ -0,0 +1,139 @@ +{ + "source": "Paid screen 40707142223 (#3544 head) ship-exploratory-plan-checks: paid-slice-10-a1 and paid-rerun-1-a1 native-events.json, every Bash command in order (parent and child); late-input receipt from train 35 rebuild gate 40664336141 paid-slice-9-a1", + "runs": { + "40707142223-slice-10": { + "root": "/home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE", + "commands": [ + { + "parent": false, + "command": "date -u +%Y-%m-%dT%H:%M:%SZ" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/host/runtime/bin/gstack-qa-deadline start /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/product/reports/deadline.json 300 2026-10-28T14:53:05.952Z" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/host/runtime/bin/gstack-qa-evidence capture /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/product/reports 001 --public --deadline /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/product/reports/deadline.json -- bun scripts/probe.ts 4" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/host/runtime/bin/gstack-qa-deadline status /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/product/reports/deadline.json" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/host/runtime/bin/gstack-qa-evidence capture /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/product/reports 002 --public --deadline /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/product/reports/deadline.json --after 001 --hypothesis 'Success holds for 4; riskiest changed edge is just past the upper bound, so 10 must exit 2 with the exact stderr message.' -- bun scripts/probe.ts 10" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/host/runtime/bin/gstack-qa-evidence capture /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/product/reports 003 --public --timeout-ms 10000 --after 002 --hypothesis 'Rejection of 10 holds; PLAN.md requires the upper boundary check: 9 must exit 0 with stdout 18 and empty stderr.' -- bun scripts/probe.ts 9" + }, + { + "parent": false, + "command": "bun run test" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/host/runtime/bin/gstack-qa-evidence materialize /home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/product/reports annotations.json" + }, + { + "parent": false, + "command": "if GATE_OUT=$(/home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/host/runtime/bin/gstack-gate-diff main 2>&1); then\n printf '%s\\n' \"$GATE_OUT\"\nelse\n GATE_EXIT=$?; printf '%s\\n' \"$GATE_OUT\"\n case \"$GATE_EXIT\" in\n 2) echo \"Gate integrity: UNAVAILABLE \u2014 $(printf '%s\\n' \"$GATE_OUT\" | grep -m1 '^GATE_ERROR=')\" ;;\n *) echo \"Gate integrity: UNAVAILABLE \u2014 helper exit $GATE_EXIT (stale install? run /gstack-upgrade)\" ;;\n esac\nfi" + }, + { + "parent": false, + "command": "date -u +%Y-%m-%dT%H:%M:%SZ" + }, + { + "parent": false, + "command": "/home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/host/runtime/bin/gstack-review-plan --base main" + }, + { + "parent": true, + "command": "git diff origin/main" + }, + { + "parent": false, + "command": "git status --short" + }, + { + "parent": false, + "command": "/home/runner/.cache/gstack-paid-shard-T57Isu/tmp/qc-9kHKwE/host/runtime/bin/gstack-review-log '{\"skill\":\"review\",\"timestamp\":\"'\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\"'\",\"status\":\"clean\",\"issues_found\":0,\"critical\":0,\"informational\":0,\"quality_score\":10.0,\"specialists\":{\"state-machines\":{\"dispatched\":true,\"findings\":0,\"critical\":0,\"informational\":0},\"red-team\":{\"dispatched\":false,\"reason\":\"scope\"}},\"findings\":[],\"commit\":\"'\"$(git rev-parse --short HEAD)\"'\",\"via\":\"ship\",\"completed\":true,\"converged\":true,\"cycles\":0,\"pass\":1,\"review_scope\":\"frozen\",\"range_base\":null}' --finish 5534d8f6-4d67-4b6f-b691-c6e83c659df2" + }, + { + "parent": false, + "command": "date -u +%Y-%m-%dT%H:%M:%SZ" + } + ] + }, + "40707142223-rerun-1": { + "root": "/home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP", + "commands": [ + { + "parent": false, + "command": "date -u +%Y-%m-%dT%H:%M:%SZ" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/host/runtime/bin/gstack-qa-deadline start /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/product/reports/deadline.json 300 2026-10-28T15:06:29.695Z" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/host/runtime/bin/gstack-qa-evidence capture /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/product/reports 001 --public --deadline /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/product/reports/deadline.json -- bun scripts/probe.ts 9" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/host/runtime/bin/gstack-qa-evidence capture /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/product/reports 002 --public --deadline /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/product/reports/deadline.json --after 001 --hypothesis 'Range rule: 10 breaks only the 0..9 bound; changed return path must not alter rejection (exit 2, exact stderr, empty stdout).' -- bun scripts/probe.ts 10" + }, + { + "parent": false, + "command": "bun run test" + }, + { + "parent": false, + "command": "if GATE_OUT=$(/home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/host/runtime/bin/gstack-gate-diff main 2>&1); then\n printf '%s\\n' \"$GATE_OUT\"\nelse\n GATE_EXIT=$?; printf '%s\\n' \"$GATE_OUT\"\n case \"$GATE_EXIT\" in\n 2) echo \"Gate integrity: UNAVAILABLE \u2014 $(printf '%s\\n' \"$GATE_OUT\" | grep -m1 '^GATE_ERROR=')\" ;;\n *) echo \"Gate integrity: UNAVAILABLE \u2014 helper exit $GATE_EXIT (stale install? run /gstack-upgrade)\" ;;\n esac\nfi" + }, + { + "parent": false, + "command": "bun /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/host/runtime/bin/gstack-qa-evidence materialize /home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/product/reports annotations.json" + }, + { + "parent": false, + "command": "git status --short" + }, + { + "parent": false, + "command": "date -u +%Y-%m-%dT%H:%M:%SZ" + }, + { + "parent": false, + "command": "/home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/host/runtime/bin/gstack-review-plan --base main" + }, + { + "parent": true, + "command": "git diff origin/main" + }, + { + "parent": false, + "command": "/home/runner/.cache/gstack-paid-shard-pi2bIV/tmp/qc-kMsNBP/host/runtime/bin/gstack-review-log '{\"skill\":\"review\",\"timestamp\":\"'\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\"'\",\"status\":\"clean\",\"issues_found\":0,\"critical\":0,\"informational\":0,\"quality_score\":10.0,\"specialists\":{\"state-machines\":{\"dispatched\":true,\"findings\":0,\"critical\":0,\"informational\":0}},\"findings\":[],\"commit\":\"'\"$(git rev-parse --short HEAD)\"'\",\"via\":\"ship\",\"completed\":true,\"converged\":true,\"cycles\":0,\"pass\":1,\"review_scope\":\"frozen\",\"range_base\":null}' --finish 94e80914-bf02-4c55-84a9-a9ea9e1adbac" + }, + { + "parent": false, + "command": "git status --short" + } + ] + } + }, + "lateInputReceipt": { + "status": "pass", + "probes": [ + "probe-1cd02598-4151-4346-9556-b245a37e9a44", + "probe-11857dac-c540-41de-8e13-ca6181ec88fc", + "probe-a8572671-a2ec-4b2d-b5bb-aee043898de1" + ], + "remaining": [ + "Step 11 native adversarial review (not dispatched)", + "review record completed:false" + ] + } +} \ No newline at end of file diff --git a/test/helpers/free-fixtures-data.ts b/test/helpers/free-fixtures-data.ts index 8e3b9ffaf..3b62728c3 100644 --- a/test/helpers/free-fixtures-data.ts +++ b/test/helpers/free-fixtures-data.ts @@ -179,7 +179,9 @@ export const FREE_FIXTURES: Readonly> = { 'test/fixtures/plan-floor-edit-wrap-37176835584.json': ['test/plan-floor-edit-wrap.test.ts'], 'test/fixtures/plan-format-kind-notes.json': ['test/plan-format-kind-note.test.ts'], 'test/fixtures/qa-caller-handoff-order-public.json': ['test/qa-exploratory-callers.test.ts'], + 'test/fixtures/qa-caller-merge-base-diff-40737741454.json': ['test/qa-caller-review-plan.test.ts'], 'test/fixtures/qa-caller-read-probe-mtb1007a.json': ['test/qa-exploratory-callers.test.ts'], + 'test/fixtures/qa-caller-review-plan-40707142223.json': ['test/qa-caller-review-plan.test.ts'], 'test/fixtures/qa-caller-section-skip-ship-small-cli.json': ['test/qa-exploratory-callers.test.ts'], 'test/fixtures/plans/autoplan-dashboard.md': ['test/autoplan-init.test.ts', 'test/autoplan-preconfigured-onboarding-ar.test.ts', 'test/autoplan-snapshot.test.ts'], 'test/fixtures/plans/autoplan-focus-appearance-design.md': [], diff --git a/test/helpers/qa-callers-fixture.ts b/test/helpers/qa-callers-fixture.ts index 9ec7a6cdd..0329b8e2c 100644 --- a/test/helpers/qa-callers-fixture.ts +++ b/test/helpers/qa-callers-fixture.ts @@ -140,6 +140,14 @@ const literalCallerListing = new RegExp(`^ls(?: -[la]+)?(?: --)?(?: ${literalCal const callerDiffMode = '(--stat|--numstat|--name-only|--name-status)'; const literalCallerDiff = new RegExp(`^git diff(?: ${callerDiffMode})?(?: (origin/main|"\\$DIFF_BASE"))?(?: ${callerDiffMode})?(?: --(?: ${literalCallerArgument})+)?$`); const callerDiffPreface = 'DIFF_BASE=$(git merge-base origin/main HEAD) && '; +const literalMergeBaseDiff = new RegExp(`^(?:DIFF_BASE=([0-9a-f]{7,40}) && )?git diff(?: ${callerDiffMode})? ([0-9a-f]{7,40}|"\\$DIFF_BASE")(?: ${callerDiffMode})?(?: --(?: ${literalCallerArgument})+)?$`); + +function mergeBaseDiffAllowed(text: string, context?: CallerDeadlineContext): boolean { + const diff = literalMergeBaseDiff.exec(text); + if (!diff || !context || (diff[2] && diff[4]) || !!diff[1] !== (diff[3] === '"$DIFF_BASE"')) return false; + const base = spawnSync('git', ['merge-base', 'origin/main', 'HEAD'], { cwd: context.fixtureRoot, encoding: 'utf8', timeout: 5000 }); + return base.status === 0 && base.stdout.trim().startsWith(diff[1] ?? diff[3]); +} function literalCallerDiffAllowed(text: string): boolean { const recomputedBase = text.startsWith(callerDiffPreface); @@ -217,9 +225,19 @@ function callerDeadlineCommand(command: string, context?: CallerDeadlineContext) return { action, stateFile }; } +const reviewPlanFlags = '(?: --pass \\d+)?(?: --last (?:none|full|fix-check))?(?: --(?:fixed-blocking|fixed-below|blocking-open|touched-gate|critical-found|adversarial-ran))*(?: --reviewed-numbers [\\w:.-]+)?'; +const reviewPlanForms = new RegExp(`^(?:--base main${reviewPlanFlags}|--fix-check-prompt --fix-list ${literalCallerArgument} --hunks ${literalCallerArgument} --tests ${literalCallerArgument}(?: --callers ${literalCallerArgument})?)$`); + +/** The review-plan helper forms ship Step 9 names, run by the fixture's exact installed path. */ +function reviewPlanCommandAllowed(text: string, runtime?: string): boolean { + const helper = runtime && `${runtime}/bin/gstack-review-plan `; + return !!helper && text.startsWith(helper) && reviewPlanForms.test(text.slice(helper.length)); +} + export function qaCallerCommandAllowed(command: string, workflowCommands: string[] = [], deadline?: CallerDeadlineContext): boolean { const text = command.trim(); if (qaHelperUsageCommand(text)) return true; + if (reviewPlanCommandAllowed(text, deadline?.runtime)) return true; if (callerEvidenceCommand(text, deadline)) return true; if (callerDeadlineCommand(text, deadline)) return true; if (/\bgstack-qa-(?:deadline|evidence)\b/.test(text) && !literalCallerCLI.test(text) @@ -227,6 +245,7 @@ export function qaCallerCommandAllowed(command: string, workflowCommands: string if (workflowCommands.includes(text)) return true; if (literalCallerCLI.test(text)) return true; if (literalCallerDiffAllowed(text)) return true; + if (mergeBaseDiffAllowed(text, deadline)) return true; if (literalCallerListingAllowed(text)) return true; let normalizedRecord = text; const substitutedFields: string[] = []; @@ -699,7 +718,7 @@ export function callerReviewRecordTemplate(fixture: Pick[0] { return { - prompt: `Load gstack's /${fixture.caller} supplied parent phase from caller-${fixture.caller}.md and resume it on the selected working-tree diff against origin/main. This excerpt comes from ${fixture.runtime}/${fixture.caller}/SKILL.md; resolve installed-relative references there, not from the excerpt file or product directory. That path identifies the asset base, not another entrypoint: do not read or invoke the full parent SKILL.md or rerun its preamble. Earlier preamble/branch/base setup is complete; use the existing local origin/main ref without fetch. Earlier-phase asset locators are ${fixture.runtime}/review/checklist.md and ${fixture.runtime}/qa/templates/functional-report-template.md. Read those files directly when referenced; recursive Glob does not follow the installed asset symlinks. Cross-project learnings are configured off in this owned fixture. ${fixture.reviewStart ? `The actual review-start helper already captured REVIEW_START=${fixture.reviewStart} for this unchanged core pass; retain that token. ` : ''} This fixture evaluates only the supplied parent phase, not later publication stages. The fixture owner recorded these read-only observations when this phase began; they are current until an input changes, so use them instead of re-running those commands, and recheck freshness before completion outputs:\n${fixture.startObservations}\n Read README.md for the project contract and commands. Use diagnostic-client commands such as \`bun scripts/probe.ts \` for exploratory discoveries and their checkpoint evidence. A required \`bun run test\` is separate suite verification: report it as verification, never as a diagnostic observation or checkpoint anchor/target. Use the production evidence helper to publish each diagnostic checkpoint as \`reports/exploration-NNN.json\`, not inside a nested directory; do not transcribe its observed payload. ${fixture.caseId === 'ship-exploratory-plan-checks' ? 'The previously discovered plan is PLAN.md.' : 'No plan file was found.'} There is no remote service and no release publication is authorized. There is no interactive approver; do not invent answers or permission. Keep normal parent decision gates. Handoff freshness check: Read HANDOFF.md and reports/HANDOFF.md if present for any concurrent collaborator update, await those results, and compare evidence with current inputs; rerun any probe whose input changed. Run the check before each of these, and dispatch each only in a later message than the check's Read results: the evidence helper's materialize, which runs once per report root and freezes the verdict; every gstack-review-log completed:true record, adversarial-review included, since a record is a completion, not preliminary bookkeeping; and the completion report. A later handoff read cannot validate an earlier materialize or completion. If a probe's snapshot differs from an earlier probe's, read reports/HANDOFF.md first; it names any collaborator change.\n\nQA fixture bindings: the deadline guard, capture, checkpoint and materialize command forms are in the installed qa/sections/exploratory.md this phase requires you to Read; use those forms verbatim with these bindings substituted literally. DEADLINE_TOOL = ${fixture.runtime}/bin/gstack-qa-deadline. EVIDENCE_TOOL = ${fixture.runtime}/bin/gstack-qa-evidence. PROBE_DIR = ${fixture.cwd}/reports. DEADLINE_FILE = ${fixture.cwd}/reports/deadline.json. SECONDS must be positive and at most 300. EARLIER_UTC is the optional caller absolute deadline: use the section clock's Hard deadline UTC, never its Runner entry UTC, reserve-start time or a clock-read time. The only native COMMAND is the existing diagnostic client, bun scripts/probe.ts, with zero or one literal argument. Resolve these exact helper and state paths; do not use variables, another helper, another state file, nested wrappers, scripts, operators or substitutions. Its diagnostic receipts are declared public/synthetic, so --public is approved. Only an explicit plan check this phase requires may use --timeout-ms 10000 in place of the deadline; every other capture, including a rerun or revalidation of an earlier probe, uses the deadline. This does not reset or bypass the smoke deadline. Only the deadline helper may create or change reports/deadline.json and its .qa-deadline- temporary files; never use Write/Edit/MultiEdit on those paths. Record the full outer command in checkpoints and evidence; keep the unchanged child JSON as observed, separate from prefixed guard diagnostics. A completed expired guard-run is not a probe or a pass: retain its unused checkpoint, report not-run coverage and do not restart the deadline. Keep the 12-probe smoke limit. Required suites and explicit plan checks are outside the bounded smoke budget, not permission to reset it. Quote arguments literally. Sources stay inside reports. Decide to execute the next probe before publishing its checkpoint, then await successful publication and dispatch that exact probe. If you defer an optional idea or stop exploration, do not publish a checkpoint for it; describe it as not run in Markdown. An unused checkpoint requires an actual authenticated expired-capture result; nearing the deadline or choosing to stop is not enough. A complete capture can truthfully retain a nonzero domain exit (including expected rejection or unavailable dependency); it is not automatically a pass. Wrapper-failed, interrupted or incomplete captures are not observations or passes; an authenticated expired capture can retain an unused checkpoint as not-run evidence. No new native child command or shell authority is granted.\n\nThe supported Bash interface is one literal documented native command or one exact generated workflow shell block with main substituted for and PLAN.md for ; even read-only commands must not be chained except for the exact DIFF_BASE preface below. Native CLI commands accept no argument or one literal argument: an unquoted shell-safe word, single-quoted text, or double-quoted text without expansion or escapes; no multiline arguments or shell composition. Inventory commands are pwd, bun --version, and ls with an optional combined -l/-a flag, optional -- separator, and literal path operands using the same quoting rules; operands beginning with - require --. Listing never permits other options, glob expansion, substitution, redirection or composition. The supported Git forms are git status --short, git status --porcelain, git branch --show-current, git rev-parse HEAD, git rev-parse --short HEAD, git merge-base origin/main HEAD, the plan-completion section's git log origin/main..HEAD --oneline, git diff (optional origin/main base and at most one output mode: --stat, --numstat, --name-only, or --name-status, before or after the base; optional literal pathspec arguments after --), git ls-files, and git ls-files --others --exclude-standard. Diff pathspecs use the same literal argument syntax as the CLI; quote globs so Git, not the shell, interprets them. The only variable-base form is DIFF_BASE=$(git merge-base origin/main HEAD) && git diff with exactly the double-quoted "$DIFF_BASE" base, the same optional output mode and literal pathspecs. That exact preface recomputes the base in the same command; other assignments and variable expansions are forbidden; the only additional command substitutions are the two installed bookkeeping fields described below. No other Git options, configuration overrides, --output, --no-index, --ext-diff, --textconv, external helpers or mutations are authorized. Exact generated workflow blocks remain allowed. Review bookkeeping permits the installed gstack-review-log start commands and single-quoted JSON records for review/adversarial-review (the installed quote-spliced double-quoted $(date -u +%Y-%m-%dT%H:%M:%SZ) is permitted only as the complete top-level timestamp value, and $(git rev-parse --short HEAD) only as the complete top-level commit value; no other substitutions, field placements or shell evaluation), gstack-review-read, gstack-specialist-stats, plus date -u +%Y-%m-%dT%H:%M:%SZ; use an original start token for a started attempt and completed:false/converged:false without a token for an unstarted one. These installed helpers are shell scripts: run them by exact installed path, never through bun or another interpreter. A review record fills this installed template, keeping its keys and adding none: \`${callerReviewRecordTemplate(fixture)}\`. A review record's status is clean only for a completed review with zero unresolved defects, otherwise issues_found${fixture.caller === 'ship' ? ' (unavailable for missing dispatched reviewer output)' : ''}; a review stopped at a gate records completed:false, never a receipt status such as blocked. Bookkeeping has an owned state directory and private Git object store; the real product, index, refs and config must stay unchanged. Outside providers are unavailable in this isolated fixture; the native reviewer is still required.${fixture.child ? ` Subagent dispatch uses a deterministic child transport instead of Agent/Task: when the workflow dispatches ${fixture.caller === 'review' ? 'the native adversarial reviewer (Step 4.8)' : 'the plan completion audit (Step 8)'}, complete its dispatch prerequisites, do not invoke Agent or Task, and Read ${fixture.child} as that child's complete final output; apply the parent's normal handling to it.` : ''} Other interpreters, shell composition outside those generated blocks or the exact DIFF_BASE preface, global setup and external operations are not authorized. Use Read/Glob/Grep for discovery instead of cat/head/tail; use native Write/Edit tools for any authorized file output. Pass these same command and write boundaries to any child; generated shell fragments remain verbatim apart from the declared substitutions.\n\nWrite the phase report to reports/review.md. Keep it compact (aim under 300 words): retain each required heading and field, link to reports/evidence.json and the checkpoints for details already recorded there, and do not restate the diff, the evidence rows or the review record. Also save reports/receipt.json with this machine-readable result shape: {\"status\":\"pass|fail|blocked|inconclusive\",\"probes\":[\"observed diagnostic receipt ids\"],\"remaining\":[\"names of incomplete contracts\"]}. The status is the overall supplied phase gate, not whether some probes passed: copy it from the verdict.status that materialize prints and writes to reports/evidence.json, and if that verdict is not pass, rerun what it lists as open before finishing when a pass is required. The probes array contains the exact id values from the captured child JSON (probe-...), never the helper's three-digit capture IDs or QA_EVIDENCE.id. Capture IDs select stored observations for checkpoint/materialize; child diagnostic IDs identify the actual probes in this final receipt. Read the child JSON's id for each probe used as evidence; do not derive it from a filename or execution counter. remaining names unresolved required contracts or gates in this phase. Pass requires no remaining required contracts or gates. Optional unavailable providers and later stages outside this excerpt are not required remainder. This format does not establish that any work succeeded. Stop at the end of this phase, or at its first unresolved approval gate.`, + prompt: `Load gstack's /${fixture.caller} supplied parent phase from caller-${fixture.caller}.md and resume it on the selected working-tree diff against origin/main. This excerpt comes from ${fixture.runtime}/${fixture.caller}/SKILL.md; resolve installed-relative references there, not from the excerpt file or product directory. That path identifies the asset base, not another entrypoint: do not read or invoke the full parent SKILL.md or rerun its preamble. Earlier preamble/branch/base setup is complete; use the existing local origin/main ref without fetch. Earlier-phase asset locators are ${fixture.runtime}/review/checklist.md and ${fixture.runtime}/qa/templates/functional-report-template.md. Read those files directly when referenced; recursive Glob does not follow the installed asset symlinks. Cross-project learnings are configured off in this owned fixture. ${fixture.reviewStart ? `The actual review-start helper already captured REVIEW_START=${fixture.reviewStart} for this unchanged core pass; retain that token. ` : ''} This fixture evaluates only the supplied parent phase, not later publication stages. The fixture owner recorded these read-only observations when this phase began; they are current until an input changes, so use them instead of re-running those commands, and recheck freshness before completion outputs:\n${fixture.startObservations}\n Read README.md for the project contract and commands. Use diagnostic-client commands such as \`bun scripts/probe.ts \` for exploratory discoveries and their checkpoint evidence. A required \`bun run test\` is separate suite verification: report it as verification, never as a diagnostic observation or checkpoint anchor/target. Use the production evidence helper to publish each diagnostic checkpoint as \`reports/exploration-NNN.json\`, not inside a nested directory; do not transcribe its observed payload. ${fixture.caseId === 'ship-exploratory-plan-checks' ? 'The previously discovered plan is PLAN.md.' : 'No plan file was found.'} There is no remote service and no release publication is authorized. There is no interactive approver; do not invent answers or permission. Keep normal parent decision gates. Handoff freshness check: Read HANDOFF.md and reports/HANDOFF.md if present for any concurrent collaborator update, await those results, and compare evidence with current inputs; rerun any probe whose input changed. Run the check before each of these, and dispatch each only in a later message than the check's Read results: the evidence helper's materialize, which runs once per report root and freezes the verdict; every gstack-review-log completed:true record, adversarial-review included, since a record is a completion, not preliminary bookkeeping; and the completion report. A later handoff read cannot validate an earlier materialize or completion. If a probe's snapshot differs from an earlier probe's, read reports/HANDOFF.md first; it names any collaborator change.\n\nQA fixture bindings: the deadline guard, capture, checkpoint and materialize command forms are in the installed qa/sections/exploratory.md this phase requires you to Read; use those forms verbatim with these bindings substituted literally. DEADLINE_TOOL = ${fixture.runtime}/bin/gstack-qa-deadline. EVIDENCE_TOOL = ${fixture.runtime}/bin/gstack-qa-evidence. PROBE_DIR = ${fixture.cwd}/reports. DEADLINE_FILE = ${fixture.cwd}/reports/deadline.json. SECONDS must be positive and at most 300. EARLIER_UTC is the optional caller absolute deadline: use the section clock's Hard deadline UTC, never its Runner entry UTC, reserve-start time or a clock-read time. The only native COMMAND is the existing diagnostic client, bun scripts/probe.ts, with zero or one literal argument. Resolve these exact helper and state paths; do not use variables, another helper, another state file, nested wrappers, scripts, operators or substitutions. Its diagnostic receipts are declared public/synthetic, so --public is approved. Only an explicit plan check this phase requires may use --timeout-ms 10000 in place of the deadline; every other capture, including a rerun or revalidation of an earlier probe, uses the deadline. This does not reset or bypass the smoke deadline. Only the deadline helper may create or change reports/deadline.json and its .qa-deadline- temporary files; never use Write/Edit/MultiEdit on those paths. Record the full outer command in checkpoints and evidence; keep the unchanged child JSON as observed, separate from prefixed guard diagnostics. A completed expired guard-run is not a probe or a pass: retain its unused checkpoint, report not-run coverage and do not restart the deadline. Keep the 12-probe smoke limit. Required suites and explicit plan checks are outside the bounded smoke budget, not permission to reset it. Quote arguments literally. Sources stay inside reports. Decide to execute the next probe before publishing its checkpoint, then await successful publication and dispatch that exact probe. If you defer an optional idea or stop exploration, do not publish a checkpoint for it; describe it as not run in Markdown. An unused checkpoint requires an actual authenticated expired-capture result; nearing the deadline or choosing to stop is not enough. A complete capture can truthfully retain a nonzero domain exit (including expected rejection or unavailable dependency); it is not automatically a pass. Wrapper-failed, interrupted or incomplete captures are not observations or passes; an authenticated expired capture can retain an unused checkpoint as not-run evidence. No new native child command or shell authority is granted.\n\nThe supported Bash interface is one literal documented native command or one exact generated workflow shell block with main substituted for and PLAN.md for ; even read-only commands must not be chained except for the two DIFF_BASE forms below. Native CLI commands accept no argument or one literal argument: an unquoted shell-safe word, single-quoted text, or double-quoted text without expansion or escapes; no multiline arguments or shell composition. Inventory commands are pwd, bun --version, and ls with an optional combined -l/-a flag, optional -- separator, and literal path operands using the same quoting rules; operands beginning with - require --. Listing never permits other options, glob expansion, substitution, redirection or composition. The supported Git forms are git status --short, git status --porcelain, git branch --show-current, git rev-parse HEAD, git rev-parse --short HEAD, git merge-base origin/main HEAD, the plan-completion section's git log origin/main..HEAD --oneline, git diff (optional origin/main base and at most one output mode: --stat, --numstat, --name-only, or --name-status, before or after the base; optional literal pathspec arguments after --), git ls-files, and git ls-files --others --exclude-standard. Diff pathspecs use the same literal argument syntax as the CLI; quote globs so Git, not the shell, interprets them. The only variable-base form is DIFF_BASE=$(git merge-base origin/main HEAD) && git diff with exactly the double-quoted "$DIFF_BASE" base, the same optional output mode and literal pathspecs. That exact preface recomputes the base in the same command. The merge base itself, the sha git merge-base origin/main HEAD prints (at least 7 characters), may replace origin/main as a literal diff base, and the review army's specialist form DIFF_BASE= && git diff "$DIFF_BASE" is the one other permitted assignment; both take the same optional output mode and literal pathspecs. Other assignments and variable expansions are forbidden; the only additional command substitutions are the two installed bookkeeping fields described below. No other Git options, configuration overrides, --output, --no-index, --ext-diff, --textconv, external helpers or mutations are authorized. Exact generated workflow blocks remain allowed. Review bookkeeping permits the installed gstack-review-log start commands and single-quoted JSON records for review/adversarial-review (the installed quote-spliced double-quoted $(date -u +%Y-%m-%dT%H:%M:%SZ) is permitted only as the complete top-level timestamp value, and $(git rev-parse --short HEAD) only as the complete top-level commit value; no other substitutions, field placements or shell evaluation), gstack-review-read, gstack-specialist-stats, ${fixture.caller === 'ship' ? 'the installed gstack-review-plan forms Step 9 names (--base main with its --pass, --last and finding flags, or --fix-check-prompt with literal --fix-list, --hunks and --tests files), ' : ''}plus date -u +%Y-%m-%dT%H:%M:%SZ; use an original start token for a started attempt and completed:false/converged:false without a token for an unstarted one. These installed helpers are shell scripts: run them by exact installed path, never through bun or another interpreter. A review record fills this installed template, keeping its keys and adding none: \`${callerReviewRecordTemplate(fixture)}\`. A review record's status is clean only for a completed review with zero unresolved defects, otherwise issues_found${fixture.caller === 'ship' ? ' (unavailable for missing dispatched reviewer output)' : ''}; a review stopped at a gate records completed:false, never a receipt status such as blocked. Bookkeeping has an owned state directory and private Git object store; the real product, index, refs and config must stay unchanged. Outside providers are unavailable in this isolated fixture; the native reviewer is still required.${fixture.caller === 'ship' ? " Step 9 dispatches its review plan's specialist and Red Team lines. The plan's adversarial-native and outside-voice lines are Step 11's voices, which Step 11 runs in a later stage outside this excerpt; they are not remaining work of this phase." : ''}${fixture.child ? ` Subagent dispatch uses a deterministic child transport instead of Agent/Task: when the workflow dispatches ${fixture.caller === 'review' ? 'the native adversarial reviewer (Step 4.8)' : 'the plan completion audit (Step 8)'}, complete its dispatch prerequisites, do not invoke Agent or Task, and Read ${fixture.child} as that child's complete final output; apply the parent's normal handling to it.` : ''} Other interpreters, shell composition outside those generated blocks or the two DIFF_BASE forms, global setup and external operations are not authorized. Use Read/Glob/Grep for discovery instead of cat/head/tail; use native Write/Edit tools for any authorized file output. Pass these same command and write boundaries to any child; generated shell fragments remain verbatim apart from the declared substitutions.\n\nWrite the phase report to reports/review.md. Keep it compact (aim under 300 words): retain each required heading and field, link to reports/evidence.json and the checkpoints for details already recorded there, and do not restate the diff, the evidence rows or the review record. Also save reports/receipt.json with this machine-readable result shape: {\"status\":\"pass|fail|blocked|inconclusive\",\"probes\":[\"observed diagnostic receipt ids\"],\"remaining\":[\"names of incomplete contracts\"]}. The status is the overall supplied phase gate, not whether some probes passed: copy it from the verdict.status that materialize prints and writes to reports/evidence.json, and if that verdict is not pass, rerun what it lists as open before finishing when a pass is required. The probes array contains the exact id values from the captured child JSON (probe-...), never the helper's three-digit capture IDs or QA_EVIDENCE.id. Capture IDs select stored observations for checkpoint/materialize; child diagnostic IDs identify the actual probes in this final receipt. Read the child JSON's id for each probe used as evidence; do not derive it from a filename or execution counter. remaining names unresolved required contracts or gates in this phase. Pass requires no remaining required contracts or gates. Optional unavailable providers and later stages outside this excerpt are not required remainder. This format does not establish that any work succeeded. Stop at the end of this phase, or at its first unresolved approval gate.`, appendSystemPrompt: `Caller execution scheduling (fixture contract): This session has at most 25 assistant turns, including required verification and final artifacts. The command boundary applies to each Bash call, not to the number of independent tool calls in an assistant turn. After required clock and approval prerequisites settle, issue independent source Reads and read-only discovery together as separate native tool calls once their paths and inputs are known. Read-only discovery is Read, Glob, Grep and the declared inventory, Git and review-read forms; a capture, a deadline guard start or run, or a diagnostic-client run is a probe, never discovery. For example, after the first clock read, one response can Read the caller excerpt, README.md, every product file and the referenced checklist and templates. Wait for their results before decisions that depend on them. diff --git a/test/qa-caller-review-plan.test.ts b/test/qa-caller-review-plan.test.ts new file mode 100644 index 000000000..0db0b6890 --- /dev/null +++ b/test/qa-caller-review-plan.test.ts @@ -0,0 +1,137 @@ +/** + * #3527 put Step 9's review plan into ship/sections/review-army.md as prose, so the qa-callers transport + * neither admitted the helper nor said where the plan's Step 11 voices run. + * - Paid screen 40707142223 (ship-exploratory-plan-checks, slice 10 and its rerun): the only refused call in + * both trials was `/bin/gstack-review-plan --base main`. + * - Train 35 rebuild gate 40664336141 (ship-exploratory-late-input, slice 9): the actor never ran the plan, + * read Step 11's native adversarial pass as owed by this Steps 8-9 excerpt, and reported pass with it remaining. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { bunIsolationArgs } from '../lib/bun-isolation'; +import { + createQaCallerFixture, qaCallerCommandAllowed, qaCallerSessionOptions, validateCallerEvidence, + type QaCallerCase, type QaCallerFixture, +} from './helpers/qa-callers-fixture'; + +const captured = JSON.parse(fs.readFileSync(path.join(import.meta.dir, 'fixtures/qa-caller-review-plan-40707142223.json'), 'utf8')); +const specialistDiff = JSON.parse(fs.readFileSync(path.join(import.meta.dir, 'fixtures/qa-caller-merge-base-diff-40737741454.json'), 'utf8')); +const SHIP_CASES: QaCallerCase[] = ['ship-exploratory-small-cli', 'ship-exploratory-unavailable', 'ship-exploratory-plan-checks', 'ship-exploratory-late-input']; +const fixtures: QaCallerFixture[] = []; +afterEach(async () => { + for (const fixture of fixtures.splice(0)) { await fixture.close(); fs.rmSync(fixture.root, { recursive: true, force: true }); } +}); +const fixtureFor = (caseId: QaCallerCase, installRuntime = false) => { + const fixture = createQaCallerFixture(caseId, { installRuntime }); + fixtures.push(fixture); + return fixture; +}; +const context = (fixture: QaCallerFixture) => ({ runtime: fixture.runtime, fixtureRoot: fixture.cwd }); + +describe('ship Step 9 review plan through the qa-callers transport', () => { + for (const [run, trial] of Object.entries(captured.runs) as [string, { root: string; commands: { command: string }[] }][]) { + test(`captured ${run}: every Bash call the actor made is inside the declared interface`, () => { + const fixture = fixtureFor('ship-exploratory-plan-checks', true); + const commands = trial.commands.map(({ command }) => command.replaceAll(trial.root, fixture.root)); + expect(commands.filter(command => command.includes('/bin/gstack-review-plan '))).toEqual([`${fixture.runtime}/bin/gstack-review-plan --base main`]); + expect(commands.filter(command => !qaCallerCommandAllowed(command, fixture.workflowCommands, context(fixture)))).toEqual([]); + }); + } + + test('the helper forms Step 9 names are admitted at the exact installed path, and nothing else is', () => { + const fixture = fixtureFor('ship-exploratory-late-input'); + const helper = `${fixture.runtime}/bin/gstack-review-plan`; + const allowed = (command: string) => qaCallerCommandAllowed(command, fixture.workflowCommands, context(fixture)); + for (const args of ['--base main', '--base main --pass 1 --last full', '--base main --pass 2 --last fix-check --fixed-below --adversarial-ran', + '--base main --pass 1 --last full --blocking-open --touched-gate --critical-found --reviewed-numbers fp:0a1b', + '--fix-check-prompt --fix-list reports/fix-list.md --hunks reports/hunks.diff --tests reports/tests.txt', + "--fix-check-prompt --fix-list 'reports/fix list.md' --hunks reports/h.diff --tests reports/t.txt --callers reports/c.txt"]) { + expect(allowed(`${helper} ${args}`), args).toBe(true); + } + for (const command of [`${helper} --base origin/main`, `${helper} --base main --json`, `${helper} --base main --force all`, + `${helper} --base main --last full --pass 1`, helper, `${helper} --base main | tee reports/plan.txt`, `${helper} --base main; rm -rf reports`, + `${helper} --base main && git reset --hard`, `${helper} --base "$(git rev-parse HEAD)"`, `${helper} --fix-check-prompt --fix-list $HOME/x --hunks a --tests b`, + `bun ${helper} --base main`, `${helper}x --base main`, `${fixture.runtime}/bin/gstack-review-planner --base main`, + '/tmp/elsewhere/bin/gstack-review-plan --base main', '~/.claude/skills/gstack/bin/gstack-review-plan --base main']) { + expect(allowed(command), command).toBe(false); + } + expect(qaCallerCommandAllowed(`${helper} --base main`, fixture.workflowCommands)).toBe(false); + }); + + test.each(SHIP_CASES)('%s: every reviewer the real plan dispatches has a declared route in the transport', caseId => { + const fixture = fixtureFor(caseId, true); + const options = qaCallerSessionOptions(fixture, 'free-review-plan'); + expect(qaCallerCommandAllowed(`${fixture.runtime}/bin/gstack-review-plan --base main`, fixture.workflowCommands, context(fixture))).toBe(true); + const plan = spawnSync(process.execPath, [...bunIsolationArgs(), path.join(fixture.runtime, 'bin/gstack-review-plan'), '--base', 'main'], + { cwd: fixture.cwd, env: { ...process.env, ...options.env }, encoding: 'utf8', timeout: 60_000 }); + expect(plan.status, plan.stderr).toBe(0); + const roles = [...plan.stdout.matchAll(/^DISPATCH: (\S+)/gm)].map(match => match[1]); + expect(roles).toContain('adversarial-native'); + const prompt = options.prompt.toLowerCase(); + for (const role of roles) expect(prompt.includes(role) || prompt.includes(role.replace('-', ' ')), role).toBe(true); + expect(options.prompt).toContain("The plan's adversarial-native and outside-voice lines are Step 11's voices, which Step 11 runs in a later stage outside this excerpt; they are not remaining work of this phase."); + }); + + test('review keeps its own Step 4.8 transport and is not told about ship Step 11', () => { + const prompt = qaCallerSessionOptions(fixtureFor('review-exploratory-small-cli'), 'free-review-plan').prompt; + expect(prompt).toContain('the native adversarial reviewer (Step 4.8)'); + expect(prompt).not.toContain('gstack-review-plan'); + expect(prompt).not.toContain("Step 11's voices"); + }); + + test('captured 40664336141 late-input receipt: pass with remaining work is still a false green', () => { + const receipt = captured.lateInputReceipt; + expect(receipt.remaining).toContain('Step 11 native adversarial review (not dispatched)'); + const reportRoot = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'qc-late-')); + const validate = (remaining: string[]) => validateCallerEvidence({ caller: 'ship', result: { exitReason: 'success', transcript: [] }, probes: [], + receipt: { status: receipt.status, probes: [], remaining }, currentSnapshot: 'none', requiredCharters: [], mutations: [], observerComplete: true, + reportRoot, checkpointFiles: {}, reportMarkdown: '' }); + try { + expect(validate(receipt.remaining)).toContain('blocked, failing or incomplete coverage reported green'); + expect(validate([])).not.toContain('blocked, failing or incomplete coverage reported green'); + } finally { fs.rmSync(reportRoot, { recursive: true, force: true }); } + }); +}); + +/** + * Paid screen 40737741454 (ship-exploratory-unavailable, slice 11 and its rerun): the plan dispatched the + * state-machines specialist, review-army tells the parent to write the merge-base sha into the specialist's + * diff command, and the only refused call in both trials was that child's `git diff `. + */ +describe('review-army specialist diff at the literal merge base', () => { + const mergeBase = (fixture: QaCallerFixture) => spawnSync('git', ['merge-base', 'origin/main', 'HEAD'], { cwd: fixture.cwd, encoding: 'utf8', timeout: 5000 }).stdout.trim(); + + for (const [run, trial] of Object.entries(specialistDiff.runs) as [string, { root: string; mergeBase: string; commands: { command: string }[] }][]) { + test(`captured ${run}: every Bash call, the specialist child's included, is inside the declared interface`, () => { + const fixture = fixtureFor('ship-exploratory-unavailable', true); + const base = mergeBase(fixture).slice(0, trial.mergeBase.length); + const commands = trial.commands.map(({ command }) => command.replaceAll(trial.root, fixture.root).replaceAll(trial.mergeBase, base)); + expect(commands).toContain(`git diff ${base}`); + expect(commands.filter(command => !qaCallerCommandAllowed(command, fixture.workflowCommands, context(fixture)))).toEqual([]); + }); + } + + test('only the fixture merge base is admitted, bare or in the specialist DIFF_BASE form, and the prompt declares it', () => { + const fixture = fixtureFor('ship-exploratory-unavailable'); + const base = mergeBase(fixture); + const allowed = (command: string) => qaCallerCommandAllowed(command, fixture.workflowCommands, context(fixture)); + for (const command of [`git diff ${base}`, `git diff ${base.slice(0, 7)}`, `git diff --stat ${base.slice(0, 12)}`, `git diff ${base} -- scale.ts`, + `DIFF_BASE=${base} && git diff "$DIFF_BASE"`, `DIFF_BASE=${base.slice(0, 9)} && git diff --name-status "$DIFF_BASE"`]) { + expect(allowed(command), command).toBe(true); + } + const other = base.replace(/^./, base[0] === '0' ? '1' : '0'); + for (const command of [`git diff ${other}`, `git diff ${base.slice(0, 6)}`, `git diff ${base} --output=reports/x`, `git diff --stat ${base} --numstat`, + `DIFF_BASE=${other} && git diff "$DIFF_BASE"`, `DIFF_BASE=${base} && git diff ${base}`, `DIFF_BASE=${base}; git diff "$DIFF_BASE"`, + `git diff "$DIFF_BASE"`, `DIFF_BASE=${base} && git diff "$DIFF_BASE" && git status --short`, `git diff ${base.toUpperCase()}`]) { + expect(allowed(command), command).toBe(false); + } + expect(qaCallerCommandAllowed(`git diff ${base}`, fixture.workflowCommands)).toBe(false); + for (const caseId of ['ship-exploratory-unavailable', 'review-exploratory-small-cli'] as QaCallerCase[]) { + expect(qaCallerSessionOptions(fixtureFor(caseId), 'free-review-plan').prompt).toContain( + 'The merge base itself, the sha git merge-base origin/main HEAD prints (at least 7 characters), may replace origin/main as a literal diff base'); + } + }); +}); diff --git a/test/shared-libs-index-flags-readonly.test.ts b/test/shared-libs-index-flags-readonly.test.ts new file mode 100644 index 000000000..c2e0c2608 --- /dev/null +++ b/test/shared-libs-index-flags-readonly.test.ts @@ -0,0 +1,41 @@ +/** + * Train gate 40619718717 (798a37d8) slice 14, shared-libs-review-index-flags, assume-unchanged trial: while + * gathering evidence the actor ran `git update-index --really-refresh`, which cleared the assume-unchanged flag + * the checker reads, so the prerequisite binding went stale. The skill's evidence step now says it is read-only. + */ +import { afterAll, expect, test } from 'bun:test'; +import { execFileSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { generateSharedCodeReuse } from '../scripts/resolvers/review-scope'; +import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; +import { fixtureGit, type SharedLibsFixture } from './helpers/shared-libs-eval-fixture'; +import { checkPathReviewPrerequisites, preparePathEligibilityFixture } from './helpers/shared-libs-path-fixture'; + +const fixtures: SharedLibsFixture[] = []; +afterAll(() => { for (const f of fixtures) fs.rmSync(f.root, { recursive: true, force: true }); }); + +test.each((['review', 'ship'] as const).flatMap(skill => (Object.keys(HOST_PATHS) as TemplateContext['host'][]).map(host => [skill, host] as const)))('%s on %s: the shared-code evidence step forbids the index refresh the captured actor ran', (skillName, host) => { + const section = generateSharedCodeReuse({ skillName, tmplPath: '', host, paths: HOST_PATHS[host] }); + const step = section.slice(section.indexOf('1. **Read the evidence.**'), section.indexOf('2. **Run the checker.**')).replace(/\s+/g, ' '); + expect(step).toContain('read-only: `git update-index`, even `--really-refresh`, clears the index flags the checker reads'); +}); + +test.each([ + ['captured refresh', 'git update-index --really-refresh >/dev/null 2>&1; echo "(no index mutation above intended)"', false], + ['captured read-only inspection', 'git ls-files -v; git status --porcelain=v1; git hash-object --no-filters -- src/retry-route.ts; git ls-files --debug src/retry-route.ts', true], +] as const)('%s against the assume-unchanged fixture: the prerequisite detector sees it (%s)', (_, command, unchanged) => { + const prepared = preparePathEligibilityFixture('assume-unchanged'); + const f = prepared.fixture; + fixtures.push(f); + const before = checkPathReviewPrerequisites(f, prepared.resumed.input); + expect(before.settled).toBe(true); + expect(fixtureGit(f, 'ls-files', '-v', 'src/retry-route.ts')).toMatch(/^h /); + execFileSync('bash', ['-c', command], { cwd: f.repo, env: { ...process.env, ...f.env }, encoding: 'utf8', timeout: 30_000 }); + const after = checkPathReviewPrerequisites(f, prepared.resumed.input); + if (unchanged) expect(after).toEqual(before); + else { + expect(fixtureGit(f, 'ls-files', '-v', 'src/retry-route.ts')).toMatch(/^H /); + expect({ current: after.current, settled: after.settled }).toEqual({ current: false, settled: false }); + } +});