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

## 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.com/claude-code)

https://claude.ai/code/session_01YT3CvxJhrcGeVCv8P3DGPD

Co-Authored-By: minsky-ai[bot] <minsky-ai[bot]@users.noreply.github.com>

Author: edobry

services/reviewer/DEPLOY.md

@@ -200,6 +200,62 @@ service start. If you see `merge_state_sweeper.missing_credentials` or
 `merge_state_sweeper.disabled` instead, the recovery layer is NOT active and
 bypass-merge PRs will not auto-sync.
 
+#### Timeout-regime watch (mt#4988)
+
+Watches the three conditions under which mt#4996's accept of the 120s toolloop-timeout cadence
+should be REOPENED. Reads `review_timing` only — no Railway credentials, no outbound call — and
+notifies through the configured alert sink at `warn` when one crosses. **It is not an incident
+channel and does not page:** a crossed trigger means a documented baseline moved, not that the
+reviewer is down.
+
+**Ships disabled.** Nothing starts unless you set the first variable, so a deploy that carries this
+code is a runtime no-op until you opt in.
+
+```bash
+# OPTIONAL — off by default. Set to "true" to start the watch.
+# railway variable set TIMEOUT_REGIME_WATCH_ENABLED=true
+
+# OPTIONAL — cycle cadence. Default 86400000 (24h). The triggers are 30-day
+# aggregates; they do not move hourly, so a shorter interval buys nothing.
+# railway variable set TIMEOUT_REGIME_WATCH_INTERVAL_MS=86400000
+
+# OPTIONAL — rolling window, in days. Default 30 — the window mt#4996 stated its
+# triggers over. Shortening it makes a single bad day able to reopen a decision.
+# railway variable set TIMEOUT_REGIME_WATCH_WINDOW_DAYS=30
+
+# OPTIONAL — completing-round cutoff, in ms. Default 118000. A round recorded at
+# or above this was censored by the 120s timeout rather than measured, so it is
+# excluded from the p99.9 (mem#1373). Raising it past the cap reintroduces the
+# censored-percentile error mt#1897 made three times.
+# railway variable set TIMEOUT_REGIME_WATCH_CAP_MS=118000
+
+# OPTIONAL — trigger 1. Crossed at or above this many unrecovered timeout events
+# in the window. Default 2; baseline is 1 in 103 days. Set to 1 and it will
+# duplicate the per-occurrence ask reviewer-pre-submit-failure/v1 already sends.
+# railway variable set TIMEOUT_REGIME_MAX_UNRECOVERED=2
+
+# OPTIONAL — trigger 2. Crossed BELOW this event-level recovery rate, in basis
+# points (9500 = 95.00%). Default 9500; baseline is 99.25%. Basis points because
+# the shared strict-positive parser is integer-only.
+# railway variable set TIMEOUT_REGIME_MIN_RECOVERY_BP=9500
+
+# OPTIONAL — trigger 3. Crossed ABOVE this completing-round p99.9, in ms.
+# Default 115000; baseline is 105000. This is the one condition that would mean
+# the 120s cap has become tight against real work.
+# railway variable set TIMEOUT_REGIME_MAX_P999_MS=115000
+```
+
+Verify activation after deploy: tail the logs and confirm `{"event":"timeout_regime.enabled", ...}`
+shortly after service start. `timeout_regime.disabled` means it is off;
+`timeout_regime.missing_db` means it was enabled but no DB is configured. Each cycle emits
+`timeout_regime.cycle_complete` carrying every reading against its threshold, so the margin is
+visible without waiting for a crossing.
+
+**Known bound:** the once-per-crossing suppression is in-process, so a redeploy re-arms it and a
+still-crossed trigger notifies again on the next cycle. `timeout_regime.trigger_crossed` carries
+`suppressionScope: "process-local"` so a repeat can be correlated against a restart rather than read
+as flapping.
+
 #### Reliability budget (mt#1810)
 
 The recovery layer carries a measurable reliability target so the next drift instance

services/reviewer/README.md

