Repository navigation
fix(reliability): validator outage no longer replays writes; every timeout and bad run reaches the repair flow - #941
Merged
Conversation
alex-mextner
marked this pull request as ready for review
November 2, 2026 05:08
This was referenced Nov 2, 2026
alex-mextner
force-pushed
the
fix/run-outcome-alerts-20261101
branch
from
November 3, 2026 16:56
529d1f4 to
661f26f
Compare
This was referenced Nov 3, 2026
Two quick confirmations in one chat start parallel agent runs that repeat the same create_event
#929
Open
…ut into one aggregated alert (Refs #935) Until now a timeout that a fallback absorbed (a provider that answered on the second try, a validator that gave up) was visible only in docker logs, and the owner learned of slow-host timeouts from users. This adds the single place the bot reports a timeout to the owner's repair flow: `reportTimeout({ kind, requestId?, elapsedMs, limitMs, recovered })` in src/services/incident/timeout-reporter.ts. Alerts are aggregated per kind: at most one per kind per 10 minutes, carrying how many timeouts were held back and their recovered / unrecovered split. Held-back timeouts are flushed when the window ends, so a lone unrecovered timeout after a recovered one is never lost. A push the queue refused is retried a minute apart five times in a row, then once per window until the queue takes it; the failure count and the backoff are shared by all kinds because they share one queue. `flushTimeoutReports()` sends what is still held and returns the kinds that stay unsent, for shutdown to log before the database closes. The alert holds no message text, user ids or titles. Until bootstrap configures the reporter (tests, dev) reports are dropped. The reporter has no callers yet: the next commit wires every timeout site, the shutdown flush and the guard test. Tests: test/services/incident/timeout-reporter.test.ts (18 tests). Review: the default staged review is refused host-wide (the codex seat is quota-exhausted until 2026-11-08 02:05Z, exit 10; the Claude/Opus seat was refused by the 80% five-hour account cap until 05:09 CET). Reviewed by the authorized reduced route, a single Opus seat: `review diff --staged --task 935 --pool 1`, three passes. Fixed from the first two: a lone incident lost after the queue stayed refusing, shutdown dropping held alerts, per-kind retry counters restarting the shared backoff, a recovered timeout arriving mid-push never flushed. Left as is after the third pass (medium-low, only reachable if the alert queue's own push reports into this reporter, which the wired queue does not): reports from another kind during a push can start a second push, and a mid-flush report can stay held without being listed as unsent. This is a single-seat pass; a full-roster review of the integration diff follows.
…s a good answer, and every timeout and badly ended run reaches the repair flow (Refs #931, #935) On 2026-11-01 a response validator that started with 2.7 s of the request budget left was cut off by that budget; the run's good answer was thrown away, the corrective retry ran after the calendar writes, and the user was told the request was interrupted. Nothing reached the owner, who learned of it from the user. Owner decision 2026-11-01: every timeout and every run that ends badly must reach the auto-fix flow. Validator verdict - The response validator returns approved / rejected / unavailable(timeout | provider_error | aborted | budget_expired) on its own 16 s clock (VALIDATION_TIMEOUT_MS) once it has started; the request deadline only decides whether it may start. An outage is never a rejection. - After calendar writes an unavailable verdict runs no retry-with-tools pass, because that pass could replay writes the user already has. The reply is a deterministic receipt built from the tool results alone (agent/receipt-confirmation.ts: exact titles, local weekday, day and time, no model prose) and the run ends unverified, never "interrupted" when every write landed. In a group, or when a write is not create/update/delete_event or names no event, the receipt is the write ledger's own summary, which hides targets in groups; lines are keyed by event id. - The receipt builder lives in its own module, not in write-outcomes.ts, which the secretary-invites branch rewrites. Integration point for that branch: planGuard in agent/turn-finish.ts calls buildReceiptConfirmation(language, hideTargets, toolEvidence, writeOutcomes), which needs only WriteOutcomes.finalNotice() and WriteOutcomes.mayHaveMutated. Delivery and incident alerts - The AI request metric carries the delivery outcome and the Telegram message ids the API returned (agent/../delivery-report.ts, TelegramStreamWriter.deliveryReport), ids and counts only. - A run that ends error / unverified / limit / with an "interrupted" notice pushes one rate-limited alert (agent/run-incident.ts, source agent-run-incident) carrying request id, tool counts, validator verdict and delivery outcome, never user text, titles or ids; the per-chat debug log gets a REQUEST ID line so the run is found in both logs. - scripts/hcb-alert-watcher.ts: the brief for the timeout and agent-run-incident sources, and probe failures are logged instead of swallowed. Runbook and CLAUDE.md updated. Timeouts - Every deadline the bot arms now reports through reportTimeout() (reporter: a417cea): the validator, provider rounds (recovered when a later provider answered), the overall request deadline, the retry store, recipient lookups, NLI classification, Telegram-session lookups and bridge spawns, voice error phrases, the admin ping, Playwright page acquisition, the render queue wait, the models-listing probe, the Google token-refresh lock wait, the agent drain and the process shutdown. SQLITE_BUSY has no timer to hook, so the bot error handler, the worker failure handler and the provider-circuit sidecar forward what they catch to reportIfDatabaseBusy() (services/incident/database-busy.ts) as db_busy. Shutdown flushes held timeout alerts before the database closes. - test/services/incident/timeout-sites.test.ts is a guard over src that checks every callable (test/helpers/timeout-site-scan.ts parses the source), not every file: AbortSignal.timeout, setTimeout, Bun.sleep, Promise.race, waitUntilFinished, a client timeout option, a TimeoutError and busy_timeout must be reported in the same callable, or be on a reasoned exemption list; an exemption that is reported elsewhere must name a callable that really reports. Result: 30 deadline callables in 23 files, 16 report themselves, 14 are exempt (9 reported by another callable, 5 are not deadlines), none unaccounted. Known limit: two deadlines in one callable with one report are not told apart. Tests (each run alone): validator-outage, response-validator, receipt-confirmation, delivery-outcome, run-incident, request-metrics, agent-turn-logic, agent-run-errors-and-budgets, agent-run-invitations-and-delivery, agent-shutdown-drain, model-registry, provider-circuit, recipient-identity/profile timeouts, retry-store timeout, nli timeout, session-bridge and service-tier timeouts, call-session timeout, playwright-pool timeout, render-service, oauth, busy-report, worker-failure-db-busy, shutdown and shutdown-timeout-report, timeout-sites, timeout-reporter, hcb-alert-watcher. Server-run: tsc --noEmit and biome check . clean. Review findings fixed before this commit (first pass): receipt leaking titles into groups, a successful run reported as interrupted, the unverifiable "no need to repeat" claim, duplicate events folded into one receipt line, model-invented tool names reaching the alert, the abandoned-login wait reported as a failure, shutdown not flushing on the deadline path, the agent catch not forwarding SQLITE_BUSY, a chain of timed-out attempts labelled provider_error, the render backlog check missing the prioritized set, the rejected-verdict branch of the unverified checklist. Known and recorded in #939: db_busy alerts are written into the database that is locked. Review: the default staged review is refused host-wide (the codex seat is quota-exhausted until 2026-11-08 02:05Z, exit 10; the Claude/Opus seat was refused by the 80% five-hour account cap until 05:09 CET). Reviewed by the authorized reduced route, a single Opus seat: `review diff --staged --task 935 --pool 1`. This is a single-seat pass; a full-roster review of the integration diff follows. Second single-seat pass, findings NOT fixed here and recorded in #935 for the next change (medium or low, no data-integrity issue): the receipt outro does not say that repeating the request is unnecessary although docs/intents/response-quality.md asks for it (the first pass asked to drop the claim when only part of a request may have been done: needs a product decision); db_busy and run-incident alerts are written into the locked database; a failed Telegram delivery (deliveryOutcome failed) on a normal run raises no incident; an interrupted supplement that wrote to the calendar raises none; the two new alert writers ignore the ADMIN_ALERT_TOKEN gate; agent_request can be reported for an unrelated error after the deadline; most timeout kinds carry no requestId; the runbook's delivery line format and the watcher test fixture are stale; no test drives a real provider-chain timeout through reportProviderTimeouts or a delete_event and non-UTC receipt. Third single-seat pass (after a refactor that kept planGuard, validateResponse and the guard scanner under the lint complexity limits). Fixed: a validator outage never sets the "interrupted" line, even when a write was declined or undelivered. Left open and recorded in whole run when one write names no event; the receipt lacks the "no need to repeat" sentence the intent doc requires (the first pass asked to drop it: owner decision); recovered-only timeout alerts can start an investigation session; logs and rows named in the incident checklist are not marked untrusted; the alert writers ignore the ADMIN_ALERT_TOKEN gate; an unavailable validator still triggers the retry on a run without writes.
alex-mextner
force-pushed
the
fix/run-outcome-alerts-20261101
branch
from
November 4, 2026 09:55
661f26f to
da6a6fd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #935, #931, #939. Rebased on main 4b2372d (version 0.3.3). Single-seat review via omp Opus (4 rounds); the full-roster review is owed in #961; open low findings are listed in #935. Hosted checks and stage-qa must be green on the exact head before gh ship.