mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 14:06:50 +01:00
B1b: structured outside-review gate fails on P0 as well as P1
validateOutsideReview() derived its structured pass/fail from a [P1] match, so a review reporting only [P0] findings passed. The wrapper now reads the classifier's findings verdict (P0 or P1), and the /review and /ship structured-gate prose requests [P0]-[P3] tags and fails on P0/P1; untagged output (OUTSIDE_STATUS: unverified) is missing coverage. The ship clarity pin moves from 'structured P1' to 'structured P0/P1' with the behavior.
This commit is contained in:
1 parent
9aba7d9bd7
commit
6f5eb600ca
4 files changed
+12
-5
No files matched your search
@@ -92,7 +92,7 @@ export function validateOutsideReview(text: string, gate: OutsideGate): { comple
|
||||
return { completed: false, reason };
|
||||
}
|
||||
if (gate === 'spec') return { completed: true, score: checked.score, gate: checked.verdict === 'clean' ? 'pass' : 'fail' };
|
||||
if (gate === 'structured') return { completed: true, gate: /\[P1\]|^P1:/m.test(plainReview(text)) ? 'fail' : 'pass' };
|
||||
if (gate === 'structured') return { completed: true, gate: checked.verdict === 'findings' ? 'fail' : 'pass' };
|
||||
return { completed: true };
|
||||
}
|
||||
|
||||
|
||||
@@ -151,7 +151,7 @@ Outside prompt (supply repository context from the parent):
|
||||
|
||||
${outsideVoiceInvocation(ctx, { timeoutMs: 540000, nativeAlreadyRequired: true, diffCommand: 'DIFF_BASE=$(git merge-base origin/<base> HEAD) && git diff "$DIFF_BASE"' })}
|
||||
|
||||
Present the full output verbatim. ${isShip ? 'An unavailable outside challenge does not block shipping by itself; supported findings still enter Step 11, and the structured P1 and non-convergence gates still apply.' : 'This outside challenge is informational; supported findings still enter Step 5 Fix-First, whose approval and convergence gates apply.'}
|
||||
Present the full output verbatim. ${isShip ? 'An unavailable outside challenge does not block shipping by itself; supported findings still enter Step 11, and the structured P0/P1 and non-convergence gates still apply.' : 'This outside challenge is informational; supported findings still enter Step 5 Fix-First, whose approval and convergence gates apply.'}
|
||||
|
||||
**Error handling:** Only this optional outside adversarial pass is non-blocking; native completion and structured-review decisions still apply.
|
||||
${outsideVoiceFailurePolicy(ctx, { timeoutMinutes: 9, onTimeout: 'missing-coverage', stderrOnEmpty: true, fallback: 'none', escape: 1 })}
|
||||
@@ -168,14 +168,14 @@ function adversarialStructuredReview(ctx: TemplateContext, isShip: boolean): str
|
||||
|
||||
If \`CODEX_MODE\` is \`ready\` and either \`DIFF_TOTAL >= 200\` or the user requested the override above:
|
||||
|
||||
Prepare a structured review prompt requesting severity-tagged findings ([P1], [P2], [P3]) or an explicit NO_FINDINGS conclusion. Preserve the base-branch scope including committed changes and working-tree changes.
|
||||
Prepare a structured review prompt requesting severity-tagged findings ([P0]-[P3]) or an explicit NO_FINDINGS conclusion. Preserve the base-branch scope including committed changes and working-tree changes.
|
||||
|
||||
${outsideVoiceInvocation(ctx, { timeoutMs: 540000, nativeAlreadyRequired: true, structuredBase: '<base>', gate: 'structured', diffCommand: 'DIFF_BASE=$(git merge-base <base> HEAD) && git diff "$DIFF_BASE"' })}
|
||||
|
||||
${outsideVoiceFor(ctx).id === 'codex' ? 'The Codex backend uses `codex review --base` without a positional prompt: those arguments are mutually exclusive. Never drop --base to resolve an argv error; prompt-only review changes the diff scope.' : 'The Claude Code backend receives the parent-captured base diff, including committed and working-tree changes, because review mode cannot execute git.'}
|
||||
|
||||
Present output under \`${outsideVoiceFor(ctx).label.toUpperCase()} SAYS (code review):\` inside a \`tool-output\` fence.
|
||||
Only a completed response with severity tags or an explicit no-findings conclusion establishes the gate. P1 findings (\`[P1]\` or native \`P1:\` labels) → GATE: FAIL. Completed without P1 → GATE: PASS. Refusal, failure, or missing markers → GATE: MISSING COVERAGE; preserve the existing user decision flow.
|
||||
Only a completed response with severity tags or an explicit no-findings conclusion establishes the gate. P0/P1 findings (\`[P0]\`/\`[P1]\` or native \`P0:\`/\`P1:\` labels; \`VERDICT: findings\`) → GATE: FAIL. Completed without P0/P1 → GATE: PASS. Refusal, failure, missing markers or \`OUTSIDE_STATUS: unverified\` → GATE: MISSING COVERAGE; preserve the existing user decision flow.
|
||||
|
||||
If GATE is FAIL, use AskUserQuestion:
|
||||
\`\`\`
|
||||
|
||||
@@ -73,6 +73,13 @@ describe('validateOutsideReview keeps the text-only shape for direct importers',
|
||||
expect(validateOutsideReview('[P2] nit', 'structured')).toEqual({ completed: true, gate: 'pass' });
|
||||
expect(validateOutsideReview('[P1] bug', 'structured')).toEqual({ completed: true, gate: 'fail' });
|
||||
});
|
||||
|
||||
test('B1b: the structured gate fails on P0 as well as P1', () => {
|
||||
for (const text of ['[P0] corrupts every row', 'P0: corrupts every row', '**P0:** corrupts every row', '[P3] nit\n[P0] corrupts every row']) {
|
||||
expect(validateOutsideReview(text, 'structured')).toEqual({ completed: true, gate: 'fail' });
|
||||
}
|
||||
expect(validateOutsideReview('[P2] naming\n[P3] nit', 'structured')).toEqual({ completed: true, gate: 'pass' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('outside-review-result CLI', () => {
|
||||
|
||||
@@ -140,7 +140,7 @@ test.each(ALL_HOST_CONFIGS.map(({ name }) => name))('%s: ship adversarial approv
|
||||
test('outside challenge availability never resets fix limits or bypasses blocking owners', () => {
|
||||
const adversarial = compact(readTemplate('ship/sections/adversarial.md'));
|
||||
expect(adversarial).toMatch(/an unavailable outside challenge does not block shipping by itself/i);
|
||||
expect(adversarial).toMatch(/structured P1 and non-convergence gates still apply/i);
|
||||
expect(adversarial).toMatch(/structured P0\/P1 and non-convergence gates still apply/i);
|
||||
expect(adversarial).toMatch(/returning here never resets Step 9's three-cycle fix limit/i);
|
||||
const standaloneReview = readTemplate('review/sections/adversarial.md');
|
||||
expect(standaloneReview).toContain('Step 5 Fix-First');
|
||||
|
||||
Reference in new issue
Block a user