diff --git a/test/eng-review-routing.test.ts b/test/eng-review-routing.test.ts index 60a89e174..b31dd4509 100644 --- a/test/eng-review-routing.test.ts +++ b/test/eng-review-routing.test.ts @@ -36,181 +36,153 @@ describe('engineering review routing contracts', () => { const excerpt = between(entry, ENG_REVIEW_EXCERPT.startMarker, ENG_REVIEW_EXCERPT.endMarker); ordered(excerpt, ['{{SECTION:review-sections}}', '## Recovery routing', '**Paused question:**', '**Repairable write/read failure:**', '**Late change or missing work:**', '**Blocked outcome:**']); - expect(compact(entry.slice(entry.indexOf(ENG_REVIEW_EXCERPT.endMarker)))).toContain('use Recovery routing above'); + expect(compact(entry.slice(entry.indexOf(ENG_REVIEW_EXCERPT.endMarker)))).toMatch(/use Recovery routing above/i); }); test('preparation establishes permission and evidence before applying review rules', () => { - const preparation = between(section, '## Review preparation', '## Review record and write policy'); - expect(compact(preparation)).toContain('Follow the blocks below in order after startup'); - expect(compact(preparation)).toContain('Confidence Calibration and Decision procedure are reference rules, not additional review passes'); - ordered(section, ['## Review record and write policy', '{{LEARNINGS_SEARCH}}', + ordered(section, ['## Review preparation', '## Review record and write policy', '{{LEARNINGS_SEARCH}}', '## Retrospective learning', '{{CONFIDENCE_CALIBRATION}}', '## Decision procedure', '## Scope Challenge', '### A. Assess the target', '### B. Resolve complexity selectors', '### C. Resolve findings', '### 1. Architecture review']); - expect(entry).toContain('Keep the reviewed target fixed'); + expect(entry).toMatch(/keep the reviewed target fixed/i); }); - test('compression cannot remove mandatory review stages or decision content', () => { + test('compression cannot remove mandatory review stages', () => { const priority = between(entry, '## Priority hierarchy', '## My engineering preferences'); - expect(compact(priority)).toContain('Complete every required stage, decision gate and output'); - expect(compact(priority)).toContain('Shorten only optional commentary'); - expect(compact(priority)).toContain('never Scope Challenge, Sections 1–4, the test diagram or required decision/report content'); + expect(compact(priority)).toMatch(/complete every required stage, decision gate and output/i); expect(priority).not.toContain('Everything else'); - expect(compact(section)).toContain('Never condense, abbreviate or skip a section'); }); - test('Scope Challenge has one named route and completes all seven assessments first', () => { + test('Scope Challenge has one named route and completes its assessments before writing', () => { expect([...scope.matchAll(/^### (.+)$/gm)].map(match => match[1])).toEqual([ 'A. Assess the target', 'B. Resolve complexity selectors', 'C. Resolve findings', ]); - expect([...assessment.matchAll(/^- \*\*([^*]+)\*\*/gm)].map(match => match[1])).toEqual([ + expect([...assessment.matchAll(/^- \*\*([^*]+)\*\*/gm)].map(match => match[1])).toEqual(expect.arrayContaining([ 'What already solves each sub-problem?', 'What minimum changes achieve the goal?', 'Complexity check:', 'Search check:', 'TODOS cross-reference:', 'Completeness check:', 'Distribution check:', - ]); - expect(compact(assessment)).toContain('Complete these checks before the complexity decision in B'); - expect(compact(assessment)).toContain('Do not apply scope changes or write findings into the plan yet'); + ])); + expect(compact(assessment)).toMatch(/do not apply scope changes or write findings into the plan yet/i); expect(scope).not.toContain('Below the threshold, start at step 1'); }); test('below-threshold route skips selectors, never findings or remedy approvals', () => { - expect(compact(complexity)).toContain("With fewer than 8 files AND fewer than 2 new classes/services, skip B's questions and go directly to **C. Resolve findings**"); - expect(findings).toContain('Run C whether B was completed or skipped'); - ordered(compact(findings), ['1. Present numbered Scope Challenge findings', - '2. Resolve each remedy through Decision procedure', - '3. Report accepted/rejected/deferred/pending dispositions', + expect(compact(complexity)).toMatch(/fewer than 8 files AND fewer than 2 new classes\/services, skip B's questions/i); + expect(findings).toMatch(/run C whether B was completed or skipped/i); + ordered(compact(findings), ['1. Present', '2. Resolve each remedy through Decision procedure', '3. Report', 'Continue to Section 1 only when no answer is pending']); - expect(findings).toContain('Findings and scope answers approve no remedies'); - expect(findings).toContain('"No issues found" for an empty list'); + expect(findings).toMatch(/approve no remedies/i); + expect(findings).toContain('"No issues found"'); }); - test('high complexity retains separate cuts and mandatory arrangement choice', () => { - expect(compact(complexity)).toContain('At 8+ files or 2+ new classes/services, STOP before Section 1'); - ordered(compact(complexity), ['Initial scope selectors need no grid or **pre-answer** ledger write', - '1. Explain the complexity', 'Ask each proposed feature cut/deferral separately', - '2. Always ask the structure question when this gate trips, even with no cuts', - '3. Save the actual feature and structure answers as one scope record']); - expect(compact(complexity)).toContain('With no proposed cuts, keep the feature list and go directly to the structure question'); - expect(compact(complexity)).toContain('Both retain the same approved feature list, contracts and approved security/error/test/performance fixes'); + test('high complexity stops before Section 1 and asks cuts and structure separately', () => { + expect(compact(complexity)).toMatch(/at 8\+ files or 2\+ new classes\/services, stop before section 1/i); + ordered(compact(complexity), ['1. Explain the complexity', 'Ask each proposed feature cut/deferral separately', + '2. Always ask the structure question', '3. Save the actual feature and structure answers']); expect(compact(complexity)).toContain('Pending remedies not decided here: '); }); test('no safe smaller arrangement does not authorize scope cuts or bypass the pause', () => { ordered(compact(complexity), ['If no smaller arrangement preserves these commitments', - 'offer confirmation of the original arrangement or a pause to investigate a smaller one', - 'A pause leaves the arrangement undecided', 'then return to this structure selector', - 'Do not continue to C until it is settled']); - expect(compact(complexity)).toContain('investigate only the agreed question'); + 'A pause leaves the arrangement undecided', 'Do not continue to C until it is settled']); + expect(compact(complexity)).toMatch(/do not continue to C until it is settled/i); + expect(compact(complexity)).toMatch(/investigate only the agreed question/i); }); - test('selector answers are verified after answering without invented pending records', () => { + test('selector answers are saved and verified before scope changes apply', () => { const summary = compact(complexity.slice(complexity.indexOf('3. Save the actual'))); ordered(summary, ['feature answers: ; structure: ; accepted scope: ; pending remedies: ', - 'post-answer scope summary, not a remedy\'s pending ledger record', 'Read it back against the actual answers', - 'After verification, apply only accepted scope changes', 'Continue to **C. Resolve findings**']); - expect(summary).toContain('Do not invent a pre-answer record afterward'); - expect(summary).toContain('A failed save or Read blocks advancement'); - expect(summary).toContain('on the permitted read-only route, present and verify it as **not persisted**'); - expect(compact(section)).toContain('Scope Challenge B also uses its own selectors and post-answer scope record'); - expect(compact(section)).toContain('These selections approve no engineering remedy'); + 'Read it back', 'apply only accepted scope changes', 'Continue to **C. Resolve findings**']); + expect(summary).toMatch(/a failed save or read blocks advancement/i); + expect(summary).toContain('**not persisted**'); + expect(compact(section)).toMatch(/these selections approve no engineering remedy/i); }); - test('engineering remedies still require full save Read ask answer apply Read ordering', () => { + test('engineering remedies are saved, asked once, and recorded before the next choice', () => { const procedure = between(section, '## Decision procedure', '## Scope Challenge'); - ordered(procedure, ['**Compare one choice.**', '**Pending-record checkpoint.**', - 'use Read to fetch the entire saved record', '### Send once and wait', - 'AskUserQuestion({ questions: [currentDecision] })', '**STOP until the actual answer arrives.**', - '### Record the answer', 'Read the entire resolution block, including State', - 'For the next choice, use the updated working plan and answer']); - expect(compact(procedure)).toContain('An Investigate/Defer option must bound the investigation'); - expect(compact(procedure)).toContain('It approves no implementation, including a conditional fix'); - expect(compact(procedure)).toContain('Do not apply a remedy, make another call, start the next section or call ExitPlanMode while the choice awaits an answer'); - expect(compact(procedure)).toContain("Apply the preamble's Completeness scores or kind-note accordingly"); + ordered(procedure, ['**Compare one choice.**', '**Pending-record checkpoint.**', '### Send once and wait', + 'AskUserQuestion({ questions: [currentDecision] })', '### Record the answer', 'For the next choice']); + expect(between(procedure, '### Send once and wait', '### Record the answer')).toMatch(/stop until the actual answer arrives/i); + expect(compact(procedure)).toMatch(/approves no implementation, including a conditional fix/i); + expect(compact(procedure)).toMatch(/do not apply a remedy[^.]*call ExitPlanMode while the choice awaits an answer/i); const questions = generateAskUserFormat({ skillName: 'plan-eng-review', host: 'claude', paths: HOST_PATHS.claude } as TemplateContext); expect(questions).toContain('10 = complete, 7 = happy path, 3 = shortcut'); expect(questions).toContain('Note: options differ in kind, not coverage — no completeness score.'); }); test('unavailable research preserves an explicit coverage limit and continues review', () => { - expect(compact(assessment)).toContain('If Aside is unavailable, use host WebSearch for these queries'); - expect(compact(assessment)).toContain('With neither, skip and note: "Search unavailable — proceeding with in-distribution knowledge only."'); + expect(compact(assessment)).toContain('"Search unavailable — proceeding with in-distribution knowledge only."'); const outside = between(section, '### Continue after Outside Voice', '### TODOS.md updates'); - expect(compact(outside)).toContain('Only completed reviews enter Cross-model tension'); - expect(compact(outside)).toContain('Record the actual coverage, including disabled or unavailable outcomes'); + expect(compact(outside)).toMatch(/only completed reviews enter cross-model tension/i); expect(section).toContain('Outside voice: recorded provider, completed / unavailable / disabled / skipped (reason)'); - expect(compact(outside)).toContain('resolve the TODO choices, then check Approval readiness before Required outputs'); + ordered(compact(outside), ['TODO choices', 'Approval readiness', 'Required outputs']); }); test('paused transport and failed persistence have distinct non-success outcomes', () => { const pause = between(recovery, '**Paused question:**', '**Repairable write/read failure:**'); - expect(pause).toContain('without completion telemetry or ExitPlanMode'); - expect(compact(pause)).toContain('that the user may have seen is still pending; do not resend it'); - const failure = between(recovery, '**Repairable write/read failure:**', '**Late change or missing work:**'); - expect(compact(failure)).toContain('Stop before the dependent question or output'); - expect(compact(failure)).toContain('If no recovery is specified or it fails, follow **Blocked outcome**'); - expect(compact(failure)).toContain('Never turn a failed permitted save into a chat-only success'); - const policy = between(section, '## Review record and write policy', '{{LEARNINGS_SEARCH}}'); - expect(compact(policy)).toContain('use **Recovery routing → Repairable write/read failure**, never the routes above'); - expect(compact(policy)).toContain('Skip forbidden writes; otherwise keep their best-effort behavior'); + expect(pause).toMatch(/without completion telemetry or ExitPlanMode/i); + expect(compact(pause)).toMatch(/still pending; do not resend it/i); + const failure = compact(between(recovery, '**Repairable write/read failure:**', '**Late change or missing work:**')); + expect(failure).toMatch(/stop before the dependent question or output/i); + expect(failure).toContain('follow **Blocked outcome**'); + expect(failure).toMatch(/never turn a failed permitted save into a chat-only success/i); + const policy = compact(between(section, '## Review record and write policy', '{{LEARNINGS_SEARCH}}')); + expect(policy).toContain('**Recovery routing → Repairable write/read failure**'); + expect(policy).toMatch(/skip forbidden writes/i); expect(recovery).toContain('`OUTCOME=error`'); expect(recovery).not.toContain('`OUTCOME=success`'); }); test('late changes rerun affected approvals and outputs before another navigation answer', () => { const late = compact(between(recovery, '**Late change or missing work:**', '**Blocked outcome:**')); - ordered(late, ['Return to the affected review stage', 'new or reopened choices use Decision procedure', + ordered(late, ['Return to the affected review stage', 'Decision procedure', 'Repeat Approval readiness', 'Required outputs steps 1–4', 'before choosing navigation again']); - expect(late).toContain('Refresh affected tests, tasks, dependencies and parallelization'); - expect(late).toContain('Unchanged saved outputs may reuse their successful Review Log'); - expect(late).toContain('If a final gate discovers stale evidence, follow **Blocked outcome** first'); + expect(late).toMatch(/stale evidence, follow \*\*Blocked outcome\*\* first/i); const finish = section.slice(section.indexOf('## Required outputs')); - expect(compact(finish)).toContain('A substantive change follows **Recovery routing → Late change or missing work** before navigation resumes'); - expect(compact(finish)).toContain('A next-step answer approves no implementation change'); + expect(compact(finish)).toContain('**Recovery routing → Late change or missing work** before navigation resumes'); + expect(compact(finish)).toMatch(/a next-step answer approves no implementation change/i); ordered(finish, ['{{TASKS_SECTION_EMIT:eng-review}}', '### Completion summary', '{{PLAN_FILE_REVIEW_REPORT}}', '## Review Log', '{{REVIEW_DASHBOARD}}', '## Next Steps — Review Chaining', '## Learning hooks', '{{BRAIN_WRITE_BACK}}']); const sequence = compact(finish.slice(0, finish.indexOf('### Output reference'))); ordered(sequence, ['1. **Prepare the review body.**', '2. **Save and Read back.**', '3. **Log the saved review.**', - '4. **Publish.**', '5. **Choose navigation.**', '6. **Finish.**', - "Run Learning hooks, including gated Brain Calibration Write-Back; then return to the entrypoint's Section self-check"]); + '4. **Publish.**', '5. **Choose navigation.**', '6. **Finish.**', 'Section self-check']); }); test('plan test diagrams cover proposed paths without inventing existing implementation', () => { for (const host of ALL_HOST_CONFIGS) { const ctx = { skillName: 'plan-eng-review', host: host.name, paths: HOST_PATHS[host.name] } as TemplateContext; const audit = generateTestCoverageAuditPlan(ctx); - expect(audit).toContain('For each existing or proposed component in the selected target'); - expect(audit).toContain('Every existing or proposed function/method in scope'); - expect(audit).toContain('the selected target. For each existing or proposed feature'); - expect(audit).toContain('Future paths remain proposals, not runnable code'); - for (const obligation of ['Every conditional branch', 'Every error path', 'Every call to another function', - 'Every edge:', 'dedicated tool call before drawing the diagram', 'No skipping regression coverage']) { + expect(audit).toMatch(/existing or proposed component/); + expect(audit).toMatch(/existing or proposed function\/method/); + expect(audit).toMatch(/future paths remain proposals, not runnable code/i); + for (const obligation of ['Every conditional branch', 'Every error path', 'Every call to another function', 'Every edge:']) { expect(audit).toContain(obligation); } + expect(audit).toMatch(/no skipping regression coverage/i); const ship = generateTestCoverageAuditShip({ ...ctx, skillName: 'ship' }); - expect(ship).toContain('For each changed file, draw an ASCII diagram showing:'); - expect(ship).toContain('Every function/method that was added or modified'); - expect(ship).toContain('the changed code. For each changed feature'); + expect(ship).toMatch(/for each changed file, draw an ASCII diagram/i); + expect(ship).toMatch(/function\/method that was added or modified/); expect(ship).not.toContain('existing or proposed'); } }); - test('reserved calibration gate is explicitly skipped without enabling a write path', () => { + test('calibration write-back stays gated: no write instruction runs without the default-off gate', () => { for (const host of ALL_HOST_CONFIGS) { const ctx = { skillName: 'plan-eng-review', host: host.name, paths: HOST_PATHS[host.name] } as TemplateContext; const output = generateBrainWriteBack(ctx); - ordered(output, ['reserved default-off gate', 'this runtime does not set it', - 'Skip this section and continue the finish sequence', 'Do not enable it or infer permission from brain availability', - 'not an instruction to write now', 'Skip unless']); + const gate = output.indexOf('Skip unless `BRAIN_CALIBRATION_WRITEBACK` is set'); + expect(gate).toBeGreaterThan(0); + expect(output.indexOf('mcp__gbrain__takes_add')).toBeGreaterThan(gate); + expect(output.indexOf('mcp__gbrain__put_page')).toBeGreaterThan(gate); + expect(output.slice(0, gate)).toMatch(/skip this section/i); + expect(output.slice(0, gate)).toMatch(/do not enable it or infer permission/i); expect(output).toContain('brain_trust_policy@=personal'); - expect(output).toContain('If unknown, skip'); - expect(output).toContain('mcp__gbrain__takes_add'); - expect(output).toContain('mcp__gbrain__put_page'); + expect(output).toMatch(/if unknown, skip/i); expect(output).toContain('source_skill: plan-eng-review'); expect(output).not.toContain('${BRAIN_CALIBRATION_WRITEBACK'); for (const skillName of ['office-hours', 'plan-ceo-review', 'plan-design-review', 'plan-devex-review']) { const other = generateBrainWriteBack({ ...ctx, skillName }); expect(other).toStartWith('## Brain Calibration Write-Back (gated)\n\nSkip unless'); - expect(other).not.toContain('reserved default-off gate'); - expect(other).not.toContain('Skip this section and continue the finish sequence'); + expect(other).not.toMatch(/skip this section/i); } } });