test(mt#1263): Cover the prose-path sanitize wiring via injected RunReviewDeps seams

[minsky-ai]

Summary

sanitizeReviewBody has 17 unit tests and decidePostSanitizeOutcome has 6, but nothing reached the wiring between them. mt#1263 has been open since 2026-04-24 for exactly that gap: a refactor could pass output.text where sanitized.body belongs and every existing test would stay green.

This adds four optional RunReviewDeps fields and six end-to-end tests that pin the seam.

Supersedes closed PR #774, which covered the same three cases via mock.module plus an eslint.config.js carve-out from custom/no-global-module-mocks. Per the principal's decision recorded in the spec's ## Direction change (2026-09-02), that PR was closed rather than rebased: it was mergeable_state: dirty against a four-month-old base, and its mechanism is now prohibited at this site by ADR-036 §2 rule 2 (Accepted 2026-08-04, mt#3565):

A seam exists, or can be added by changing ≤1 production file with no exported-type change (an optional deps parameter with a real default counts as no change, per ADR-026 rule 2)? Use or build it; patching banned at this site.

runReview already takes deps: RunReviewDeps = {}, so rule 2 fires. No mock.module and no eslint.config.js change appear in this diff — eslint.config.js is untouched, and the only occurrence of the string mock.module is docblock prose explaining what this replaces.

Key changes

services/reviewer/src/review-worker.ts — four optional fields on RunReviewDeps, each defaulting to the real implementation, on top of the three mt#4895 shipped for the marker path:

field	replaces	call site
reviewerCaller	callReviewer passed into callReviewerWithRetry	the model call
bodySanitizer	sanitizeReviewBody(output.text)	prose-path sanitize
reviewSubmitter	submitReview(...)	errored path
guardedSubmitter	submitReviewWithGuards({...})	reviewed path

bodySanitizer is not a convenience — the original mt#1263 criterion asked for a seam letting a case choose its SanitizeResult rather than reverse-engineering a leaked body that produces one.

services/reviewer/src/runreview-sanitize-wiring.test.ts (new) — six tests following mt#4895's shipped pattern (runreview-concurrent-inflight.test.ts).

Two confounds the tests discriminate

1. Reaching the prose path. outputToolsActive = toolsActive && config.provider === "openai", so the test config uses provider: "anthropic". An OpenAI config routes to the output-tools path and its separate log-only scratchSanitized check, and none of these assertions would run.
2. Which text the event came from. Asserting the submitted body alone leaves the event unpinned, so the two texts parse to different events: RAW_MODEL_TEXT ends in APPROVE, STRIPPED_BODY in REQUEST_CHANGES. A wiring reading output.text would yield APPROVE.

Deliberately not seamed

The sanitizeReviewBody call on the output-tools path (the scratchSanitized check emitting reviewer.cot_leak_detected_in_scratch). It is log-only — gates no submission, changes no return value — so it is a different decision from the prose-path wiring, and mt#1263 scopes itself to the prose path. Called out here rather than left as a silent asymmetry.

Does this unblock mt#2725?

The spec requires a sentence either way. Half of it, and the remaining half is now a test to write rather than a harness to build.

review-worker.test.ts:1156-1163 defers "the full output-tools-path invocation — i.e. that the production path actually calls this" to "the mt#1263 runReview integration harness / the mt#2731 finalize-tail extraction."

• mt#2731 has since shipped (DONE). persistConvergenceMetric now lives at review-finalize.ts:68 and is called once from finalizeReviewSuccess (review-finalize.ts:332) for both success paths — so it is one call site, not two.
• The prose half is proven here. The sixth test asserts deps.metricsRecorder receives exactly one write with verdict: "request_changes", which means execution ran past the submit, through finalizeReviewSuccess, into the shared metric tail.
• The output-tools half is not exercised. It needs provider: "openai" plus a reviewerCaller returning toolCalls. Every seam that requires now exists; nothing further needs building.

Testing

Execution evidence:

New test file, all six cases — covering the spec's three named sanitize actions (stripped, errored, passthrough), the errored path's non-propagating submit failure, and the mt#2725 reachability probe:

$ cd services/reviewer && bun test --preload ../../tests/setup.ts src/runreview-sanitize-wiring.test.ts
(pass) ... > sanitize action: stripped > submits sanitized.body — not output.text — and derives the event from it
(pass) ... > sanitize action: stripped > emits reviewer.cot_leak_detected carrying the sanitize action and lengths
(pass) ... > sanitize action: errored > posts the error notice as COMMENT and returns status=error with NO review
(pass) ... > sanitize action: errored > a submitReview failure does not propagate — it is logged and the error status still returns
(pass) ... > sanitize action: passthrough > submits the raw output text and emits no cot_leak_detected log
(pass) ... > the harness drives execution through the finalize tail, not just the submit
 6 pass
 0 fail
Ran 6 tests across 1 file. [195.00ms]

Full reviewer suite (regression check on the four production call-site edits):

$ cd services/reviewer && bun test --preload ../../tests/setup.ts src/
 2213 pass
 0 fail
 4976 expect() calls
Ran 2213 tests across 79 files. [4.62s]

Note the invocation: root-level bun test cannot see reviewer tests — bunfig.toml:52 sets pathIgnorePatterns = ["services/**", "src/cockpit/web/**"].

Negative control — sanitized body reaching the submit: reverted the wiring by swapping sanitized.body → output.text at both annotateReviewBody call sites, and observed the stripped and errored cases fail.

The spec requires this be reproduced rather than cited from PR #774's body. Mutation confirmed applied before running (grep -c went 2 → 0 for the original form, 0 → 2 for the mutated form), so the control is a revert of the whole wiring rather than a partial one that could pass for the wrong reason:

$ sed -i '' 's/annotateReviewBody(sanitized\.body, .../annotateReviewBody(output.text, .../g' src/review-worker.ts
error: expect(received).toContain(expected)
(fail) ... > sanitize action: stripped > submits sanitized.body — not output.text — and derives the event from it
error: expect(received).toContain(expected)
(fail) ... > sanitize action: errored > posts the error notice as COMMENT and returns status=error with NO review
 3 pass
 2 fail

Passthrough passing under the mutation is correct, not a hole: on passthrough the sanitized body is output.text, so there is nothing for the swap to change. PR #774 recorded the same result. Mutation reverted and re-verified (grep -c back to 2 original / 0 mutated, 6 pass / 0 fail).

Negative control — the R1 leak-log assertion: narrowed the production guard from action !== "passthrough" to action === "stripped", and observed the errored case fail.

Added because R1's BLOCKING finding was a missing assertion, and an assertion added in response to a reviewer is exactly the kind that can be written to pass rather than to discriminate:

$ sed -i '' 's/if (sanitized.action !== "passthrough") {/if (sanitized.action === "stripped") {/' src/review-worker.ts
(fail) ... > sanitize action: errored > posts the error notice as COMMENT and returns status=error with NO review
 5 pass
 1 fail

Reverted and re-verified (1 original form / 0 residual mutation, 6 pass / 0 fail).

What these controls do not buy. Each proves the probe can fail for the one thing reverted. Neither establishes coverage of the defect class — e.g. a wiring passing sanitized.body to annotateReviewBody but the raw text to decidePostSanitizeOutcome is a different defect, which the event assertions (confound 2 above) are what cover.

Typecheck — clean across all 8 projects (., packages/domain, packages/shared, services/reviewer, services/site, src/cockpit/web, tsconfig.hooks.json, tsconfig.scripts.json); infra/tsconfig.json skipped with its documented reason. Lint — 0 errors, 0 warnings across 4337 files. Two warning classes were introduced and removed during the work, both fatal under CI's --max-warnings=0 gate: three no-non-null-assertion (replaced by an onlySubmission helper that also keeps the assertions non-vacuous under noUncheckedIndexedAccess) and two no-magic-string-duplication (the sanitizer reason literals, extracted to named constants). format:check clean.

Deploy verification: isDeploySurfaceFile returns true for both changed files, so this is deploy surface and carries no [no-deploy-impact] tag. The change is additive optional-parameter plumbing with no behavioral change on any path where the new deps are absent — but that is intent, not evidence, so post-merge I will wait on the deployment bound to this merge (notBefore = merge time, expectCommitSha = merge SHA) and assert the health body's service identity rather than the status code.

Branch note

The remote task/mt-1263 branch still carried PR #774's commit 350367837, so this work could not fast-forward onto it. With the principal's approval the branch was deleted and re-pushed rather than force-pushed. 350367837 remains permanently reachable at refs/pull/774/head — verified before deletion, not assumed. No MCP tool covers remote-branch deletion; that capability gap is filed as mt#4923.

Spec deviation

The original Success Criteria place these tests in services/reviewer/src/review-worker.test.ts. They are in a new runreview-sanitize-wiring.test.ts instead, matching mt#4895's shipped pattern that the re-scoped criteria direct this task to follow. review-worker.test.ts is already 1832 lines, past the repo's 1500-line error ceiling, so adding ~300 lines would deepen an existing violation.

Had Claude do the analysis and the implementation; AI-authored.

[minsky-reviewer]

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 136K prompt, 4K completion | Duration: 68s
Mode: normal

Commands

• /review — request a fresh review

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Good progress: the PR cleanly injects four optional RunReviewDeps seams and adds a focused end-to-end test suite that drives the prose-path sanitize wiring without module patching, consistent with ADR-036. The stripped, errored, and passthrough cases are covered, with additional checks for log emission and finalize-tail reachability. However, one success-criterion hole remains: the stripped-path test does not assert the required header annotation, so a regression dropping annotateReviewBody would pass undetected. I’m requesting a targeted fix for that. I also noted two non-blocking nits: add an assertion that reviewer.cot_leak_detected also fires on the errored path, and simplify the octokitFactory stub typing for readability. Address the blocking item and this should be ready to merge.

Findings

• [BLOCKING] services/reviewer/src/runreview-sanitize-wiring.test.ts:164 — Stripped-path test does not assert the required header annotation, leaving a regression hole
  The Success Criteria require asserting that submitReview receives sanitized.body + header annotation on the stripped path. In the stripped case test (services/reviewer/src/runreview-sanitize-wiring.test.ts:141-190), the assertions check that the submitted body contains STRIPPED_BODY and does not contain the scratch fragment, but there is no assertion that the annotation header produced by annotateReviewBody(...) is present. A refactor that dropped the header while still submitting the sanitized body would pass this test, violating the criterion and leaving a silent-behavior-change gap. Please assert for a distinctive header token (e.g., the known annotation prefix) to pin that the annotation is included, not just the sanitized text.
• [NON-BLOCKING] services/reviewer/src/runreview-sanitize-wiring.test.ts:214 — No assertion that reviewer.cot_leak_detected also fires on the errored sanitize action
  The top-level goal calls out that the structured CoT-leak event should emit "only when sanitize did something" (i.e., on stripped and errored, not passthrough). The stripped-path test asserts the event (lines 192-205), and the passthrough test asserts its absence (lines 289-305), but the errored block (lines 206-279) never checks for the presence of reviewer.cot_leak_detected. This leaves a small coverage gap: a regression that stops logging on the errored path would not be caught. Consider adding findLogEvent(logs, COT_LEAK_EVENT) presence assertion to the first errored test (and optionally validate action/lengths like the stripped case).
• [NON-BLOCKING] services/reviewer/src/runreview-sanitize-wiring.test.ts:129 — Overly complex type cast for octokitFactory stub reduces readability without adding safety
  In baseDeps, octokitFactory is stubbed as async () => ({}) as Awaited<ReturnType<RunReviewDeps["octokitFactory"] & {}>> (around lines 122–135). The intersection with {} and deep Awaited<ReturnType<...>> casting adds noise and could mask a future accidental use of octokit properties by making everything type-assert to success. Since the stubbed submitters do not consume octokit, a simpler and clearer as any or an explicit minimal mock type is sufficient. Consider replacing with a minimal typed mock or as unknown as ReturnType<typeof createOctokit> to improve clarity.

Spec verification

Criterion	Status	Evidence
New integration test(s) in services/reviewer/src/review-worker.test.ts that exercise runReview end-to-end with mocked octokit, callReviewer, and getAppIdentity.	N/A	Superseded by the re-scoped criteria under “Direction change (2026-09-02)”: tests may live in a sibling file following mt#4895’s pattern. This PR adds services/reviewer/src/runreview-sanitize-wiring.test.ts covering the end-to-end wiring via injected RunReviewDeps seams.
Test case: sanitize returns stripped → assert submitReview receives sanitized.body (not output.text) + header annotation, event matches parseReviewEvent on the stripped body, status="reviewed" returned, [cot-leakage: stripped] in reason.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:141-190 asserts: guarded submitter receives a body containing STRIPPED_BODY and not the SCRATCH_FRAGMENT (proves sanitized.body used), submitted.event === "REQUEST_CHANGES" (pins event from sanitized body), result.status === "reviewed" and result.reason contains "[cot-leakage: stripped]", and result.review equals SUBMITTED.
Test case: sanitize returns errored → assert submitReview receives the error notice body, event forced to "COMMENT", status="error" returned with NO review field populated, posting failure does not propagate (try/catch).	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:208-254 checks plain submitter receives ERRORED_BODY and not the scratch fragment; submitted.event === "COMMENT"; result.status === "error"; result.review is undefined. services/reviewer/src/runreview-sanitize-wiring.test.ts:256-279 injects a throwing reviewSubmitter and asserts status remains "error" and a "reviewer.submit_error_notice_failed" log is emitted.
Test case: sanitize returns passthrough → assert submitReview receives the raw output.text, no reviewer.cot_leak_detected log emitted.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:281-305 asserts guarded submitter body contains RAW_MODEL_TEXT and submitted.event === "APPROVE"; findLogEvent(..., "reviewer.cot_leak_detected") returns null.
Use a test seam for sanitizeReviewBody (mock it to control what action is returned) so test cases don't need to construct elaborate leaked bodies.	Met	Production seam added at services/reviewer/src/review-worker.ts:1465 — (deps.bodySanitizer ?? sanitizeReviewBody)(output.text). Tests pass bodySanitizer via RunReviewDeps in baseDeps (services/reviewer/src/runreview-sanitize-wiring.test.ts:122-168) to choose stripped/errored/passthrough results directly.
PR #774 is CLOSED with a comment naming the Direction change section as the reason — not merged, not left open.	Unverifiable	This criterion depends on repository/PR state outside this diff. The review cannot verify PR #774’s state from in-repo files; no in-repo artifact records the close action.
The three original test cases (stripped, errored, passthrough) ship, asserting exactly what the criteria above specify.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts adds three describe blocks covering stripped (lines ~141-205), errored (lines ~206-279), and passthrough (lines ~281-305) with the specified assertions.
They are built on RunReviewDeps injection, following mt#4895’s pattern. No mock.module, and no new eslint.config.js exception. If some collaborator cannot be reached through deps, extend RunReviewDeps rather than reaching for a patch.	Met	services/reviewer/src/review-worker.ts:367-389 adds optional RunReviewDeps fields (reviewerCaller, bodySanitizer, reviewSubmitter, guardedSubmitter) with real defaults; call sites route through deps.* ?? real. services/reviewer/src/runreview-sanitize-wiring.test.ts constructs deps and does not use mock.module. No changes to eslint.config.js appear in the diff.
A negative control per case, in the shape #774 already demonstrated: swapping sanitized.body for output.text must fail cases 1 and 2. Reproduce it rather than citing it.	Unverifiable	The PR body includes a ‘Negative control’ section with local run output, but no in-repo artifact encodes this mutation test. As reviewers we cannot reproduce live; no code change in this diff commits a negative-control harness.
State in the PR body whether the harness is now reusable enough to unblock mt#2725’s production-path assertion, or whether that still needs mt#2731’s finalize-tail extraction. A sentence either way.	Unverifiable	The required artifact is PR-body prose rather than a repo file. While the PR description contains such a statement, the review tools require file:line evidence; no in-repo file encodes this declaration.

Documentation impact

• no-update-needed — This PR adds optional injection seams to an internal type (RunReviewDeps) and new tests covering existing behavior. No user-facing commands, routes, or documented behaviors change, and no docs are updated in the diff. I also spot-checked for any docs referencing the sanitize wiring or RunReviewDeps and found none in this PR’s changes.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

The PR cleanly introduces DI seams for the prose-path sanitize wiring and adds comprehensive end-to-end tests for stripped/errored/passthrough, plus a finalize-tail reachability probe. However, there is a normative gap against the task spec: the errored sanitize case lacks an assertion that reviewer.cot_leak_detected is emitted, even though the spec requires the event to be logged for both stripped and errored (and the production code logs whenever action ≠ passthrough). Please add that assertion in the errored block. Also noted a minor type-casting nit in the test’s octokitFactory stub that could be simplified. Once the logging assertion is added, this looks ready to merge.

Findings

• [BLOCKING] services/reviewer/src/runreview-sanitize-wiring.test.ts:270 — Spec gap: no assertion that reviewer.cot_leak_detected is emitted on the errored sanitize path
  The task’s Success Criteria require that the sanitize wiring "emits reviewer.cot_leak_detected log emitted only for stripped/errored". This test suite asserts the event is present for stripped (lines ~255–268) and absent for passthrough (lines ~353–362), but the errored block (lines ~275–317) does not assert that the CoT-leak detection event was emitted. The production code logs whenever sanitized.action !== "passthrough" (review-worker.ts:1467-1474 after this diff), so the errored case should also be covered. Please add an assertion in the errored describe to verify that reviewer.cot_leak_detected appears with the expected fields, mirroring the stripped-path check. This is a normative acceptance-test gap and must be closed before merge.
• [NON-BLOCKING] services/reviewer/src/runreview-sanitize-wiring.test.ts:167 — Type assertion for octokitFactory return type is needlessly complex and may be ill-typed
  In baseDeps, octokitFactory is implemented as async () => ({}) as Awaited<ReturnType<RunReviewDeps["octokitFactory"] & {}>>.
• RunReviewDeps["octokitFactory"] is an optional function type, so taking ReturnType<…> over a possibly-undefined type is generally ill-formed in TypeScript.
• Intersecting with {} (& {}) does not reliably narrow away undefined; this risks a type error under stricter TS settings and makes the intent hard to read.
  Suggestion: simplify to a concrete, accurate type or unknown/any for the stub, e.g. as any, or extract the actual Octokit client type from createOctokit and use that directly. This keeps the test clear and avoids brittle meta-type gymnastics.

Spec verification

Criterion	Status	Evidence
PR #774 is CLOSED with a comment naming the Direction change (2026-09-02) section as the reason.	Unverifiable	This is GitHub state outside the repo. The diff cannot prove PR #774’s closure or the presence of a close-comment. Reported Unverifiable per the Critic Constitution; reviewer cannot fetch external PR state from this diff.
The three original sanitize-path cases (stripped, errored, passthrough) ship, asserting the specified wiring (body/event/reason/logs) end-to-end.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:94-119 defines RAW_MODEL_TEXT/STRIPPED_BODY/ERRORED_BODY. Tests cover: stripped path submission uses sanitized body, derives event from sanitized text, includes header and tier (lines ~208-249); emits reviewer.cot_leak_detected with action/lengths (lines ~255-268). Errored path forces COMMENT, returns status=error with no review, and logs submit_error_notice_failed on failure (lines ~275-317, ~319-337). Passthrough submits raw text and emits no cot_leak_detected (lines ~343-362).
Harness built on RunReviewDeps injection; no mock.module and no new eslint.config.js carve-out. Extend RunReviewDeps if needed.	Met	services/reviewer/src/review-worker.ts:367-389 adds optional deps: reviewerCaller, bodySanitizer, reviewSubmitter, guardedSubmitter, each defaulted at callsites (lines ~1000, ~1465, ~1499, ~1536). No eslint.config.js diff appears; no mock.module usages in the new test file.
Negative control per case: swapping sanitized.body → output.text must fail stripped/errored cases; reproduce rather than cite.	Unverifiable	The mutation control is described in the PR body’s execution log, but it is not committed as code. Review cannot execute tests; PR-body logs are untrusted per policy. Unable to verify within the diff; mark Unverifiable.
State whether the harness unblocks mt#2725’s production-path assertion, or still needs mt#2731 finalize-tail extraction. Provide a sentence either way.	Met	PR description’s “Does this unblock mt#2725?” section states: prose half is proven here via a metricsRecorder write after finalize tail; output-tools half still unexercised and would need provider=openai and toolCalls. The sentence is present in the PR body provided.

Documentation impact

• no-update-needed — Change is internal test coverage plus optional dependency-injection seams on RunReviewDeps with defaults preserving behavior. No CLI, API, or user-facing behavior changed; no docs in docs/ reference these internal seams. Verified only services/reviewer/src/* touched; no docs updated or invalidated.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Re-verification complete. The prior BLOCKING gap — missing assertion that the leak guard logs on the errored sanitize path — is addressed by the new assertions in the errored test case, and I see no new defects introduced by this fix. The added test file cleanly exercises the prose-path sanitize wiring via injected RunReviewDeps seams, covers stripped/errored/passthrough, and includes a finalize-tail reachability check. No production behavior changes; no new public exports. I find no critical issues in the current diff. Event: APPROVE.

Spec verification

Criterion	Status	Evidence
New integration test(s) in services/reviewer/src/review-worker.test.ts that exercise runReview end-to-end with mocked octokit, callReviewer, and getAppIdentity.	N/A	Superseded by the re-scoped criteria in “## Direction change (2026-09-02)”: tests are allowed in a sibling file to avoid deepening an existing 1500-line ceiling violation. Implemented in services/reviewer/src/runreview-sanitize-wiring.test.ts.
Test case: sanitize returns stripped → assert submitReview receives sanitized.body (not output.text) + header annotation, event matches parseReviewEvent on the stripped body, status="reviewed" returned, [cot-leakage: stripped] in reason.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:129-170 — asserts guarded submit uses body containing STRIPPED_BODY and not SCRATCH_FRAGMENT; contains header markers (Chinese-wall, Tier: 3); event is REQUEST_CHANGES; result.status === "reviewed" and reason contains "[cot-leakage: stripped]"; result.review equals SUBMITTED.
Test case: sanitize returns errored → assert submitReview receives the error notice body, event forced to "COMMENT", status="error" returned with NO review field populated, posting failure does not propagate (try/catch).	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:178-236 — plain submit receives ERRORED_BODY and not SCRATCH_FRAGMENT; event === "COMMENT"; result.status === "error" and review is undefined. 238-257 — injecting a throwing reviewSubmitter still returns status=error and logs reviewer.submit_error_notice_failed.
Test case: sanitize returns passthrough → assert submitReview receives the raw output text, no reviewer.cot_leak_detected log emitted.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:263-286 — guarded submit receives RAW_MODEL_TEXT and event === "APPROVE"; result.status === "reviewed"; findLogEvent(logs, COT_LEAK_EVENT) returns null.
Use a test seam for sanitizeReviewBody (mock it to control what action is returned) so test cases don't need to construct elaborate leaked bodies.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:206-219 (baseDeps) wires deps.bodySanitizer to return the supplied SanitizeResult; all three cases pass controlled SanitizeResult values.
PR #774 is CLOSED with a comment naming this section as the reason — not merged, not left open. The work is superseded by mechanism, not by subject.	Unverifiable	This criterion depends on repository/PR state outside this diff. The review surface does not include cross-repo API state; cannot verify whether PR #774 is currently closed with the prescribed comment.
The three original test cases (stripped, errored, passthrough) ship, asserting exactly what the criteria above specify — that content is unchanged and still correct.	Met	All three cases present and asserting the required behaviors: stripped (lines 129-170, 172-176), errored (lines 178-236, 238-257), passthrough (lines 263-286).
They are built on RunReviewDeps injection, following mt#4895's shipped pattern. No mock.module, and no new eslint.config.js exception. If some collaborator genuinely cannot be reached through deps, extend RunReviewDeps rather than reaching for a patch — and say so in the PR body.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:206-219 builds a deps object with injected seams: octokitFactory, prContextFetcher, appIdentityFetcher, reviewerCaller, bodySanitizer, reviewSubmitter, guardedSubmitter, timing/metrics emitters. The diff shows no eslint.config.js changes and no use of mock.module.
A negative control per case, in the shape #774 already demonstrated: swapping sanitized.body for output.text must fail cases 1 and 2. #774's PR body records that mutation test passing; reproduce it rather than citing it.	Met	PR body includes executed negative-control transcript showing two failures when mutating annotateReviewBody(sanitized.body, …) → annotateReviewBody(output.text, …); tests explicitly construct discriminating inputs (RAW_MODEL_TEXT vs STRIPPED_BODY) to make the swap detectable. Note: negative-control evidence is reported in the PR description; no separate committed test is expected by the spec.
State in the PR body whether the harness is now reusable enough to unblock mt#2725’s production-path assertion, or whether that still needs mt#2731’s finalize-tail extraction. A sentence either way — do not silently leave the question open.	Met	PR body section “Does this unblock mt#2725?” states half proven (prose path) and remaining half needs an OpenAI/tools path; the last test (services/reviewer/src/runreview-sanitize-wiring.test.ts:293-330) asserts deps.metricsRecorder receives a write, evidencing passage through finalize tail.

Documentation impact

• no-update-needed — This PR adds an integration test file (services/reviewer/src/runreview-sanitize-wiring.test.ts) and does not alter production behavior or public APIs. No user-facing docs reference are impacted, and no existing docs are invalidated by test-only additions.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Re-verification complete. The prior BLOCKING gap is addressed: the errored sanitize-path test now asserts the leak-guard log, and all three sanitize wiring cases (stripped, errored, passthrough) are covered end-to-end via injected RunReviewDeps seams. The production changes are additive optional seams with real defaults; call sites route through them correctly. No module patching or eslint carve-outs were introduced, matching the re-scoped criteria. I find no new critical defects or behavior changes. Documentation impact: none. Event: APPROVE.

Spec verification

Criterion	Status	Evidence
New integration test(s) in services/reviewer/src/review-worker.test.ts that exercise runReview end-to-end with mocked octokit, callReviewer, and getAppIdentity.	N/A	Superseded by the re-scoped criteria in “## Direction change (2026-09-02)”. Tests are placed in a new sibling file following mt#4895’s pattern: services/reviewer/src/runreview-sanitize-wiring.test.ts (see added file in this PR).
Test case: sanitize returns stripped → assert submitReview receives sanitized.body (not output.text) + header annotation, event matches parseReviewEvent on the stripped body, status="reviewed" returned, [cot-leakage: stripped] in reason.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:116-168 — asserts guarded submit used, submitted.body contains STRIPPED_BODY and not the scratch fragment; header annotation present; submitted.event === "REQUEST_CHANGES"; result.status === "reviewed"; result.reason contains [cot-leakage: stripped]; result.review equals SUBMITTED.
Test case: sanitize returns errored → assert submitReview receives the error notice body, event forced to "COMMENT", status="error" returned with NO review field populated, posting failure does not propagate (try/catch).	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:170-224 — asserts plain submit path used, body contains ERRORED_BODY, submitted.event === "COMMENT", result.status === "error", result.review is undefined, and the subsequent test at :226-248 forces a submit failure and asserts it is logged and does not change the returned error status.
Test case: sanitize returns passthrough → assert submitReview receives the raw output.text, no reviewer.cot_leak_detected log emitted.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:250-282 — asserts guarded submit used with body containing RAW_MODEL_TEXT, event APPROVE, result.status === "reviewed", and findLogEvent(logs, COT_LEAK_EVENT) returns null (no leak log).
Use a test seam for sanitizeReviewBody (mock it to control what action is returned) so test cases don't need to construct elaborate leaked bodies.	Met	services/reviewer/src/review-worker.ts:367-404,1465 — RunReviewDeps gains optional bodySanitizer with a real default; tests supply bodySanitizer via baseDeps() (runreview-sanitize-wiring.test.ts:189-205) to return specific SanitizeResults directly.
PR #774 is CLOSED with a comment naming this section as the reason — not merged, not left open. The work is superseded by mechanism, not by subject.	Unverifiable	This criterion refers to GitHub PR state outside the repository diff. The current PR does not carry API evidence of #774’s closure. Reviewer cannot verify external PR state from repository contents alone.
The three original test cases (stripped, errored, passthrough) ship, asserting exactly what the criteria above specify — that content is unchanged and still correct.	Met	All three cases are implemented in services/reviewer/src/runreview-sanitize-wiring.test.ts with assertions mirroring the original criteria (see lines 116-168, 170-248, 250-282).
They are built on RunReviewDeps injection, following mt#4895's shipped pattern. No mock.module, and no new eslint.config.js exception. If some collaborator genuinely cannot be reached through deps, extend RunReviewDeps rather than reaching for a patch — and say so in the PR body.	Met	services/reviewer/src/review-worker.ts adds optional reviewerCaller, bodySanitizer, reviewSubmitter, guardedSubmitter with real defaults and routes callsites through them (:1000, :1465, :1499, :1536). New tests consume these seams. The added test file contains no mock.module usage, and no eslint.config.js changes appear in the diff.
A negative control per case, in the shape #774 already demonstrated: swapping sanitized.body for output.text must fail cases 1 and 2. #774's PR body records that mutation test passing; reproduce it rather than citing it.	Met	The PR body’s “Testing” section shows explicit mutation runs: replacing annotateReviewBody(sanitized.body, …) → annotateReviewBody(output.text, …) caused the stripped and errored tests to fail, while passthrough appropriately still passed. The commands and failing assertions are included in the PR description.
State in the PR body whether the harness is now reusable enough to unblock mt#2725's production-path assertion, or whether that still needs mt#2731's finalize-tail extraction. A sentence either way — do not silently leave the question open.	Met	PR body section “Does this unblock mt#2725?” documents that mt#2731 shipped and that this harness proves the prose half (via a test asserting a metricsRecorder write) while the output-tools half remains to be exercised; see the added test …drives execution through the finalize tail… at services/reviewer/src/runreview-sanitize-wiring.test.ts:284-330.

Documentation impact

• no-update-needed — Change is limited to adding optional dependency-injection seams in services/reviewer/src/review-worker.ts and adding a new test file. No user-facing behavior, commands, or configuration changed; no docs reference these internal seams. Searched the diff for docs/ changes — none present.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Re-verification complete. The prior BLOCKING gap (missing assertion that the leak-guard log fires on the errored sanitize path) is addressed in the new test file, which now asserts reviewer.cot_leak_detected for both stripped and errored and ensures no log on passthrough. The production changes add four optional RunReviewDeps seams with real defaults and correctly route call sites through them; no behavior change when deps are absent. No eslint carve-outs or module patching were introduced, and I find no new critical defects from this fix. Documentation impact: none. Event: APPROVE.

Spec verification

Criterion	Status	Evidence
PR #774 is CLOSED with a comment naming this section as the reason — not merged, not left open. The work is superseded by mechanism, not by subject.	Unverifiable	This criterion concerns the state of an external PR (#774) and its closure comment, which are outside the repository diff. I cannot verify repository-external PR state from this review context.
The three original test cases (stripped, errored, passthrough) ship, asserting exactly what the criteria above specify — that content is unchanged and still correct.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts — three describe blocks exercise: (1) stripped: asserts annotateReviewBody receives sanitized.body and event derived from sanitized text; (2) errored: asserts error notice posted via plain submitter, forced COMMENT, status=error with no review, and leak-guard log present; (3) passthrough: asserts raw output submitted and no cot_leak_detected log.
They are built on RunReviewDeps injection, following mt#4895's shipped pattern. No mock.module, and no new eslint.config.js exception. If some collaborator cannot be reached through deps, extend RunReviewDeps rather than patching — and say so in the PR body.	Met	Production: services/reviewer/src/review-worker.ts adds optional reviewerCaller, bodySanitizer, reviewSubmitter, guardedSubmitter to RunReviewDeps (L~367-406) and routes call sites through deps.* ?? real. Tests: services/reviewer/src/runreview-sanitize-wiring.test.ts constructs deps via baseDeps(...). No changes to eslint.config.js appear in the diff; no usage of mock.module in the test file.
A negative control per case, in the shape #774 already demonstrated: swapping sanitized.body for output.text must fail cases 1 and 2. Reproduce it rather than citing it.	Unverifiable	Negative-control executions and their observed failures are not committed artifacts; they are described in the PR body. I cannot run tests here. The test file focuses on the positive assertions; the mutation run evidence lives outside the diff.
State in the PR body whether the harness is now reusable enough to unblock mt#2725’s production-path assertion, or whether that still needs mt#2731’s finalize-tail extraction. A sentence either way — do not silently leave the question open.	Met	The PR description includes a dedicated section “Does this unblock mt#2725?” stating that half is proven here (prose path reaches finalize tail via metricsRecorder seam) and the output-tools half remains to be exercised with provider=openai and toolCalls.

Documentation impact

• no-update-needed — Change adds optional test seams to RunReviewDeps and a new integration test file covering existing behavior. No user-facing behavior, routes, or CLI changed. No docs in docs/ reference these internals; behavior on production paths remains the same when deps are absent.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Re-verification complete. The prior BLOCKING finding (missing assertion that the leak-guard log fires on the errored sanitize path) is addressed by the new test assertions. The production diff adds four optional RunReviewDeps seams with real defaults and correctly routes the call sites through them; no behavior change when deps are absent. The new end-to-end tests cover stripped/errored/passthrough via injected seams, pin the event derivation to the sanitized body, confirm the error path’s non-propagating submit failure, and prove finalize-tail reachability. I find no new critical defects introduced by the fixes. Documentation impact: none. Event: APPROVE.

Spec verification

Criterion	Status	Evidence
New integration test(s) in services/reviewer/src/review-worker.test.ts that exercise runReview end-to-end with mocked octokit, callReviewer, and getAppIdentity.	N/A	Superseded by the re-scoped criteria in “## Direction change (2026-09-02)”: tests may live in a sibling file following mt#4895’s pattern. This PR adds services/reviewer/src/runreview-sanitize-wiring.test.ts rather than editing review-worker.test.ts.
Test case: sanitize returns stripped → assert submitReview receives sanitized.body (not output.text) + header annotation, event matches parseReviewEvent on the stripped body, status="reviewed" returned, [cot-leakage: stripped] in reason.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:121-177 — asserts guarded submit received sanitized body and not SCRATCH_FRAGMENT, header annotation lines present, event === "REQUEST_CHANGES", result.status === "reviewed", and result.reason contains [cot-leakage: stripped].
Test case: sanitize returns errored → assert submitReview receives the error notice body, event forced to "COMMENT", status="error" returned with NO review field populated, posting failure does not propagate (try/catch).	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:179-235 — asserts plain submit received the error notice body, event === "COMMENT", result.status === "error", result.review is undefined, and at :237-258 a simulated submit failure logs reviewer.submit_error_notice_failed and does not change status.
Test case: sanitize returns passthrough → assert submitReview receives the raw output.text, no reviewer.cot_leak_detected log emitted.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:260-290 — asserts raw text is submitted on the guarded path, event === "APPROVE", and findLogEvent(logs, COT_LEAK_EVENT) returns null.
Use a test seam for sanitizeReviewBody (mock it to control what action is returned) so test cases don't need to construct elaborate leaked bodies.	Met	services/reviewer/src/review-worker.ts:1465 routes through deps.bodySanitizer ?? sanitizeReviewBody; tests inject bodySanitizer via baseDeps(...) at runreview-sanitize-wiring.test.ts:211-220 to choose SanitizeResult directly.
PR #774 is CLOSED with a comment naming this section as the reason — not merged, not left open. The work is superseded by mechanism, not by subject.	Unverifiable	This criterion concerns GitHub PR state outside the repo. The diff cannot show whether PR #774 was closed with the required comment. Marking Unverifiable per the review contract for out-of-repo artifacts.
The three original test cases (stripped, errored, passthrough) ship, asserting exactly what the criteria above specify — that content is unchanged and still correct.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts contains three describes covering stripped (lines 121-177), errored (179-258), and passthrough (260-290) with the specified assertions.
They are built on RunReviewDeps injection, following mt#4895's shipped pattern. No mock.module, and no new eslint.config.js exception. If some collaborator cannot be reached through deps, extend RunReviewDeps.	Met	services/reviewer/src/review-worker.ts:367-389 adds four optional RunReviewDeps seams with real defaults; call sites route through them at :1000, :1465, :1499, :1536. Test file injects via baseDeps(...). No occurrences of mock.module in the added test, and no eslint.config.js changes appear in the diff.
A negative control per case, in the shape #774 already demonstrated: swapping sanitized.body for output.text must fail cases 1 and 2. Reproduce it rather than citing it.	Unverifiable	The PR body includes console transcripts demonstrating the mutation test runs, but the diff does not add automated mutation tests. Live-run evidence is out-of-repo; cannot be verified from code. Marking Unverifiable per contract.
State in the PR body whether the harness is now reusable enough to unblock mt#2725's production-path assertion, or whether that still needs mt#2731's finalize-tail extraction. A sentence either way.	Met	services/reviewer/src/runreview-sanitize-wiring.test.ts:292-339 adds a test that asserts deps.metricsRecorder receives exactly one write with verdict: "request_changes", proving the harness reaches the finalize tail. The PR body also states the output-tools half remains to be exercised; the seams now exist to do so.

Documentation impact

• no-update-needed — Production changes add optional RunReviewDeps seams and route existing call sites through them with real defaults (services/reviewer/src/review-worker.ts:1000,1465,1499,1536). No external behavior or public API surface changed; new tests only exercise existing wiring. No docs reference or user-visible behavior was altered.