mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 06:24:41 +01:00
fix(review): severity follows the blocking floor, not the checklist pass or category (GSTA-160)
The checklist bound severity to its passes (Pass 1 CRITICAL, Pass 2 INFORMATIONAL) and the core floor had no security clause, so GSTA-11's replay saw real bugs filed INFORMATIONAL. One floor sentence now reaches every reviewer prompt; the passes set check order only; shell injection covers shell sinks; a state-swap-before-failure check joins Pass 1 (GSTA-159); the Testing specialist reports a floor-level product defect as category correctness so classify keeps it blocking. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
21277291c8
commit
da65a356cf
23 files changed
+211
-82
No files matched your search
@@ -3,6 +3,8 @@
|
||||
* step whose fixes do not earn a full pass. It reads the fixes, never the whole diff. The parent
|
||||
* writes every input in literally: a reviewer never sees the parent's shell.
|
||||
*/
|
||||
import { BLOCKING_FLOOR } from './review-floor';
|
||||
|
||||
export interface FixCheckInput { fixList: string; touchedHunks: string; callers: string; testResults: string }
|
||||
|
||||
export const FIX_CHECK_INSTRUCTIONS = `You are the fix-check reviewer for a pre-landing review. An earlier review pass found
|
||||
@@ -14,8 +16,7 @@ fails open (accepts what it refused) or fails closed (refuses what it accepted),
|
||||
behaviour the code had before the fix, an error or unreadable state now read as success, a test
|
||||
made weaker. Name the input or call site that reaches each defect.
|
||||
|
||||
Rate each finding against the blocking floor: CRITICAL only when it would fail a test, break
|
||||
production, or lose data or secrets; everything else is INFORMATIONAL. Output one JSON object per
|
||||
Rate each finding against the blocking floor. ${BLOCKING_FLOOR} Output one JSON object per
|
||||
line, {"severity":"CRITICAL|INFORMATIONAL","path":"<file>","line":<n>,"summary":"<what breaks, for which input>"},
|
||||
or the two words NO FINDINGS when the fixes are sound.`;
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ import type { TemplateContext } from './types';
|
||||
import { CC_BACKGROUND_DEFAULT_SINCE, FOREGROUND_IF_AVAILABLE, BACKGROUND_RECOVERY } from './constants';
|
||||
import { learningsCapture, LEARNINGS_VERDICT } from './learnings';
|
||||
import { DEFENSIVE_REVIEW_FRAMING, FIXTURE_SUMMARY_MODE } from './defensive-review';
|
||||
import { BLOCKING_FLOOR } from './review-floor';
|
||||
|
||||
function generateSpecialistSelection(ctx: TemplateContext): string {
|
||||
const isShip = ctx.skillName === 'ship';
|
||||
@@ -146,14 +147,13 @@ Every other specialist, including Testing, reads the full diff with fixtures.
|
||||
|
||||
Required fields: severity, confidence, path, category, summary, specialist.
|
||||
Optional: line, fix, fingerprint, evidence, test_stub, advisory, evidence_paths, helper_target, cause.
|
||||
CRITICAL means a test that would fail, a production break, or a security or data-loss defect; everything else is INFORMATIONAL.
|
||||
${BLOCKING_FLOOR}
|
||||
A defect the diff causes but that shows elsewhere (a caller broken by a changed signature, a consumer of a removed export) keeps the victim as \`path\` and names the changed line in \`cause\`: {\\"cause\\":{\\"path\\":\\"file\\",\\"line\\":N}}. Without it the finding reads as pre-existing code.
|
||||
|
||||
Leave extraction advice to the core shared-code check; report real defects in duplicated code independently, keep advisory metadata on structural advice, and never call a demonstrated defect advisory because a shared helper could fix it.
|
||||
|
||||
If you can write a test that would catch this issue, include it in the \`test_stub\` field.
|
||||
Use the detected test framework ({TEST_FW}). Write a minimal skeleton — describe/it/test
|
||||
blocks with clear intent. Skip test_stub for architectural or design-only findings.
|
||||
For a finding a test could catch, add a minimal \`test_stub\` skeleton in the detected test
|
||||
framework ({TEST_FW}); skip it for architectural or design-only findings.
|
||||
|
||||
Output nothing else: no preamble, no summary or commentary.
|
||||
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
/**
|
||||
* The blocking floor every gstack reviewer prompt carries (docs/review-policy.md#the-blocking-floor).
|
||||
* One sentence, so the core checklist, each specialist, the Red Team and the fix-check rate
|
||||
* severity the same way. test/review-severity-floor.test.ts keeps the markdown copies in step.
|
||||
*/
|
||||
export const BLOCKING_FLOOR = 'CRITICAL is the blocking floor: a finding that would fail a test, break production, lose data or secrets, or open a security hole (injection, auth bypass, secret exposure). Everything else is INFORMATIONAL.';
|
||||
Reference in new issue
Block a user