diff --git a/test/review-workflow-clarity.test.ts b/test/review-workflow-clarity.test.ts index 1bb6b30e2..e3a63df17 100644 --- a/test/review-workflow-clarity.test.ts +++ b/test/review-workflow-clarity.test.ts @@ -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); });