diff --git a/devex-review/SKILL.md b/devex-review/SKILL.md index 3bc9020a3..2ff7c844b 100644 --- a/devex-review/SKILL.md +++ b/devex-review/SKILL.md @@ -800,7 +800,7 @@ Pick the evidence source by surface: (the helper refuses those flags without `--approved`). Unapproved paths are recorded unverified. ```bash TARGET_CLI="${TARGET_CLI:?set TARGET_CLI to the CLI under audit}" - ~/.claude/skills/gstack/bin/gstack-error-capture --launch-check -- "$TARGET_CLI" + ~/.claude/skills/gstack/bin/gstack-error-capture -- "$TARGET_CLI" --help ~/.claude/skills/gstack/bin/gstack-error-capture -- "$TARGET_CLI" --gstack-unknown-flag ``` Swap `--gstack-unknown-flag` for each rejected form the help shows. @@ -823,7 +823,7 @@ when `secret_findings` is non-zero; `other_masked` (emails, paths, IPs) is not a `scan_complete: false` or a withheld stream leaves redaction unknown. You judge what, fix, recovery, code, context and trace. -`launch: failed` means the containment broke the CLI before it ran: report it as a +`launch: failed` means the target never started in the containment (exit 127): report it as a containment limit and offer an approved rerun with `--pass-env`. A reached missing-auth or missing-config error is a real environment-class path: score it. Record per path the containment settings, passed env names and the error class reached; classes you never diff --git a/devex-review/SKILL.md.tmpl b/devex-review/SKILL.md.tmpl index 008477f97..6dd69f932 100644 --- a/devex-review/SKILL.md.tmpl +++ b/devex-review/SKILL.md.tmpl @@ -125,7 +125,7 @@ Pick the evidence source by surface: (the helper refuses those flags without `--approved`). Unapproved paths are recorded unverified. ```bash TARGET_CLI="${TARGET_CLI:?set TARGET_CLI to the CLI under audit}" - ~/.claude/skills/gstack/bin/gstack-error-capture --launch-check -- "$TARGET_CLI" + ~/.claude/skills/gstack/bin/gstack-error-capture -- "$TARGET_CLI" --help ~/.claude/skills/gstack/bin/gstack-error-capture -- "$TARGET_CLI" --gstack-unknown-flag ``` Swap `--gstack-unknown-flag` for each rejected form the help shows. @@ -148,7 +148,7 @@ when `secret_findings` is non-zero; `other_masked` (emails, paths, IPs) is not a `scan_complete: false` or a withheld stream leaves redaction unknown. You judge what, fix, recovery, code, context and trace. -`launch: failed` means the containment broke the CLI before it ran: report it as a +`launch: failed` means the target never started in the containment (exit 127): report it as a containment limit and offer an approved rerun with `--pass-env`. A reached missing-auth or missing-config error is a real environment-class path: score it. Record per path the containment settings, passed env names and the error class reached; classes you never diff --git a/lib/dx-error-capture.ts b/lib/dx-error-capture.ts index 9f0f82ad2..9f9af4a7e 100644 --- a/lib/dx-error-capture.ts +++ b/lib/dx-error-capture.ts @@ -67,13 +67,17 @@ Flags: (repeatable, needs --approved; symlinks and non-regular files are refused). --cwd Run in this directory instead of a temp one (needs --approved). --approved The user approved the --pass-env, --copy-config and --cwd values given. - --launch-check Run " --version" in the same containment and report launch ok|failed. + --launch-check Run " --version" in the same containment; launch says whether it started, + exit_code is the probe's own result. --help Print this help. Containment: stdin is /dev/null, no controlling terminal, own process group, temp cwd and temp HOME/XDG_CONFIG_HOME in a 0700 directory under the gstack state root (removed on exit), env limited -to PATH, LANG, LC_*, TMPDIR, TERM=dumb and version-manager shims (NVM_DIR, NVM_BIN, MISE_*, VOLTA_HOME, -ASDF_*, PYENV_ROOT, RBENV_ROOT; prefixed names that look like credentials are dropped). +to PATH, LANG, LC_*, TMPDIR, TERM=dumb and version-manager locations (NVM_DIR, NVM_BIN, VOLTA_HOME, +PYENV_ROOT, RBENV_ROOT, MISE_DATA_DIR, ASDF_DIR, ASDF_DATA_DIR); config, trust and env-loading +variables (MISE_*, ASDF_* and the rest) need --pass-env. Containment lasts while the helper runs: +a SIGKILLed helper, or a descendant that leaves the process group, outlives it, so capture only +read-only invocations. Record (one JSON object on stdout): version, argv (redacted), exit_code (null when killed or signalled), signal, timed_out (the helper's own timer only), deadline_s, terminal {verdict: blocking | @@ -130,8 +134,8 @@ function parseArgs(argv: string[]): Options { } const OWNED_ENV = new Set(['PATH', 'HOME', 'XDG_CONFIG_HOME', 'TERM', 'USERPROFILE']); -const SHIM_NAMES = new Set(['PATH', 'LANG', 'TMPDIR', 'NVM_DIR', 'NVM_BIN', 'VOLTA_HOME', 'PYENV_ROOT', 'RBENV_ROOT']); -const SHIM_PREFIXES = ['LC_', 'MISE_', 'ASDF_']; +const SHIM_NAMES = new Set(['PATH', 'LANG', 'TMPDIR', 'NVM_DIR', 'NVM_BIN', 'VOLTA_HOME', 'PYENV_ROOT', 'RBENV_ROOT', 'MISE_DATA_DIR', 'ASDF_DIR', 'ASDF_DATA_DIR']); +const SHIM_PREFIXES = ['LC_']; const WINDOWS_NAMES = new Set(['SYSTEMROOT', 'COMSPEC', 'PATHEXT', 'WINDIR']); const CREDENTIAL_NAME = /TOKEN|SECRET|PASSW|KEY|AUTH|CREDENTIAL|COOKIE|SESSION/i; @@ -261,12 +265,12 @@ const bump = (map: Map, id: string, by = 1) => map.set(id, (map. /** Sanitize, mask pass-env values literally, then mask every finding span. Null means withhold. */ function clean(text: string, literals: string[], tally?: Tally, maxBytes = 4 * SCAN_CAP): string | null { - let value = sanitize(text); - for (const literal of literals) { - const parts = value.split(literal); + const mask = (input: string) => literals.reduce((acc, literal) => { + const parts = acc.split(literal); if (parts.length > 1 && tally) bump(tally.secret, 'passed_env.value', parts.length - 1); - value = parts.join(PASS_MARK); - } + return parts.join(PASS_MARK); + }, input); + const value = mask(sanitize(mask(text))); const result = scan(value, { maxBytes }); if (result.oversize) return null; if (tally) for (const finding of result.findings) bump(finding.tier === 'HIGH' ? tally.secret : tally.other, finding.id); @@ -279,7 +283,10 @@ function literalForms(names: string[]): string[] { const value = process.env[name] ?? ''; if (value.length < MIN_MASKED_VALUE) continue; const base64 = Buffer.from(value, 'utf8').toString('base64'); - for (const form of [value, encodeURIComponent(value), base64, base64.replace(/=+$/, ''), Buffer.from(value, 'utf8').toString('base64url')]) forms.add(form); + for (const form of [value, encodeURIComponent(value), JSON.stringify(value).slice(1, -1), base64, base64.replace(/=+$/, ''), Buffer.from(value, 'utf8').toString('base64url')]) { + forms.add(form); + forms.add(sanitize(form)); + } } return [...forms].sort((a, b) => b.length - a.length); } @@ -451,7 +458,7 @@ async function capture(options: Options): Promise<{ record: Record chunk.stream === 'stderr').map(chunk => chunk.bytes)).toString('utf8')); const terminal = classifyTerminal(stderrText, { exitCode, timedOut: run.timedOut, launched, quietMs: run.quietMs, outputBytes: run.outputBytes, deadlineS }); return { literals, record: { ...base, exit_code: exitCode, signal: run.signal, timed_out: run.timedOut, deadline_s: deadlineS, terminal, - launch: !launched || (options.launchCheck && exitCode !== 0) ? 'failed' : 'ok', + launch: launched ? 'ok' : 'failed', stdout: released.text.stdout, stderr: released.text.stderr, withheld_reason: released.withheld, secret_findings: tallyOut(released.tally.secret), other_masked: tallyOut(released.tally.other), scan_complete: released.scanComplete, truncated: released.truncated, passed_env: options.passEnv, copied_config: copied, @@ -462,9 +469,19 @@ async function capture(options: Options): Promise<{ record: Record new Promise(resolve => { stream.write(text, () => resolve()); }); -const errorLine = (what: string, fix: string, code: string) => `${PROG}: ${what}; fix: ${fix} (${code})\n`; +const stringFields = (value: unknown): string[] => typeof value === 'string' ? [value] + : value && typeof value === 'object' ? Object.values(value).flatMap(stringFields) : []; + +/** Every diagnostic passes the record's redaction; a line that still matches prints without its details. */ +function errorLine(what: string, fix: string, code: DxCaptureCodeName, literals: string[]): string { + const safe = clean(`${PROG}: ${what}; fix: ${fix} (${code})`, literals); + const leaked = safe === null || literals.some(literal => safe.includes(literal)) + || scan(safe, { maxBytes: 2 * Buffer.byteLength(safe, 'utf8') + 1 }).findings.length > 0; + return `${leaked ? `${PROG}: details withheld by redaction; fix: ${DX_CAPTURE_CODES[code].fix} (${code})` : safe}\n`; +} export async function dxErrorCaptureMain(argv: string[]): Promise { + let literals: string[] = []; try { const options = parseArgs(argv); if (options.help) { await writeTo(process.stdout, DX_CAPTURE_HELP); return 0; } @@ -474,26 +491,28 @@ export async function dxErrorCaptureMain(argv: string[]): Promise { `ask the user to approve ${gated.join(' and ')} as given, then rerun with --approved`); } checkPassEnv(options.passEnv); + literals = literalForms(options.passEnv); if (options.cwd && !fs.statSync(options.cwd, { throwIfNoEntry: false })?.isDirectory()) { throw new CaptureError('DX_CAPTURE_BAD_CWD', `--cwd ${JSON.stringify(options.cwd)} is not an existing directory`); } - const { record, literals } = await capture(options); + const { record } = await capture(options); const json = JSON.stringify(record); - if (literals.some(literal => json.includes(literal)) || scan(json, { maxBytes: 2 * json.length + 1 }).findings.some(finding => finding.tier === 'HIGH')) { + if (stringFields(record).some(field => literals.some(literal => field.includes(literal)) + || scan(field, { maxBytes: 2 * Buffer.byteLength(field, 'utf8') + 1 }).findings.some(finding => finding.tier === 'HIGH'))) { throw new CaptureError('DX_CAPTURE_REDACTION_FAILED', 'the finished record still matched a secret check; nothing was printed'); } await writeTo(process.stdout, json + '\n'); if (record.launch !== 'not_found') return 0; await writeTo(process.stderr, errorLine(`${JSON.stringify(path.basename(options.command))} does not resolve to an executable file; nothing ran`, - DX_CAPTURE_CODES.DX_CAPTURE_NOT_FOUND.fix, 'DX_CAPTURE_NOT_FOUND')); + DX_CAPTURE_CODES.DX_CAPTURE_NOT_FOUND.fix, 'DX_CAPTURE_NOT_FOUND', literals)); return 1; } catch (error) { if (error instanceof CaptureError) { - await writeTo(process.stderr, errorLine(error.what, error.fix, error.code)); + await writeTo(process.stderr, errorLine(error.what, error.fix, error.code, literals)); return DX_CAPTURE_CODES[error.code].exit; } const frame = ((error as Error)?.stack ?? '').split('\n').find(line => line.trim().startsWith('at '))?.trim() ?? 'no stack frame'; - await writeTo(process.stderr, errorLine(`unexpected ${(error as Error)?.name ?? 'error'} ${frame}`, DX_CAPTURE_CODES.DX_CAPTURE_INTERNAL.fix, 'DX_CAPTURE_INTERNAL')); + await writeTo(process.stderr, errorLine(`unexpected ${(error as Error)?.name ?? 'error'} ${frame}`, DX_CAPTURE_CODES.DX_CAPTURE_INTERNAL.fix, 'DX_CAPTURE_INTERNAL', literals)); return 1; } } diff --git a/test/dx-error-capture.test.ts b/test/dx-error-capture.test.ts index af9d7f379..5ece0fa9d 100644 --- a/test/dx-error-capture.test.ts +++ b/test/dx-error-capture.test.ts @@ -41,6 +41,8 @@ const FIXTURES: Record = { "process.exit(1);", ].join('\n'), 'cross.ts': "const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms));\nawait new Promise(resolve => process.stdout.write(process.argv[2], resolve));\nawait sleep(50);\nawait new Promise(resolve => process.stderr.write(process.argv[3], resolve));\n", + 'echo-json-env.ts': "console.log(JSON.stringify({ value: process.env.DX_TEST_VALUE }));\n", + 'cjk.ts': "console.error('\u9519'.repeat(5000));\nprocess.exit(1);\n", 'echo-env.ts': "const value = process.env.DX_TEST_VALUE ?? '';\nconsole.log('value=' + value);\nconsole.error('encoded=' + Buffer.from(value).toString('base64'));\n", 'big.ts': "const token = process.argv[2] + process.argv[3];\nprocess.stdout.write('a '.repeat(45) + token + ' ' + 'b'.repeat(200) + '\\n');\n", 'bad-utf8.ts': "process.stdout.write(Buffer.from([0x6f, 0x6b, 0xff, 0xfe, 0x0a]));\nprocess.stderr.write('fine\\n');\n", @@ -49,6 +51,7 @@ const FIXTURES: Record = { 'env.ts': "import { readFileSync, statSync } from 'node:fs';\nconst config = process.env.HOME + '/.config/tool/config.json';\nlet copied = null;\ntry { copied = { text: readFileSync(config, 'utf8'), mode: statSync(config).mode & 0o777 }; } catch {}\nconsole.log(JSON.stringify({ cwd: process.cwd(), home: process.env.HOME, xdg: process.env.XDG_CONFIG_HOME, term: process.env.TERM, keys: Object.keys(process.env).sort(), copied }));\n", }; for (const [name, source] of Object.entries(FIXTURES)) fs.writeFileSync(path.join(ROOT, name), source); +fs.writeFileSync(path.join(ROOT, 'no-version.sh'), '#!/bin/sh\necho "unknown option: $1" >&2\nexit 2\n', { mode: 0o755 }); fs.mkdirSync(path.join(FAKE_HOME, '.config/tool'), { recursive: true }); fs.writeFileSync(path.join(FAKE_HOME, '.config/tool/config.json'), '{"color":false}\n'); fs.symlinkSync(path.join(FAKE_HOME, '.config/tool/config.json'), path.join(FAKE_HOME, '.linked.json')); @@ -172,6 +175,25 @@ describe('gstack-error-capture redaction', () => { expect(result.record.containment.env).toContain('DX_TEST_VALUE'); }); + test('a --pass-env value the target JSON-escapes is masked, and decoding stdout cannot rebuild it', () => { + const value = 'quo"te' + randomAlnum(12); + const result = capture(['--pass-env', 'DX_TEST_VALUE', '--approved'], 'echo-json-env.ts', [], { DX_TEST_VALUE: value }); + expect(result.stdout).not.toContain(JSON.stringify(value).slice(1, -1)); + expect(JSON.stringify(JSON.parse(result.record.stdout))).not.toContain(value.slice(6)); + expect(result.record.secret_findings.count).toBeGreaterThan(0); + }); + + test('a forwarded value equal to a JSON literal does not trip the final secret check', () => { + const { record } = capture(['--pass-env', 'CI', '--approved'], 'exit0.ts', [], { CI: 'true' }); + expect(record.exit_code).toBe(0); + }); + + test('multi-byte output under the publish cap is recorded, not withheld', () => { + const { record } = capture([], 'cjk.ts'); + expect(record.stderr).toBe('\u9519'.repeat(5000) + '\n'); + expect(record.truncated.stderr).toBe(false); + }); + test('truncation runs after redaction and drops the run cut at --max-bytes', () => { const result = capture(['--max-bytes', '100'], 'big.ts', TOKEN_HALVES); expect(leaksToken(result.stdout)).toBe(false); @@ -201,7 +223,8 @@ describe('gstack-error-capture containment', () => { test('the target gets a temp cwd and HOME, a scrubbed env and TERM=dumb; the run dir is removed', () => { const staleDir = path.join(STATE, 'error-capture', 'run-' + spawnSync(process.execPath, ['-e', ''], { timeout: 10_000 }).pid + '-stale'); fs.mkdirSync(staleDir, { recursive: true }); - const { record } = capture([], 'env.ts', [], { DX_UNLISTED_SECRET_NAME: 'x', MISE_GITHUB_TOKEN: 'y', MISE_ENV: 'dev' }); + const { record } = capture([], 'env.ts', [], { DX_UNLISTED_SECRET_NAME: 'x', MISE_GITHUB_TOKEN: 'y', MISE_ENV: 'dev', + MISE_GLOBAL_CONFIG_FILE: path.join(FAKE_HOME, 'mise.toml'), MISE_TRUSTED_CONFIG_PATHS: FAKE_HOME, MISE_DATA_DIR: path.join(FAKE_HOME, '.mise') }); const seen = JSON.parse(record.stdout); expect(seen.cwd.startsWith(fs.realpathSync(STATE))).toBe(true); expect(seen.home.startsWith(fs.realpathSync(STATE))).toBe(true); @@ -209,7 +232,8 @@ describe('gstack-error-capture containment', () => { expect(seen.term).toBe('dumb'); expect(seen.keys).not.toContain('DX_UNLISTED_SECRET_NAME'); expect(seen.keys).not.toContain('MISE_GITHUB_TOKEN'); - expect(seen.keys).toContain('MISE_ENV'); + for (const name of ['MISE_ENV', 'MISE_GLOBAL_CONFIG_FILE', 'MISE_TRUSTED_CONFIG_PATHS']) expect(seen.keys).not.toContain(name); + expect(seen.keys).toContain('MISE_DATA_DIR'); expect(record.containment).toEqual({ cwd: 'temp', home: 'temp', env: seen.keys }); expect(fs.readdirSync(path.join(STATE, 'error-capture'))).toEqual([]); expect(fs.statSync(path.join(STATE, 'error-capture')).mode & 0o777).toBe(0o700); @@ -229,6 +253,14 @@ describe('gstack-error-capture containment', () => { expect(result.stderr).toContain('(DX_CAPTURE_CONFIG_REFUSED)'); }); + test('--launch-check reports a started target as launch ok even when --version is rejected', () => { + const result = helper(['--launch-check', '--', path.join(ROOT, 'no-version.sh')]); + expect(result.status).toBe(0); + const record = JSON.parse(result.stdout); + expect(record.launch).toBe('ok'); + expect(record.exit_code).toBe(2); + }); + test('--launch-check runs --version inside the containment', () => { const result = helper(['--launch-check', '--', process.execPath]); expect(result.status).toBe(0); @@ -241,6 +273,14 @@ describe('gstack-error-capture containment', () => { }); describe('gstack-error-capture own errors', () => { + test('its own diagnostics are redacted: a token in an unknown flag never reaches stderr', () => { + const result = helper(['--' + TOKEN, '--', process.execPath]); + expect(result.status).toBe(2); + expect(result.stderr).toMatch(ERROR_LINE); + expect(result.stderr).toContain('(DX_CAPTURE_USAGE)'); + expect(leaksToken(result.stderr)).toBe(false); + }); + test('--pass-env without --approved is refused with exit 3', () => { const result = helper(['--pass-env', 'DX_TEST_VALUE', '--', process.execPath, path.join(ROOT, 'echo-env.ts')], { DX_TEST_VALUE: PASSED_VALUE }); expect(result.status).toBe(3); diff --git a/test/dx-error-contract-grader.test.ts b/test/dx-error-contract-grader.test.ts index b1441c3a1..4b601604c 100644 --- a/test/dx-error-contract-grader.test.ts +++ b/test/dx-error-contract-grader.test.ts @@ -2,7 +2,8 @@ import { describe, expect, test } from 'bun:test'; import { DX_ERROR_COMPACT, DX_ERROR_ITEMS } from '../lib/dx-error-contract'; import { DX_ERROR_CONTRACT_PLAN, capFindings, gradeDxContractReview } from './fixtures/dx-error-contract-case'; -const fence = (rows: Array>) => ['INPUT: dx abc', 'Review prose.', '```gstack-findings', ...rows.map(row => JSON.stringify(row)), '```'].join('\n'); +const SHA = 'a'.repeat(64); +const fence = (rows: Array>) => [`INPUT: dx ${SHA}`, 'Review prose.', '```gstack-findings', ...rows.map(row => JSON.stringify(row)), '```'].join('\n'); const PASSING = [ { id: 'F1', severity: 'Critical', title: 'CAP4:P1:redaction --debug prints the bearer token' }, { id: 'F2', severity: 'Critical', title: 'CAP4:P1:channel push failure exits 0 and blocks on a y/N prompt' }, @@ -27,14 +28,14 @@ describe('dx-error-contract-review grader (free controls for the paid case)', () }); test('a review meeting every bar passes', () => { - const grade = gradeDxContractReview(fence(PASSING)); + const grade = gradeDxContractReview(fence(PASSING), SHA); expect(grade.reasons).toEqual([]); expect(grade.pass).toBe(true); }); test('command-named paths map to their labels', () => { const rows = PASSING.map(row => ({ ...row, title: String(row.title).replace(':P1:', ':acmesync push:').replace(':P5:', ':acmesync migrate:') })); - expect(gradeDxContractReview(fence(rows)).pass).toBe(true); + expect(gradeDxContractReview(fence(rows), SHA).pass).toBe(true); }); test.each([ @@ -45,16 +46,20 @@ describe('dx-error-contract-review grader (free controls for the paid case)', () ['unsafe fix missed', PASSING.filter(row => row.id !== 'F3'), 'P1 missing CAP4:fix'], ['context gap missed', PASSING.filter(row => row.id !== 'F4'), 'P1 missing a CAP finding on context'], ['silent path missed', PASSING.filter(row => row.id !== 'F5'), 'P5 (unspecified error handling) not capped'], + ['malformed severity on a decoy row', [...PASSING, { id: 'F7', severity: 'Banana', title: 'Looks fine' }], 'result:'], ['CAP4 under-rated', PASSING.map(row => row.id === 'F2' ? { ...row, severity: 'High' } : row), 'CAP4 labelled High, want Critical'], ])('%s fails', (_name, rows, reason) => { - const grade = gradeDxContractReview(fence(rows as Array>)); + const grade = gradeDxContractReview(fence(rows as Array>), SHA); expect(grade.pass).toBe(false); expect(grade.reasons.join('\n')).toContain(reason); }); - test('a review with no findings fence fails', () => { - const grade = gradeDxContractReview('INPUT: dx abc\nLooks fine to me.'); - expect(grade.pass).toBe(false); - expect(grade.reasons[0]).toContain('no gstack-findings fence'); + test('a review with no findings fence, or bound to another snapshot, fails', () => { + const unfenced = gradeDxContractReview(`INPUT: dx ${SHA}\nLooks fine to me.`, SHA); + expect(unfenced.pass).toBe(false); + expect(unfenced.reasons.join('\n')).toContain('gstack-findings'); + const rebound = gradeDxContractReview(fence(PASSING), 'b'.repeat(64)); + expect(rebound.pass).toBe(false); + expect(rebound.reasons[0]).toContain('result: receipt names'); }); }); diff --git a/test/fixtures/dx-error-contract-case.ts b/test/fixtures/dx-error-contract-case.ts index 7aae347af..d0e078486 100644 --- a/test/fixtures/dx-error-contract-case.ts +++ b/test/fixtures/dx-error-contract-case.ts @@ -4,7 +4,8 @@ * prompt, run headless). The bars below were fixed before any paid trial and * are not tuned after seeing results. * - * A trial PASSES when the review's gstack-findings fence: + * A trial PASSES when the bound result parses with no errors (the INPUT receipt + * for this snapshot and a valid gstack-findings fence) and the fence: * - flags path P1 (agent-hostile push) with Critical CAP4 findings naming * redaction, channel and fix, and names context in some CAP finding on P1; * - raises no CAP finding on P2 (agent-ready decoy), P3 (remote API error with @@ -12,7 +13,7 @@ * - raises at least one CAP finding on P5 (error handling never specified); * - labels every CAP4 finding Critical and every CAP6 finding High. */ -import { parseRecordFindings } from '../../lib/autoplan-reconcile'; +import { parseRecordFindings, parseReviewResult } from '../../lib/autoplan-reconcile'; import { DX_ERROR_ITEMS } from '../../lib/dx-error-contract'; export const DX_ERROR_CONTRACT_PLAN = `# Plan: acmesync 2.0 CLI error handling @@ -90,10 +91,8 @@ export function capFindings(review: string): CapFinding[] { return caps; } -export function gradeDxContractReview(review: string): DxContractGrade { - const reasons: string[] = []; - const parsed = parseRecordFindings(review); - if (parsed.source !== 'fence') reasons.push(`no gstack-findings fence (source=${parsed.source})`); +export function gradeDxContractReview(review: string, sha256: string): DxContractGrade { + const reasons = parseReviewResult(review, { phase: 'dx', sha256 }).errors.map(error => `result: ${error}`); const caps = capFindings(review); for (const cap of caps) { if (!ITEMS.has(cap.item)) reasons.push(`unknown item in "${cap.title}"`); diff --git a/test/skill-e2e-dx-error-contract.test.ts b/test/skill-e2e-dx-error-contract.test.ts index 1dc22be4a..13195665f 100644 --- a/test/skill-e2e-dx-error-contract.test.ts +++ b/test/skill-e2e-dx-error-contract.test.ts @@ -53,15 +53,15 @@ describeIfSelected('DX error contract native reviewer (arm A)', ['dx-error-contr }); logCost(NAME, result); const review = fs.existsSync(prompts.nativeResult) ? fs.readFileSync(prompts.nativeResult, 'utf8') : ''; - const grade = gradeDxContractReview(review); + const grade = gradeDxContractReview(review, snapshot.sha256); console.log(`dx-error-contract-review caps: ${grade.caps.map(cap => `${cap.severity}:${cap.title}`).join(' | ') || 'none'}`); recordE2E(collector, NAME, 'DX error contract native reviewer', result, { passed: result.exitReason === 'success' && grade.pass, ...(grade.reasons.length ? { error: grade.reasons.join('; ') } : {}), }); expect(result.exitReason).toBe('success'); - expectContract(review.startsWith(`INPUT: dx ${snapshot.sha256}`) && review.includes('```gstack-findings'), - 'dx-error-contract-review: the reviewer did not write a bound result with a gstack-findings fence', { collector, name: NAME }); + expectContract(!grade.reasons.some(reason => reason.startsWith('result: ')), + `dx-error-contract-review: the bound result does not parse: ${grade.reasons.filter(reason => reason.startsWith('result: ')).join('; ')}`, { collector, name: NAME }); expect(grade.reasons).toEqual([]); }, CAPTURE_MS + 30_000); });