mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-12 19:44:48 +01:00
test(W7c): review-workflow-clarity keeps order, tokens and gates, drops judged prose
W7c (t06 #12). One test file. Each removed pin was the text fix for a past `review/SKILL.md workflow` judge finding; that judge (gate lane) still grades the prose, so the free test keeps only structure, tokens and gates. Removed assertion groups (invariant -> remaining protection): - Plan-audit sentences ("retain the exact command ...", "A mixed item contributes to both lists", "Keep external-state ...", "If only behavioral checks remain ...", "File-existence checks ... are static audit checks") -> audited deliverables vs deferred behavioral checks: kept free checks on the separation, never DONE from a diff, zero-deliverable no-waive, UNVERIFIABLE -> Step 4.7, read-only audit (no apps/APIs/state) and inspect -before-invoke order; gate E2E review-exploratory-small-cli (gate). - QA loop/report duplicates of qa-probe-gates / qa-caller-authority (probe loop sentences, shared contracts, report layout prose, "Only the parent runs report-only discovery", "Read agent/user updates ...", "Prepare one provisional QA section", smoke composition) -> owned there (free, every host); here only `5 minutes/12 probes`, headings, link-every-checkpoint, no second report, never overwrite another run, freshness vs current inputs. - Persistence/finalization prose (~45: "final zero-edit pass, reconcile", "by structural identity", "without requiring deleted pre-extraction blocks", "raw bytes equal the bound snapshot blobs", "Keep both through re-reviews", "Earlier core tokens remain unused", "Re-read its final- snapshot supporting source ...", "Fixed advice needs no skip coverage", "The required in-host adversarial result controls native completion", "Optional outside attempts keep their own incomplete records ...", etc.) -> kept free token list (REVIEW_START/PASS_START, COMPLETED, CONVERGED, CYCLES=0 once, `review_binding`, `snapshot_covered_paths`, sharedLibsFingerprint, `completed:false`, evidence_paths/helper_target), cycle limit (at most 3, Below 3 repeat, At 3 persist `converged:false`), section order, and the integrity gates below; judge `review/SKILL.md workflow` (gate lane); gate E2Es review-army-delivery-audit, review-army-json-findings, review-army-quality-score (gate). - Confidence-calibration prose (data loss/outage examples, missing field / nullable / race quoting, metaclass/ORM sentence, missing-literal sentence, learning-log sentence) -> kept the score table as a routing table (each band's action), `[CRITICAL]` / `[CRITICAL|INFORMATIONAL]`, no P0-2, `file:line`, force 4-5, never invent 7+, unverified-without-quote, gate before table; judge `review/SKILL.md workflow` (gate lane). - Shared-code reuse safeguard enumeration (assume-unchanged/skip-worktree, sparse index, symlinks, submodules, Git filters, encodings, fsmonitor, textconv, provenance, binding wording) -> enforced in code and covered by free tests test/shared-libs-snapshot-check.test.ts, test/shared-libs-evidence.test.ts, test/shared-libs-checker-interface- evidence.test.ts; kept the checker command/heredoc/JSON tokens and the fail-closed lines. - Misc definitions ("A probe is affected when ...", "An invocation is this /review run ...", "Step 1.5's provisional notes ...", "Otherwise record each considered specialist", "Core findings use the confidence gates below", "highest confidence", "distinct specialists", "all source names", "active or unknown reader/writer", Greptile "Before edits, inspect ...") -> judge `review/SKILL.md workflow` (gate lane). Kept as case-insensitive meaning checks (stop/consent/approval/safety): no edits before Step 5, readers returned or confirmed stopped, no edits in Step 4.8, stop without edits on unconfirmed readers (review and ship), stop at 3 cycles with no clean summary or fourth pass, failed/blocked probes mean incomplete, ship named-risk cannot complete review, native vs outside cannot substitute, structured-review gate, never carry coverage forward, never invent a binding, skipped advisory cannot suppress a defect, user-skip-only suppression, Greptile reply != code approval and wait for approval before editing, HIGH-impact gate options (A ends, read-only audit, no shipping authorization), never downgrade severity, never trust a supplied hash, missing metadata cannot dedupe, test creation needs approval, report-only review never runs setup/installs/cookie import. Kept unchanged: all marker-order arrays, manifest order, Pre-Landing summary after the logger, finish-token count and negatives, stale-wording bans (incl. `git diff origin/main`), ship/review routing negatives. Cosmetic pins dropped: STOP / NON / COUNT capitals; the test titled "resolves the judged ... ambiguities" is renamed to say what it keeps. Mutation log: .context/mutation-review-workflow-clarity.md (23 mutations, 0 unexpected: 3 reorders, 4 markers, 11 safety red; 5 rewordings green).
This commit is contained in:
garrytan committed
1 parent
851328b35f
commit
9c5aedea12
1 file changed
+158
-268
@@ -4,7 +4,7 @@ import { join } from 'node:path';
|
||||
import { generateReviewArmy } from '../scripts/resolvers/review-army';
|
||||
import { generateCrossReviewDedup, generateSharedCodeReuse, generateScopeDrift } from '../scripts/resolvers/review-scope';
|
||||
import { generatePlanCompletionAuditReview, generatePlanCompletionAuditShip } from '../scripts/resolvers/plan-gates';
|
||||
import { generateQAExploratory, generateQAReview } from '../scripts/resolvers/qa';
|
||||
import { generateQAReview } from '../scripts/resolvers/qa';
|
||||
import { generateConfidenceCalibration } from '../scripts/resolvers/confidence';
|
||||
import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types';
|
||||
|
||||
@@ -15,36 +15,27 @@ const adversarial = readFileSync(join(root, 'review/sections/adversarial.md.tmpl
|
||||
test('review audits deliverables before deferring behavioral plan checks to the QA preflight', () => {
|
||||
const ctx: TemplateContext = { skillName: 'review', tmplPath: '', host: 'claude', paths: HOST_PATHS.claude };
|
||||
const audit = generatePlanCompletionAuditReview(ctx).replace(/\s+/g, ' ');
|
||||
for (const contract of [
|
||||
'Separate static audit evidence from behavioral checks',
|
||||
'retain the exact command, expected outcome and source for Step 4.7',
|
||||
'They remain pending execution, never DONE from a diff',
|
||||
'A mixed item contributes to both lists',
|
||||
'Zero audited deliverables do not waive these checks',
|
||||
'Keep external-state and human-only checks under the existing audit rules',
|
||||
'If only behavioral checks remain, report zero audited deliverables and retain their pending Step 4.7 list',
|
||||
'For each audited deliverable, run the verification dispatch',
|
||||
'File-existence checks and verified read-only content validators are static audit checks, not behavioral probes',
|
||||
'Inspect the validator and its hooks before running it; verify read-only effects and access to the target',
|
||||
'leave the item UNVERIFIABLE and defer the command to Step 4.7',
|
||||
'Do not start applications, exercise APIs or mutate state during this audit',
|
||||
'If found and verified safe above, invoke it',
|
||||
]) expect(audit).toContain(contract);
|
||||
expect(audit.indexOf('Inspect the validator and its hooks')).toBeLessThan(audit.indexOf('If found and verified safe above, invoke it'));
|
||||
for (const rule of [
|
||||
/separate static audit evidence from behavioral checks/i,
|
||||
/never DONE from a diff/,
|
||||
/zero audited deliverables do not waive these checks/i,
|
||||
/for each audited deliverable/i,
|
||||
/leave the item UNVERIFIABLE and defer the command to Step 4\.7/,
|
||||
/do not start applications, exercise APIs or mutate state during this audit/i,
|
||||
]) expect(audit).toMatch(rule);
|
||||
const inspect = audit.search(/inspect the validator and its hooks before running it/i);
|
||||
expect(inspect).toBeGreaterThan(-1);
|
||||
expect(inspect).toBeLessThan(audit.search(/if found and verified safe above, invoke it/i));
|
||||
expect(audit).not.toContain('For each extracted plan item, run the verification dispatch');
|
||||
const qa = generateQAReview(ctx);
|
||||
expect(qa).toContain('Then run required plan checks and revalidation, even after smoke expires');
|
||||
expect(qa).toContain('Report clean/completed only when all required checks pass on current inputs');
|
||||
});
|
||||
|
||||
test('review prior-Skip matching includes adversarial and Greptile findings without relaxing eligibility', () => {
|
||||
const dedup = generateCrossReviewDedup({ skillName: 'review', tmplPath: '', host: 'claude', paths: HOST_PATHS.claude });
|
||||
expect(dedup).toContain('For every combined finding, including core, specialist, exploratory QA, adversarial and valid actionable Greptile findings, check:');
|
||||
expect(dedup).toContain('Suppress only when all conditions hold: the user skipped the same unchanged finding');
|
||||
expect(dedup).toContain('same advisory/defect kind');
|
||||
expect(dedup).toContain('Never use a skipped advisory to suppress a real defect');
|
||||
expect(dedup).toContain('Only suppress `skipped` findings — never `fixed` or `auto-fixed`');
|
||||
expect(skill).toContain('Run Step 5.0 severity/prior-skip dedup on all');
|
||||
expect(dedup).toMatch(/for every combined finding, including core, specialist, exploratory QA, adversarial and valid actionable Greptile findings/i);
|
||||
expect(dedup).toMatch(/suppress only when all conditions hold: the user skipped the same unchanged finding/i);
|
||||
expect(dedup).toMatch(/never use a skipped advisory to suppress a real defect/i);
|
||||
expect(dedup).toMatch(/only suppress `skipped` findings — never `fixed` or `auto-fixed`/i);
|
||||
expect(skill).toContain('Step 5.0 severity/prior-skip dedup');
|
||||
});
|
||||
|
||||
test('Review audit and prior-Skip clarifications do not route Ship through Review steps', () => {
|
||||
@@ -74,42 +65,40 @@ test('review collects every source before its single parent fix phase', () => {
|
||||
expect(positions.every(position => position >= 0)).toBe(true);
|
||||
expect(positions).toEqual([...positions].sort((a, b) => a - b));
|
||||
expect(skill.match(/## Step 5: Fix-First Review/g)).toHaveLength(1);
|
||||
expect(skill.replace(/\s+/g, ' ')).toContain('Do not edit reviewed source until Step 5');
|
||||
expect(skill.replace(/\s+/g, ' ')).toContain('every dispatched reader has returned or is confirmed stopped');
|
||||
expect(skill.replace(/\s+/g, ' ')).toMatch(/do not edit reviewed source until Step 5/i);
|
||||
expect(skill.replace(/\s+/g, ' ')).toMatch(/every dispatched reader has returned or is confirmed stopped/i);
|
||||
});
|
||||
|
||||
test('review settles adversarial attempts before fixing and has one full-pass back edge', () => {
|
||||
const generated = readFileSync(join(root, 'review/sections/adversarial.md'), 'utf8');
|
||||
const decision = skill.slice(skill.indexOf('## Step 5.8: Persist Eng Review result')).replace(/\s+/g, ' ');
|
||||
expect(generated).toContain('## Step 4.8: Adversarial review');
|
||||
expect(generated).toContain("queued for the parent's Fix-First handling at Step 5; do not edit during Step 4.8");
|
||||
expect(generated.replace(/\s+/g, ' ')).toContain('before the parent applies queued fixes');
|
||||
expect(generated).toContain('Return all findings and structured-review decisions to Step 5');
|
||||
expect(decision).toContain('A pass covers Steps 3–5, including all reviewers before fixes');
|
||||
expect(generated).toMatch(/do not edit during Step 4\.8/i);
|
||||
expect(generated).toMatch(/return all findings and structured-review decisions to Step 5/i);
|
||||
expect(decision).toMatch(/a pass covers Steps 3–5/i);
|
||||
expect(decision).toContain('Below 3, repeat Steps 3–5 with a new REVIEW_START');
|
||||
expect(decision).not.toContain('Route Step');
|
||||
expect(decision).not.toContain('Steps 5.0–5d');
|
||||
expect(decision).toContain('without a clean summary or a fourth pass');
|
||||
expect(decision).toMatch(/without a clean summary or a fourth pass/i);
|
||||
});
|
||||
|
||||
test('review small-diff and failed-reader paths retain QA and the required adversarial pass', () => {
|
||||
const army = generateReviewArmy({ skillName: 'review', tmplPath: 'review/SKILL.md.tmpl',
|
||||
host: 'claude', paths: HOST_PATHS.claude });
|
||||
expect(army).toContain("Continue to Step 4.6 with the core findings and an empty specialist list, then the parent's Exploratory QA step and Step 4.8 (adversarial review), then Step 5");
|
||||
expect(army.replace(/\s+/g, ' ')).toMatch(/Continue to Step 4\.6 [^.]*Exploratory QA step and Step 4\.8 \(adversarial review\), then Step 5/i);
|
||||
expect(army).not.toContain('design-lite');
|
||||
expect(army).toContain('Missing dispatched coverage remains incomplete, never completed or clean');
|
||||
expect(army).toContain('Continue independent Step 4.7 QA and Step 4.8 adversarial review');
|
||||
expect(army.replace(/\s+/g, ' ')).toMatch(/missing dispatched coverage remains incomplete, never completed or clean/i);
|
||||
expect(army.replace(/\s+/g, ' ')).toMatch(/independent Step 4\.7 QA and Step 4\.8 adversarial review/i);
|
||||
expect(army).not.toContain("Exploratory QA step, then continue to Step 5.");
|
||||
expect(army).not.toContain('If the Red Team subagent fails or times out, skip silently');
|
||||
});
|
||||
|
||||
test('review defines QA confidence, severity, impact selection and numeric version comparison', () => {
|
||||
const flat = skill.replace(/\s+/g, ' ');
|
||||
expect(flat).toContain('For QA findings, assign confidence (1–10) from replay/code evidence');
|
||||
expect(flat).toContain("retain Step 4.7's severity, not a severity inferred from confidence");
|
||||
expect(flat).toContain('A probe is affected when its entrypoint, dependencies, contract or replay inputs change');
|
||||
expect(flat).toContain('If impact is uncertain, rerun it');
|
||||
expect(flat).toContain('Compare dotted version components as integers from left to right');
|
||||
expect(flat).toContain('confidence (1–10)');
|
||||
expect(flat).toMatch(/retain Step 4\.7's severity/i);
|
||||
expect(flat).toMatch(/if impact is uncertain, rerun it/i);
|
||||
expect(flat).toMatch(/compare dotted version components as integers/i);
|
||||
});
|
||||
|
||||
test('review emits scope check after the plan audit and before the checklist', () => {
|
||||
@@ -121,36 +110,30 @@ test('review emits scope check after the plan audit and before the checklist', (
|
||||
expect(positions.every(position => position >= 0)).toBe(true);
|
||||
expect(positions).toEqual([...positions].sort((a, b) => a - b));
|
||||
const audit = readFileSync(join(root, 'review/sections/plan-completion.md'), 'utf8').replace(/\s+/g, ' ');
|
||||
expect(audit).toContain('When continuing after the audit (no HIGH-impact gate, or option B/C), emit the single final Scope Check');
|
||||
expect(audit).toContain("Step 1.5's provisional notes and this plan context");
|
||||
expect(audit).toContain('Emit Step 1.5\'s Scope Check once without plan fields');
|
||||
expect(audit).toMatch(/emit the single final Scope Check/i);
|
||||
expect(skill).not.toContain('Finish Step 1.5 here');
|
||||
});
|
||||
|
||||
test('review composes confidence-tagged findings into one final report with explicit incomplete coverage', () => {
|
||||
const flat = skill.replace(/\s+/g, ' ');
|
||||
expect(flat).toContain('Use CRITICAL/INFORMATIONAL labels in the finding format');
|
||||
expect(flat).toContain("Step 5.8 combines these finding lines with the checklist's action groups");
|
||||
expect(flat).toContain('CRITICAL/INFORMATIONAL');
|
||||
const report = flat.slice(flat.indexOf('### Report the final review'), flat.indexOf('{{LEARNINGS_LOG}}'));
|
||||
expect(report).toContain('Emit one final report, merging all reviewers rather than concatenating their reports');
|
||||
expect(report).toContain('counts final unresolved non-advisory defects');
|
||||
expect(report).toContain('State INCOMPLETE if `COMPLETED` is false, even when N=0');
|
||||
expect(report).toContain("Use the checklist's action groups with confidence-tagged finding lines");
|
||||
expect(report).toContain('Keep fixed, skipped and advisory items separate from unresolved defects; retain their dispositions');
|
||||
expect(report).toContain("Append Step 4.7's single `## Exploratory QA and Verification Results` section");
|
||||
expect(report).toContain('Neither coverage gaps nor advice are defects');
|
||||
expect(report).toMatch(/one final report/i);
|
||||
expect(report).toMatch(/unresolved non-advisory defects/i);
|
||||
expect(report).toMatch(/[Ss]tate INCOMPLETE if `COMPLETED` is false/);
|
||||
expect(report).toContain('`## Exploratory QA and Verification Results`');
|
||||
expect(report).toMatch(/neither coverage gaps nor advice are defects/i);
|
||||
});
|
||||
|
||||
test('small-diff persistence uses an empty specialist map without manufacturing skipped coverage', () => {
|
||||
const army = generateReviewArmy({ skillName: 'review', tmplPath: 'review/SKILL.md.tmpl',
|
||||
host: 'claude', paths: HOST_PATHS.claude });
|
||||
expect(army).toContain('For DIFF_LINES < 50, keep `specialists: {}`; do not manufacture per-specialist scope records');
|
||||
expect(army).toContain('Otherwise record each considered specialist');
|
||||
expect(skill).toContain("Use Step 4.6's `specialists` object unchanged, including its empty small-diff map");
|
||||
expect(army).toContain('DIFF_LINES < 50, keep `specialists: {}`');
|
||||
expect(skill.replace(/\s+/g, ' ')).toMatch(/Step 4\.6's `specialists` object unchanged/i);
|
||||
const ship = readFileSync(join(root, 'ship/sections/review-army.md.tmpl'), 'utf8');
|
||||
expect(ship).toContain('`specialists`: `{}` for a small-diff skip');
|
||||
expect(skill).toContain('every required Step 4.7 probe passes');
|
||||
expect(ship).toContain('all required probes pass');
|
||||
expect(skill.replace(/\s+/g, ' ')).toMatch(/every required Step 4\.7 probe passes/i);
|
||||
expect(ship.replace(/\s+/g, ' ')).toMatch(/all required probes pass/i);
|
||||
});
|
||||
|
||||
test('caller QA runs charter and setup after resource loading and has a severity for unmatched functional failures', () => {
|
||||
@@ -165,13 +148,13 @@ test('caller QA runs charter and setup after resource loading and has a severity
|
||||
const readiness = body.indexOf('**2. Check readiness and list required checks.**');
|
||||
expect(readiness).toBeGreaterThan(preparation);
|
||||
expect(probes).toBeGreaterThan(readiness);
|
||||
expect(body.slice(preparation, readiness).replace(/\s+/g, ' ')).toContain('complete the shared isolation/permission preflight before setup');
|
||||
expect(body.slice(preparation, readiness).replace(/\s+/g, ' ')).toMatch(/isolation\/permission preflight before setup/i);
|
||||
} else expect(preparation).toBeGreaterThan(body.indexOf('**2. List required checks.**'));
|
||||
expect(probes).toBeGreaterThan(preparation);
|
||||
expect(body.replace(/\s+/g, ' ')).toContain('unmatched functional failures are `functional-contract`, `CRITICAL`');
|
||||
expect(body).toContain('Setup/permission blockers are not defects');
|
||||
expect(body).toContain('Test creation needs user approval');
|
||||
expect(body).toContain('Return verified defects to Fix-First');
|
||||
expect(body.replace(/\s+/g, ' ')).toContain('`functional-contract`, `CRITICAL`');
|
||||
expect(body).toMatch(/setup\/permission blockers are not defects/i);
|
||||
expect(body).toMatch(/test creation needs user approval/i);
|
||||
expect(body).toMatch(/return verified defects to Fix-First/i);
|
||||
expect(body).not.toContain('for parent approval');
|
||||
}
|
||||
});
|
||||
@@ -185,34 +168,24 @@ test('review prepares context and deduplicates before classifying findings', ()
|
||||
].map(marker => skill.indexOf(marker));
|
||||
expect(positions.every(position => position >= 0)).toBe(true);
|
||||
expect(positions).toEqual([...positions].sort((a, b) => a - b));
|
||||
expect(skill).toContain('findings before Step 5a classification');
|
||||
expect(skill).toMatch(/before Step 5a classification/i);
|
||||
});
|
||||
|
||||
test('review owns the complete persistence contract after the adversarial read', () => {
|
||||
const step = skill.slice(skill.indexOf('## Step 5.8: Persist Eng Review result'));
|
||||
expect(skill.indexOf('{{SECTION:adversarial}}')).toBeLessThan(skill.indexOf('## Step 5.8: Persist Eng Review result'));
|
||||
expect(adversarial).not.toContain('### Before persisting Eng Review (Step 5.8)');
|
||||
for (const contract of [
|
||||
'repeat Steps 3–5', 'at most 3 fix cycles', 'final zero-edit pass, reconcile',
|
||||
'by structural identity', 'advisory/defect kind',
|
||||
'original `evidence_paths`/`helper_target`', 'without requiring deleted pre-extraction blocks',
|
||||
'Current findings determine recurring defects and unresolved counts', 'earlier fixes do not suppress them',
|
||||
'snapshot_covered_paths',
|
||||
'raw bytes equal the bound snapshot blobs', 'prior-cycle, supplied or prior-record coverage',
|
||||
'REVIEW_START', 'COMPLETED', 'CONVERGED', 'CYCLES', 'Step 4.7',
|
||||
'native Step 4.8 adversarial pass', 'means false, as does a failed native review',
|
||||
'optional outside', 'their own incomplete records when unavailable', 'named-risk',
|
||||
'zero counts', '`completed:false`', '`specialists`', '`findings`',
|
||||
'verified exploratory QA findings',
|
||||
'approved **and completed**', 'explicit Skip', 'sharedLibsFingerprint',
|
||||
'`review_binding`', 'validated captured branch',
|
||||
]) expect(step.toLowerCase().replace(/\s+/g, ' ')).toContain(contract.toLowerCase());
|
||||
for (const token of [
|
||||
'repeat Steps 3–5', 'at most 3 fix cycles', 'original `evidence_paths`/`helper_target`',
|
||||
'snapshot_covered_paths', 'REVIEW_START', 'COMPLETED', 'CONVERGED', 'CYCLES', 'Step 4.7',
|
||||
'native Step 4.8 adversarial pass', 'named-risk', 'zero counts', '`completed:false`',
|
||||
'`specialists`', '`findings`', 'explicit Skip', 'sharedLibsFingerprint', '`review_binding`',
|
||||
]) expect(step.toLowerCase().replace(/\s+/g, ' ')).toContain(token.toLowerCase());
|
||||
expect(step.indexOf('### 1. Re-review after edits'))
|
||||
.toBeLessThan(step.indexOf('### 2. Fill the record'));
|
||||
expect(step.indexOf('### 2. Fill the record'))
|
||||
.toBeLessThan(step.indexOf('~/.claude/skills/gstack/bin/gstack-review-log'));
|
||||
expect(step).toContain('`quality_score` is Step 4.6\'s specialist score');
|
||||
expect(step).toContain('unresolved non-advisory core defects still count');
|
||||
expect(step).toContain('`quality_score`');
|
||||
expect(step.indexOf('Pre-Landing Review: N issues (X critical, Y informational)'))
|
||||
.toBeGreaterThan(step.indexOf('~/.claude/skills/gstack/bin/gstack-review-log'));
|
||||
expect(step).toContain('`## Exploratory QA and Verification Results`');
|
||||
@@ -220,26 +193,20 @@ test('review owns the complete persistence contract after the adversarial read',
|
||||
|
||||
test('review distinguishes required native coverage from optional outside coverage', () => {
|
||||
const section = readFileSync(join(root, 'review/sections/adversarial.md'), 'utf8');
|
||||
expect(section).toContain('Only this optional outside adversarial pass is non-blocking');
|
||||
expect(section).toMatch(/only this optional outside adversarial pass is non-blocking/i);
|
||||
expect(section).not.toContain('All errors are non-blocking');
|
||||
expect(section).toContain('The native pass is required for Step 5.8 completion');
|
||||
expect(skill).toContain('Core findings use the confidence gates below');
|
||||
expect(skill).toContain('Step 4.6 applies its specialist gates');
|
||||
expect(section).toMatch(/native pass is required for Step 5\.8 completion/i);
|
||||
});
|
||||
|
||||
test('review identifies probe selection, report assets and the detected diff base', () => {
|
||||
const generated = generateQAReview({ skillName: 'review', tmplPath: 'review/SKILL.md.tmpl',
|
||||
host: 'claude', paths: HOST_PATHS.claude });
|
||||
const checklist = readFileSync(join(root, 'review/checklist.md'), 'utf8');
|
||||
expect(generated).toContain('Smoke: 5 minutes/12 probes, one success and the riskiest changed failure/edge');
|
||||
expect(generated).toContain('Required even for small diffs or missing plans/servers');
|
||||
expect(generated.replace(/\s+/g, ' ')).toContain("Use checklist severity; unmatched functional failures are `functional-contract`, `CRITICAL`");
|
||||
expect(generated).toContain('Setup/permission blockers are not defects');
|
||||
expect(generated).toContain('Test creation needs user approval');
|
||||
expect(generated).toContain("Read QA\'s `templates/functional-report-template.md`. Title it");
|
||||
expect(generated).toContain('Link every checkpoint');
|
||||
expect(generated).toContain('No second report');
|
||||
expect(checklist).toContain('merge-base diff from the caller');
|
||||
expect(generated).toContain('5 minutes/12 probes');
|
||||
expect(generated).toContain("Read QA\'s `templates/functional-report-template.md`");
|
||||
expect(generated).toMatch(/link every checkpoint/i);
|
||||
expect(generated).toMatch(/no second report/i);
|
||||
expect(checklist).toMatch(/merge-base diff from the caller/i);
|
||||
expect(checklist).not.toContain('git diff origin/main');
|
||||
});
|
||||
|
||||
@@ -247,37 +214,16 @@ test('caller QA defines execution, evidence ownership and report adaptation befo
|
||||
for (const skillName of ['review', 'ship']) {
|
||||
const generated = generateQAReview({ skillName, tmplPath: `${skillName}/SKILL.md.tmpl`,
|
||||
host: 'claude', paths: HOST_PATHS.claude }).replace(/\s+/g, ' ');
|
||||
for (const contract of [
|
||||
'Only the parent runs report-only discovery',
|
||||
'Never overwrite another run',
|
||||
'Follow the shared Probe loop for smoke checks and replays until the smoke limit',
|
||||
'using the same procedure but no smoke guard; never reset the clock',
|
||||
'Read agent/user updates and await results without batching them with reporting/logging',
|
||||
'Compare each probe\'s recorded source, tests, contracts, commands and fixtures (or input fingerprint) with current inputs, even without updates',
|
||||
'Re-review changed or uncertain coverage',
|
||||
"Use checklist severity",
|
||||
]) expect(generated).toContain(contract);
|
||||
const shared = generateQAExploratory({ skillName: 'qa', tmplPath: '', host: 'claude', paths: HOST_PATHS.claude });
|
||||
for (const contract of ['First demonstrate success: output AND durable effects',
|
||||
'Wait for successful checkpoint publication before dispatch',
|
||||
'Replay the exact failing command/request from the same initial fixture state']) {
|
||||
expect(shared).toContain(contract);
|
||||
}
|
||||
expect(generated).toMatch(/never overwrite another run/i);
|
||||
expect(generated).toMatch(/with current inputs, even without updates/i);
|
||||
expect(generated).toContain('Use checklist severity');
|
||||
if (skillName === 'review') {
|
||||
expect(generated).toContain('Title it `## Exploratory QA and Verification Results`');
|
||||
expect(generated).toContain('keep metadata/outcome tables');
|
||||
expect(generated).toContain('demote other headings one level');
|
||||
expect(generated).toContain('include it here under `### Browser results`');
|
||||
expect(generated).toContain('other headings demoted two levels');
|
||||
expect(generated).toContain('Keep browser/functional scores and outcomes separate');
|
||||
expect(generated).toContain('save browser baseline/evidence normally');
|
||||
expect(generated).toContain('Prepare one provisional QA section');
|
||||
expect(generated).toContain('Update affected outcomes/checkpoint links through repairs/revalidation');
|
||||
expect(generated).toContain('Continue to Step 4.8 even if blocked');
|
||||
expect(generated).toContain('Step 5.8 appends this section once after final findings and decides completion');
|
||||
expect(generated).toContain('`## Exploratory QA and Verification Results`');
|
||||
expect(generated).toContain('`### Browser results`');
|
||||
expect(generated).toMatch(/continue to Step 4\.8 even if blocked/i);
|
||||
expect(generated).toMatch(/Step 5\.8 appends this section once/i);
|
||||
} else {
|
||||
expect(generated).toContain('PR section `## Exploratory QA`');
|
||||
expect(generated).toContain('fields as subsections');
|
||||
expect(generated).toContain('`## Exploratory QA`');
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -291,15 +237,14 @@ test('review section index follows the actual pre-fix execution order', () => {
|
||||
|
||||
test('review finalization ownership: initialize invocation state and capture the core token before reading', () => {
|
||||
const start = skill.slice(skill.indexOf('## Step 3: Get the diff'), skill.indexOf('## Step 3.4:'));
|
||||
expect(start).toContain('one invocation action list and CYCLES=0');
|
||||
expect(start).toContain('Keep both through re-reviews');
|
||||
expect(start).toContain('CYCLES=0');
|
||||
expect(skill.match(/CYCLES=0/g)).toHaveLength(1);
|
||||
expect(start).toContain('gstack-review-log --start review\ngit diff "$DIFF_BASE"');
|
||||
expect(start).toContain('Save the printed REVIEW_START for this core candidate before reading its diff');
|
||||
expect(start).toContain('Each re-review captures a new token before reading, never at log time');
|
||||
expect(start.replace(/\s+/g, ' ')).toContain('Earlier core tokens remain unused');
|
||||
expect(start).toContain('Native/outside reviewer attempts own separate PASS_START tokens, not REVIEW_START');
|
||||
expect(start).toContain('Step 5.8 finishes only the final core token');
|
||||
const flat = start.replace(/\s+/g, ' ');
|
||||
expect(flat).toMatch(/REVIEW_START for this core candidate before reading its diff/i);
|
||||
expect(flat).toMatch(/new token before reading, never at log time/i);
|
||||
expect(flat).toMatch(/separate PASS_START tokens, not REVIEW_START/);
|
||||
expect(flat).toMatch(/finishes only the final core token/i);
|
||||
});
|
||||
|
||||
test('review finalization ownership: late findings use Fix-First before the bounded parent transition', () => {
|
||||
@@ -317,46 +262,31 @@ test('review finalization ownership: late findings use Fix-First before the boun
|
||||
const positions = markers.map(marker => flat.indexOf(marker));
|
||||
expect(positions.every(position => position >= 0)).toBe(true);
|
||||
expect(positions).toEqual([...positions].sort((a, b) => a - b));
|
||||
expect(flat).toContain('Structured approval does not waive advisory/test_stub ASK gates');
|
||||
expect(flat).toContain('A pass covers Steps 3–5, including all reviewers before fixes');
|
||||
expect(flat).toContain('at most 3 fix cycles');
|
||||
expect(flat).toContain('Below 3, repeat Steps 3–5 with a new REVIEW_START');
|
||||
expect(flat).toContain('At 3, persist `converged:false` and remaining findings');
|
||||
expect(flat).toContain('At 3, persist `converged:false`');
|
||||
const limit = flat.slice(flat.indexOf('At 3,'), flat.indexOf('No edits:'));
|
||||
expect(limit).toContain('by filling and saving the record below');
|
||||
expect(limit).toContain('Report nonconvergence and coverage gaps, then STOP this invocation');
|
||||
expect(limit).toContain('without a clean summary or a fourth pass');
|
||||
expect(limit).toMatch(/then stop this invocation/i);
|
||||
expect(limit).toMatch(/without a clean summary or a fourth pass/i);
|
||||
});
|
||||
|
||||
test('review finalization ownership: affected QA reuse cannot replace a full review or erase decisions', () => {
|
||||
const step = skill.slice(skill.indexOf('## Step 5.8: Persist Eng Review result'));
|
||||
const flat = step.replace(/\s+/g, ' ');
|
||||
expect(flat).toContain('On a repeat, execute Steps 3–5 in order');
|
||||
expect(flat).toContain('rerun affected probes after source, test, contract, command or fixture changes');
|
||||
expect(flat).toContain("At Step 4.7, reuse only this invocation's unchanged-input QA evidence");
|
||||
expect(flat).toContain('Reusing a probe never skips a review step');
|
||||
expect(flat).toContain("final zero-edit pass, reconcile this invocation's actions with current findings");
|
||||
expect(flat).toContain('original `evidence_paths`/`helper_target`');
|
||||
expect(flat).toContain('Current findings determine recurring defects and unresolved counts; earlier fixes do not suppress them');
|
||||
expect(flat).toContain('Re-read its final-snapshot supporting source and reconfirm the decision');
|
||||
expect(flat).toContain('otherwise report its history without a reusable skip');
|
||||
expect(flat).toContain('The logger computes `snapshot_covered_paths` from eligible paths whose raw bytes equal the bound snapshot blobs');
|
||||
expect(flat).toContain('Never carry prior-cycle, supplied or prior-record coverage forward or build this proof yourself');
|
||||
expect(flat).toContain('Fixed advice needs no skip coverage');
|
||||
expect(flat).toContain('include this invocation\'s revalidated decisions');
|
||||
expect(flat).toContain('execute Steps 3–5 in order');
|
||||
expect(flat).toMatch(/reuse only this invocation's unchanged-input QA evidence/i);
|
||||
expect(flat).toMatch(/reusing a probe never skips a review step/i);
|
||||
expect(flat).toMatch(/earlier fixes do not suppress them/i);
|
||||
expect(flat).toContain('logger computes `snapshot_covered_paths`');
|
||||
expect(flat).toMatch(/never carry prior-cycle, supplied or prior-record coverage forward or build this proof yourself/i);
|
||||
});
|
||||
|
||||
test('review finalization ownership: required native completion and optional outside records stay separate', () => {
|
||||
const step = skill.slice(skill.indexOf('### 2. Fill the record'));
|
||||
const flat = step.replace(/\s+/g, ' ');
|
||||
expect(flat).toContain('native Step 4.8 adversarial pass finish');
|
||||
expect(flat).toContain('every required Step 4.7 probe passes');
|
||||
expect(flat).toContain('Any failed, blocked, inconclusive or not-run required probe means false, as does a failed native review');
|
||||
expect(flat).toContain('`/ship` named-risk acceptance cannot complete `/review`');
|
||||
expect(flat).toContain('The required in-host adversarial result controls native completion');
|
||||
expect(flat).toContain('Optional outside attempts keep their own incomplete records when unavailable');
|
||||
expect(flat).toContain('cannot substitute for the native result, or vice versa');
|
||||
expect(flat).toContain('structured-review gate still applies');
|
||||
expect(flat).toMatch(/any failed, blocked, inconclusive or not-run required probe means false, as does a failed native review/i);
|
||||
expect(flat).toMatch(/`\/ship` named-risk acceptance cannot complete `\/review`/i);
|
||||
expect(flat).toMatch(/cannot substitute for the native result, or vice versa/i);
|
||||
expect(flat).toMatch(/structured-review gate still applies/i);
|
||||
expect(flat).toContain('zero counts and `completed:false`');
|
||||
expect(flat).toContain('`CONVERGED`: true only for a completed zero-edit pass');
|
||||
});
|
||||
@@ -367,35 +297,33 @@ test('review finalization ownership: finish only the final core token without lo
|
||||
expect(step.match(/--finish REVIEW_START/g)).toHaveLength(1);
|
||||
expect(step).not.toContain('--start review');
|
||||
expect(step).not.toContain('--finish PASS_START');
|
||||
expect(flat).toContain('Never invent a binding or replace REVIEW_START at log time');
|
||||
expect(flat).toContain('finish only the final core token');
|
||||
expect(flat).toMatch(/never invent a binding or replace REVIEW_START at log time/i);
|
||||
expect(flat).toMatch(/finish only the final core token/i);
|
||||
expect(step).toContain('"completed":COMPLETED,"converged":CONVERGED,"cycles":CYCLES');
|
||||
});
|
||||
|
||||
test('review finalization ownership: the plan audit retains its high-impact gate before the final scope check', () => {
|
||||
const plan = readFileSync(join(root, 'review/sections/plan-completion.md.tmpl'), 'utf8');
|
||||
expect(plan).toContain('INFORMATIONAL except for the HIGH-impact discrepancy question below');
|
||||
expect(plan).toContain('resolve that gate before the final Scope Check');
|
||||
expect(plan).toContain('HIGH-impact discrepancy question');
|
||||
expect(plan).toMatch(/resolve that gate before the final Scope Check/i);
|
||||
expect(plan).not.toContain('never blocks the review');
|
||||
expect(plan).toContain('{{PLAN_COMPLETION_AUDIT_REVIEW}}');
|
||||
const audit = readFileSync(join(root, 'review/sections/plan-completion.md'), 'utf8');
|
||||
const gate = audit.indexOf('**HIGH-impact plan-file discrepancies** trigger AskUserQuestion');
|
||||
expect(gate).toBeGreaterThan(-1);
|
||||
expect(gate).toBeLessThan(audit.indexOf('When continuing after the audit (no HIGH-impact gate, or option B/C)'));
|
||||
expect(audit).toContain('then it gates via AskUserQuestion');
|
||||
expect(audit).toContain('A ends this invocation before code review or implementation');
|
||||
expect(audit).toContain('after implementation, start a fresh /review');
|
||||
expect(audit).toContain('B queues the approved TODO changes for Step 5, not this read-only audit');
|
||||
expect(audit).toContain('B/C continue to the final Scope Check and Step 2');
|
||||
expect(audit).toContain('None of these choices authorizes shipping or waives required verification');
|
||||
expect(skill).toContain("including Step 1.5's approved TODO changes");
|
||||
const flatAudit = audit.replace(/\s+/g, ' ');
|
||||
expect(flatAudit).toMatch(/A ends this invocation before code review or implementation/i);
|
||||
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);
|
||||
});
|
||||
|
||||
test('review scope notes remain provisional until the plan section emits the only final scope check', () => {
|
||||
const scope = generateScopeDrift({ skillName: 'review', tmplPath: 'review/SKILL.md.tmpl',
|
||||
host: 'claude', paths: HOST_PATHS.claude }).replace(/\s+/g, ' ');
|
||||
expect(scope).toContain('Keep these notes provisional. Next, execute the plan-completion section');
|
||||
expect(scope).toContain('it resolves the HIGH-impact decision and emits the single final Scope Check before Step 2');
|
||||
expect(scope).toMatch(/keep these notes provisional/i);
|
||||
expect(scope).toMatch(/single final Scope Check before Step 2/i);
|
||||
expect(scope).not.toContain('Scope Check: [CLEAN');
|
||||
expect(scope).not.toContain('available plan-audit results');
|
||||
});
|
||||
@@ -404,29 +332,20 @@ test('review confidence uses its severity labels without an undefined P0 excepti
|
||||
const ctx: TemplateContext = { skillName: 'review', tmplPath: 'review/SKILL.md.tmpl',
|
||||
host: 'claude', paths: HOST_PATHS.claude };
|
||||
const confidence = generateConfidenceCalibration(ctx);
|
||||
expect(confidence).toContain('Only report a suspected release-blocking catastrophe');
|
||||
expect(confidence).toContain('widespread data loss, total outage or system-wide compromise');
|
||||
expect(confidence).toContain('label it CRITICAL and explicitly speculative');
|
||||
expect(confidence).toContain('[CRITICAL]');
|
||||
expect(confidence).toContain('[CRITICAL|INFORMATIONAL]');
|
||||
expect(confidence).not.toMatch(/\bP[012]\b/);
|
||||
expect(confidence).toContain('If you cannot quote the motivating line(s), the finding is unverified');
|
||||
const flat = confidence.replace(/\s+/g, ' ');
|
||||
for (const rule of [
|
||||
'| 9-10 | Specific code verifies a concrete bug or exploit. | Show normally |',
|
||||
'| 7-8 | High-confidence pattern match; very likely correct. | Show normally |',
|
||||
'| 5-6 | Moderate; could be a false positive. | Show with caveat:',
|
||||
'Medium confidence, verify this is actually an issue',
|
||||
'| 3-4 | Suspicious but may be fine. | Suppress from main report. Include in appendix only. |',
|
||||
'| 1-2 | Speculation. | Only report a suspected release-blocking catastrophe',
|
||||
'Quote the specific code line', 'file:line and verbatim text',
|
||||
'For a missing field, quote its class definition; for a nullable value, its initialization; for a race, both sides',
|
||||
'Force its confidence to 4-5: use 4 for appendix-only reporting, or 5 only when the finding belongs in the main report with the medium-confidence caveat below',
|
||||
'Never invent speculative confidence 7+',
|
||||
'read and quote their generating metaclass, descriptor, ORM Meta block, migration, decorator or schema',
|
||||
'Missing literal names in the class body or grep results do not prove absence',
|
||||
'If the user confirms a reported finding scored < 7 is real, log the corrected pattern as a learning',
|
||||
]) expect(flat).toContain(rule);
|
||||
for (const row of [
|
||||
/\| 9-10 \|[^|]*\| Show normally \|/, /\| 7-8 \|[^|]*\| Show normally \|/,
|
||||
/\| 5-6 \|[^|]*\| Show with caveat/, /\| 3-4 \|[^|]*\| Suppress from main report/,
|
||||
/\| 1-2 \|[^|]*\| Only report a suspected release-blocking catastrophe/,
|
||||
]) expect(flat).toMatch(row);
|
||||
expect(flat).toMatch(/explicitly speculative/i);
|
||||
expect(flat).toMatch(/the finding is unverified/i);
|
||||
expect(flat).toContain('file:line');
|
||||
expect(flat).toMatch(/force its confidence to 4-5/i);
|
||||
expect(flat).toMatch(/never invent speculative confidence 7\+/i);
|
||||
expect(confidence.indexOf('Pre-emit verification gate')).toBeLessThan(confidence.indexOf('| Score |'));
|
||||
expect(confidence).not.toContain('FP classes the gate kills');
|
||||
expect(confidence).not.toContain('1539-framework-aware-review.md');
|
||||
@@ -435,28 +354,20 @@ test('review confidence uses its severity labels without an undefined P0 excepti
|
||||
|
||||
test('review names the lifecycle and record owners before using their persistence rules', () => {
|
||||
const start = skill.slice(skill.indexOf('## Step 3: Get the diff'), skill.indexOf('## Step 3.4:')).replace(/\s+/g, ' ');
|
||||
expect(start).toContain('An invocation is this /review run; a pass reviews one candidate before any fixes');
|
||||
expect(start).toContain('REVIEW_START / PASS_START | Opaque start receipts from the logger');
|
||||
expect(start).toContain('a matching key alone never proves a prior Skip is reusable');
|
||||
expect(start).toContain("`review_binding` | The logger's proof tying a finished review to its captured candidate");
|
||||
expect(start).toContain('`snapshot_covered_paths` | Supporting advice files the logger proved byte-identical to that candidate');
|
||||
expect(start).toContain('Used by the prior-Skip checker, never supplied by the reviewer');
|
||||
expect(start).toContain('REVIEW_START / PASS_START |');
|
||||
expect(start).toContain('`review_binding` |');
|
||||
expect(start).toContain('`snapshot_covered_paths` |');
|
||||
expect(start).toMatch(/a matching key alone never proves a prior Skip is reusable/i);
|
||||
expect(start).toMatch(/never supplied by the reviewer/i);
|
||||
});
|
||||
|
||||
test('review invocation-local advice reuse keeps raw-source and changed-decision gates', () => {
|
||||
const decisions = skill.slice(skill.indexOf('**Keep decisions through fix cycles:**'),
|
||||
skill.indexOf('### Step 5a:')).replace(/\s+/g, ' ');
|
||||
for (const contract of [
|
||||
'Immediately save completed AUTO-FIX/fix and explicit Skip actions',
|
||||
'keeping defects separate from advice',
|
||||
"retain the helper's fingerprint, `advisory`, `evidence_paths` and `helper_target`",
|
||||
're-read every supporting caller and helper destination',
|
||||
'including secondary callers and transformed/indirect paths',
|
||||
'Compare their raw source with the decision evidence',
|
||||
'Unrelated auto-fixes do not reopen unchanged identity, contract and tradeoffs',
|
||||
'Material proposal, behavior, migration or risk changes require a new question',
|
||||
"cannot suppress new/recurring defects or replace Step 5.0's prior-review checker",
|
||||
]) expect(decisions).toContain(contract);
|
||||
expect(decisions).toMatch(/save completed AUTO-FIX\/fix and explicit Skip actions/i);
|
||||
expect(decisions).toContain("`advisory`, `evidence_paths` and `helper_target`");
|
||||
expect(decisions).toMatch(/require a new question/i);
|
||||
expect(decisions).toMatch(/cannot suppress new\/recurring defects/i);
|
||||
});
|
||||
|
||||
for (const skillName of ['review', 'ship']) {
|
||||
@@ -466,16 +377,14 @@ for (const skillName of ['review', 'ship']) {
|
||||
const flat = army.replace(/\s+/g, ' ');
|
||||
|
||||
test(`${skillName} clarity: terminal failure permits independent work but never certifies coverage`, () => {
|
||||
expect(flat).toContain('Confirm that each task has finished or is stopped');
|
||||
expect(flat).toContain('A timeout alone does not prove termination');
|
||||
expect(flat).toContain("If a reader or writer is still active, wait; if its state is unknown, inspect its task/process status");
|
||||
expect(flat).toContain("If you cannot confirm it stopped, use the parent's Fix-First stop path without edits");
|
||||
expect(flat).toContain('Continue independent evidence collection after a terminal failure');
|
||||
expect(flat).toContain('Missing dispatched coverage remains incomplete, never completed or clean');
|
||||
expect(flat).toMatch(/confirm that each task has finished or is stopped/i);
|
||||
expect(flat).toMatch(/a timeout alone does not prove termination/i);
|
||||
expect(flat).toMatch(/stop path without edits/i);
|
||||
expect(flat).toMatch(/missing dispatched coverage remains incomplete, never completed or clean/i);
|
||||
expect(flat).not.toContain('Specialists are additive — partial results are better than no results');
|
||||
const redTeam = flat.slice(flat.indexOf('### Red Team dispatch'));
|
||||
expect(redTeam).toContain('confirm it stopped and record its review as incomplete, just as for other specialists');
|
||||
expect(redTeam).toContain('original specialist outputs and rerun stages 1–7');
|
||||
expect(redTeam).toMatch(/confirm it stopped and record its review as incomplete/i);
|
||||
expect(redTeam).toContain('stages 1–7');
|
||||
});
|
||||
|
||||
test(`${skillName} clarity: ordered specialist merge separates validation from scoring and provenance`, () => {
|
||||
@@ -487,26 +396,21 @@ for (const skillName of ['review', 'ship']) {
|
||||
expect(positions.every(position => position >= 0)).toBe(true);
|
||||
expect(positions).toEqual([...positions].sort((a, b) => a - b));
|
||||
const validation = flat.slice(flat.indexOf('#### 2.'), flat.indexOf('#### 3.'));
|
||||
expect(validation).toContain('core and specialist findings');
|
||||
expect(validation).toContain('remove `advisory` and retain its `CRITICAL` severity');
|
||||
expect(validation).toContain('Never downgrade severity');
|
||||
expect(validation).toContain('Valid INFORMATIONAL advisories remain advisory');
|
||||
expect(validation).toMatch(/never downgrade severity/i);
|
||||
const merge = flat.slice(flat.indexOf('#### 3.'), flat.indexOf('#### 4.'));
|
||||
expect(merge.indexOf('Partition defects and advisories')).toBeLessThan(merge.indexOf('grouping by fingerprint'));
|
||||
for (const gate of ['sharedLibsFingerprint', 'literal JSON on stdin', 'never trust a supplied hash',
|
||||
'Missing/malformed metadata cannot deduplicate', 'highest confidence', '+1 (cap at 10)',
|
||||
'distinct specialists', 'all source names', 'Core findings never earn a specialist confidence boost']) {
|
||||
expect(merge).toContain(gate);
|
||||
}
|
||||
for (const token of ['sharedLibsFingerprint', 'literal JSON on stdin', '+1 (cap at 10)']) expect(merge).toContain(token);
|
||||
for (const rule of [/never trust a supplied hash/i, /missing\/malformed metadata cannot deduplicate/i,
|
||||
/core findings never earn a specialist confidence boost/i]) expect(merge).toMatch(rule);
|
||||
for (const gate of ['Confidence 7+', 'Confidence 5-6', 'Confidence 3-4', 'Confidence 1-2']) expect(army).toContain(gate);
|
||||
const scoring = flat.slice(flat.indexOf('#### 5.'), flat.indexOf('#### 6.'));
|
||||
expect(scoring).toContain('Only specialist findings enter this header and `quality_score`; core findings do not');
|
||||
expect(scoring).toContain('NON-advisory');
|
||||
expect(scoring).toMatch(/only specialist findings enter this header and `quality_score`/i);
|
||||
expect(scoring).toMatch(/non-advisory/i);
|
||||
expect(scoring).toContain('quality_score = max(0, 10 - (critical_count * 2 + informational_count * 0.5))');
|
||||
expect(scoring).toContain('unresolved-defect totals');
|
||||
expect(flat).toContain('Advisory findings COUNT in the stats `findings` field');
|
||||
expect(flat).toContain('Count only findings that specialist actually returned');
|
||||
expect(flat).toContain('core-only advice must not create a specialist dispatch or finding');
|
||||
expect(flat).toMatch(/advisory findings count in the stats `findings` field/i);
|
||||
expect(flat).toMatch(/count only findings that specialist actually returned/i);
|
||||
expect(flat).toMatch(/core-only advice must not create a specialist dispatch or finding/i);
|
||||
expect(flat).toContain('ASK-only');
|
||||
});
|
||||
|
||||
@@ -517,29 +421,20 @@ for (const skillName of ['review', 'ship']) {
|
||||
const positions = markers.map(marker => reuse.indexOf(marker));
|
||||
expect(positions.every(position => position >= 0)).toBe(true);
|
||||
expect(positions).toEqual([...positions].sort((a, b) => a - b));
|
||||
for (const gate of ['first-party authored provenance', 'all supporting callers and the helper destination',
|
||||
'--check-shared-libs REVIEW_START', "<<'GSTACK_SHARED_LIBS_REUSE_JSON'", 'literal JSON on stdin',
|
||||
'`reusable: true`', 'False, command failure or unreadable output', 'never suppression',
|
||||
'Do not supply your own snapshot, prior record or coverage', 'sharedLibsFingerprint',
|
||||
'without consuming/replacing it', 'actual repo, raw branch and current snapshot',
|
||||
'completed/converged', 'verified binding', 'explicit Skip', 'logger-versioned `snapshot_covered_paths`',
|
||||
'older unversioned coverage', 'Sanitized branch names are not identity', 'canReuseSharedLibsAdvisory',
|
||||
'byte-for-byte with its blob', 'assume-unchanged, skip-worktree', 'sparse index', 'symlinks/ancestors',
|
||||
'submodules', 'ignored/outside or unreadable', 'active/unknown Git filters', 'encodings and line conversion',
|
||||
'fsmonitor and optional locks', 'never uses external diff/textconv', 'Unknown evidence fails closed',
|
||||
'logger recomputes final coverage', 'Real defects retain normal Fix-First handling independently']) {
|
||||
expect(reuse).toContain(gate);
|
||||
}
|
||||
for (const token of ['--check-shared-libs REVIEW_START', "<<'GSTACK_SHARED_LIBS_REUSE_JSON'", 'literal JSON on stdin',
|
||||
'`reusable: true`', 'sharedLibsFingerprint', 'explicit Skip', 'logger-versioned `snapshot_covered_paths`',
|
||||
'canReuseSharedLibsAdvisory']) expect(reuse).toContain(token);
|
||||
for (const rule of [/false, command failure or unreadable output[^.]*never suppression/i,
|
||||
/do not supply your own snapshot, prior record or coverage/i, /unknown evidence fails closed/i,
|
||||
/real defects retain normal Fix-First handling/i]) expect(reuse).toMatch(rule);
|
||||
});
|
||||
}
|
||||
|
||||
test('review clarity: settlement gates edits separately from incomplete required coverage', () => {
|
||||
const fix = skill.slice(skill.indexOf('## Step 5: Fix-First Review'), skill.indexOf('{{CROSS_REVIEW_DEDUP}}')).replace(/\s+/g, ' ');
|
||||
expect(fix).toContain('every dispatched reader has returned or is confirmed stopped');
|
||||
expect(fix).toContain('active or unknown reader/writer');
|
||||
expect(fix).toContain('persist incomplete at Step 5.8 and STOP without edits');
|
||||
expect(fix).toContain('Terminal failure does not block fixes from independent evidence');
|
||||
expect(fix).toContain('Missing required output still makes the pass incomplete');
|
||||
expect(fix).toMatch(/every dispatched reader has returned or is confirmed stopped/i);
|
||||
expect(fix).toMatch(/persist incomplete at Step 5\.8 and stop without edits/i);
|
||||
expect(fix).toMatch(/missing required output still makes the pass incomplete/i);
|
||||
});
|
||||
|
||||
test('review clarity: Greptile reply choices never substitute for Fix-First approval', () => {
|
||||
@@ -547,43 +442,38 @@ test('review clarity: Greptile reply choices never substitute for Fix-First appr
|
||||
expect(fix).toContain('VALID & ACTIONABLE Greptile findings');
|
||||
const greptile = skill.slice(skill.indexOf('### Greptile comment resolution'), skill.indexOf('## Step 5.8:'));
|
||||
const flat = greptile.replace(/\s+/g, ' ');
|
||||
expect(flat).toContain('Step 5c alone supplies A) Fix / B) Skip');
|
||||
expect(flat).toMatch(/Step 5c alone supplies A\) Fix \/ B\) Skip/i);
|
||||
expect(flat).not.toContain('A: Fix it now, B: Acknowledge, C: False positive');
|
||||
expect(flat).toContain('reply decisions, not code approval');
|
||||
expect(flat).toMatch(/reply decisions, not code approval/i);
|
||||
expect(flat).toContain('B) Propose a code change');
|
||||
expect(flat).toContain('return to Steps 5c–5d with an ASK proposal');
|
||||
expect(flat).toContain('Show the exact change and any `test_stub`; wait for approval before editing');
|
||||
expect(flat).toContain('no new fix permission');
|
||||
expect(flat).toContain('Steps 5c–5d');
|
||||
expect(flat).toMatch(/wait for approval before editing/i);
|
||||
expect(flat).toMatch(/no new fix permission/i);
|
||||
});
|
||||
|
||||
test('ship review clarity: parent settlement gate precedes classification and cannot waive coverage', () => {
|
||||
const ship = readFileSync(join(root, 'ship/sections/review-army.md.tmpl'), 'utf8');
|
||||
const gate = ship.slice(ship.indexOf('## Step 9.4:'), ship.indexOf('1. **Classify')).replace(/\s+/g, ' ');
|
||||
expect(gate).toContain("Before edits, inspect every dispatched reader/writer's handle");
|
||||
expect(gate).toContain('Wait for return or confirm termination');
|
||||
expect(gate).toContain('otherwise log incomplete through items 5–6 and STOP without edits');
|
||||
expect(gate).toContain('After terminal failure, independent evidence may support fixes');
|
||||
expect(gate).toContain('missing dispatched output still blocks continuation, even with a QA exception');
|
||||
expect(gate).toMatch(/wait for return or confirm termination/i);
|
||||
expect(gate).toMatch(/log incomplete through items 5–6 and stop without edits/i);
|
||||
expect(gate).toMatch(/missing dispatched output still blocks continuation, even with a QA exception/i);
|
||||
});
|
||||
|
||||
test('review resolves the judged smoke-clock, setup-authority, plan-gate and findings-source ambiguities', () => {
|
||||
test('review keeps its smoke-clock, setup-authority, plan-gate and findings-source rules', () => {
|
||||
const ctx = { skillName: 'review', tmplPath: 'review/SKILL.md.tmpl', host: 'claude', paths: HOST_PATHS.claude } as TemplateContext;
|
||||
const qa = generateQAReview(ctx).replace(/\s+/g, ' ');
|
||||
expect(qa).toContain('Follow the shared Probe loop for smoke checks and replays until the smoke limit');
|
||||
expect(qa).toContain('Then run required plan checks and revalidation, even after smoke expires, using the same procedure but no smoke guard');
|
||||
expect(qa).toContain('/review sets none; only an invoker-supplied EARLIER_UTC counts');
|
||||
expect(qa).toContain('report-only /review never runs setup, installs or cookie import');
|
||||
expect(qa).toContain('After a grant, recheck readiness and run affected checks; otherwise they stay blocked');
|
||||
expect(qa).toMatch(/\/review sets none; only an invoker-supplied EARLIER_UTC counts/);
|
||||
expect(qa).toMatch(/report-only \/review never runs setup, installs or cookie import/i);
|
||||
expect(qa).toMatch(/otherwise they stay blocked/i);
|
||||
expect(qa).not.toContain('Ask for setup/permission');
|
||||
expect(generateQAReview({ ...ctx, skillName: 'ship', tmplPath: 'ship/SKILL.md.tmpl' })).not.toContain('/review sets none');
|
||||
const shared = generateQAExploratory({ ...ctx, skillName: 'qa' }).replace(/\s+/g, ' ');
|
||||
expect(shared).toContain('Explicit plan checks and revalidation remain required beyond this smoke budget');
|
||||
const audit = generatePlanCompletionAuditReview(ctx).replace(/\s+/g, ' ');
|
||||
expect(audit).toContain('No plan file found → say "No plan file detected." and use the Fallback Intent Sources below');
|
||||
expect(audit).toContain('"No plan file detected."');
|
||||
expect(audit).toContain('Fallback Intent Sources');
|
||||
expect(audit).not.toContain('skip with "No plan file detected — skipping."');
|
||||
expect(audit).toContain('**HIGH-impact plan-file discrepancies** trigger AskUserQuestion');
|
||||
expect(audit).toContain('Discrepancies derived only from fallback sources (commit messages, TODOS.md, PR description) never trigger this question');
|
||||
expect(audit).toMatch(/derived only from fallback sources[\s\S]{0,80}never trigger this question/i);
|
||||
expect(generatePlanCompletionAuditShip({ ...ctx, skillName: 'ship', tmplPath: 'ship/SKILL.md.tmpl' })).toContain('skip with "No plan file detected — skipping."');
|
||||
const persist = skill.slice(skill.indexOf('### 2. Fill the record')).replace(/\s+/g, ' ');
|
||||
expect(persist).toContain("Step 5's combined final-pass findings (core, specialist, adversarial, actionable Greptile, verified exploratory QA findings)");
|
||||
expect(persist).toMatch(/combined final-pass findings \(core, specialist, adversarial, actionable Greptile, verified exploratory QA findings\)/i);
|
||||
});
|
||||
Reference in new issue
Block a user