feat(ops): report deterministic remediation facts on the incident packet - #351
Merged
Conversation
The first draft graded each signal into observe/auto-pr/approval-required from a table keyed by the signal's domain. Two problems made that unsafe. Evidence readiness was inverted. Only github-actions, sentry and fly have deep inspectors; every other source returns unsupportedInspection, which hard-codes an empty gap list. Demoting only on evidenceGaps.length meant social, product, costs, customers and posthog signals were judged evidence ready precisely because nothing had been gathered. Coverage now comes from the inspection status, so "no inspector ran" is visibly different from "inspection came back clean". The grading axis was wrong. Whether a repair is safe depends on the kind of change it needs, and a signal cannot carry change kind because that is only knowable after root-cause diagnosis. The server now reports facts it can prove -- observer trust, inspection coverage, exposure, terminal state and blockers -- and leaves the change-kind judgement to the incident-remediation skill that already owns it. Folding the facts into the incident packet also removes the standalone tool's double provider fan-out, its duplicate fingerprint lookup, and its re-declared EvidenceGap type. evidenceNumber moves to a shared module rather than being copied between the priority engine and the policy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MCP.md now describes the facts block on the investigation packet rather than a standalone assessment tool, states that no-inspector is a caveat instead of a blocker, and says plainly that exposure covers only what the investigated signal proves so an agent also reads the packet's correlated customerImpact. The incident-remediation skill gains the rule that consumes it and loses two line-number citations into MCP.md that this change would have invalidated. Section names replace them, since line cites go stale by construction. The STOP table gains the rationalization this whole block exists to prevent: an empty gap list is not the same as complete evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Intent
Give an agent a deterministic, machine-readable answer to "is this incident safe to act on yet?" and put it where the agent already looks: on the
ops_investigatepacket. Operational priority stays what it is — impact and urgency — and never doubles as permission.The server reports facts, not a verdict. It does not grade autonomy, because it structurally cannot: whether a repair is safe depends on the kind of change it needs, and one signal can require either a one-line guard or a schema migration. Change kind is only knowable after an agent has diagnosed the root cause, so
.agents/skills/ops-incident-remediationkeeps that judgement and this packet block supplies the facts a skill cannot see for itself.This PR adds no write capability. It does not add GitHub rerun, social requeue, Fly, database, customer-state, environment, or any other provider/runtime mutation.
Context
zap-pilot-opsalready had strong read-side boundaries:ops_status, bounded investigation/inspection, deterministic operational priority, and one narrowly allowlisted Sentry resolve mutation. What was missing was a machine-readable statement of whether an incident is evidenced well enough to act on.The priority engine weights
customers,socialandinfrahighest precisely because a wrong change there costs the most. An agent that took the top ofops_status.prioritiesas its work queue would therefore be pointed at the least safe targets first. That is the gap this closes.Scope
RemediationFactsto theops_investigateincident packetunknownstate, observer failure, a fingerprint absent from the snapshot, an unavailable or not-found inspection, any unresolved evidence gap, and any non-zeroaumAtRiskUsdall become blockersno-inspectoras a caveat rather than a blocker, and forbid calling such an incident production-verifieddirectMutationAllowedunconditionallyfalseevidenceNumberhelper thatprioritize.tsand the policy both neededapps/control-center/MCP.md.agents/skills/ops-incident-remediationthroughremediation.blockersand repair its two staleMCP.md:<line>citationsOut of scope
Product contract / invariants
The Ops MCP remains read-first and
ops_resolve_sentry_issueremains the sole narrowly allowlisted provider mutation, unchanged. Operational priority remains deterministic and continues to represent impact rather than permission.directMutationAllowed: falseis an advisory contract statement, not an enforcement mechanism: the Sentry resolve tool has always enforced its own credential and shape gate and does not consult this block. The documentation says so explicitly rather than implying the packet gates writes.Implementation
apps/control-center/src/server/services/operations/autonomy.tsownsbuildRemediationFacts(). It reads the signal's status and fingerprint kind for observer trust, maps the primarySignalInspectionstatus to a coverage value, and reads proven exposure out of signal evidence through the new sharedevidence.tshelper.investigateOperationalSignal()calls it once, after gap de-duplication, using the same snapshot, the same priority list and the same primary inspection the rest of the packet is built from. There is no second provider fan-out and no second fingerprint lookup.Why the design changed since the first draft of this PR
The first draft added a standalone
ops_assess_remediationtool that graded each signal intoobserve/auto-pr/approval-requiredfrom a table keyed by the signal's domain. Review found three problems that the current design fixes.The safety check was inverted. Only
github-actions,sentryandflyhave deep inspectors. Every other source returnsunsupportedInspection, which hard-codes an empty gap list. The old policy demoted only onevidenceGaps.length > 0, sosocial,product,costs,customersandposthogsignals were reported as evidence-ready and reachedauto-prbecause nothing had been gathered. Coverage is now derived from the inspection status, and "no inspector ran" is visibly different from "inspection came back clean".The grading axis was wrong. Domain does not determine whether a repair is safe; change kind does, and a signal cannot carry change kind. The domain table simultaneously over-authorized (a
jobssignal whose real fix is a broad timeout policy change, which the skill forbids) and over-restricted (aninfrasignal whose real fix is a one-line config typo). The grades are gone; the skill's existing change-kind rules are the single home for that judgement.Two supporting defects. The standalone tool fetched a snapshot and then called
ops_investigate, which fetched the snapshot again — two full provider fan-outs inside the 30-second MCP function budget, with the signal and the gaps coming from different snapshots. It also re-implementedprojectSignaland re-declaredEvidenceGap. Folding the facts into the packet removes all of that: one snapshot, one fan-out, and no tool that nothing calls.Acceptance criteria
ops_investigatereturnsremediationwith policy version, operational priority score, observer trust, inspection coverage, exposure, terminal state, blockers and reasons.directMutationAllowedis alwaysfalse.unknownstate, observer failure, and a fingerprint absent from the snapshot all block.aumAtRiskUsdblocks.MCP.mdcitations resolve.Contract alignment
AGENTS.mdremains accurate.apps/control-center/has no scopedAGENTS.md, so the repository-root one applies and is unaffected.apps/control-center/README.mddescribes the write boundary, which is unchanged;MCP.mdis updated in this PR.Validation
Run in the branch worktree against real gates:
vitest run— 46 files, 311 tests passturbo run type-check— all three tsconfig projects passturbo run lint deadcode— 0 errors (only pre-existingcurlywarnings ininvestigation.ts)turbo run format:check— cleanvitest run --coverage— exits 0node scripts/lint/run-jscpd.mjs src— the two clones this PR introduced are gone; 8 clones remain, all pre-existing onmainand none touching the changed filesValidation gaps
The remote MCP surface was not exercised against a live deployment; the HTTP protocol path is covered by the integration tests instead. No production provider was called during validation, so the facts block has been verified against fixtures and the real code path, not against live provider evidence.
Known unrelated failures
code-qualitystays red untilmainis repaired.mainfailsdup:checkon 8 pre-existing Control Center clones introduced by #350; #353 repairs them and is still open.The count is the evidence that none of them belong to this PR. Run
33599314677on the previous head reported 10 clones: those 8 plus 2 from the first draft'sautonomy.ts. Run33604577272on the current head reports 8, the same setmainreports. Every other job passes:quick-gates,tests,coverage,e2e,security,check-dead-env,deploy-gates.Reviewer notes
This change was reviewed by five independent lenses (facts-model logic, integration wiring, documentation truthfulness, test adequacy, repository guardrails), each finding adversarially verified against the source. Nineteen candidate findings were raised and all nineteen were refuted. The exposure-scope note above is the sharpest surviving objection and is answered by documentation plus the skill rule rather than by code.
Please read
autonomy.tsas a facts reporter, not as a ranking heuristic. The invariant is that the server states only what it can prove — whether the reading is trustworthy, how much evidence backs it, and what exposure is demonstrated — and refuses to pretend it knows whether a given repair is safe.remediation.exposurereports only what the investigated signal itself proves. A correlated incident — analpha-etlcron failure behind stale priority portfolios, say — carries its customer exposure in the packet's separatecustomerImpact, so the skill rule andMCP.mdboth tell an agent to read both. Attributing another signal's AUM to a job failure insideexposurewould be an inference rather than a fact, which is the one thing this block must not do. Making that correlation blocking on the server is a reasonable follow-up; it is deliberately not in this PR.no-inspectorbeing a caveat rather than a blocker is deliberate. Making it a blocker would permanently freeze everysocial,product,costsandcustomersincident, since none of those sources has a deep inspector. Such incidents can still be repaired from repository evidence; what they cannot be called is production-verified.A follow-up can add action-specific bounded executors, but each should carry its own server-generated precondition and verification contract rather than widening this block implicitly.