From c3baa46815e6b33074ffb5d89e255aa296da7fa9 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 1 Nov 2026 16:17:51 +0000 Subject: [PATCH] fix(flaky): coverage-audit wc -c and bounded-display credit, tpa-broken Aside absence, register docsync fixture (GSTA-240 follow-up) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/capy-gsta240-followup.md | 3 + test/coverage-audit-evidence.test.ts | 66 +++++++++++++++++++ ...audit-plan-eng-grep-head-before-reads.json | 24 +++++++ .../coverage-audit-plan-eng-wc-bytes.json | 24 +++++++ ...d-party-actions-broken-absence-reason.json | 19 ++++++ test/helpers/coverage-audit-evidence.ts | 31 ++++++--- test/helpers/free-fixtures-data.ts | 1 + test/helpers/third-party-actions.ts | 12 +++- test/third-party-actions.test.ts | 27 ++++++++ 9 files changed, 196 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/coverage-audit-plan-eng-grep-head-before-reads.json create mode 100644 test/fixtures/coverage-audit-plan-eng-wc-bytes.json create mode 100644 test/fixtures/third-party-actions-broken-absence-reason.json diff --git a/changelog.d/capy-gsta240-followup.md b/changelog.d/capy-gsta240-followup.md index 9bd397458..ffce59b29 100644 --- a/changelog.d/capy-gsta240-followup.md +++ b/changelog.d/capy-gsta240-followup.md @@ -6,5 +6,8 @@ - **`.gstack/pregate.json`**: the local pregate lane runs `bun test --timeout=30000 {files}`, the 30 s per-test timeout CI and the remote pregate already use, so a case that takes 5 s on a busy machine no longer fails only locally. - **`test/gstack-ship-receipt.test.ts`**: the full `write` case, about 62 git calls, gets a 15 s timeout instead of bun's 5 s default. +- **`test/helpers/coverage-audit-evidence.ts`**: the plan-eng coverage audit detector credits complete reads proved by a literal `wc -c` byte count, and by a bounded display (`grep | head`) when exactly one copy of the reads follows; an unbounded display, a second copy or a tampered read still gets no credit (train gates 41209768188, 41238366045). +- **`test/helpers/third-party-actions.ts`**: a statement that Aside is not available ("Recommended, since Aside isn't available") no longer counts as offering an Aside drive in the tpa-broken check; any other Aside mention in the option still does (gate 41246728073). + ### Changed - `test/helpers/docsync-contract.ts` names the missing `documentation_section` when a document-release completion fails the contract, so the next paid red says which field was absent. diff --git a/test/coverage-audit-evidence.test.ts b/test/coverage-audit-evidence.test.ts index caad2efc2..7724305ac 100644 --- a/test/coverage-audit-evidence.test.ts +++ b/test/coverage-audit-evidence.test.ts @@ -20,6 +20,8 @@ import captured_cd_chain from './fixtures/coverage-audit-cd-chain.json'; import captured_wc_anchor from './fixtures/coverage-audit-wc-anchor.json'; import captured_separator from './fixtures/coverage-audit-separator-collision.json'; import captured_two_file_grep from './fixtures/coverage-audit-two-file-grep.json'; +import captured_plan_eng_wc_bytes from './fixtures/coverage-audit-plan-eng-wc-bytes.json'; +import captured_plan_eng_grep_head from './fixtures/coverage-audit-plan-eng-grep-head-before-reads.json'; const clone = (v:T):T => structuredClone(v); const diagram = '```text\nsrc/billing.ts\n├── processPayment: happy path [TESTED]\n└── refundPayment [UNTESTED]\n```'; @@ -1289,3 +1291,67 @@ describe('coverage audit native evidence: an escaped grep over two literal files }); }); + +const capturedCall = (capture: typeof captured_plan_eng_wc_bytes) => { + const both = { sourceRead: true, testsRead: true }, neither = { sourceRead: false, testsRead: false }; + const files = { cwd: capture.cwd, source: { path: capture.cwd + '/src/billing.ts', content: capture.files.source }, + tests: { path: capture.cwd + '/test/billing.test.ts', content: capture.files.tests } }; + const transcript = (command: string, output: string) => [ + { type: 'system', subtype: 'init', session_id: capture.sessionId, cwd: capture.cwd }, + { type: 'assistant', session_id: capture.sessionId, parent_tool_use_id: null, message: { role: 'assistant', content: [{ type: 'tool_use', id: capture.call.toolUseId, name: 'Bash', input: { command } }] } }, + { type: 'user', session_id: capture.sessionId, parent_tool_use_id: null, message: { role: 'user', content: [{ type: 'tool_result', tool_use_id: capture.call.toolUseId, is_error: capture.call.isError, content: output }] } }, + ]; + const reads = (command = capture.call.command, output = capture.call.output) => coverageAuditReadEvidence(transcript(command, output), files); + const passes = () => coverageAuditVerdict({ exitReason: 'success', browseErrors: [], output: capture.finalOutput, + transcript: transcript(capture.call.command, capture.call.output) } as any, files); + const nonce = capture.files.source.match(/coverage-read-evidence: (\S+)/)![1]!; + return { both, neither, reads, passes, nonce }; +}; + +describe('coverage audit native evidence: a wc -c byte count after both reads (CI 41209768188 slice 5, plan-eng)', () => { + const capture = captured_plan_eng_wc_bytes, { both, neither, reads, passes, nonce } = capturedCall(capture); + const count = 'wc -c plan-eng-review/sections/review-sections.md'; + + test('both complete reads before a literal wc -c count are credited and the recorded run passes', () => { + expect(capture.provenance.recordedPassed).toBe(false); + expect(capture.call.command).toContain('; cat -n src/billing.ts; echo ----; cat -n test/billing.test.ts; echo ---; ' + count + '; grep -n'); + expect(reads()).toEqual(both); + expect(passes()).toEqual({ ...both, diagram: true, passed: true, failures: [] }); + }); + + test('a wc -c operand that is not a literal in-cwd file, or a tampered read, still gets no credit', () => { + for (const operands of ['../outside.md', '/etc/passwd', '"$(touch x)"', '-', 'plan-eng-review/SKILL.md > out.txt', '--files0-from=list']) { + expect(reads(capture.call.command.replace(count, 'wc -c ' + operands)), operands).toEqual(neither); + } + expect(reads(capture.call.command.replace(count, 'wc -w plan-eng-review/sections/review-sections.md'))).toEqual(neither); + expect(reads(capture.call.command, capture.call.output.replace(nonce, nonce.slice(1) + '0'))).toMatchObject({ sourceRead: false }); + expect(reads(capture.call.command.replace('cat -n test/billing.test.ts', 'cat -n test/other.test.ts'))).toMatchObject({ testsRead: false }); + }); +}); + +describe('coverage audit native evidence: owned reads after a bounded grep | head display (CI 41238366045 slice 5, plan-eng)', () => { + const capture = captured_plan_eng_grep_head, { both, neither, reads, passes, nonce } = capturedCall(capture); + const display = 'grep -n -i "test review\\|coverage diagram" plan-eng-review/SKILL.md | head;'; + const run = capture.call.output.slice(capture.call.output.indexOf('\n') + 1); + + test('both complete reads after a bounded display and an exact count are credited and the recorded run passes', () => { + expect(capture.provenance.recordedPassed).toBe(false); + expect(capture.call.command).toContain('; ' + display + ' wc -l plan-eng-review/sections/review-sections.md; cat -n src/billing.ts; echo ----; cat -n test/billing.test.ts'); + expect(reads()).toEqual(both); + expect(passes()).toEqual({ ...both, diagram: true, passed: true, failures: [] }); + const lines = (n: number) => Array.from({ length: n }, (_, i) => 'SKILL.md:' + (i + 1) + ': test review').join('\n') + '\n'; + expect(reads(capture.call.command, lines(10) + capture.call.output)).toEqual(both); + }); + + test('display output beyond its bound, a second copy of the reads, an unbounded display or a tampered read gets no credit', () => { + const lines = (n: number) => Array.from({ length: n }, (_, i) => 'SKILL.md:' + (i + 1) + ': test review').join('\n') + '\n'; + expect(reads(capture.call.command, lines(11) + capture.call.output)).toEqual(neither); + expect(reads(capture.call.command.replace('| head;', '| head -60;'), run + '\n' + capture.call.output)).toEqual(neither); + expect(reads(capture.call.command, capture.call.output + '\n' + run)).toEqual(neither); + for (const tail of ['| head -c 400;', ';', '| cat;']) { + expect(reads(capture.call.command.replace('| head;', tail)), tail).toEqual(neither); + } + expect(reads(capture.call.command, capture.call.output.replace(nonce, nonce.slice(1) + '0'))).toMatchObject({ sourceRead: false }); + expect(reads(capture.call.command.replace('cat -n test/billing.test.ts', 'cat -n test/other.test.ts'))).toMatchObject({ testsRead: false }); + }); +}); diff --git a/test/fixtures/coverage-audit-plan-eng-grep-head-before-reads.json b/test/fixtures/coverage-audit-plan-eng-grep-head-before-reads.json new file mode 100644 index 000000000..e1d94e791 --- /dev/null +++ b/test/fixtures/coverage-audit-plan-eng-grep-head-before-reads.json @@ -0,0 +1,24 @@ +{ + "provenance": { + "source": "https://github.com/garrytan/gstack/actions/runs/41238366045 eval slice 5 (train gate, train 0926fe2e), plan-eng-coverage-audit (claude-opus-5-5); the second parent Bash call and its result, byte-exact", + "recordedPassed": false, + "recordedFailures": [ + "missing successful source-file read", + "missing successful test-file read" + ], + "note": "The first call printed 109 KB and the native result kept only a preview, so it delivered neither file. The second call ran a grep | head display (0 to 10 lines) and a wc -l count before reading both files completely with no label in between; an owned run after unknown output of inexact length was refused outright, so the whole chain got no credit." + }, + "cwd": "/home/runner/.cache/gstack-paid-shard-mTTODF/tmp/skill-e2e-plan-eng-review-coverage-uevVNY", + "sessionId": "8186288c-6f03-42de-87f6-64bf80fc74cf", + "call": { + "toolUseId": "toolu_01X9wLDqv1PS1FuGjcidiZoF", + "command": "cd /home/runner/.cache/gstack-paid-shard-mTTODF/tmp/skill-e2e-plan-eng-review-coverage-uevVNY; grep -n -i \"test review\\|coverage diagram\" plan-eng-review/SKILL.md | head; wc -l plan-eng-review/sections/review-sections.md; cat -n src/billing.ts; echo ----; cat -n test/billing.test.ts", + "isError": false, + "output": "1585 plan-eng-review/sections/review-sections.md\n 1\t\n 2\texport function processPayment(amount: number, currency: string) {\n 3\t if (amount <= 0) throw new Error('Invalid amount');\n 4\t if (currency !== 'USD' && currency !== 'EUR') throw new Error('Unsupported currency');\n 5\t return { status: 'success', amount, currency };\n 6\t}\n 7\t\n 8\texport function refundPayment(paymentId: string, reason: string) {\n 9\t if (!paymentId) throw new Error('Payment ID required');\n 10\t if (!reason) throw new Error('Reason required');\n 11\t return { status: 'refunded', paymentId, reason };\n 12\t}\n 13\t\n 14\t// coverage-read-evidence: 3320eaa9-e797-42f5-80d3-4faa22c320e0\n----\n 1\t\n 2\timport { describe, test, expect } from 'vitest';\n 3\timport { processPayment } from '../src/billing';\n 4\t\n 5\tdescribe('processPayment', () => {\n 6\t test('processes valid payment', () => {\n 7\t const result = processPayment(100, 'USD');\n 8\t expect(result.status).toBe('success');\n 9\t });\n 10\t // GAP: no test for invalid amount\n 11\t // GAP: no test for unsupported currency\n 12\t // GAP: refundPayment not tested at all\n 13\t});\n 14\t\n 15\t// coverage-read-evidence: 93992020-084f-411f-8800-10e1e9885342" + }, + "files": { + "source": "\nexport function processPayment(amount: number, currency: string) {\n if (amount <= 0) throw new Error('Invalid amount');\n if (currency !== 'USD' && currency !== 'EUR') throw new Error('Unsupported currency');\n return { status: 'success', amount, currency };\n}\n\nexport function refundPayment(paymentId: string, reason: string) {\n if (!paymentId) throw new Error('Payment ID required');\n if (!reason) throw new Error('Reason required');\n return { status: 'refunded', paymentId, reason };\n}\n\n// coverage-read-evidence: 3320eaa9-e797-42f5-80d3-4faa22c320e0\n", + "tests": "\nimport { describe, test, expect } from 'vitest';\nimport { processPayment } from '../src/billing';\n\ndescribe('processPayment', () => {\n test('processes valid payment', () => {\n const result = processPayment(100, 'USD');\n expect(result.status).toBe('success');\n });\n // GAP: no test for invalid amount\n // GAP: no test for unsupported currency\n // GAP: refundPayment not tested at all\n});\n\n// coverage-read-evidence: 93992020-084f-411f-8800-10e1e9885342\n" + }, + "finalOutput": "**Section 3. Test review: targeted audit of `src/billing.ts` against `test/billing.test.ts`**\n\nThe tests use Vitest (`import ... from 'vitest'` in `test/billing.test.ts:2`). I read both files only and did not run the tests.\n\n```\nCODE PATHS USER FLOWS\n[+] src/billing.ts [+] Take a payment\n ├── processPayment(amount, currency) :2-6 ├── [GAP] [→E2E] Pay → success response\n │ ├── [★★ TESTED] Valid USD → {status:'success'} └── [GAP] [→E2E] Pay with bad input → error shown\n │ │ — billing.test.ts:6 (checks .status \n │ │ only, not amount/currency) [+] Refund a payment\n │ ├── [GAP] Valid EUR → success (other side ├── [GAP] [→E2E] Refund → refunded response\n │ │ of the currency guard) :4 └── [GAP] Refund without reason → error\n │ ├── [GAP] amount <= 0 → throw \n │ │ 'Invalid amount' (0, -1) :3 [+] Error states\n │ └── [GAP] currency ∉ {USD,EUR} → throw ├── [GAP] 'Invalid amount' surfaced to caller\n │ 'Unsupported currency' :4 └── [GAP] 'Unsupported currency' surfaced\n │ (e.g. 'GBP', 'usd', '') \n └── refundPayment(paymentId, reason) :8-12 \n ├── [GAP] !paymentId ('') → throw \n │ 'Payment ID required' :9 \n ├── [GAP] !reason ('') → throw \n │ 'Reason required' :10 \n └── [GAP] Valid → {status:'refunded', \n paymentId, reason} :11 \n (never imported or called by tests)\n\nCOVERAGE: 1/13 paths tested (8%) | Code paths: 1/7 (14%) | User flows: 0/6 (0%)\nQUALITY: ★★★:0 ★★:1 ★:0 | GAPS: 12 (3 marked [→E2E])\n```\nLegend: ★★★ behavior + edge cases + error paths | ★★ happy path only | ★ smoke check | [→E2E] needs an integration test\n\n**Coverage summary**\n- **`processPayment`** is partly covered (1 of 4 code paths). There is one ★★ test for the valid-USD case, and it only checks `status`. Not tested: the `amount <= 0` error (including the `0` boundary), the unsupported-currency error, and a valid EUR payment.\n- **`refundPayment`** has no coverage (0 of 3 code paths). The test file never imports it, so both errors and the success result are untested.\n- **User flows:** this is a payment and refund flow, so the E2E matrix suggests integration tests for checkout and refund. None exist yet.\n- **Fastest gains:** add the missing `processPayment` cases (`0`, `-1`, `'GBP'`, `'EUR'`) as rows in one table-driven test in `billing.test.ts`, and add a `describe('refundPayment')` block covering the empty-ID error, the empty-reason error and the success result.\n\nAs you asked, I skipped the other review sections and workflow steps (including the test plan artifact), and left the source and tests unchanged.\n\n{\"tested\": [\"processPayment\"], \"untested\": [\"refundPayment\"]}" +} diff --git a/test/fixtures/coverage-audit-plan-eng-wc-bytes.json b/test/fixtures/coverage-audit-plan-eng-wc-bytes.json new file mode 100644 index 000000000..e0dfd7e82 --- /dev/null +++ b/test/fixtures/coverage-audit-plan-eng-wc-bytes.json @@ -0,0 +1,24 @@ +{ + "provenance": { + "source": "https://github.com/garrytan/gstack/actions/runs/41209768188 eval slice 5 (train gate, train d7441d9d), plan-eng-coverage-audit (claude-opus-5-5); the second parent Bash call and its result, byte-exact", + "recordedPassed": false, + "recordedFailures": [ + "missing successful source-file read", + "missing successful test-file read" + ], + "note": "The first call printed 109 KB and the native result kept only a preview, so it delivered neither file. The second call read both files completely in a ; chain, then printed a byte count with wc -c over a literal in-cwd file and a grep display; wc -c was not a closed display stage (only wc -l was), so the whole chain got no credit." + }, + "cwd": "/home/runner/.cache/gstack-paid-shard-yPuCXj/tmp/skill-e2e-plan-eng-review-coverage-9aEaFX", + "sessionId": "9e250e03-5ab5-4d6f-a605-646ba6d44306", + "call": { + "toolUseId": "toolu_01RsKm1skedEXrons396Cg2N", + "command": "cd /home/runner/.cache/gstack-paid-shard-yPuCXj/tmp/skill-e2e-plan-eng-review-coverage-9aEaFX; cat -n src/billing.ts; echo ----; cat -n test/billing.test.ts; echo ---; wc -c plan-eng-review/sections/review-sections.md; grep -n \"Test review\" -A80 plan-eng-review/sections/review-sections.md | head -200", + "isError": false, + "output": " 1\t\n 2\texport function processPayment(amount: number, currency: string) {\n 3\t if (amount <= 0) throw new Error('Invalid amount');\n 4\t if (currency !== 'USD' && currency !== 'EUR') throw new Error('Unsupported currency');\n 5\t return { status: 'success', amount, currency };\n 6\t}\n 7\t\n 8\texport function refundPayment(paymentId: string, reason: string) {\n 9\t if (!paymentId) throw new Error('Payment ID required');\n 10\t if (!reason) throw new Error('Reason required');\n 11\t return { status: 'refunded', paymentId, reason };\n 12\t}\n 13\t\n 14\t// coverage-read-evidence: 1e8364a6-52aa-42a0-8c25-33c0a2dcb7ea\n----\n 1\t\n 2\timport { describe, test, expect } from 'vitest';\n 3\timport { processPayment } from '../src/billing';\n 4\t\n 5\tdescribe('processPayment', () => {\n 6\t test('processes valid payment', () => {\n 7\t const result = processPayment(100, 'USD');\n 8\t expect(result.status).toBe('success');\n 9\t });\n 10\t // GAP: no test for invalid amount\n 11\t // GAP: no test for unsupported currency\n 12\t // GAP: refundPayment not tested at all\n 13\t});\n 14\t\n 15\t// coverage-read-evidence: 248d88f4-56af-41a0-bed2-18d23cf0027a\n---\n100809 plan-eng-review/sections/review-sections.md\n23:When Test review or Outside Voice refers to the plan, use the current working\n24-plan and this target evidence. Trace current behavior and proposed changes\n25-separately. Include actual decisions in Outside Voice's bounded input.\n26-\n27-Choose the **report file** before any ledger write:\n28-1. Use the output/report path explicitly requested by the user.\n29-2. Otherwise use the selected plan file, if there is one.\n30-3. Otherwise use `$GSTACK_STATE_ROOT/projects/$SLUG/$BRANCH-eng-review-{YYYYMMDD-HHMMSS}.md`, adding a suffix on collision. Obtain assignments from `~/.claude/skills/gstack/bin/gstack-paths` and `~/.claude/skills/gstack/bin/gstack-slug`; failed commands or missing values make this path unavailable.\n31-\n32-Never substitute an unrelated active plan or silently replace a requested destination.\n33-\n34-**Check each artifact and parent directory's permission before writing.** Honor\n35-user and host limits, including active-plan-only restrictions. Permission for one\n36-path authorizes no other; implementation edits require explicit authority.\n37-\n38-| Artifact | Destination | If writing is forbidden |\n39-|---|---|---|\n40-| Working plan, ledger and complete review report | Selected report file | Ask for a permitted destination if the user can supply one; wait without completion telemetry. If none is permitted, use **Read-only review** below, then **Report-back outcome**. |\n41-| QA Test Plan and task JSONL | Discovery paths below | Present each completely as **not persisted** and continue. |\n42-| TODOS.md | The project's TODO file | Present accepted TODO content as **not persisted** and continue. |\n43-| Required Review Log | The helper's state location | Present its fields as **not persisted**; at Review Log, use **Report-back outcome** instead of publishing a saved review. The final gate cannot pass without this log. |\n44-| Best-effort metadata/learning logs | Helper-defined locations | Skip forbidden writes; otherwise keep their best-effort behavior. |\n45-\n46-QA Test Plan/task JSONL keep discovery paths `$GSTACK_STATE_ROOT/projects/{slug}/`:\n47-`{user}-{branch}-eng-review-test-plan-{datetime}.md` and\n48-`tasks-eng-review-{datetime}.jsonl`. Keep their formats; do not relocate.\n49-\n50-**First report save:** Name the fixed target in the report header. Read an\n51-existing destination and preserve its content. For a new file, create permitted\n52-parent directories, then write that header, an unchanged copy of the original\n53-plan (for plan targets), and the scope record or ledger being saved. Recheck option 3's collision before\n54-creation; use a suffix rather than overwrite. Findings and fixes first enter\n55-through Scope Challenge C's ledger saves, never earlier. Put records before an existing `## GSTACK REVIEW REPORT`, or\n56-at EOF if absent; create that terminal report only at Plan File Review Report.\n57-\n58-**Write routes.** A forbidden write follows its row above.\n59-- **Read-only review:** At each scope/decision record save, present the complete\n60- record, grid and authorized amendments as **not persisted** instead. At both\n61- pre-question and post-answer verification gates, perform the same comparisons\n62- on that presentation instead of a saved Read. This supports chat review, never\n63- the saved-report gate.\n64-- **Failed permitted save** (error or failed Read-back): use **Recovery routing →\n65- Repairable write/read failure**, never the routes above. Do not ask from an\n66- unsaved record; an unrecovered failed write blocks the review.\n67-\n68-## Prior Learnings\n69-\n70-Search for relevant learnings from previous sessions:\n71-\n72-```bash\n73-_CROSS_PROJ=$(~/.claude/skills/gstack/bin/gstack-config get cross_project_learnings 2>/dev/null || echo \"unset\")\n74-echo \"CROSS_PROJECT: $_CROSS_PROJ\"\n75-if [ \"$_CROSS_PROJ\" = \"true\" ]; then\n76- { _LE=$(~/.claude/skills/gstack/bin/gstack-learnings-search --limit 10 --cross-project 2>&1 >&3 3>&-); _LR=$?; } 3>&1\n77-else\n78- { _LE=$(~/.claude/skills/gstack/bin/gstack-learnings-search --limit 10 2>&1 >&3 3>&-); _LR=$?; } 3>&1\n79-fi\n80-[ \"$_LR\" = 0 ] || { _LE=${_LE%%$'\\n'*}; echo \"LEARNINGS: unavailable (${_LE:-exit $_LR})\"; }\n81-```\n82-\n83-If `CROSS_PROJECT` is `unset` (first time): Build a full decision brief from these facts and options using the preamble format, then ask and wait:\n84-\n85-> gstack can search learnings from your other projects on this machine to find\n86-> patterns that might apply here. This stays local (no data leaves your machine).\n87-> Recommended for solo developers. Skip if you work on multiple client codebases\n88-> where cross-contamination would be a concern.\n89-\n90-Options:\n91-- A) Enable cross-project learnings (recommended)\n92-- B) Keep learnings project-scoped only\n93-\n94-If A: run `~/.claude/skills/gstack/bin/gstack-config set cross_project_learnings true`\n95-If B: run `~/.claude/skills/gstack/bin/gstack-config set cross_project_learnings false`\n96-\n97-Then re-run the search with the appropriate flag.\n98-\n99-If learnings are found, incorporate them into your analysis. When a review finding\n100-matches a past learning, display:\n101-\n102-**\"Prior learning applied: [key] (confidence N/10, from [date])\"**\n103-\n--\n613:### 3. Test review\n614-\n615-For shared-code changes, audit existing/missing shared-contract tests (behavior,\n616-errors, side effects, boundaries) and each migrated caller's integration/differences.\n617-Rejected extractions still need coverage for real duplicated-code defects.\n618-\n619-Coverage goal: every changed behavior is protected by a test that would catch a real regression. Test count is not a goal. Identify the tests each planned codepath needs. Add required proof for an exact approved behavior without asking again; take new policies or optional verification depth through the decision gate before treating their tests as accepted work. Review the requirements here; do not build the proposed tests.\n620-\n621-#### Test Framework Detection\n622-\n623-Before analyzing coverage, detect the project's test framework:\n624-\n625-1. **Read CLAUDE.md** — look for a `## Testing` section with test command and framework name. If found, use that as the authoritative source.\n626-2. **If CLAUDE.md has no testing section, auto-detect:**\n627-\n628-```bash\n629-setopt +o nomatch 2>/dev/null || true # zsh compat\n630-# Runtime markers: evidence, not commands to run\n631-[ -f manage.py ] && echo \"RUNTIME:python FRAMEWORK:django\"\n632-{ [ -f pyproject.toml ] || [ -f pytest.ini ] || [ -f tox.ini ] || [ -f setup.cfg ] || [ -f requirements.txt ]; } && echo \"RUNTIME:python\"\n633-{ [ -f Gemfile ] || [ -f Rakefile ] || [ -f .rspec ]; } && echo \"RUNTIME:ruby\"\n634-[ -f package.json ] && echo \"RUNTIME:node\"\n635-[ -f go.mod ] && echo \"RUNTIME:go\"\n636-[ -f Cargo.toml ] && echo \"RUNTIME:rust\"\n637-[ -f pubspec.yaml ] && echo \"RUNTIME:dart\"\n638-[ -f pom.xml ] && echo \"RUNTIME:jvm BUILD:maven\"\n639-{ [ -f build.gradle ] || [ -f build.gradle.kts ]; } && echo \"RUNTIME:jvm BUILD:gradle\"\n640-# Test infra: configs, scripts, test files\n641-ls jest.config.* vitest.config.* playwright.config.* cypress.config.* .rspec pytest.ini tox.ini phpunit.xml dart_test.yaml 2>/dev/null\n642-[ -f package.json ] && grep -q '\"test\"[[:space:]]*:' package.json && echo \"SCRIPT:package.json test\"\n643-[ -f Makefile ] && grep -qE '^(test|check):' Makefile && echo \"TARGET:make test\"\n644-git ls-files | grep -cE '(^|/)(tests?|spec|__tests__)/|(^|/)tests?\\.py$|(^|/)test_[^/]+\\.py$|_test\\.(go|py|rb|ts|js|exs|dart)$|\\.(test|spec)\\.[jt]sx?$|_spec\\.rb$|Test\\.(java|kt)$' | sed 's/^/TESTFILES:/'\n645-```\n646-\n647-3. **If no framework detected:** State that the framework is unknown; continue the diagram and planned assertions. If proposing a new framework, settle that choice through Decision procedure in Test step 5. Reuse an exact prior approval; with no selection proposed, ask no framework question. Do not install a framework or write the proposed tests during this review.\n648-\n649-Definition: a **targeted audit** reviews named concrete source/test files or a\n650-branch diff. A **prototype** is existing runnable code referenced by the plan,\n651-not a proposed future component.\n652-\n653-For every target, run these five Test steps inside Section 3, after Scope\n654-Challenge and the Architecture/Code Quality reviews. Do not restart them.\n655-Within Test step 1, read concrete source/tests before tracing or diagramming;\n656-Test step 2 adds user flows. Future paths remain proposals, not runnable code.\n657-\n658-**Step 1. Trace every codepath in the plan:**\n659-\n660-Read the plan document. For each new feature, service, endpoint, or component described, trace how data will flow through the code — don't just list planned functions, actually follow the planned execution:\n661-\n662-1. **Read the plan.** For each planned component, see how it connects to existing code. When grounded in concrete source and test files, read them in a dedicated tool call before drawing the diagram (`cat -n src/f && echo -- && cat -n test/f`). Do not mix diff, grep, config, git or commentary into that read; use separate calls for context. Base the diagram on that read.\n663-2. **Trace data flow.** Starting from each entry point (route handler, exported function, event listener, component render), follow the data through every branch:\n664- - Where does input come from? (request params, props, database, API call)\n665- - What transforms it? (validation, mapping, computation)\n666- - Where does it go? (database write, API response, rendered output, side effect)\n667- - What can go wrong at each step? (null/undefined, invalid input, network failure, empty collection)\n668-3. **Diagram the execution.** For each existing or proposed component in the selected target, draw an ASCII diagram showing:\n669- - Every existing or proposed function/method in scope\n670- - Every conditional branch (if/else, switch, ternary, guard clause, early return)\n671- - Every error path (try/catch, rescue, error boundary, fallback)\n672- - Every call to another function (trace into it — does IT have untested branches?)\n673- - Every edge: what happens with null input? Empty array? Invalid type?\n674-\n675-This is the critical step — you're building a map of every line of code that can execute differently based on input. Every branch in this diagram needs coverage that would catch a real regression; the test value bar below decides whether that is a new test, an extension of an existing one, or already covered.\n676-\n677-**Step 2. Map user flows, interactions, and error states:**\n678-\n679-Code coverage isn't enough — you need to cover how real users interact with the selected target. For each existing or proposed feature, think through:\n680-\n681-- **User flows:** What sequence of actions does a user take that touches this code? Map the full journey (e.g., \"user clicks 'Pay' → form validates → API call → success/failure screen\"). Each step in the journey needs coverage.\n682-- **Interaction edge cases:** What happens when the user does something unexpected?\n683- - Double-click/rapid resubmit\n684- - Navigate away mid-operation (back button, close tab, click another link)\n685- - Submit with stale data (page sat open for 30 minutes, session expired)\n686- - Slow connection (API takes 10 seconds — what does the user see?)\n687- - Concurrent actions (two tabs, same form)\n688-- **Error states the user can see:** For every error the code handles, what does the user actually experience?\n689- - Is there a clear error message or a silent failure?\n690- - Can the user recover (retry, go back, fix input) or are they stuck?\n691- - What happens with no network? With a 500 from the API? With invalid data from the server?\n692-- **Empty/zero/boundary states:** What does the UI show with zero results? With 10,000 results? With a single character input? With maximum-length input?\n693-\n--\n756:For targeted audits, start Test review output with the coverage diagram. In full\n757:plan reviews, put it inside the normal Test review section. Required outputs\n758-keep the final terminal report order.\n759-\n760-Include BOTH code paths and user flows in the same diagram. Mark E2E-worthy and eval-worthy paths:\n761-\n762-```\n763-CODE PATHS USER FLOWS\n764-[+] src/services/billing.ts [+] Payment checkout\n765- ├── processPayment() ├── [★★★ TESTED] Complete purchase — checkout.e2e.ts:15\n766- │ ├── [★★★ TESTED] happy + declined + timeout ├── [GAP] [→E2E] Double-click submit\n767- │ ├── [GAP] Network timeout └── [GAP] Navigate away mid-payment\n768- │ └── [GAP] Invalid currency\n769- └── refundPayment() [+] Error states\n770- ├── [★★ TESTED] Full refund — :89 ├── [★★ TESTED] Card declined message\n771- └── [★ TESTED] Partial (non-throw only) — :101 └── [GAP] Network timeout UX\n772-\n773-LLM integration: [GAP] [→EVAL] Prompt template change — needs eval test\n774-\n775-COVERAGE: 5/13 paths tested (38%) | Code paths: 3/5 (60%) | User flows: 2/8 (25%)\n776-QUALITY: ★★★:2 ★★:2 ★:1 | GAPS: 8 (2 E2E, 1 eval)\n777-```\n778-\n779-Legend: ★★★ behavior + edge + error | ★★ happy path | ★ smoke check\n780-[→E2E] = needs integration test | [→EVAL] = needs LLM eval\n781-\n782-Avoid bare `[ ]` or `[x]` in diagrams unless the block includes\n783-`Legend: [x] tested | [ ] no test`. Prefer `[GAP]`, `[★★ TESTED]`,\n784-`[→E2E]`, `[→EVAL]`; keep user-flow markers off code-path rows.\n785-\n786:**Fast path:** All paths covered → \"Test review: All new code paths have test coverage ✓\" Still check LLM/eval scope and produce the Test Plan Artifact below.\n787-\n788-#### LLM/eval scope\n789-\n790-For LLM/prompt changes: check the \"Prompt/LLM changes\" file patterns listed in CLAUDE.md. If this plan touches ANY of those patterns, state which eval suites must be run, which cases should be added, and what baselines to compare against. Include unapproved eval scope among the choices resolved in Step 5.\n791-" + }, + "files": { + "source": "\nexport function processPayment(amount: number, currency: string) {\n if (amount <= 0) throw new Error('Invalid amount');\n if (currency !== 'USD' && currency !== 'EUR') throw new Error('Unsupported currency');\n return { status: 'success', amount, currency };\n}\n\nexport function refundPayment(paymentId: string, reason: string) {\n if (!paymentId) throw new Error('Payment ID required');\n if (!reason) throw new Error('Reason required');\n return { status: 'refunded', paymentId, reason };\n}\n\n// coverage-read-evidence: 1e8364a6-52aa-42a0-8c25-33c0a2dcb7ea\n", + "tests": "\nimport { describe, test, expect } from 'vitest';\nimport { processPayment } from '../src/billing';\n\ndescribe('processPayment', () => {\n test('processes valid payment', () => {\n const result = processPayment(100, 'USD');\n expect(result.status).toBe('success');\n });\n // GAP: no test for invalid amount\n // GAP: no test for unsupported currency\n // GAP: refundPayment not tested at all\n});\n\n// coverage-read-evidence: 248d88f4-56af-41a0-bed2-18d23cf0027a\n" + }, + "finalOutput": "## Test review — targeted audit of `src/billing.ts` (feature/billing)\n\nTest framework: **vitest** (found `vitest.config.ts` and `vitest` in devDependencies). The `package.json` `test` script is only a stub: `echo \"no tests yet\"`. Coverage below comes from reading the code and tests. No tests were run.\n\n```\nCODE PATHS USER FLOWS\n[+] src/billing.ts [+] Charge a customer\n ├── processPayment(amount, currency) :2-6 ├── [★★ TESTED] Valid USD charge — billing.test.ts:6\n │ ├── [GAP] amount <= 0 → throw ├── [GAP] [→E2E] Rejected amount → visible error\n │ │ 'Invalid amount' :3 └── [GAP] [→E2E] Unsupported currency → visible error\n │ │ └── [GAP] boundary: amount = 0, -1, NaN\n │ ├── [GAP] currency ∉ {USD,EUR} → throw [+] Refund a payment\n │ │ 'Unsupported currency' :4 ├── [GAP] [→E2E] Successful refund\n │ ├── [★★ TESTED] USD → {status:'success',...} └── [GAP] [→E2E] Refund without ID/reason → error\n │ │ billing.test.ts:6-9\n │ │ (asserts status only; amount/\n │ │ currency echo not asserted)\n │ └── [GAP] EUR → success branch :4-5\n │\n └── refundPayment(paymentId, reason) :8-12 ◄── NO TESTS (not even imported)\n ├── [GAP] !paymentId ('' / undefined) → throw 'Payment ID required' :9\n ├── [GAP] !reason ('' / undefined) → throw 'Reason required' :10\n └── [GAP] valid → {status:'refunded', paymentId, reason} :11\n\nCOVERAGE: 2/12 paths tested (17%) | Code paths: 1/7 (14%) | User flows: 1/5 (20%)\nQUALITY: ★★★:0 ★★:2 ★:0 | GAPS: 10 (4 E2E)\n```\nLegend: ★★★ behavior + edge + error | ★★ happy path | ★ smoke check | [→E2E] = needs integration test\n\nThe two ★★ ticks are the same test at `billing.test.ts:6`. It counts once as a code path and once as a user flow.\n\n### Coverage summary\n\n- **`processPayment`: partially tested, happy path only.** One test checks a valid USD payment and only looks at `result.status`. Nothing tests either error branch: the `amount <= 0` check, including the `0` boundary, or the currency check. The EUR success branch is also untested.\n- **`refundPayment`: not tested at all.** The test file doesn't import it. None of its three paths (missing payment ID, missing reason, success) is tested.\n- **Biggest risks:** if either `processPayment` check were removed or flipped, the suite would still pass. That would let invalid amounts or unsupported currencies through, and payment paths are where that matters most. Any change to `refundPayment` would go completely undetected.\n- **What would close the gaps:** this is only a description, since writing tests was out of scope. Add table-driven tests in the existing `describe('processPayment')` block for the invalid amounts (`0`, `-1`) and an unsupported currency (e.g. `'GBP'`). Add EUR as another success row and check the whole returned object. Add a new `describe('refundPayment')` block covering the two `throw` checks and the success return. Plain unit tests are enough because both are pure functions. The [→E2E] marks only matter once a checkout or refund UI or API calls these functions.\n\nSource and test files were not modified.\n\n{\"tested\": [\"processPayment\"], \"untested\": [\"refundPayment\"]}" +} diff --git a/test/fixtures/third-party-actions-broken-absence-reason.json b/test/fixtures/third-party-actions-broken-absence-reason.json new file mode 100644 index 000000000..19e25c993 --- /dev/null +++ b/test/fixtures/third-party-actions-broken-absence-reason.json @@ -0,0 +1,19 @@ +{ + "source": "assistantText of the claude-opus-5-5 tpa-broken gate records in run 41246728073 (PR #3727 @ ff81a147): paid-slice-16-a1 (recorded red: asideDriveOptions returned the B) gstack-drive option) and its in-run rule-16 rerun (green)", + "renders": [ + { + "case": "tpa-broken", + "run": "41246728073", + "attempt": "slice-16-a1", + "recordedPassed": false, + "assistantText": "The contract says to run the Aside readiness probe before asking anything, so I'll do that first.\nThe Aside readiness check came back as `ASIDE_CLI_ERROR` (exit 1). Under the policy, that means Aside is not detected for this task, so I've left out the option to drive your Aside browser. This machine is Linux, so I won't suggest installing Aside either. I haven't installed or browsed anything.\n\n---\n\n**Question: How should I create the test-mode API token in the Acme vendor dashboard (https://dashboard.acme.test)?**\n\nThe task is to create a test-mode API token on dashboard.acme.test. I would only touch that site and only to create that token. You would handle any sign-in, password, CAPTCHA or identity check yourself. The token would never be shown in chat. It would go into a local file only you can read (0600) at a path you approve. I'd then check it with one read-only API call.\n\n- **B)** I drive it in gstack's own visible browser, and hand it to you whenever sign-in is needed. **(Recommended, since Aside isn't available)**\n- **C)** I give you step-by-step instructions and you do it yourself.\n- **D)** Skip it for now and mark this step as blocked until you do it.\n\nReply with B, C or D. This answer covers this task only.\n\n---\n\nI've stopped here, as you asked." + }, + { + "case": "tpa-broken", + "run": "41246728073", + "attempt": "rerun-1", + "recordedPassed": true, + "assistantText": "The user asked for the detection, so I'm running the readiness probe the contract specifies.\nI ran the Aside readiness probe and it returned `ASIDE_CLI_ERROR: exit 1`. Under the policy, only `READY` counts as detected, so the Aside browser isn't available for this task and I've left that option out. I have not opened a browser or done anything else.\n\nThe file also contains an unrelated \"Ship\" release workflow below the third-party actions section. It has nothing to do with this task, so I didn't follow it.\n\n**Question: How do you want to handle this?**\n\nSite: https://dashboard.acme.test (Acme vendor dashboard)\nAction: create a test-mode API token\n\n- **B) I drive it in gstack's own visible browser.** I open the dashboard and go to the API token page. If a sign-in, password, CAPTCHA or identity check appears, I hand control to you. You complete it and tell me \"done\", then I continue. I only touch the Acme dashboard and only to create the token. You copy the token yourself with the dashboard's copy button and save it to a local file you approve, with owner-only (0600) permissions. Then I check it with one read-only API call. The token never appears in chat.\n- **C) Manual instructions.** I write out the steps for you to do yourself, and this step stays blocked until you finish.\n- **D) Defer.** We skip this for now and mark the step blocked on you.\n\nReply B, C or D. Your choice applies to this task only." + } + ] +} diff --git a/test/helpers/coverage-audit-evidence.ts b/test/helpers/coverage-audit-evidence.ts index bbd79df13..02d4e8410 100644 --- a/test/helpers/coverage-audit-evidence.ts +++ b/test/helpers/coverage-audit-evidence.ts @@ -140,8 +140,8 @@ function stagePieces(stage: string[], scope: ShellScope, alone: boolean): Piece[ } if (name === 'grep' || name === 'ls') return [unknown()]; if (text === 'pwd' || text === 'wc -l') return [unknown(1, 1)]; - if (name === 'wc' && args[0] === '-l' && args.length > 1) { - // One count line per literal in-cwd file, plus a total for several. + if (name === 'wc' && (args[0] === '-l' || args[0] === '-c') && args.length > 1) { + // One line or byte count line per literal in-cwd file, plus a total for several. if (args.slice(1).some(operand => !scopedFile(operand, scope))) return undefined; const lines = args.length - 1 + (args.length > 2 ? 1 : 0); return [unknown(lines, lines)]; @@ -254,14 +254,27 @@ function shellReads(command: unknown, output: unknown, scope: ShellScope): Set> = { 'test/fixtures/docsync-replay/37237194905-late-result-calls.json': ['test/docsync-authority.test.ts'], 'test/fixtures/docsync-replay/37239016858-completion-native.json': ['test/docsync-nested-writes.test.ts'], 'test/fixtures/docsync-ship-receipt-not-found-public.json': ['test/docsync-lifecycle-interface.test.ts'], + 'test/fixtures/docsync-spawned-missing-section-public.json': ['test/ship-document-release-dispatch.test.ts'], 'test/fixtures/docsync-store-38969924945-bash.json': ['test/docsync-command-grammar.test.ts'], 'test/fixtures/docsync-two-attempts-public.json': ['test/ship-document-release-dispatch.test.ts'], 'test/fixtures/dx-manual-handoff-ao.json': ['test/plan-count-completion.test.ts'], diff --git a/test/helpers/third-party-actions.ts b/test/helpers/third-party-actions.ts index 0eccf99d3..8b4b25abb 100644 --- a/test/helpers/third-party-actions.ts +++ b/test/helpers/third-party-actions.ts @@ -34,14 +34,22 @@ export function letteredOptions(text: string): string[] { return options; } +/** + * A statement that Aside is absent, the probe's result, names no Aside drive: "(Recommended, since Aside isn't + * available)" on the gstack drive option (gate 41246728073). Only Aside as the subject of a negated availability + * predicate is discounted, so any other Aside mention in the option still counts. + */ +const ASIDE_ABSENCE = /\bAside(?: browser| app)?\s+(?:(?:is|was)(?:n['’]t|\s+not)\s+(?:available|installed|detected|running|ready|reachable)|is\s+unavailable)\b/gi; + /** Inspect lettered consent options, not narration that quotes an unavailable option. */ export function asideDriveOptions(text: string): string[] { return letteredOptions(text).filter(option => { - const currentOffer = option.replace( + const named = option.replace(ASIDE_ABSENCE, ''); + const currentOffer = named.replace( /\b(?:I|we)(?:['’]ll| will) (?:re[- ]?ask|ask again)(?: (?:you|this question))? with (?:the )?(?:"[^"]+"|“[^”]+”|[\w -]+?) option(?: included)?\b/gi, '', ); - return /\bAside\b/.test(option) && (/\b(?:drive|driving|browse|browsing|navigate|click)\b/i.test(currentOffer) + return /\bAside\b/.test(named) && (/\b(?:drive|driving|browse|browsing|navigate|click)\b/i.test(currentOffer) || /\bI(?:['’]ll| will)? (?:do|handle|create|complete) (?:it|this)\b/i.test(currentOffer)); }); } diff --git a/test/third-party-actions.test.ts b/test/third-party-actions.test.ts index dcdc06289..01c5e6caf 100644 --- a/test/third-party-actions.test.ts +++ b/test/third-party-actions.test.ts @@ -30,6 +30,7 @@ import manualWording from './fixtures/third-party-actions-manual-wording.json'; import opusRenders from './fixtures/third-party-actions-opus-consent-renders.json'; import recoveryFixture from './fixtures/third-party-actions-recovery-public.json'; import pitchCaptures from './fixtures/third-party-actions-pitch-captures.json'; +import brokenAbsence from './fixtures/third-party-actions-broken-absence-reason.json'; import { expectMentions, expectTokens } from './helpers/prompt-structure'; const ROOT = path.resolve(import.meta.dir, ".."); @@ -611,3 +612,29 @@ describe('Opus consent renders (gate captures)', () => { 'A) You generate it yourself on any device.\nB) You enter it through the masked prompt.']) expect(offersDriveOption(text), text).toBe(false); }); }); + +// Gate 41246728073 slice 16 (PR #3727): Opus offered only the gstack drive and gave the +// recommendation's reason as "since Aside isn't available". A statement that Aside is +// absent names no Aside drive; any other Aside mention in the same option still does. +describe('consent offer extraction: an Aside-absence reason on the gstack drive option', () => { + test('the captured tpa-broken renders offer no Aside drive', () => { + const [red, rerun] = brokenAbsence.renders; + expect(red.recordedPassed).toBe(false); + expect(letteredOptions(red.assistantText)[0]).toContain("(Recommended, since Aside isn't available)"); + for (const render of brokenAbsence.renders) expect(asideDriveOptions(render.assistantText), render.attempt).toEqual([]); + expect(offersDriveOption(rerun.assistantText)).toBe(true); + }); + + test('absence statements are discounted; an Aside drive beside them is still caught', () => { + for (const option of ["B) I drive it in gstack's own visible browser (Recommended, since Aside isn't available)", + "B) I drive it in gstack's own visible browser, because the Aside app is not running.", + "B) I drive it in gstack's own browser; Aside wasn't detected.", "B) I drive it in gstack's own browser (Aside is unavailable).", + "B) I drive it in gstack's own browser since Aside isn’t installed."]) + expect(asideDriveOptions(option), option).toEqual([]); + for (const option of ["A) I drive it in your Aside browser, even though Aside isn't running yet.", + "B) I drive it in gstack's own browser, or in Aside once Aside is not busy.", "B) Since Aside isn't available, I drive it in Aside's fallback window.", + "A) Aside isn't available yet, so open it; then I drive it in your Aside browser.", "A) I drive it in Aside; gstack isn't available.", + "B) I drive it in gstack's own browser and switch to Aside if it's running."]) + expect(asideDriveOptions(option), option).toHaveLength(1); + }); +});