feat(mt#4988): Watch mt#4996's reopen triggers so the accept is not decide-and-forget

[minsky-ai]

Summary

mt#4988 was filed to build burst detection plus a pipeline capturing perishable Railway deploy logs.
mt#4996 (merged today, PR #3650) removed the reason for both, and this PR ships what survived.

The task was explicitly queued behind that decision — mt#5006 filed it as "Reconsider scope first:
largely obviated if mt#4996 accepts the cadence." The re-scope is recorded as
## AMENDMENT 2026-09-05 on the task, with superseding criteria and the originals left visible.

Retired. The Railway-log capture lost its consumer: mt#4996's three reopen triggers are all
answerable from review_timing alone, so nothing reads a deploy log. And surfacing every recovered
burst became alert noise — a recovered burst is now documented expected behaviour at a ~13-day
cadence, and mt#2719's SC5 already excludes provider_timeout from paging on a self-healing premise
mt#4996 measured holding at 99.25%.

Shipped. mt#4996's accept installed three thresholds and nothing evaluated them — a gap
created today. Only one had partial coverage: reviewer-pre-submit-failure/v1 (mt#4881) pages
per-occurrence on an unrecovered timeout, covering trigger 1's single case but not its aggregate,
and neither of the other two, which are distributional and would go unnoticed indefinitely.

Key changes

• services/reviewer/src/timeout-regime-watch.ts (new) — a daily in-process check over a
  rolling 30 days of review_timing, following findings-aggregation.ts's scheduler shape
  (enabled flag, parsePositiveIntEnv, isRunning re-entrancy guard, cycle never throws). The
  evaluator is a pure function of its inputs; the query takes an injected nowMs with a real
  default. Records all three readings against their thresholds every cycle, and notifies only on a
  new crossing.
• services/reviewer/src/server.ts — registration beside the other schedulers, opt-in via
  TIMEOUT_REGIME_WATCH_ENABLED.
• Defaults are mt#4996's recorded values, not round numbers: 2 unrecovered events, 95.00% recovery,
  115s p99.9, 30-day window, 118s completing-round cap.

Deliberately NOT an operator incident. ask-emitter.ts's operator-incident path renders
"Reviewer is down — …" and "Only you can clear this — the reviewer cannot recover on its own."
Both are false here: nothing is down, and the remedy is re-running mt#4996's analysis, which an
agent can do. So this notifies through the existing AlertSink at warn, and the body says so
outright — "Nothing is broken and no one is paged" — with a test asserting that sentence so the
framing cannot erode later. The OperatorIncidentContext union is untouched, so the consumer
set the spec enumerated for it is not disturbed and no contract propagates. Recorded on the task as
## Implementation record 2026-09-05.

Two edge cases the thresholds turn on

• A quiet window reports recovery as not-computable, never as a number. Zero timeout events is
  the common case in this corpus. Reporting 0% would fire the trigger on every quiet window;
  reporting 100% would make it unfalsifiable exactly where there is nothing to measure. The value is
  null, and a null never crosses.
• The p99.9 is computed only over rounds BELOW the cap. A round recorded at the cap is censored
  by the timeout mechanism rather than measured (mem#1373) — the correction that changed mt#1897's
  conclusion after three passes computed percentiles over cap artifacts. Encoding it here keeps the
  trigger from inheriting the same error.

Correction to the spec's own burst figures

The summary asserted "37 of 47 timeouts fall on 6 of 42 days; bursts arrive roughly monthly."
Re-run under the spec's own tool_use_active is true filter: 7 burst days in 93 (≈ one per 13
days, so more frequent than monthly) and 59 of 115 timeout rows on burst days (51%, so less
concentrated than 79%); over the same last-42-day window, 2 burst days and 24 of 39 rows, not 6 and
37. Recorded as measured, not diagnosed — this pass did not establish why the inherited numbers
differ, and says so rather than guessing.

Acceptance tests

AT1, AT2 and AT4 were superseded by the amendment (AT1′/AT2′/AT4′); AT3 is retained unchanged
and is the no-duplicate-with-mt#4881 constraint. Numbering below follows the task spec.

Execution evidence:

$ cd services/reviewer && bun test --preload ../../tests/setup.ts src/timeout-regime-watch.test.ts

(pass) evaluateTimeoutRegime > AT2': the measured baseline crosses nothing
(pass) evaluateTimeoutRegime > AT3: ONE unrecovered event does not cross — mt#4881 already pages per occurrence
(pass) evaluateTimeoutRegime > AT4': two unrecovered events cross the aggregate trigger
(pass) evaluateTimeoutRegime > AT1': recovery below 95% crosses
(pass) evaluateTimeoutRegime > recovery exactly at the threshold does not cross
(pass) evaluateTimeoutRegime > a quiet window reports recovery as NOT COMPUTABLE, never as zero
(pass) evaluateTimeoutRegime > p99.9 above 115s crosses; at the threshold it does not
(pass) evaluateTimeoutRegime > no completing rounds reports null, and null never crosses
(pass) sampleTimeoutRegime > Postgres string aggregates are coerced to numbers
(pass) sampleTimeoutRegime > a NULL percentile (no completing rounds) stays null rather than becoming 0
(pass) runTimeoutRegimeWatchCycle > AT1'/SC2': a crossing surfaces exactly once, and not again while it persists
(pass) runTimeoutRegimeWatchCycle > a trigger that clears and re-crosses notifies again
(pass) runTimeoutRegimeWatchCycle > AT2'/AT3: a baseline window with one unrecovered event surfaces nothing
(pass) runTimeoutRegimeWatchCycle > a query failure is swallowed, not thrown — the watch never crashes the service
(pass) runTimeoutRegimeWatchCycle > a missing alert sink does not prevent the crossing being detected
(pass) buildTimeoutRegimeAlertBody > names the crossed trigger, its value, its threshold, and where to reopen
(pass) buildTimeoutRegimeAlertBody > says plainly that this is not an incident
(pass) loadTimeoutRegimeWatchConfig > defaults are mt#4996's recorded values, not round numbers
(pass) loadTimeoutRegimeWatchConfig > the recovery rate is carried as basis points

 19 pass / 0 fail / 53 expect() calls

AT1′ — recovery below 95% surfaces exactly once: covered by the AT1' evaluator case (92.5%
crosses) and the AT1'/SC2' cycle case, which asserts one alert on the first cycle and no second
alert while the crossing persists. AT2′ — the baseline window crosses nothing (evaluator +
cycle). AT3 (retained) — one unrecovered event does not cross, so no duplicate lands alongside
reviewer-pre-submit-failure/v1; asserted at both the evaluator and cycle level, and confirmed on
REAL data below. AT4′ — two unrecovered events cross the aggregate trigger.

SC1′ (every reading recorded with its threshold each cycle) — asserted by
result.readings having all three entries on a healthy cycle. SC2′ (surfaces only on a
crossing, at most once per crossing) — the suppression pair above, plus the clears-and-re-crosses
case proving suppression is not permanent. SC4′ (config, defaults are mt#4996's values) — the
two loadTimeoutRegimeWatchConfig cases. SC3/SC5 — see Live verification.

Full reviewer suite (server.ts is touched, so the whole package ran):
2584 pass / 0 fail across 98 files.

Local checks (session be46aa3f): typecheck pass, 0 errors, 8 projects including
services/reviewer (whose tsconfig sets noUncheckedIndexedAccess); lint 0 errors / 0 warnings
across 4,391 files; prettier clean. validatedWorkspace confirmed as the session dir on both.

Live verification

The unit tests inject a fake DB, so they say nothing about whether the SQL is valid or returns what
the evaluator expects. The module's query was run verbatim against production review_timing, twice.

1. The real 30-day window — the healthy case.

reviews_with_timeout | timeout_events | unrecovered_events |   p999_ms   | completing_rounds
                  36 |             51 |                  1 | 108004.608  |             27170

Through the shipped thresholds: unrecovered 1 < 2 (not crossed); recovery 50/51 = 98.04% ≥ 95%
(not crossed); p99.9 108.0s < 115s (not crossed). All three read healthy — the correct
verdict today, matching mt#4996's accept. This also independently confirms the figures the re-scope
reasoned from: 51 events with 1 unrecovered over 30 days is exactly what mt#4996's per-day table
predicts.

2. SC5 — replayed over the REAL 2026-09-04 burst, which produces a real crossing.

A healthy window cannot show that the triggers FIRE, so the same query was replayed over the burst
day that started this whole investigation:

reviews_with_timeout | timeout_events | unrecovered_events | recovery_pct |   p999_ms   | completing_rounds
                   9 |              9 |                  1 |        88.89 | 116141.456  |              1455

Through the shipped thresholds: recovery 88.89% < 95% → CROSSES; p99.9 116.1s > 115s →
CROSSES; unrecovered 1 < 2 → does not cross. So on real burst data the watch fires on two of
three triggers and stays silent on the one reviewer-pre-submit-failure/v1 already paged for —
which is AT3's no-duplicate constraint holding on production data rather than on a fixture. This
discharges SC5 as written ("verified against a real or replayed burst, not only a synthetic one").

Worth stating plainly so the numbers are not over-read: the shipped window is 30 days, and one
burst day does not move a 30-day p99.9 (108.0s above). That is the intended behaviour — a single bad
day should not reopen a settled decision — and the replay above is a narrowed window used to prove
the trigger arithmetic fires on genuine degradation, not a claim that today's 30-day regime crosses.

Deploy verification:

All three changed files return true from isDeploySurfaceFile, verified by running the predicate
over this PR's actual changed-file list rather than recalling a pattern set:

$ bun -e 'import { isDeploySurfaceFile } from "./packages/domain/src/deployment/deploy-surface.ts";
  for (const f of process.argv.slice(1)) console.log(isDeploySurfaceFile(f), f);' \
  services/reviewer/src/timeout-regime-watch.ts services/reviewer/src/timeout-regime-watch.test.ts \
  services/reviewer/src/server.ts
true services/reviewer/src/timeout-regime-watch.ts
true services/reviewer/src/timeout-regime-watch.test.ts
true services/reviewer/src/server.ts

So this is not [no-deploy-impact]. After merge I will run deployment_wait-for-latest against
the reviewer service with notBefore set to the merge timestamp and expectCommitSha set to the
merge SHA, read buildIdentity, and assert the /health body's service field is
minsky-reviewer rather than accepting the status code.

The watch itself ships disabled (TIMEOUT_REGIME_WATCH_ENABLED defaults to false, matching
findings-aggregation.ts), so the deploy carries the code without starting the scheduler. That is
deliberate: it makes this merge a no-op at runtime, and enabling it is a one-variable change once
the deploy is confirmed healthy. No new external-system integration — the check reads a
first-party Postgres table through the DB handle the service already holds and notifies through an
alert sink it already constructs — so no credential or scope is required and no live-exercise beyond
deploy health is owed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YT3CvxJhrcGeVCv8P3DGPD

[minsky-reviewer]

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 534K prompt, 5K completion | Duration: 161s
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

---

Solid, well-tested addition that follows the service’s scheduler pattern. However, one blocking correctness gap: the SQL in sampleTimeoutRegime does not apply the tool_use_active is true filter that the spec’s own burst queries and this PR’s reasoning rely on. That widens the window and can skew counts/recovery/p99.9, creating false positives/negatives relative to the accepted baseline — please add the filter (or update the amended criteria/tests with a justified rationale). Non-blocking notes: restart-local suppression means re-notify after deploys; consider a rearm signal or persistence if noise emerges. Clarify the retry_outcomes counting invariant. Finally, document the new watch and its env in services/reviewer/README.md alongside the other schedulers — docs currently omit this surface and thresholds. With the SQL filter fixed and docs added, the change looks ready to proceed.

Findings

• [BLOCKING] services/reviewer/src/timeout-regime-watch.ts:134 — Reopen-trigger SQL omits tool_use_active is true filter used by the spec’s measurements and reasoning
  In sampleTimeoutRegime the WITH w AS (...) CTE selects from review_timing using only created_at >= ${cutoff} (lines ~170–196) and never filters tool_use_active is true. The task’s prior measurements and the PR body’s “Correction to the spec’s own burst figures” explicitly re-run “under the spec’s own tool_use_active is true filter,” and the original burst query in the spec (Context section) includes this filter. Dropping it widens the window to non-active rows and can shift counts, the recovery denominator, and the p99.9, causing false crossings or missed crossings compared to the accepted baseline. Please add AND tool_use_active is true to the w CTE (or otherwise justify the change and update the amended criteria/tests accordingly).
• [NON-BLOCKING] services/reviewer/src/timeout-regime-watch.ts:373 — Once-per-crossing suppression is process-local; restart re-notifies — consider noting this in structured logs or persisting if flapping becomes noisy
  startTimeoutRegimeWatch tracks alreadyCrossed in-memory (lines ~355–439), which you document in the header as a deliberate ‘Does NOT cover’. That’s fine, but operators will see a fresh alert after any redeploy while a trigger still holds. Consider adding an explicit rearmed: true flag to the first post-boot timeout_regime.trigger_crossed event or designing a persisted suppression table if/when triggers begin to fire frequently. Not blocking — just surfacing the operational consequence.
• [NON-BLOCKING] services/reviewer/src/timeout-regime-watch.ts:258 — unrecovered_events counted via unnest(retry_outcomes) assumes at most one terminal timeout-unrecovered per review
  The query computes unrecovered_events by counting rows in unnest(w.retry_outcomes) where outcome = 'timeout-unrecovered'. If the source array could (pathologically) contain multiple 'timeout-unrecovered' entries for a single review, this would overcount. If the invariant is ‘at most one terminal outcome per review’, consider asserting it in code/tests or switching to a COUNT(DISTINCT ...)-by-review aggregation. Non-blocking because the current schema likely enforces the invariant, but it’s worth making explicit.
• [NON-BLOCKING] services/reviewer/src/server.ts:1978 — New scheduler env surface is undocumented alongside other reviewer schedulers
  TIMEOUT_REGIME_WATCH_ENABLED, TIMEOUT_REGIME_WATCH_INTERVAL_MS, TIMEOUT_REGIME_WATCH_WINDOW_DAYS, TIMEOUT_REGIME_MAX_UNRECOVERED, TIMEOUT_REGIME_MIN_RECOVERY_BP, TIMEOUT_REGIME_MAX_P999_MS, and TIMEOUT_REGIME_WATCH_CAP_MS are introduced and wired, but services/reviewer/README.md’s scheduler sections (Sweeper, PR-watch, Asks-reconcile, Findings-aggregation, Merge-state, Adoption) don’t mention this watch or its env. Consider adding a short “Timeout-regime watch” section mirroring the others so operators know how to enable/tune it. Not blocking, but helps adoption and prevents drift.

Documentation impact

• blocking-needs-update — The PR adds a new operator-facing scheduler (timeout-regime watch) with multiple env vars, enabled via TIMEOUT_REGIME_WATCH_ENABLED and related thresholds. services/reviewer/README.md documents other schedulers and alerting paths but contains no mention of this watch or its env. This is an omission (new surface not documented). Please add a short section mirroring existing scheduler docs (enablement, cadence, env, behavior, non-incident framing).
  Affected: services/reviewer/README.md

[minsky-reviewer]

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

---

Verification round: the prior BLOCKING issue remains unresolved. The SQL in sampleTimeoutRegime still omits the tool_use_active is true filter the spec and PR reasoning depend on, which can skew counts/recovery/p99.9 and lead to false positives/negatives; please add the filter (or justify the change and adjust tests/spec accordingly). Documentation for the new timeout-regime watch and its env vars is also still absent from services/reviewer/README.md. Non-blocking notes: suppression is process-local (expect re-notify after redeploy) and the unrecovered-events count assumes a single terminal outcome per review — consider making that invariant explicit. With the SQL filter added and README updated, the PR looks solid to merge.

Findings

• [BLOCKING] services/reviewer/src/timeout-regime-watch.ts:180 — Reopen-trigger SQL omits tool_use_active is true filter used by the spec’s measurements and rationale
  In sampleTimeoutRegime the WITH w AS (...) CTE selects from review_timing using only created_at >= ${cutoff} and does not filter tool_use_active is true (see services/reviewer/src/timeout-regime-watch.ts:170-201). The task’s reasoning and the PR body’s corrections explicitly re-run “under the spec’s own tool_use_active is true filter,” and the original burst query in the spec (Context section) includes this filter. Dropping it widens the window to non-active rows and can shift counts, the recovery denominator, and the p99.9, creating false positives/negatives relative to the accepted baseline. Please add AND tool_use_active is true to the w CTE (or justify the change and update the amended criteria/tests accordingly).
• [BLOCKING] services/reviewer/README.md:1 — New timeout-regime watch and its env surface are undocumented alongside other reviewer schedulers
  This PR introduces a new operator-facing scheduler (startTimeoutRegimeWatch) with multiple env vars: TIMEOUT_REGIME_WATCH_ENABLED, TIMEOUT_REGIME_WATCH_INTERVAL_MS, TIMEOUT_REGIME_WATCH_WINDOW_DAYS, TIMEOUT_REGIME_MAX_UNRECOVERED, TIMEOUT_REGIME_MIN_RECOVERY_BP, TIMEOUT_REGIME_MAX_P999_MS, and TIMEOUT_REGIME_WATCH_CAP_MS (see services/reviewer/src/server.ts:1953-1963 and services/reviewer/src/timeout-regime-watch.ts). services/reviewer/README.md documents other schedulers and alerting paths but contains no mention of this watch or its env, which is a user-facing omission. Please add a short “Timeout-regime watch” section mirroring the others (enablement, cadence, env, behavior, non-incident framing).
• [NON-BLOCKING] services/reviewer/src/timeout-regime-watch.ts:307 — Once-per-crossing suppression is process-local; restart re-notifies
  runTimeoutRegimeWatchCycle tracks crossed triggers in the in-memory alreadyCrossed set and clears/rebuilds it each cycle. This matches the module doc’s "Does NOT cover" note, but operators will see a fresh alert after any redeploy while a trigger still holds. Consider adding an explicit rearmed: true flag on the first post-boot timeout_regime.trigger_crossed event or persisting suppression if future flapping becomes noisy. Not blocking — surfacing the operational consequence.
• [NON-BLOCKING] services/reviewer/src/timeout-regime-watch.ts:216 — unrecovered_events via unnest(retry_outcomes) assumes at most one 'timeout-unrecovered' per review
  The un CTE counts timeout-unrecovered by unnesting retry_outcomes. If the source array could (pathologically) contain multiple 'timeout-unrecovered' entries for a single review, this would overcount events. If the invariant is “at most one terminal outcome per review,” consider asserting it in code/tests or switching to a per-review aggregation (e.g., COUNT(*) FILTER (...) grouped by review). Non-blocking since the schema likely enforces the invariant.

Spec verification

Criterion	Status	Evidence
SC1′. A scheduled check evaluates all three of mt#4996's reopen triggers against review_timing over a rolling 30 days, and records each trigger's current value alongside its threshold on every cycle — so a reader can see the margin, not only a boolean.	Met	services/reviewer/src/timeout-regime-watch.ts:64-123 define the three triggers and thresholds; evaluateTimeoutRegime computes all three readings. startTimeoutRegimeWatch schedules the check, and runTimeoutRegimeWatchCycle logs every reading each cycle via log.info("timeout_regime.cycle_complete", { readings, sample }) at services/reviewer/src/timeout-regime-watch.ts:296-307. The config default windowDays: 30 is loaded at services/reviewer/src/timeout-regime-watch.ts:111-118.
SC2′. The check surfaces to the operator only when a trigger crosses, at most once per trigger per crossing. No output on a healthy cycle beyond the recorded values.	Met	services/reviewer/src/timeout-regime-watch.ts:309-338 computes notified as crossed minus alreadyCrossed, then emits a single alert per new crossing via alertSink?.notify("warn", ...). Healthy cycles still log readings (cycle_complete) but do not call the sink. Suppression state is tracked in-memory and cleared when the trigger clears (see tests in services/reviewer/src/timeout-regime-watch.test.ts:152-198).
SC4′. The three thresholds and the 30-day window are configurable via strict-positive env parse, matching every other scheduler in this service, and their defaults are the values mt#4996 recorded — not round numbers.	Met	services/reviewer/src/timeout-regime-watch.ts:100-121 implements loadTimeoutRegimeWatchConfig() using parsePositiveIntEnv for interval/window/thresholds. Defaults: windowDays 30, maxUnrecoveredEvents 2, minRecoveryRate from TIMEOUT_REGIME_MIN_RECOVERY_BP default 9500 (95.00%), maxRoundP999Ms 115000. Tests assert defaults and basis-point handling at services/reviewer/src/timeout-regime-watch.test.ts:284-357.
AT1′. Seed a window whose recovery rate is below 95% → the check surfaces exactly once.	Met	Unit tests cover this: services/reviewer/src/timeout-regime-watch.test.ts:83-104 asserts recovery_rate crosses at 92.5%, and services/reviewer/src/timeout-regime-watch.test.ts:152-178 asserts one alert on first cycle and suppression on the next while the crossing persists.
AT2′. Seed a window at the measured baseline (99.25% recovery, p99.9 ≈ 105s, ≤1 unrecovered) → the check surfaces nothing.	Met	services/reviewer/src/timeout-regime-watch.test.ts:55-66 baseline crosses nothing (all three readings present, zero crossings). services/reviewer/src/timeout-regime-watch.test.ts:201-228 asserts that with one unrecovered event and healthy p99.9 no alerts are emitted.
AT4′. Seed a window with 2 unrecovered rows → the check surfaces once, and AT3 still holds: no duplicate alongside reviewer-pre-submit-failure/v1.	Met	services/reviewer/src/timeout-regime-watch.test.ts:71-81 shows unrecovered_count crosses at 2. The suite also asserts that one unrecovered does not cross (AT3 preserved) at services/reviewer/src/timeout-regime-watch.test.ts:67-70 and no duplicate is emitted in the baseline cycle test :201-228.
SC3 (retained): The check must not duplicate mt#4881: an unrecovered timeout already emits an operator ask per occurrence; this check must not emit a second notification for that case.	Met	The implementation only surfaces on aggregate >= 2 unrecovered events (threshold 2; services/reviewer/src/timeout-regime-watch.ts:142-151). Tests assert that one unrecovered event does not cross or surface (services/reviewer/src/timeout-regime-watch.test.ts:67-70, :201-228). No operator-incident ask is minted — the channel is the AlertSink at warn (services/reviewer/src/timeout-regime-watch.ts:327-336).
SC5 (retained): Verified against a real or replayed burst, not only a synthetic one.	Unverifiable	Live verification described in the PR body references production review_timing query results, which are out-of-repo and not present in the diff. The reviewer cannot run those queries. Therefore this criterion's evidence is outside the repository and cannot be verified here.
AT3 (retained). Seed a day containing an unrecovered row → the check does not emit a duplicate notification alongside the reviewer-pre-submit-failure/v1 ask.	Met	Unit tests assert a single unrecovered event does not cross or notify (services/reviewer/src/timeout-regime-watch.test.ts:67-70, :201-228). The implementation surfaces only above the aggregate threshold and uses the non-incident AlertSink, avoiding duplication of mt#4881’s per-occurrence operator-ask channel.

Documentation impact

• blocking-needs-update — The PR introduces a new operator-visible scheduler (timeout-regime watch) with multiple environment variables and an alerting behavior distinct from incident paging (AlertSink.warn, non-incident framing). services/reviewer/README.md documents other schedulers and alert paths in detail but contains no mention of this watch or its env surface. This is a user-facing omission (new surface not documented) and should be added before merge.
  Affected: services/reviewer/README.md

[minsky-reviewer]

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

---

Re-verification against the current diff shows the prior blocking concerns have been addressed. The new timeout-regime watch is implemented as a pure, testable evaluator with a safe in-process scheduler, stateful once-per-crossing suppression, and strict-positive config defaults matching mt#4996. The server wiring follows existing scheduler patterns and ships disabled by default. I found one minor doc nit: the README now contains the "What watches those triggers (mt#4988)" section twice; consider deduplicating. No critical new defects surfaced, and spec criteria SC1′, SC2′, and SC4′ are met; SC5 is unverifiable from the diff alone. Overall, this is solid and ready to merge.

Findings

• [NON-BLOCKING] services/reviewer/README.md:620 — README now contains the "What watches those triggers (mt#4988)" section twice
  The block documenting timeout-regime-watch.ts and its env vars appears twice in services/reviewer/README.md — once added at ~line 620 in this PR and again later under the existing "The 120s model timeout is settled — do not tune it (mt#4996)" section. Duplicated sections can drift independently and confuse readers. Suggest removing one copy and keeping a single canonical section (ideally the later one that sits near the mt#4996 decision context and already existed), or deduplicate by linking to the single section.

Spec verification

Criterion	Status	Evidence
SC1′. A scheduled check evaluates all three of mt#4996's reopen triggers against review_timing over a rolling 30 days, and records each trigger's current value alongside its threshold on every cycle — so a reader can see the margin, not only a boolean.	Met	Implemented in services/reviewer/src/timeout-regime-watch.ts: sampleTimeoutRegime queries a rolling window (param windowDays, default 30) and returns TimeoutRegimeSample (lines ~120–205). evaluateTimeoutRegime computes all three readings (unrecovered_count, recovery_rate, round_p999_ms) with value, threshold, and crossed (lines ~77–119, 150–187). Each cycle logs timeout_regime.cycle_complete with both readings and the full sample, including thresholds, at runTimeoutRegimeWatchCycle (lines ~246–286). Registered scheduler in services/reviewer/src/server.ts (lines ~1870–1893) runs it on a cadence.
SC2′. The check surfaces to the operator only when a trigger crosses, at most once per trigger per crossing. No output on a healthy cycle beyond the recorded values.	Met	runTimeoutRegimeWatchCycle filters crossed = readings.filter((r) => r.crossed) and computes notified only for names not in alreadyCrossed (lines ~267–279). It then updates alreadyCrossed to the current set (lines ~279–281), ensuring one alert per crossing. It emits log.warn("timeout_regime.trigger_crossed", …) and calls alertSink.notify("warn", …) only when notified.length > 0 (lines ~283–307). Healthy cycles always log only timeout_regime.cycle_complete (lines ~260–275).
SC4′. The three thresholds and the 30-day window are configurable via strict-positive env parse, matching every other scheduler in this service, and their defaults are the values mt#4996 recorded — not round numbers.	Met	loadTimeoutRegimeWatchConfig parses env via parsePositiveIntEnv and sets defaults that match the mt#4996 values: windowDays 30, completingRoundCapMs 118000, maxUnrecoveredEvents 2, minRecoveryRate carried as BP (9500 => 0.95), maxRoundP999Ms 115000 (lines ~94–119). These are documented in README (services/reviewer/README.md, section "What watches those triggers (mt#4988)").
SC3 (retained). Must not duplicate mt#4881: an unrecovered timeout already emits an operator ask per occurrence; this check must not emit a second notification for that case.	Met	The unrecovered trigger only crosses at or above the aggregate threshold (default 2) — see evaluateTimeoutRegime (crossed: sample.unrecoveredEvents >= thresholds.maxUnrecoveredEvents, lines ~165–172) with default maxUnrecoveredEvents: 2 (config lines ~108–114). A single unrecovered event (the case mt#4881 already alerts per-occurrence) does not cross and therefore does not emit. Surfacing path is through AlertSink.warn, not the operator-incident path, further reducing risk of duplicated paging (timeout-regime-watch.ts header and lines ~291–307).
SC5 (retained). Verified against a real or replayed burst, not only a synthetic one.	Unverifiable	Live verification evidence is described in the PR body, not in-repo artifacts. This criterion depends on external run results; no live-run logs or fixtures are committed in this diff. Per the review contract, artifacts outside the repo are not verifiable here.

Documentation impact

• no-update-needed — The PR adds a new scheduler and updates documentation in-place: services/reviewer/README.md now documents the timeout-regime watch and its env vars. No existing documented behavior was changed or removed; server wiring matches existing scheduler patterns. I checked README content and saw the new section present. No other docs reference this mechanism.

[minsky-reviewer]

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

---

Verification round focused on the follow-up commit documenting the new scheduler’s env vars and a spot-check of the watchdog implementation and its wiring. Prior BLOCKING items appear addressed: the watch is opt-in, evaluates all three mt#4996 triggers over a 30-day window, records readings each cycle, and notifies only on new crossings with in-process suppression. The added DEPLOY.md section accurately documents all seven TIMEOUT_REGIME_* variables with defaults matching the code, resolving the CI documentation census failure. I found no new critical defects in the touched files. Tests cover evaluator logic, sampling normalization, suppression, and alert-body wording. Given the narrow diff scope (DEPLOY.md plus previously reviewed code), this iteration is APPROVE.

Findings

• [NON-BLOCKING] services/reviewer/DEPLOY.md:200 — Env-var documentation added in DEPLOY.md matches the code’s keys and defaults
  Verified the newly documented TIMEOUT_REGIME_* variables against loadTimeoutRegimeWatchConfig() in services/reviewer/src/timeout-regime-watch.ts. The keys and defaults match exactly:
• TIMEOUT_REGIME_WATCH_ENABLED (default false)
• TIMEOUT_REGIME_WATCH_INTERVAL_MS (default 86400000)
• TIMEOUT_REGIME_WATCH_WINDOW_DAYS (default 30)
• TIMEOUT_REGIME_WATCH_CAP_MS (default 118000)
• TIMEOUT_REGIME_MAX_UNRECOVERED (default 2)
• TIMEOUT_REGIME_MIN_RECOVERY_BP (default 9500 → 0.95)
• TIMEOUT_REGIME_MAX_P999_MS (default 115000)
  This addresses the earlier CI failure described in the commit message. No action needed — noting the verification.

Spec verification

Criterion	Status	Evidence
SC1′. A scheduled check evaluates all three of mt#4996's reopen triggers over a rolling 30 days and records each trigger's current value alongside its threshold on every cycle.	Met	Implementation: evaluateTimeoutRegime() computes unrecovered_count, recovery_rate (nullable on quiet window), and round_p999_ms; sampleTimeoutRegime() queries a 30-day window by default; runTimeoutRegimeWatchCycle() logs timeout_regime.cycle_complete with full readings each cycle. See services/reviewer/src/timeout-regime-watch.ts:72-154, 189-264, 268-297.
SC2′. The check surfaces to the operator only when a trigger crosses, at most once per trigger per crossing.	Met	runTimeoutRegimeWatchCycle() filters crossed, computes notified only for readings not present in alreadyCrossed, and awaits alertSink?.notify('warn', ...) only when notified.length > 0. Suppression state is maintained per-process. See services/reviewer/src/timeout-regime-watch.ts:268-319.
SC4′. Thresholds and 30-day window are configurable via strict-positive env parse; defaults are mt#4996's recorded values.	Met	loadTimeoutRegimeWatchConfig() uses parsePositiveIntEnv for interval, windowDays, cap, and thresholds; minRecovery is basis points divided by 10_000. Defaults: 30d, cap 118000, unrecovered 2, minRecovery 0.95, p999 115000. See services/reviewer/src/timeout-regime-watch.ts:116-147. DEPLOY.md documents all keys (services/reviewer/DEPLOY.md:200+).
AT1′. Seed a window whose recovery rate is below 95% → the check surfaces exactly once.	Met	Unit test runTimeoutRegimeWatchCycle > AT1'/SC2' asserts one alert on first crossing and none while it persists. See services/reviewer/src/timeout-regime-watch.test.ts:107-133.
AT2′. Baseline window (99.25% recovery, p99.9 ≈105s, ≤1 unrecovered) → no surfacing.	Met	evaluateTimeoutRegime > AT2' and runTimeoutRegimeWatchCycle > AT2'/AT3 assert no crossings for baseline-like samples. See services/reviewer/src/timeout-regime-watch.test.ts:40-52 and 158-177.
AT4′. Seed a window with 2 unrecovered rows → surfaces once; AT3 still holds (no duplicate beside mt#4881).	Met	evaluateTimeoutRegime > AT4' asserts crossing at count=2; cycle tests keep unrecovered below threshold when testing SC2. Non-duplication with mt#4881 is enforced by threshold at 2 and separate alert channel. See services/reviewer/src/timeout-regime-watch.ts:138-147 and test at services/reviewer/src/timeout-regime-watch.test.ts:58-65.
SC3/SC5 retained from original: no-duplicate with mt#4881 and verified against a real or replayed burst.	Met	Non-duplication ensured by trigger-1 threshold >=2 and using alert-sink (warn) rather than operator-incident path used by mt#4881; tests assert one-unrecovered path does not surface (AT3). Live verification evidence is described in PR body (replayed 2026-09-04 burst showing crossings for recovery and p999, not for unrecovered). Code supports this; unit tests use injected DB and clock seams. See services/reviewer/src/timeout-regime-watch.test.ts and design notes in services/reviewer/src/timeout-regime-watch.ts header.

Documentation impact

• no-update-needed — This commit only adds env-var documentation for the new scheduler in services/reviewer/DEPLOY.md and code that is disabled by default. No existing documented behavior was changed, and the deployment doc now correctly lists the seven TIMEOUT_REGIME_* vars. I verified the keys/defaults match the code in services/reviewer/src/timeout-regime-watch.ts.

[minsky-reviewer]

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

---

Re-verification focused on the latest commits wiring the timeout-regime watchdog and documenting its env vars. Prior blocking concerns appear addressed: the watch is opt-in, follows the established scheduler pattern, samples a rolling 30-day window, records all readings each cycle, and notifies only on new crossings with in-process suppression. Registration in server.ts is clear and non-invasive. Unit tests cover evaluator math, string-to-number normalization from Postgres, suppression behavior, and alert body wording; we could not independently verify the “live replayed burst” evidence from the repo and recorded that criterion as Unverifiable. I found no new critical defects introduced by this iteration. Documentation updates in DEPLOY.md/README.md match the code’s keys and defaults, and the change does not alter existing documented behavior. Event is APPROVE.

Spec verification

Criterion	Status	Evidence
SC1′. A scheduled check evaluates all three of mt#4996's reopen triggers against review_timing over a rolling 30 days, and records each trigger's current value alongside its threshold on every cycle — so a reader can see the margin, not only a boolean.	Met	Code computes and records all three readings per cycle: see evaluateTimeoutRegime() and runTimeoutRegimeWatchCycle() which log timeout_regime.cycle_complete with readings containing unrecovered_count, recovery_rate, and round_p999_ms. Files: services/reviewer/src/timeout-regime-watch.ts:88-141, 247-297.
SC2′. The check surfaces to the operator only when a trigger crosses, at most once per trigger per crossing. No output on a healthy cycle beyond the recorded values.	Met	runTimeoutRegimeWatchCycle() computes crossed, then derives notified only for names not present in the alreadyCrossed set; it updates the set and only calls alertSink?.notify when notified.length > 0. Files: services/reviewer/src/timeout-regime-watch.ts:268-319.
SC4′. The three thresholds and the 30-day window are configurable via strict-positive env parse, matching every other scheduler in this service, and their defaults are the values mt#4996 recorded — not round numbers.	Met	loadTimeoutRegimeWatchConfig() uses parsePositiveIntEnv for all numeric configs; minRecoveryRate comes from basis points. Defaults: windowDays=30, completingRoundCapMs=118000, maxUnrecoveredEvents=2, minRecoveryRate=0.95, maxRoundP999Ms=115000. Files: services/reviewer/src/timeout-regime-watch.ts:116-147. Also registered in server: services/reviewer/src/server.ts:1959-1970 (end-of-file block).
AT1′. Seed a window whose recovery rate is below 95% → the check surfaces exactly once.	Met	Unit test runTimeoutRegimeWatchCycle > AT1'/SC2' asserts a single alert on first crossing and suppression on subsequent cycles while still crossed. File: services/reviewer/src/timeout-regime-watch.test.ts:107-133.
AT2′. Seed a window at the measured baseline (99.25% recovery, p99.9 ≈ 105s, ≤1 unrecovered) → the check surfaces nothing.	Met	Tests evaluateTimeoutRegime > AT2' and runTimeoutRegimeWatchCycle > AT2'/AT3 assert no crossings and verify readings still recorded. File: services/reviewer/src/timeout-regime-watch.test.ts:40-52, 158-177.
AT4′. Seed a window with 2 unrecovered rows → the check surfaces once, and AT3 still holds: no duplicate alongside reviewer-pre-submit-failure/v1.	Met	evaluateTimeoutRegime > AT4' asserts crossing at unrecoveredEvents=2 and threshold is 2; the cycle tests keep unrecovered below threshold when focusing on suppression behavior. File: services/reviewer/src/timeout-regime-watch.test.ts:58-65.
SC3/SC5 (retained): no-duplicate with mt#4881 and verified against a real or replayed burst, not only a synthetic one.	Unverifiable	Live verification evidence (real or replayed burst) is described in the PR body, not in-repo. The code enforces no-duplication via threshold>=2 and a distinct alert channel, and unit tests assert the one-unrecovered path does not surface (services/reviewer/src/timeout-regime-watch.test.ts:45-52, 158-177), but external replay evidence cannot be verified from the diff alone.

Documentation impact

• no-update-needed — This PR adds a new watchdog (disabled by default) and registers it in server.ts, and updates DEPLOY.md/README.md with environment variables and behavior notes for the watch. No existing documented behavior was changed or invalidated; docs were updated in this PR to cover the new TIMEOUT_REGIME_* variables.