Skip to content

feat(mt#2719): Reviewer operator-incident paging tier — reach the page, not just the marker - #3564

Merged
edobry merged 4 commits into
mainfrom
task/mt-2719
Sep 2, 2026
Merged

edobry merged 4 commits into
mainfrom
task/mt-2719

Conversation

@minsky-ai

@minsky-ai minsky-ai Bot commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The reviewer could already record that it was structurally unable to review; it could not tell
anyone. mt#4881 shipped the classified failure stream and an inbox ask. This adds the tier above
it: the two conditions only the operator can clear — sustained GitHub App auth failure, and
sustained provider credit exhaustion — now create a severity: "incident" + forceImmediate ask
and page the principal, with the remediation URL in the notification body.

Originating incident: mt#3433 — ~4h of reviewer downtime the principal found by reading chat
scroll, because remediation (add credits) was operator-only by construction.

The defect this had to work around

Setting severity: "incident" on the reviewer's emit path would have paged nobody.

  • CreateAskInput carries severity and forceImmediate (packages/domain/src/ask/repository.ts:262,270), so the naive change typechecks and looks complete.
  • The page is fired by pagePrincipalForAsk. A repo-wide grep finds exactly one non-test importer: src/adapters/shared/commands/asks.ts.
  • That call sits inside createAsk, after persistRouteOutcome — not in repo.create. Neither create implementation calls anything paging-related.
  • The reviewer's DomainAskEmitter calls repo.create directly and never calls createAsk, by design.

So the marker would have been written to a row no paging code reads: typechecks, deploys, returns
healthy, inert in production. The mt#2435 shape.

Two findings that shaped the fix: the reviewer's original reason for bypassing createAsk is
itself stale (mt#3491 made an explicit routingTarget: "operator" win over the kind→target
default), and mt#3851 already forces operator routing for any severity: "incident" ask. But
createAsk lives in the adapter layer, which the reviewer does not import — hence the extraction
rather than a call.

Key changes

  • packages/domain/src/ask/principal-page-dispatch.ts (new) — the production page dispatch, moved out of asks.ts so both producers reach one seam. A pure relocation: every collaborator (notifyPrincipal, resolvePersistenceProvider, emitSystemEventFromProvider, pagePrincipalForAsk) was already domain-side. createAsk's behaviour is unchanged — it calls the extracted function exactly where it called its private copy.
  • AskEmitter.emitOperatorIncidentAlert — one method, one discriminated OperatorIncidentContext, for both sources. See the SC1 amendment in the spec for why this replaced the spec's named emitAuthHealthAlert plus a near-identical sibling.
  • auth-health.ts — pages on trip, deduped by the tracker's existing tripped flag (no new dedup state). Additive to the mt#2717 alert sink; the three surfaces degrade independently.
  • failure-alert.ts — escalates an operator-actionable class on the threshold crossing, evaluated before mt#4881's per-PR suppression so a single-PR outage still reaches the count that proves it sustained. provider_unavailable / provider_timeout never page — they self-heal.
  • services/reviewer/scripts/smoke-operator-incident-page.ts (new) — dry by default; --execute persists a real ask and sends a real page.

Thresholds reuse mt#4881's 60-minute window and auth-health's count of 3 rather than minting a
third number for the same judgment (SC8).

Spec criteria

Criterion Evidence
SC1 (amended) emitOperatorIncidentAlert + DomainAskEmitter impl; deviation recorded in the spec
SC2 configureGithubAuthHealthAskEmitter called at boot, server.ts
SC3 one-shot via the tracker's tripped flag; fail-open on no-repo / throwing repo
SC4 13 emitter tests, 2 auth-health tests, 9 escalation tests, 7 dispatch tests
SC5 consumes mt#4881's ReviewFailureClass; no second scan over review_error
SC6 AT3 + the live smoke below
SC7 AT5 + the regression test on the rendered page body
SC8 PROVIDER_ESCALATION_THRESHOLD derivation in its docblock

Testing

Execution evidence:

$ bun test --preload ./tests/setup.ts packages/domain/src/ask/principal-page-dispatch.test.ts
 7 pass / 0 fail / 12 expect() calls — Ran 7 tests across 1 file. [197.00ms]

$ cd services/reviewer && bun test --preload ../../tests/setup.ts src/ask-emitter.test.ts
 13 pass / 0 fail  (AT3, AT5, and the page-body regression)

$ cd services/reviewer && bun test --preload ../../tests/setup.ts src/failure-alert.test.ts
 47 pass / 0 fail / 121 expect() calls  (AT4 + 9 new escalation cases)

$ cd services/reviewer && bun test --preload ../../tests/setup.ts src/auth-health.test.ts
 18 pass / 0 fail  (AT1, AT2)

$ cd services/reviewer && bun test --preload ../../tests/setup.ts
 2446 pass / 0 fail / 5369 expect() calls — Ran 2446 tests across 92 files. [9.40s]

$ bun scripts/run-related-tests.ts src/adapters/shared/commands/asks.ts packages/domain/src/ask/principal-page-dispatch.ts
 685 pass / 0 fail — Ran 685 tests across 38 files. [22.53s]
 (includes asks.severity-page.test.ts, which covers the block that moved)

AT1/AT2 — auth-health trip with an emitter creates exactly one operator-routed ask; with no
emitter wired it still logs and does not throw. AT3 — see the negative control below. AT4 — a
sustained provider_credits_exhausted run crosses the threshold and emits exactly one incident;
a provider_unavailable run 8 long emits none. AT5 — the remediation URL is asserted by substring,
in the rendered page rather than only the ask.

Negative control — AT3, the dispatch is what pages:

Reverted the await dispatchPrincipalPage(repo, ask, this.pageDeps) call and re-ran:

Expected length: 1
Received length: 0
(fail) DomainAskEmitter.emitOperatorIncidentAlert (mt#2719) > actually pages — the assertion the whole task turns on
 11 pass / 1 fail

The other 11 still passed, which is the defect in miniature: the ask is created, correctly marked
severity: "incident", routingTarget: "operator" — and nothing pages.

Negative control — the escalation must precede the suppression:

Moved the escalation after mt#4881's duplicate-suppression return and re-ran:

(fail) recordReviewFailure operator escalation (mt#2719) > escalates even when the ordinary alert is suppressed as a duplicate
 46 pass / 1 fail

Both files were restored and verified byte-identical before committing.

Live verification

The dry smoke, run against the real production Telegram credentials (read into shell variables
from the linked Railway project, never printed):

$ TELEGRAM_BOT_TOKEN=… TELEGRAM_CHAT_ID=… bun services/reviewer/scripts/smoke-operator-incident-page.ts
channel resolved: configured via env
PASS (dry): emitter → repo.create → dispatchPrincipalPage → send reached
  ask.severity=incident routingTarget=operator
  page title: Incident — needs you

configured via env confirms the resolution order the design depends on — resolvePrincipalChannel
reads TELEGRAM_* before falling back to Pulumi, which a container cannot do.

This run found a real defect that every unit test missed. buildPageMessage excerpts the ask's
question at 300 chars; the remediation URL sat at the end of the body and was cut from the
notification the principal actually reads, while remaining present in the ask. SC7 was silently
untrue. Fixed by leading with the remediation (not by widening the shared excerpt, which bounds
every page and is not one caller's to move), plus a regression test that asserts the rendered page
body rather than the ask.

--execute is UNVERIFIED, deliberately, and its risky part is exercised. A full run sends a
real notification to the principal's phone, consumes one of the substrate's 3-per-24h page budget
(principal-page.ts PAGE_RATE_LIMIT_MAX), and — since R1 below — writes a real ask row. That is
an outward-facing action I have not taken unilaterally; it needs the operator's go-ahead. Per
§7a's dual-mode rule the branch's own imports must still resolve at runtime (the mt#2760 class),
so that was exercised directly and bounded:

$ bun -e 'import "reflect-metadata"; …'
factory.resolvePersistenceProvider: function
repository.DrizzleAskRepository: function
listByClassifierVersion on prototype: function

scripts/verify-ask-principal-page.ts (mt#3595) already proves the final transport leg end-to-end.

Review rounds

R1 — BLOCKING, --execute used FakeAskRepository while announcing a real ask. Correct
finding, and a pointed one: it is this task's own defect class — a path reporting success for work
it did not do — reproduced inside the script written to catch that class. Fixed by changing the
behaviour rather than the wording (be53c506e): --execute now resolves the real persistence
provider, builds a DrizzleAskRepository, and reads the row BACK before claiming success,
asserting the persisted severity and principalPagedAt instead of trusting the emit's return
value. It skips cleanly when no provider or connection is available. This also closed the gap the
reviewer named — with the fake repo the mode exercised neither the insert nor the
severity/forceImmediate columns, so it could not have caught a persistence-layer rejection.

External preconditions

Verified provisioned, no new provisioning required. TELEGRAM_CHAT_ID, TELEGRAM_BOT_TOKEN
present and ALERT_SINK_TYPE=telegram on the production minsky-reviewer Railway project
(key names projected, values never rendered). Note infra/index.ts:338-345 declares these
conditionally as a per-stack opt-in — a stack without reviewer-telegram-chat-id degrades to a
logged PageDecisionReason rather than failing loudly, which is correct but means "the page fired"
must never be inferred from "the ask was created."

Deploy verification: this PR touches services/reviewer/src, packages/domain and
src/adapters/shared/commands — all deploy surface per isDeploySurfaceFile (checked with the
predicate, not from memory). After merge I will run deployment_wait-for-latest for the reviewer
service with notBefore set to the merge timestamp and expectCommitSha set to the merge commit,
and read buildIdentity rather than treating SUCCESS alone as proof.

Parallel work

Open PR #3412 (mt#4639, IN-REVIEW) touches ask-emitter.ts, auth-health.ts and server.ts — a
mechanical err.message → getLoggableErrorSummary sweep, established from its actual changed-file
list. It does not touch failure-alert.ts, principal-page.ts or ask/repository.ts, where the
substantive work here lives. Overlap is additive; a watch is armed on #3412.

…e, not just the marker

The reviewer could already record that it was structurally unable to review;
it could not tell anyone. mt#4881 shipped the classified failure stream and an
inbox ask. This adds the tier above it: the two conditions only the operator can
clear — sustained GitHub App auth failure, and sustained provider credit
exhaustion — now create a `severity: "incident"` + `forceImmediate` ask AND page
the principal, with the remediation URL in the body.

The defect this had to work around: setting `severity: "incident"` on the
reviewer's emit path would have paged nobody. `pagePrincipalForAsk`'s only
production caller was `createAsk` in the command adapter, and the reviewer's
emitter deliberately writes through `repo.create` instead — so the marker would
have persisted to a row nothing reads. Typechecks, deploys, healthy, inert.

- Extract the production page dispatch to `@minsky/domain/ask/principal-page-dispatch`
  so both `createAsk` and the reviewer reach one seam. Pure relocation: every
  collaborator was already domain-side, and `createAsk`'s behaviour is unchanged.
- `AskEmitter.emitOperatorIncidentAlert` — one method, discriminated context, for
  both sources (they produce the same ask shape; see the spec's SC1 amendment).
- `auth-health.ts` pages on trip, deduped by the tracker's existing `tripped` flag.
- `failure-alert.ts` escalates an operator-actionable class on the threshold
  CROSSING, evaluated before the per-PR suppression so a single-PR outage still
  reaches the count that proves it sustained. Self-healing classes never page.

Thresholds reuse mt#4881's 60-minute window and auth-health's count of 3 rather
than minting a third number for the same judgment.

Deploy impact: touches services/reviewer/src and packages/domain — deploy
surface per isDeploySurfaceFile.
…cated it away

Ships the live smoke for the reviewer's paging composition, and fixes the defect
that smoke immediately found.

`buildPageMessage` excerpts an ask's question at PAGE_QUESTION_EXCERPT_CHARS
(300). The remediation URL sat at the END of the incident body, so it was cut
from the notification the principal actually reads on a phone — while remaining
present in the ask. SC7 asks the operator to be able to act from the
notification without investigating; that was silently not true.

The unit tests could not see it: they asserted the URL was in `ask.question`,
which was true and was not the requirement. Fixed by leading with the
remediation rather than by widening the shared excerpt, which is a deliberate
bound on every page and not one caller's to move. Added a regression test that
asserts the rendered PAGE body instead of the ask.

- `services/reviewer/scripts/smoke-operator-incident-page.ts` — dry by default
  (resolves the real channel, drives the real emitter, records instead of
  sending); `--execute` sends a real page and is opt-in because it spends one of
  the substrate's 3-per-24h page budget.

Deploy impact: touches services/reviewer/src — deploy surface.
@minsky-ai minsky-ai Bot added the authorship/co-authored Co-authored by human and AI agent label Sep 2, 2026
@minsky-reviewer

minsky-reviewer Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

Minsky Reviewer Status

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

Commands

  • /review — request a fresh review

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


Solid structural move extracting the principal-page dispatch into the domain and correctly wiring the reviewer’s emit path to reach it. Tests exercise the negative control and the decision-path well. One blocking issue: the live-smoke --execute path claims to create “a real ask” but still uses FakeAskRepository, so no persistence is exercised; this is misleading and leaves the repo-backed seam unverified. Either use a real DrizzleAskRepository from the domain container in --execute or soften the claim and checks to reflect that only the page send is real. I limited my sweep to the changed files; broader docs and other modules were not re-read. With the execute-mode fix, this looks ready.

Findings

  • [BLOCKING] services/reviewer/scripts/smoke-operator-incident-page.ts:116 — --execute mode claims to create a real Ask but uses FakeAskRepository — misleading live verification and no persistence exercised
    In services/reviewer/scripts/smoke-operator-incident-page.ts:116-132, the --execute path constructs const repo = new FakeAskRepository(); and then const emitter = new DomainAskEmitter(() => Promise.resolve(repo));. The banner prints “this WILL create a real ask and notify the principal’s phone,” and later asserts ask.principalPagedAt, but the repository is an in-memory fake — no real Ask row is created or persisted, and no database/persistence wiring is exercised. This produces a misleading success message and does not verify the repo-backed dispatch seam (only the send). Fix one of:
  • Use a real DrizzleAskRepository from the domain container/persistence provider in --execute so a genuine Ask row is created and the substrate claim is written; or
  • Amend --execute wording and checks to stop asserting a “real ask” was created when it was not, and make clear it only sends a real page without persistence. As written, operators will read a stronger guarantee than the code provides, and the path under test is materially different from production (no DB).

Documentation impact

  • no-update-needed — Internal plumbing changes: extracted a domain-level paging dispatch and wired reviewer emitters to call it. No new public CLI or MCP surface was added, and existing shared commands retain behavior (asks.create still pages via the extracted dispatch). The user-facing docs likely do not describe internal reviewer paging internals; no docs were updated in this PR and no externally documented behavior appears to have changed.

…s it claimed

The reviewer was right and the finding was correctly BLOCKING: `--execute`
announced "this WILL create a real ask" while running against
`FakeAskRepository`. That is the exact defect class this task is about — a path
reporting success for work it did not do — reproduced inside the script written
to catch it.

Fixed by changing the BEHAVIOR, not the wording: `--execute` now resolves the
real persistence provider, builds a `DrizzleAskRepository`, and reads the row
BACK before claiming success, asserting the persisted `severity` and
`principalPagedAt` rather than trusting the emit's return value. It skips
cleanly when no provider or connection is available.

This also closes a real gap the reviewer named: with the fake repo the mode
exercised neither the insert nor the `severity`/`forceImmediate` columns, so it
could not have caught a persistence-layer rejection — one of the two things the
smoke exists for.

Deploy impact: none — script-only change under services/reviewer/scripts.

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


R2 verification: The R1 BLOCKING finding on the smoke script’s --execute path was correctly addressed — it now resolves the real persistence provider and repository, persists a real ask, and verifies principalPagedAt. The paging seam was cleanly extracted to the domain and adopted by both createAsk and the reviewer emitter. Auth-health wiring, provider escalation logic, and paging dispatch behave fail-open and match the spec; tests and the smoke script cover the page actually firing and the remediation link on the rendered page. I found no new critical defects introduced by the fix. Approving this PR.

Spec verification

Criterion Status Evidence
A new AskEmitter.emitOperatorIncidentAlert(ctx) method (+ DomainAskEmitter impl) creates an operator-routed (routingTarget: "operator") Ask when the auth-health tracker trips.

AMENDED at implementation, 2026-09-02 — shipped as emitOperatorIncidentAlert(ctx).
One method with a discriminated OperatorIncidentContext (source: "github_auth" \| "provider") rather than this criterion's name plus a near-identical sibling for the
provider half. The rationale is this spec's own 2026-07-31 extension, which says the two
conditions "are both 'the reviewer is structurally unable to review, and only the operator
can fix it'": they produce the same ask shape, the same severity: "incident" marker and
the same page, differing only in prose and remediation URL. Two methods would have been
duplication with two places to forget the page. Everything this criterion actually requires
— a new interface method, a DomainAskEmitter impl, routingTarget: "operator", firing on
the tracker trip — is satisfied; only the name and arity changed. | Met | services/reviewer/src/ask-emitter.ts:120-186 (type OperatorIncidentContext) and :330-343 (AskEmitter includes emitOperatorIncidentAlert). Implementation at :594-671 sets routingTarget: "operator" and severity: "incident". Wiring from tracker at services/reviewer/src/auth-health.ts:177-214 calls emitOperatorIncidentAlert on trip. |
| The global githubAuthHealth singleton is configured with the ask-emitter at reviewer server boot (alongside the existing configureGithubAuthHealthAlertSink call), reusing the booted domain container (makeContainerAskRepoProvider). | Met | services/reviewer/src/server.ts:1456-1470 — configureGithubAuthHealthAlertSink(alertSink) and configureGithubAuthHealthAskEmitter(new DomainAskEmitter(makeContainerAskRepoProvider(domainServices.container))) at boot. |
| One-shot dedup: at most one Ask per trip (re-armed after recovery), mirroring the circuit-breaker alerted semantics; fail-open (no container/DB → skip, never crash the tracker callback). | Met | services/reviewer/src/auth-health.ts:146-176 — tracker uses tripped flag to fire once per trip; on missing emitter it logs and continues. services/reviewer/src/ask-emitter.ts:612-640 returns "skipped" when no repo; try/catch prevents throws (fail-open). |
| Unit tests for the new emitter method and the trip→Ask wiring. | Met | services/reviewer/src/ask-emitter.test.ts:72-187 — tests for emitOperatorIncidentAlert including routing, paging, remediation link; also regression asserting rendered page contains remediation URL. PR description lists passing test runs. |
| SC5 — provider-failure detection consumes mt#4881's classified stream; fires only on operator-actionable classes (credits exhaustion), not on self-healing classes. | Met | services/reviewer/src/failure-alert.ts:74-124 defines OPERATOR_ACTIONABLE_CLASSES = {"provider_credits_exhausted"}. Escalation gate shouldEscalateToOperator(...) at :523-535 only pages when class is operator-actionable and threshold crossing; unavailable/timeout are excluded. |
| SC6 — the page actually fires for an Ask emitted by this path with severity: "incident", or logs a structured reason if not. | Met | packages/domain/src/ask/principal-page-dispatch.ts:86-135 builds production page deps; dispatchPrincipalPage at :160-198 calls pagePrincipalForAsk and logs rate-limit suppression. services/reviewer/src/ask-emitter.ts:659-666 invokes dispatchPrincipalPage after repo.create. Live smoke script services/reviewer/scripts/smoke-operator-incident-page.ts executes dry path and --execute path verifying principalPagedAt (lines ~112-175). |
| SC7 — remediation link included in Ask body for both sources (provider billing URL and GitHub App settings). | Met | services/reviewer/src/ask-emitter.ts:700-742 buildOperatorIncidentQuestion leads with Remediation: ${ctx.remediationUrl}; constants at :76-105 include GITHUB_APP_SETTINGS_URL and per-provider URLs. Unit tests assert page body contains the URL: services/reviewer/src/ask-emitter.test.ts:191-223. |
| SC8 — thresholds derived, not picked; reuse existing 60-minute window and auth-health threshold of 3 where answering the same question. | Met | services/reviewer/src/failure-alert.ts:28-73 SUPPRESSION_WINDOW_MS comment derives 60min; :99-135 PROVIDER_ESCALATION_THRESHOLD = 3 with rationale; services/reviewer/src/auth-health.ts:126-143 DEFAULT_AUTH_HEALTH_THRESHOLD parses env with default 3, with justification in header docs. |

Adoption sweep

Symbol Kind Consumers found Classification Notes
@minsky/domain/ask/principal-page-dispatch.dispatchPrincipalPage function src/adapters/shared/commands/asks.ts:66 — imported and called within createAsk paging block, services/reviewer/src/ask-emitter.ts:659 — dynamically imported and invoked after repo.create for incident asks Adopted Paging seam extracted to domain and used by both createAsk and DomainAskEmitter per spec; avoids duplication of page decision.

Documentation impact

  • no-update-needed — This PR adds internal paging wiring and a smoke script; it does not change public CLI/MCP command surfaces or documented user workflows. The movement of the paging seam into the domain preserves createAsk behavior (asks.ts header notes unchanged behavior). No docs under docs/ were modified, and behavior visible to users (operators) remains consistent except for incident pages actually firing, which is an internal reliability fix.

@edobry
edobry merged commit 15305c0 into main Sep 2, 2026
20 checks passed
@edobry
edobry deleted the task/mt-2719 branch September 2, 2026 08:31

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


R3 verification: The prior BLOCKING finding on the smoke script’s --execute path was addressed — it now uses the real persistence provider/repository and verifies a real page via principalPagedAt. The paging seam was cleanly extracted to the domain (principal-page-dispatch), adopted both by createAsk and the reviewer emitter, and auth-health wiring at server boot is present. Provider escalation logic pages only on the threshold crossing for operator-actionable classes and precedes mt#4881 suppression as required. Tests cover the new paths and a live dry smoke validates channel resolution and page body (including remediation URL). I find no new critical defects introduced by these fixes. Approving this PR.

Spec verification

Criterion Status Evidence
A new AskEmitter.emitOperatorIncidentAlert(ctx) method (+ DomainAskEmitter impl) creates an operator-routed (routingTarget: "operator") Ask when the auth-health tracker trips.

AMENDED at implementation, 2026-09-02 — shipped as emitOperatorIncidentAlert(ctx)... | Met | services/reviewer/src/ask-emitter.ts:282-451 — adds emitOperatorIncidentAlert(ctx: OperatorIncidentContext) on DomainAskEmitter, sets routingTarget: "operator", severity: "incident", forceImmediate: true; services/reviewer/src/auth-health.ts:179-212,244-268 — wires the global tracker to call emitOperatorIncidentAlert on trip. |
| The global githubAuthHealth singleton is configured with the ask-emitter at reviewer server boot (alongside the existing configureGithubAuthHealthAlertSink call), reusing the booted domain container (makeContainerAskRepoProvider). | Met | services/reviewer/src/server.ts:1738-1752 — calls configureGithubAuthHealthAskEmitter(new DomainAskEmitter(makeContainerAskRepoProvider(domainServices.container))) immediately after wiring the alert sink; comment cites single-instance convention. |
| One-shot dedup: at most one Ask per trip (re-armed after recovery), mirroring the circuit-breaker alerted semantics; fail-open (no container/DB → skip, never crash the tracker callback). | Met | services/reviewer/src/auth-health.ts:244-268 — emits inside onTrip (fires once per trip), and wraps in Promise.resolve(...).catch(...) to be fail-open; services/reviewer/src/ask-emitter.ts:320-338 — logs and returns "skipped" when no repo, returns "failed" on error without throwing. |
| Unit tests for the new emitter method and the trip→Ask wiring. | Met | services/reviewer/src/ask-emitter.test.ts:156-333 — tests emitOperatorIncidentAlert including page dispatch and body; services/reviewer/src/auth-health.test.ts:201-268 — tests trip triggers exactly one operator incident and degrades cleanly when no emitter wired. |
| SC5 — provider-failure detection consumes mt#4881's classified stream and fires only on operator-actionable classes (provider_credits_exhausted), NOT on self-healing ones. | Met | services/reviewer/src/failure-alert.ts:566-614 — escalation runs before suppression and uses shouldEscalateToOperator; services/reviewer/src/failure-alert.ts:480-506 — pure shouldEscalateToOperator with set OPERATOR_ACTIONABLE_CLASSES={"provider_credits_exhausted"}; services/reviewer/src/failure-alert.test.ts:620-739 — asserts credits pages and provider_unavailable/provider_timeout do not. |
| SC6 — the page actually fires: an Ask with severity: "incident" reaches pagePrincipalForAsk and records a principalPagedAt (or a logged reason otherwise). | Met | packages/domain/src/ask/principal-page-dispatch.ts:129-172 — dispatchPrincipalPage calls pagePrincipalForAsk and logs rate-limit; services/reviewer/src/ask-emitter.ts:420-451 — calls dispatchPrincipalPage after repo.create; services/reviewer/scripts/smoke-operator-incident-page.ts:133-177 — --execute persists via DrizzleAskRepository and verifies principalPagedAt was written. |
| SC7 — remediation link included: provider billing URL for credits-exhaustion, GitHub App settings for auth-health, asserted on the notification body. | Met | services/reviewer/src/ask-emitter.ts:76-116,446-451,494-552 — defines GITHUB_APP_SETTINGS_URL, providerBillingUrl, and composes question leading with remediation URL; services/reviewer/src/ask-emitter.test.ts:208-247,270-333 — asserts remediation URL present in ask question and rendered page body; services/reviewer/scripts/smoke-operator-incident-page.ts:104-132 — dry-run asserts the page body includes the remediation URL. |
| SC8 — thresholds derived, not picked; reuse mt#4881's 60-minute window and a count of 3 rather than minting new numbers. | Met | services/reviewer/src/failure-alert.ts:85-116,420-478 — documents and sets PROVIDER_ESCALATION_THRESHOLD = 3, reusing SUPPRESSION_WINDOW_MS = 60min; tests at services/reviewer/src/failure-alert.test.ts:636-739 assert crossing behavior and window minutes carried. |

Adoption sweep

Symbol Kind Consumers found Classification Notes
@minsky/domain/ask/principal-page-dispatch.dispatchPrincipalPage function src/adapters/shared/commands/asks.ts:1568 — called after ask routing/persist to perform severity-page dispatch, services/reviewer/src/ask-emitter.ts:446 — invoked (lazy import) after repo.create to ensure paging on reviewer emit path, packages/domain/src/ask/principal-page-dispatch.test.ts:28,69 — unit tests exercising dispatch semantics Adopted Extracted from asks.ts so both createAsk and the reviewer emitter can reach one paging seam per the spec.
@minsky/domain/ask/principal-page-dispatch.makeProductionPageDeps function — Missing consumers Exported helper to construct production PrincipalPageDeps; not used outside the module in this PR. Non-blocking; available for future producers if needed.

Recommendation: file a follow-up adoption task to wire 1 missing consumer.

Documentation impact

  • no-update-needed — This PR adds an internal reviewer paging tier and extracts a domain helper for severity-page dispatch without changing any documented public CLI, API, or user-facing contract. The behavior matches the existing severity transport binding (mt#3595) and wires the reviewer to use it; no docs under docs/ were modified or need updates based on the code changes reviewed. Checked server wiring and emitter semantics; no externally-visible routes/flags changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

authorship/co-authored Co-authored by human and AI agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant