mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
C2: name the flows judges found implicit in review resolvers
Structured outside review: MISSING COVERAGE asks no fix question and is never reported clean (was "preserve the existing user decision flow"). /review's HIGH-impact plan gate: spawned or non-interactive runs report REQUIREMENTS MISSING and continue, writing no TODOs. Value cards: the byte clamp applies to card lines; JSON records you write keep full values.
This commit is contained in:
1 parent
78d4a2a343
commit
1c105acb31
6 files changed
+17
-2
No files matched your search
@@ -176,7 +176,7 @@ ${outsideVoiceInvocation(ctx, { timeoutMs: 540000, nativeAlreadyRequired: true,
|
||||
${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. 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.
|
||||
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: no fix question; say so and never report the review clean.
|
||||
|
||||
If GATE is FAIL, use AskUserQuestion:
|
||||
\`\`\`
|
||||
|
||||
@@ -472,6 +472,7 @@ The plan completion results augment the existing Scope Drift Detection. If a pla
|
||||
- Options: A) Stop this review for implementation, B) Continue this review with P1 TODOs, C) Record the items as intentionally dropped
|
||||
- A ends this invocation before code review or implementation. List the missing work; after implementation, start a fresh /review.
|
||||
- B queues the approved TODO changes for Step 5, not this read-only audit. B/C continue to the final Scope Check and Step 2. None of these choices authorizes shipping or waives required verification.
|
||||
- Spawned or non-interactive runs ask nothing: report REQUIREMENTS MISSING with the items and continue, writing no TODOs and dropping nothing.
|
||||
|
||||
This is **INFORMATIONAL** unless HIGH-impact plan-file discrepancies are found (then it gates via AskUserQuestion).
|
||||
Discrepancies derived only from fallback sources (commit messages, TODOS.md, PR description) never trigger
|
||||
|
||||
@@ -129,7 +129,7 @@ function cardRules(mode: TestValueBarMode, skillName: string): string {
|
||||
qa: skillName === 'qa-only' ? 'Put it under each proposed test.' : 'Put it in the 8e.5 record.',
|
||||
audit: 'Read cards from test header comments when present.',
|
||||
}[mode];
|
||||
return `Value card: \`Value: protects=<...>; fails_when=<...>; why_new=<...>; seam=none\` (seam: \`none\` or its name); each field at most ${CARD_FIELD_MAX_BYTES} UTF-8 bytes here (clamp to 157 plus \`...\`; JSON keeps full values). ${where} A missing upstream card never blocks: derive it; ignore unknown fields.
|
||||
return `Value card: \`Value: protects=<...>; fails_when=<...>; why_new=<...>; seam=none\` (seam: \`none\` or its name); each field at most ${CARD_FIELD_MAX_BYTES} UTF-8 bytes here (clamp to 157 plus \`...\`; JSON records you write keep full values). ${where} A missing upstream card never blocks: derive it; ignore unknown fields.
|
||||
|
||||
Example: ${EXAMPLE_CARD}
|
||||
${EXAMPLE_REJECTED}`;
|
||||
|
||||
@@ -28,6 +28,11 @@ test('adversarial outside failures retain the required native pass without dupli
|
||||
expect(output).not.toContain("Use the caller's fallback");
|
||||
expect(output).toContain('Only this optional outside adversarial pass is non-blocking');
|
||||
expect(output).toContain('GATE: MISSING COVERAGE');
|
||||
// C2: the gate names what missing coverage does instead of an unnamed "existing user decision flow".
|
||||
if (output.includes('Only a completed response with severity tags')) {
|
||||
expect(output).toContain('→ GATE: MISSING COVERAGE: no fix question; say so and never report the review clean.');
|
||||
expect(output).not.toContain('preserve the existing user decision flow');
|
||||
}
|
||||
expect(outsideVoiceInvocation(ctx)).toContain("Use the caller's fallback; missing coverage is never clean/PASS.");
|
||||
const disabled = outsideVoicePreflight(ctx, { disabledBehavior: 'skip-all' });
|
||||
expect(disabled).toMatch(/(?:do NOT fall back|Disabled ends this entire extra review step)/);
|
||||
|
||||
@@ -317,6 +317,9 @@ test('review finalization ownership: the plan audit retains its high-impact gate
|
||||
expect(flatAudit).toMatch(/not this read-only audit/i);
|
||||
expect(flatAudit).toContain('B/C continue to the final Scope Check and Step 2');
|
||||
expect(flatAudit).toMatch(/none of these choices authorizes shipping or waives required verification/i);
|
||||
// C2: a spawned or non-interactive review cannot answer the question; its default writes and drops nothing.
|
||||
expect(flatAudit).toContain('Spawned or non-interactive runs ask nothing: report REQUIREMENTS MISSING with the items and continue, writing no TODOs and dropping nothing.');
|
||||
expect(flatAudit.indexOf('Spawned or non-interactive runs ask nothing')).toBeLessThan(flatAudit.indexOf('When continuing after the audit'));
|
||||
});
|
||||
|
||||
test('review scope notes remain provisional until the plan section emits the only final scope check', () => {
|
||||
|
||||
@@ -83,6 +83,12 @@ describe('test value bar render contract', () => {
|
||||
expect(Buffer.byteLength(rendered, 'utf8')).toBeLessThanOrEqual(TEST_VALUE_BAR_MAX_BYTES[mode]);
|
||||
});
|
||||
|
||||
test('the card clamp applies to card lines only; JSON records keep full values (C2)', () => {
|
||||
for (const mode of TEST_VALUE_BAR_MODES) {
|
||||
expect(bar(mode)).toContain(`each field at most ${CARD_FIELD_MAX_BYTES} UTF-8 bytes here (clamp to 157 plus \`...\`; JSON records you write keep full values)`);
|
||||
}
|
||||
});
|
||||
|
||||
test('qa mode names only the invoking skill\'s card location (/qa-only has no 8e.5 record)', () => {
|
||||
const forSkill = (skillName: string) => generateTestValueBar({ ...ctx, skillName }, ['qa']);
|
||||
expect(forSkill('qa')).toContain('Put it in the 8e.5 record.');
|
||||
|
||||
Reference in new issue
Block a user