From 3e44a5af3ca16dc5cf65801df98e38b6e42cf28c Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 23:25:21 +0000 Subject: [PATCH 1/5] fix(ship): Step 9.2.1 QA writes reports under a git-excluded run dir, not the product tree (GST-969) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/qa-evidence.ts | 27 ++++++- scripts/resolvers/qa.ts | 2 +- ship/sections/review-army.md | 1 + test/qa-report-dir.test.ts | 77 +++++++++++++++++++ .../ship/gst-969-qa-tree-writes.json | 4 + 5 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 test/qa-report-dir.test.ts create mode 100644 test/size-ledger/ship/gst-969-qa-tree-writes.json diff --git a/lib/qa-evidence.ts b/lib/qa-evidence.ts index 4fc9eead6..64801f465 100644 --- a/lib/qa-evidence.ts +++ b/lib/qa-evidence.ts @@ -388,7 +388,28 @@ function materialize(root: string, source: string) { next: `Include every reportLinks entry in the Markdown report, and report the overall status as ${verdict.status}${verdict.open.length ? ` (open: ${verdict.open.join('; ')})` : ''}; this verdict is final. Next, write the Markdown report beside evidence.json.` }; } -const QA_EVIDENCE_USAGE = 'capture ROOT ID [--public] --deadline FILE|--timeout-ms MS [--after PREVIOUS_CAPTURE --hypothesis TEXT] -- COMMAND ARGS (--after publishes checkpoint ID linking PREVIOUS_CAPTURE to this probe; required after the first complete capture unless a checkpoint was published) | checkpoint ROOT ID CAPTURE OBSERVATION_COMMAND HYPOTHESIS NEXT_COMMAND | checkpoint ROOT ID INTENT_FILE | materialize ROOT ANNOTATIONS (annotations: {evidence: [{capture, command, contract, expected, classification: pass|superseded|product-defect|fail|setup-blocked|blocked|inconclusive}], limits: [..]}; revision, runtime, cwd and learning are filled in)'; +/** A fresh report root for /review and /ship QA under the repository's git-excluded .gstack/tmp, so probes never change the reviewed tree. */ +function reportDir(): Record { + const top = spawnSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8', timeout: 5000 }); + if (top.status !== 0 || !top.stdout.trim()) throw new QaEvidenceError('report-dir needs a git work tree'); + const repoRoot = fs.realpathSync(top.stdout.trim()); + const exclude = spawnSync('git', ['rev-parse', '--git-path', 'info/exclude'], { encoding: 'utf8', timeout: 5000 }); + if (exclude.status !== 0 || !exclude.stdout.trim()) throw new QaEvidenceError('report-dir cannot locate the git exclude file'); + const excludeFile = path.resolve(exclude.stdout.trim()); + fs.mkdirSync(path.dirname(excludeFile), { recursive: true }); + const excluded = fs.existsSync(excludeFile) ? fs.readFileSync(excludeFile, 'utf8') : ''; + if (!excluded.split(/\r?\n/).includes('/.gstack/tmp/')) fs.appendFileSync(excludeFile, `${excluded && !excluded.endsWith('\n') ? '\n' : ''}/.gstack/tmp/\n`); + const parent = path.join(repoRoot, '.gstack', 'tmp', 'qa-reports'); + fs.mkdirSync(parent, { recursive: true, mode: 0o700 }); + const stamp = new Date().toISOString().replace(/[-:]/g, '').replace(/\.\d+Z$/, 'Z'); + for (let n = 1; ; n++) { + const dir = path.join(parent, `run-${stamp}${n > 1 ? `-${n}` : ''}`); + try { fs.mkdirSync(dir, { mode: 0o700 }); } catch (error: any) { if (error?.code === 'EEXIST') continue; throw error; } + return { action: 'report-dir', status: 'complete', reportDir: dir, exitCode: 0 }; + } +} + +const QA_EVIDENCE_USAGE = 'report-dir (prints a fresh git-excluded report root for /review and /ship) | capture ROOT ID [--public] --deadline FILE|--timeout-ms MS [--after PREVIOUS_CAPTURE --hypothesis TEXT] -- COMMAND ARGS (--after publishes checkpoint ID linking PREVIOUS_CAPTURE to this probe; required after the first complete capture unless a checkpoint was published) | checkpoint ROOT ID CAPTURE OBSERVATION_COMMAND HYPOTHESIS NEXT_COMMAND | checkpoint ROOT ID INTENT_FILE | materialize ROOT ANNOTATIONS (annotations: {evidence: [{capture, command, contract, expected, classification: pass|superseded|product-defect|fail|setup-blocked|blocked|inconclusive}], limits: [..]}; revision, runtime, cwd and learning are filled in)'; export async function qaEvidenceMain(args: string[]): Promise { return withQaReceiptOutput(false, 'qa-evidence-receipt', value => value.event === 'observation' @@ -400,6 +421,10 @@ export async function qaEvidenceMain(args: string[]): Promise { emit('stdout', { action: 'help', status: 'complete', usage: QA_EVIDENCE_USAGE, exitCode: 0 }); return 0; } + if (action === 'report-dir' && args.length === 1) { + emit('stdout', reportDir()); + return 0; + } const root = qaEvidenceRoot(reportRoot); let receipt: Record; const publicOutput = action === 'capture' && rest[1] === '--public'; diff --git a/scripts/resolvers/qa.ts b/scripts/resolvers/qa.ts index 330b8b43f..43dda71d8 100644 --- a/scripts/resolvers/qa.ts +++ b/scripts/resolvers/qa.ts @@ -244,7 +244,7 @@ export function generateQAReview(ctx: TemplateContext): string { Only the parent runs report-only discovery. Never overwrite another run's reports. Batch only independent Reads. - +${ship ? `Without a caller REPORT_DIR, use the \`reportDir\` that \`bun ${quoteSafePath(ctx.paths.binDir)}/gstack-qa-evidence report-dir\` prints: git-excluded, so QA never changes the reviewed tree.\n` : ''} ${ship ? `**1. Load methods before any QA or explicit-verification probe.** ${generateQAReviewPreflight(ctx)}` : `**1. Set the charter and isolation.** diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index f78dda7d1..1ae2738fe 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -594,6 +594,7 @@ If the Red Team fails or times out, confirm it stopped and record its review as Only the parent runs report-only discovery. Never overwrite another run's reports. Batch only independent Reads. +Without a caller REPORT_DIR, use the `reportDir` that `bun $HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded, so QA never changes the reviewed tree. **1. Load methods before any QA or explicit-verification probe.** diff --git a/test/qa-report-dir.test.ts b/test/qa-report-dir.test.ts new file mode 100644 index 000000000..ee8b33e63 --- /dev/null +++ b/test/qa-report-dir.test.ts @@ -0,0 +1,77 @@ +import { afterAll, 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 { RESOLVERS } from '../scripts/resolvers'; +import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; + +const BIN = path.resolve(import.meta.dir, '../bin'); +const ROOT = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'qa-report-dir-')); +afterAll(() => fs.rmSync(ROOT, { recursive: true, force: true })); + +const IDENTITY = { GIT_AUTHOR_NAME: 'fixture', GIT_AUTHOR_EMAIL: 'fixture@example.com', GIT_COMMITTER_NAME: 'fixture', GIT_COMMITTER_EMAIL: 'fixture@example.com' }; + +function product() { + const root = fs.mkdtempSync(path.join(ROOT, 'product-')); + const git = (...args: string[]) => { + const r = spawnSync('git', args, { cwd: root, encoding: 'utf8', timeout: 10_000, env: { ...process.env, ...IDENTITY } }); + expect(r.status, r.stderr).toBe(0); + return r.stdout; + }; + git('init', '-q'); + fs.writeFileSync(path.join(root, 'app.ts'), 'export const x = 1;\n'); + git('add', 'app.ts'); + git('commit', '-q', '-m', 'seed'); + const tool = (name: string, ...args: string[]) => { + const r = spawnSync(process.execPath, [path.join(BIN, name), ...args], { cwd: root, encoding: 'utf8', timeout: 15_000 }); + expect(r.error, r.error?.message).toBeUndefined(); + return r; + }; + const tree = () => git('status', '--porcelain', '--untracked-files=all', '--ignored=no') + git('ls-files', '--others', '--exclude-standard'); + return { root, tool, tree }; +} + +function reportDir(f: ReturnType): string { + const r = f.tool('gstack-qa-evidence', 'report-dir'); + expect(r.status, r.stderr).toBe(0); + return JSON.parse(r.stdout.trim().slice('QA_EVIDENCE '.length)).reportDir; +} + +test('the /ship QA report root leaves a product tree without a .gstack ignore rule unchanged', () => { + const f = product(); + expect(f.tree()).toBe(''); + const dir = reportDir(f); + expect(path.relative(f.root, dir).split(path.sep).slice(0, 3)).toEqual(['.gstack', 'tmp', 'qa-reports']); + const deadline = path.join(dir, 'deadline.json'); + expect(f.tool('gstack-qa-deadline', 'start', deadline, '300').status).toBe(0); + const capture = f.tool('gstack-qa-evidence', 'capture', dir, '001', '--public', '--deadline', deadline, '--', process.execPath, '-e', 'console.log(JSON.stringify({ ok: true }))'); + expect(capture.status, capture.stderr).toBe(0); + fs.writeFileSync(path.join(dir, 'report.md'), '## Exploratory QA\n'); + expect(f.tree()).toBe(''); +}); + +test('each report-dir call owns a fresh run so a second QA run never overwrites the first', () => { + const f = product(); + const first = reportDir(f); + const second = reportDir(f); + expect(second).not.toBe(first); + expect(fs.readdirSync(first)).toEqual([]); + expect(fs.readdirSync(second)).toEqual([]); +}); + +test('report-dir refuses outside a git work tree instead of guessing a location', () => { + const outside = fs.mkdtempSync(path.join(ROOT, 'plain-')); + const r = spawnSync(process.execPath, [path.join(BIN, 'gstack-qa-evidence'), 'report-dir'], { cwd: outside, encoding: 'utf8', timeout: 15_000, env: { ...process.env, GIT_CEILING_DIRECTORIES: ROOT } }); + expect(r.status).toBe(2); + expect(r.stderr).toContain('report-dir needs a git work tree'); + expect(fs.readdirSync(outside)).toEqual([]); +}); + +test('/ship Step 9.2.1 resolves REPORT_DIR through report-dir, not the product tree', () => { + const ctx: TemplateContext = { host: 'claude', skillName: 'ship', tmplPath: '', paths: HOST_PATHS.claude }; + expect(RESOLVERS.QA_REVIEW(ctx)).toContain('/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded'); + const rendered = fs.readFileSync(path.resolve(import.meta.dir, '../ship/sections/review-army.md'), 'utf8'); + expect(rendered).toContain('gstack-qa-evidence report-dir` prints: git-excluded'); +}); diff --git a/test/size-ledger/ship/gst-969-qa-tree-writes.json b/test/size-ledger/ship/gst-969-qa-tree-writes.json new file mode 100644 index 000000000..f168aa918 --- /dev/null +++ b/test/size-ledger/ship/gst-969-qa-tree-writes.json @@ -0,0 +1,4 @@ +{ + "bytes": 183, + "why": "GST-969: /ship Step 9.2.1 QA writes its reports under gstack-qa-evidence report-dir (git-excluded .gstack/tmp), not the product tree" +} From 7672a4f07e0d9796d58dfe613edc418aad130887 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 23:35:32 +0000 Subject: [PATCH 2/5] fix: pre-landing review fixes Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- scripts/resolvers/qa.ts | 2 +- ship/sections/review-army.md | 2 +- test/qa-report-dir.test.ts | 2 +- test/size-ledger/ship/gst-969-qa-tree-writes.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/resolvers/qa.ts b/scripts/resolvers/qa.ts index 43dda71d8..11ab11c9c 100644 --- a/scripts/resolvers/qa.ts +++ b/scripts/resolvers/qa.ts @@ -244,7 +244,7 @@ export function generateQAReview(ctx: TemplateContext): string { Only the parent runs report-only discovery. Never overwrite another run's reports. Batch only independent Reads. -${ship ? `Without a caller REPORT_DIR, use the \`reportDir\` that \`bun ${quoteSafePath(ctx.paths.binDir)}/gstack-qa-evidence report-dir\` prints: git-excluded, so QA never changes the reviewed tree.\n` : ''} +${ship ? `Without a caller REPORT_DIR, use the \`reportDir\` that \`${quoteSafePath(ctx.paths.binDir)}/gstack-qa-evidence report-dir\` prints: git-excluded, so QA never changes the reviewed tree.\n` : ''} ${ship ? `**1. Load methods before any QA or explicit-verification probe.** ${generateQAReviewPreflight(ctx)}` : `**1. Set the charter and isolation.** diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 1ae2738fe..83c0f10e2 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -594,7 +594,7 @@ If the Red Team fails or times out, confirm it stopped and record its review as Only the parent runs report-only discovery. Never overwrite another run's reports. Batch only independent Reads. -Without a caller REPORT_DIR, use the `reportDir` that `bun $HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded, so QA never changes the reviewed tree. +Without a caller REPORT_DIR, use the `reportDir` that `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded, so QA never changes the reviewed tree. **1. Load methods before any QA or explicit-verification probe.** diff --git a/test/qa-report-dir.test.ts b/test/qa-report-dir.test.ts index ee8b33e63..d5c437fb1 100644 --- a/test/qa-report-dir.test.ts +++ b/test/qa-report-dir.test.ts @@ -71,7 +71,7 @@ test('report-dir refuses outside a git work tree instead of guessing a location' test('/ship Step 9.2.1 resolves REPORT_DIR through report-dir, not the product tree', () => { const ctx: TemplateContext = { host: 'claude', skillName: 'ship', tmplPath: '', paths: HOST_PATHS.claude }; - expect(RESOLVERS.QA_REVIEW(ctx)).toContain('/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded'); + expect(RESOLVERS.QA_REVIEW(ctx)).toContain('that `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded'); const rendered = fs.readFileSync(path.resolve(import.meta.dir, '../ship/sections/review-army.md'), 'utf8'); expect(rendered).toContain('gstack-qa-evidence report-dir` prints: git-excluded'); }); diff --git a/test/size-ledger/ship/gst-969-qa-tree-writes.json b/test/size-ledger/ship/gst-969-qa-tree-writes.json index f168aa918..b6930b3bb 100644 --- a/test/size-ledger/ship/gst-969-qa-tree-writes.json +++ b/test/size-ledger/ship/gst-969-qa-tree-writes.json @@ -1,4 +1,4 @@ { - "bytes": 183, + "bytes": 179, "why": "GST-969: /ship Step 9.2.1 QA writes its reports under gstack-qa-evidence report-dir (git-excluded .gstack/tmp), not the product tree" } From 5805f00417c1c711e56cb2e80792be4bb3c5dbd9 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 23:52:06 +0000 Subject: [PATCH 3/5] fix: harden report-dir (linked .gstack, .gitignore negation, error cause) per Step 11 voices Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/qa-evidence.ts | 19 ++++++++++--- scripts/resolvers/qa.ts | 2 +- ship/sections/review-army.md | 2 +- test/qa-report-dir.test.ts | 28 ++++++++++++++++++- .../ship/gst-969-qa-tree-writes.json | 2 +- 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/lib/qa-evidence.ts b/lib/qa-evidence.ts index 64801f465..111872c4c 100644 --- a/lib/qa-evidence.ts +++ b/lib/qa-evidence.ts @@ -400,16 +400,25 @@ function reportDir(): Record { const excluded = fs.existsSync(excludeFile) ? fs.readFileSync(excludeFile, 'utf8') : ''; if (!excluded.split(/\r?\n/).includes('/.gstack/tmp/')) fs.appendFileSync(excludeFile, `${excluded && !excluded.endsWith('\n') ? '\n' : ''}/.gstack/tmp/\n`); const parent = path.join(repoRoot, '.gstack', 'tmp', 'qa-reports'); - fs.mkdirSync(parent, { recursive: true, mode: 0o700 }); + for (const dir of [path.join(repoRoot, '.gstack'), path.dirname(parent), parent]) { + try { + if (fs.lstatSync(dir).isSymbolicLink()) throw new QaEvidenceError(`report-dir refuses the linked ${path.relative(repoRoot, dir)}`); + } catch (error: any) { + if (error?.code !== 'ENOENT') throw error; + fs.mkdirSync(dir, { mode: 0o700 }); + } + } + const ignored = spawnSync('git', ['check-ignore', '-q', path.join(parent, 'report.md')], { cwd: repoRoot, encoding: 'utf8', timeout: 5000 }); + if (ignored.status !== 0) throw new QaEvidenceError('report-dir: .gstack/tmp is not git-ignored here (a .gitignore rule overrides info/exclude)'); const stamp = new Date().toISOString().replace(/[-:]/g, '').replace(/\.\d+Z$/, 'Z'); for (let n = 1; ; n++) { const dir = path.join(parent, `run-${stamp}${n > 1 ? `-${n}` : ''}`); try { fs.mkdirSync(dir, { mode: 0o700 }); } catch (error: any) { if (error?.code === 'EEXIST') continue; throw error; } - return { action: 'report-dir', status: 'complete', reportDir: dir, exitCode: 0 }; + return { action: 'report-dir', status: 'complete', reportDir: qaEvidenceRoot(dir), exitCode: 0 }; } } -const QA_EVIDENCE_USAGE = 'report-dir (prints a fresh git-excluded report root for /review and /ship) | capture ROOT ID [--public] --deadline FILE|--timeout-ms MS [--after PREVIOUS_CAPTURE --hypothesis TEXT] -- COMMAND ARGS (--after publishes checkpoint ID linking PREVIOUS_CAPTURE to this probe; required after the first complete capture unless a checkpoint was published) | checkpoint ROOT ID CAPTURE OBSERVATION_COMMAND HYPOTHESIS NEXT_COMMAND | checkpoint ROOT ID INTENT_FILE | materialize ROOT ANNOTATIONS (annotations: {evidence: [{capture, command, contract, expected, classification: pass|superseded|product-defect|fail|setup-blocked|blocked|inconclusive}], limits: [..]}; revision, runtime, cwd and learning are filled in)'; +const QA_EVIDENCE_USAGE = 'report-dir (prints a fresh git-excluded report root for /ship QA) | capture ROOT ID [--public] --deadline FILE|--timeout-ms MS [--after PREVIOUS_CAPTURE --hypothesis TEXT] -- COMMAND ARGS (--after publishes checkpoint ID linking PREVIOUS_CAPTURE to this probe; required after the first complete capture unless a checkpoint was published) | checkpoint ROOT ID CAPTURE OBSERVATION_COMMAND HYPOTHESIS NEXT_COMMAND | checkpoint ROOT ID INTENT_FILE | materialize ROOT ANNOTATIONS (annotations: {evidence: [{capture, command, contract, expected, classification: pass|superseded|product-defect|fail|setup-blocked|blocked|inconclusive}], limits: [..]}; revision, runtime, cwd and learning are filled in)'; export async function qaEvidenceMain(args: string[]): Promise { return withQaReceiptOutput(false, 'qa-evidence-receipt', value => value.event === 'observation' @@ -422,7 +431,9 @@ export async function qaEvidenceMain(args: string[]): Promise { return 0; } if (action === 'report-dir' && args.length === 1) { - emit('stdout', reportDir()); + let created: Record; + try { created = reportDir(); } catch (error: any) { throw error instanceof QaEvidenceError ? error : new QaEvidenceError(`report-dir failed: ${error?.code ?? 'unknown error'}`); } + emit('stdout', created); return 0; } const root = qaEvidenceRoot(reportRoot); diff --git a/scripts/resolvers/qa.ts b/scripts/resolvers/qa.ts index 11ab11c9c..7ff67ecb3 100644 --- a/scripts/resolvers/qa.ts +++ b/scripts/resolvers/qa.ts @@ -244,7 +244,7 @@ export function generateQAReview(ctx: TemplateContext): string { Only the parent runs report-only discovery. Never overwrite another run's reports. Batch only independent Reads. -${ship ? `Without a caller REPORT_DIR, use the \`reportDir\` that \`${quoteSafePath(ctx.paths.binDir)}/gstack-qa-evidence report-dir\` prints: git-excluded, so QA never changes the reviewed tree.\n` : ''} +${ship ? `Without a caller REPORT_DIR, set it to the \`reportDir\` that \`${quoteSafePath(ctx.paths.binDir)}/gstack-qa-evidence report-dir\` prints: git-excluded, so QA never changes the reviewed tree.\n` : ''} ${ship ? `**1. Load methods before any QA or explicit-verification probe.** ${generateQAReviewPreflight(ctx)}` : `**1. Set the charter and isolation.** diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 83c0f10e2..81040c5f8 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -594,7 +594,7 @@ If the Red Team fails or times out, confirm it stopped and record its review as Only the parent runs report-only discovery. Never overwrite another run's reports. Batch only independent Reads. -Without a caller REPORT_DIR, use the `reportDir` that `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded, so QA never changes the reviewed tree. +Without a caller REPORT_DIR, set it to the `reportDir` that `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded, so QA never changes the reviewed tree. **1. Load methods before any QA or explicit-verification probe.** diff --git a/test/qa-report-dir.test.ts b/test/qa-report-dir.test.ts index d5c437fb1..e084b8830 100644 --- a/test/qa-report-dir.test.ts +++ b/test/qa-report-dir.test.ts @@ -61,6 +61,32 @@ test('each report-dir call owns a fresh run so a second QA run never overwrites expect(fs.readdirSync(second)).toEqual([]); }); +test('report-dir refuses a linked .gstack instead of creating directories through it', () => { + const f = product(); + const elsewhere = fs.mkdtempSync(path.join(ROOT, 'elsewhere-')); + fs.symlinkSync(elsewhere, path.join(f.root, '.gstack')); + const r = f.tool('gstack-qa-evidence', 'report-dir'); + expect(r.status).toBe(2); + expect(r.stderr).toContain('report-dir refuses the linked .gstack'); + expect(fs.readdirSync(elsewhere)).toEqual([]); +}); + +test('report-dir refuses when a product .gitignore un-ignores .gstack/tmp', () => { + const f = product(); + fs.writeFileSync(path.join(f.root, '.gitignore'), '!/.gstack/tmp/\n!/.gstack/tmp/**\n'); + const r = f.tool('gstack-qa-evidence', 'report-dir'); + expect(r.status).toBe(2); + expect(r.stderr).toContain('.gstack/tmp is not git-ignored here'); +}); + +test('report-dir names the filesystem cause when .gstack is not a directory', () => { + const f = product(); + fs.writeFileSync(path.join(f.root, '.gstack'), 'not a directory\n'); + const r = f.tool('gstack-qa-evidence', 'report-dir'); + expect(r.status).toBe(2); + expect(r.stderr).toContain('report-dir failed: ENOTDIR'); +}); + test('report-dir refuses outside a git work tree instead of guessing a location', () => { const outside = fs.mkdtempSync(path.join(ROOT, 'plain-')); const r = spawnSync(process.execPath, [path.join(BIN, 'gstack-qa-evidence'), 'report-dir'], { cwd: outside, encoding: 'utf8', timeout: 15_000, env: { ...process.env, GIT_CEILING_DIRECTORIES: ROOT } }); @@ -71,7 +97,7 @@ test('report-dir refuses outside a git work tree instead of guessing a location' test('/ship Step 9.2.1 resolves REPORT_DIR through report-dir, not the product tree', () => { const ctx: TemplateContext = { host: 'claude', skillName: 'ship', tmplPath: '', paths: HOST_PATHS.claude }; - expect(RESOLVERS.QA_REVIEW(ctx)).toContain('that `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded'); + expect(RESOLVERS.QA_REVIEW(ctx)).toContain('set it to the `reportDir` that `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded'); const rendered = fs.readFileSync(path.resolve(import.meta.dir, '../ship/sections/review-army.md'), 'utf8'); expect(rendered).toContain('gstack-qa-evidence report-dir` prints: git-excluded'); }); diff --git a/test/size-ledger/ship/gst-969-qa-tree-writes.json b/test/size-ledger/ship/gst-969-qa-tree-writes.json index b6930b3bb..b64cc6568 100644 --- a/test/size-ledger/ship/gst-969-qa-tree-writes.json +++ b/test/size-ledger/ship/gst-969-qa-tree-writes.json @@ -1,4 +1,4 @@ { - "bytes": 179, + "bytes": 185, "why": "GST-969: /ship Step 9.2.1 QA writes its reports under gstack-qa-evidence report-dir (git-excluded .gstack/tmp), not the product tree" } From d29af566af1f9773b2b20eb1200cce56e4c8fbb9 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 23:58:22 +0000 Subject: [PATCH 4/5] docs: CHANGELOG fragment for GST-969 Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gst-969-qa-tree-writes.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/gst-969-qa-tree-writes.md diff --git a/changelog.d/gst-969-qa-tree-writes.md b/changelog.d/gst-969-qa-tree-writes.md new file mode 100644 index 000000000..604321faa --- /dev/null +++ b/changelog.d/gst-969-qa-tree-writes.md @@ -0,0 +1,3 @@ +### Fixed + +- **/ship's exploratory QA no longer writes reports into your project (GST-969).** Step 9.2.1 used to put its QA reports, deadline file and captured evidence in `.gstack/qa-reports/` inside the repository under review. In a project that doesn't ignore `.gstack/`, those files showed up as untracked changes, so they could land in the PR and change the tree the review certified. The new `gstack-qa-evidence report-dir` command creates a fresh run directory under `.gstack/tmp/qa-reports/` and adds `/.gstack/tmp/` to the repository's git exclude file, and /ship's QA step now uses it. It refuses a symlinked `.gstack`, a `.gitignore` rule that un-ignores `.gstack/tmp`, and a directory outside a git repository. /review's Step 4.7 still uses `.gstack/qa-reports`: /review's prompt is already at its size cap. From 9b384c86dd991db7c33dc2a8b56dbbaec30b3051 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 00:03:45 +0000 Subject: [PATCH 5/5] fix: move the QA report-dir fallback into the shared exploratory section (review-army.md stays at its no-loss pin); classify qa-reports for gstack-clean Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gst-969-qa-tree-writes.md | 2 +- lib/qa-evidence.ts | 4 ++-- qa/sections/exploratory.md | 2 +- scripts/resolvers/qa.ts | 4 ++-- ship/sections/review-army.md | 1 - test/gstack-clean-producers.test.ts | 1 + test/qa-report-dir.test.ts | 11 ++++++----- test/size-ledger/ship/gst-969-qa-tree-writes.json | 4 ---- 8 files changed, 13 insertions(+), 16 deletions(-) delete mode 100644 test/size-ledger/ship/gst-969-qa-tree-writes.json diff --git a/changelog.d/gst-969-qa-tree-writes.md b/changelog.d/gst-969-qa-tree-writes.md index 604321faa..8962ded61 100644 --- a/changelog.d/gst-969-qa-tree-writes.md +++ b/changelog.d/gst-969-qa-tree-writes.md @@ -1,3 +1,3 @@ ### Fixed -- **/ship's exploratory QA no longer writes reports into your project (GST-969).** Step 9.2.1 used to put its QA reports, deadline file and captured evidence in `.gstack/qa-reports/` inside the repository under review. In a project that doesn't ignore `.gstack/`, those files showed up as untracked changes, so they could land in the PR and change the tree the review certified. The new `gstack-qa-evidence report-dir` command creates a fresh run directory under `.gstack/tmp/qa-reports/` and adds `/.gstack/tmp/` to the repository's git exclude file, and /ship's QA step now uses it. It refuses a symlinked `.gstack`, a `.gitignore` rule that un-ignores `.gstack/tmp`, and a directory outside a git repository. /review's Step 4.7 still uses `.gstack/qa-reports`: /review's prompt is already at its size cap. +- **/ship and /review exploratory QA no longer write reports into your project (GST-969).** /ship Step 9.2.1 and /review Step 4.7 used to put its QA reports, deadline file and captured evidence in `.gstack/qa-reports/` inside the repository under review. In a project that doesn't ignore `.gstack/`, those files showed up as untracked changes, so they could land in the PR and change the tree the review certified. The new `gstack-qa-evidence report-dir` command creates a fresh run directory under `.gstack/tmp/qa-reports/` and adds `/.gstack/tmp/` to the repository's git exclude file, and both QA steps now use it. It refuses a symlinked `.gstack`, a `.gitignore` rule that un-ignores `.gstack/tmp`, and a directory outside a git repository. /qa keeps its `.gstack/qa-reports` output directory. diff --git a/lib/qa-evidence.ts b/lib/qa-evidence.ts index 111872c4c..841cb2354 100644 --- a/lib/qa-evidence.ts +++ b/lib/qa-evidence.ts @@ -388,7 +388,7 @@ function materialize(root: string, source: string) { next: `Include every reportLinks entry in the Markdown report, and report the overall status as ${verdict.status}${verdict.open.length ? ` (open: ${verdict.open.join('; ')})` : ''}; this verdict is final. Next, write the Markdown report beside evidence.json.` }; } -/** A fresh report root for /review and /ship QA under the repository's git-excluded .gstack/tmp, so probes never change the reviewed tree. */ +/** A fresh report root for caller QA (/review and /ship) under the repository's git-excluded .gstack/tmp, so probes never change the reviewed tree. */ function reportDir(): Record { const top = spawnSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8', timeout: 5000 }); if (top.status !== 0 || !top.stdout.trim()) throw new QaEvidenceError('report-dir needs a git work tree'); @@ -418,7 +418,7 @@ function reportDir(): Record { } } -const QA_EVIDENCE_USAGE = 'report-dir (prints a fresh git-excluded report root for /ship QA) | capture ROOT ID [--public] --deadline FILE|--timeout-ms MS [--after PREVIOUS_CAPTURE --hypothesis TEXT] -- COMMAND ARGS (--after publishes checkpoint ID linking PREVIOUS_CAPTURE to this probe; required after the first complete capture unless a checkpoint was published) | checkpoint ROOT ID CAPTURE OBSERVATION_COMMAND HYPOTHESIS NEXT_COMMAND | checkpoint ROOT ID INTENT_FILE | materialize ROOT ANNOTATIONS (annotations: {evidence: [{capture, command, contract, expected, classification: pass|superseded|product-defect|fail|setup-blocked|blocked|inconclusive}], limits: [..]}; revision, runtime, cwd and learning are filled in)'; +const QA_EVIDENCE_USAGE = 'report-dir (prints a fresh git-excluded report root for /review and /ship QA) | capture ROOT ID [--public] --deadline FILE|--timeout-ms MS [--after PREVIOUS_CAPTURE --hypothesis TEXT] -- COMMAND ARGS (--after publishes checkpoint ID linking PREVIOUS_CAPTURE to this probe; required after the first complete capture unless a checkpoint was published) | checkpoint ROOT ID CAPTURE OBSERVATION_COMMAND HYPOTHESIS NEXT_COMMAND | checkpoint ROOT ID INTENT_FILE | materialize ROOT ANNOTATIONS (annotations: {evidence: [{capture, command, contract, expected, classification: pass|superseded|product-defect|fail|setup-blocked|blocked|inconclusive}], limits: [..]}; revision, runtime, cwd and learning are filled in)'; export async function qaEvidenceMain(args: string[]): Promise { return withQaReceiptOutput(false, 'qa-evidence-receipt', value => value.event === 'observation' diff --git a/qa/sections/exploratory.md b/qa/sections/exploratory.md index 0c62850b0..1364afdb9 100644 --- a/qa/sections/exploratory.md +++ b/qa/sections/exploratory.md @@ -19,7 +19,7 @@ Missing or unreadable assets, prerequisites or permission block affected probes, ## 1. Charter and preflight -Reuse resolved REPORT_DIR; otherwise own a fresh `.gstack/qa-reports` subdirectory. +Reuse resolved REPORT_DIR; otherwise use the `reportDir` `EVIDENCE_TOOL report-dir` prints. Write a **charter** per behavior: contract, risk, entrypoint, isolation, exit condition, source, commands and inputs. Save charters as Markdown in the report. For /review and /ship, no plan/server is required. diff --git a/scripts/resolvers/qa.ts b/scripts/resolvers/qa.ts index 7ff67ecb3..76be4da89 100644 --- a/scripts/resolvers/qa.ts +++ b/scripts/resolvers/qa.ts @@ -66,7 +66,7 @@ full contents. If either required Read is missing, complete it now before Charte ## 1. Charter and preflight -Reuse resolved REPORT_DIR; otherwise own a fresh \`.gstack/qa-reports\` subdirectory. +${reportOnly ? 'Reuse resolved REPORT_DIR; otherwise own a fresh \`.gstack/qa-reports\` subdirectory.' : 'Reuse resolved REPORT_DIR; otherwise use the \`reportDir\` \`EVIDENCE_TOOL report-dir\` prints.'} Write a **charter** per behavior: contract, risk, entrypoint, isolation, exit condition, source, commands and inputs. Save charters as Markdown in the report. ${reportOnly ? '' : `For /review and /ship, no plan/server is required. @@ -244,7 +244,7 @@ export function generateQAReview(ctx: TemplateContext): string { Only the parent runs report-only discovery. Never overwrite another run's reports. Batch only independent Reads. -${ship ? `Without a caller REPORT_DIR, set it to the \`reportDir\` that \`${quoteSafePath(ctx.paths.binDir)}/gstack-qa-evidence report-dir\` prints: git-excluded, so QA never changes the reviewed tree.\n` : ''} + ${ship ? `**1. Load methods before any QA or explicit-verification probe.** ${generateQAReviewPreflight(ctx)}` : `**1. Set the charter and isolation.** diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 81040c5f8..f78dda7d1 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -594,7 +594,6 @@ If the Red Team fails or times out, confirm it stopped and record its review as Only the parent runs report-only discovery. Never overwrite another run's reports. Batch only independent Reads. -Without a caller REPORT_DIR, set it to the `reportDir` that `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded, so QA never changes the reviewed tree. **1. Load methods before any QA or explicit-verification probe.** diff --git a/test/gstack-clean-producers.test.ts b/test/gstack-clean-producers.test.ts index daad0df41..937eb19cb 100644 --- a/test/gstack-clean-producers.test.ts +++ b/test/gstack-clean-producers.test.ts @@ -42,6 +42,7 @@ const KEPT: Record = { 'review-*': '/ship review loop records', 'fix-check-*': '/ship fix-check inputs', 'tasks-*': 'gstack-tasks rows files (bin/gstack-tasks)', + 'qa-reports': 'caller QA report runs from gstack-qa-evidence report-dir (lib/qa-evidence.ts)', 'autoplan': 'autoplan phase artifacts (autoplan/SKILL.md.tmpl)', 'eval-bisect': '/investigate eval bisect cache', 'price-gate': 'gstack-eval-plan price receipts', diff --git a/test/qa-report-dir.test.ts b/test/qa-report-dir.test.ts index e084b8830..97a121703 100644 --- a/test/qa-report-dir.test.ts +++ b/test/qa-report-dir.test.ts @@ -95,9 +95,10 @@ test('report-dir refuses outside a git work tree instead of guessing a location' expect(fs.readdirSync(outside)).toEqual([]); }); -test('/ship Step 9.2.1 resolves REPORT_DIR through report-dir, not the product tree', () => { - const ctx: TemplateContext = { host: 'claude', skillName: 'ship', tmplPath: '', paths: HOST_PATHS.claude }; - expect(RESOLVERS.QA_REVIEW(ctx)).toContain('set it to the `reportDir` that `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence report-dir` prints: git-excluded'); - const rendered = fs.readFileSync(path.resolve(import.meta.dir, '../ship/sections/review-army.md'), 'utf8'); - expect(rendered).toContain('gstack-qa-evidence report-dir` prints: git-excluded'); +test('caller QA without a resolved REPORT_DIR (/review, /ship) falls back to report-dir, not the product tree', () => { + const line = 'Reuse resolved REPORT_DIR; otherwise use the `reportDir` `EVIDENCE_TOOL report-dir` prints.'; + const ctx: TemplateContext = { host: 'claude', skillName: 'qa', tmplPath: '', paths: HOST_PATHS.claude }; + expect(RESOLVERS.QA_EXPLORATORY(ctx)).toContain(line); + expect(fs.readFileSync(path.resolve(import.meta.dir, '../qa/sections/exploratory.md'), 'utf8')).toContain(line); + expect(fs.readFileSync(path.resolve(import.meta.dir, '../qa/SKILL.md.tmpl'), 'utf8')).toContain('Use the directory as `REPORT_DIR`'); }); diff --git a/test/size-ledger/ship/gst-969-qa-tree-writes.json b/test/size-ledger/ship/gst-969-qa-tree-writes.json deleted file mode 100644 index b64cc6568..000000000 --- a/test/size-ledger/ship/gst-969-qa-tree-writes.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "bytes": 185, - "why": "GST-969: /ship Step 9.2.1 QA writes its reports under gstack-qa-evidence report-dir (git-excluded .gstack/tmp), not the product tree" -}