@@ -620,6 +620,34 @@ condition that would make the cap genuinely tight against real work. The unrecov
 on its own via `reviewer-pre-submit-failure/v1` (mt#4881). Full reasoning and the queries:
 mt#4996 `## DECISION 2026-09-05`.
 
+### What watches those triggers (mt#4988)
+
+`timeout-regime-watch.ts` evaluates all three of the triggers above on a schedule, so the accept is
+not decide-and-forget. It reads `review_timing` only — no Railway credentials, no external call —
+records every reading against its threshold each cycle (`timeout_regime.cycle_complete`), and
+notifies through the alert sink at `warn` only when one crosses, at most once per trigger per
+crossing.
+
+**It is not an incident channel and deliberately does not page.** A crossed trigger means a
+documented baseline moved, not that the reviewer is down, and the remedy is to reopen mt#4996's
+analysis. The alert body says so in as many words.
+
+| Variable                           | Default          | What it does                                                                                                       |
+| ---------------------------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------ |
+| `TIMEOUT_REGIME_WATCH_ENABLED`     | `false`          | Opt-in, matching the other non-urgent schedulers. Set `true` to start it.                                          |
+| `TIMEOUT_REGIME_WATCH_INTERVAL_MS` | `86400000` (24h) | Cycle cadence. The triggers are 30-day aggregates; they do not move hourly.                                        |
+| `TIMEOUT_REGIME_WATCH_WINDOW_DAYS` | `30`             | The rolling window mt#4996 stated its triggers over.                                                               |
+| `TIMEOUT_REGIME_WATCH_CAP_MS`      | `118000`         | Rounds at or above this are censored at the timeout cap, not measured, and are excluded from the p99.9 (mem#1373). |
+| `TIMEOUT_REGIME_MAX_UNRECOVERED`   | `2`              | Crossed at or above this many unrecovered events. Baseline: 1 in 103 days.                                         |
+| `TIMEOUT_REGIME_MIN_RECOVERY_BP`   | `9500`           | Crossed below this event-level recovery rate, in basis points (9500 = 95.00%). Baseline: 99.25%.                   |
+| `TIMEOUT_REGIME_MAX_P999_MS`       | `115000`         | Crossed above this completing-round p99.9. Baseline: 105.0s.                                                       |
+
+Every default is a value mt#4996 measured rather than a round number, per
+`decision-defaults.mdc §Thresholds`. **Known bound:** the once-per-crossing suppression is
+in-process, so a redeploy re-arms it and a still-crossed trigger notifies again on the next cycle.
+The `timeout_regime.trigger_crossed` log line carries `suppressionScope: "process-local"` so a
+repeat can be correlated against a restart rather than read as flapping.
+
 ## Running a model A/B on production traffic (mt#4569)
 
 `REVIEWER_EXPERIMENT_MODEL` runs a second model alongside the incumbent so the two can be compared. **Unset — the default — no experiment runs and every PR uses `REVIEWER_MODEL`.**

services/reviewer/src/server.ts

@@ -39,6 +39,7 @@ import { safeTruncate } from "@minsky/shared/safe-truncate";
 import { callMcp } from "./mcp-client";
 import { loadMergeStateSweeperConfig, startMergeStateSweeper } from "./merge-state-sweeper";
 import { loadAdoptionSweeperConfig, startAdoptionSweeper } from "./adoption-sweeper";
+import { loadTimeoutRegimeWatchConfig, startTimeoutRegimeWatch } from "./timeout-regime-watch";
 import { getDb, type ReviewerDb } from "./db/client";
 import { applyMigrations } from "./db/migrate";
 import { recoverPendingReviews, loadBootRecoveryConfig } from "./boot-recovery";
@@ -1952,6 +1953,18 @@ if (import.meta.main) {
   // Disabled by default; set ADOPTION_SWEEPER_ENABLED=true to activate.
   startAdoptionSweeper(config, loadAdoptionSweeperConfig());
 
+  // Start the timeout-regime watch (mt#4988).
+  // mt#4996 accepted the 120s toolloop-timeout cadence on a measured baseline
+  // and recorded three conditions under which that question should be REOPENED.
+  // Nothing evaluated them; this does, daily, over a 30-day window of
+  // review_timing. It notifies through the alert sink at `warn` rather than
+  // minting an operator incident — a drifted baseline is not an outage and is
+  // not operator-only to resolve.
+  // Configurable via TIMEOUT_REGIME_WATCH_ENABLED, TIMEOUT_REGIME_WATCH_INTERVAL_MS,
+  // TIMEOUT_REGIME_WATCH_WINDOW_DAYS, and the three threshold vars.
+  // Opt-in: disabled by default; set TIMEOUT_REGIME_WATCH_ENABLED=true to activate.
+  startTimeoutRegimeWatch(db, loadTimeoutRegimeWatchConfig(), alertSink);
+
   // Start the webhook-event retention pruner (mt#1372).
   // Deletes reviewer_webhook_events rows older than MINSKY_REVIEWER_WEBHOOK_EVENT_RETENTION_DAYS
   // (default: 90 days). Runs once every 24 hours. The first prune fires after