Skip to content

write-protocol lifetime ownership and recovery barriers#378

Open
ragnorc wants to merge 16 commits into
mainfrom
claude/interrupted-mutations-graph-drift-njvits
Open

write-protocol lifetime ownership and recovery barriers#378
ragnorc wants to merge 16 commits into
mainfrom
claude/interrupted-mutations-graph-drift-njvits

Conversation

@ragnorc

@ragnorc ragnorc commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What & why

This RFC proposes three coordinated changes to close structural defects in write-protocol cancellation safety and recovery residual handling:

  1. W1 — Engine-owned write-protocol lifetime: Shield armed write protocols from caller cancellation by spawning protocol execution as a detached task. Caller disconnection means "stopped waiting for the result", never "abandoned the protocol".

  2. W2 — Full recovery at the write-entry barrier: Escalate rollback-class recovery residuals to RecoveryMode::Full under exclusive admission at the write-entry heal, instead of parking them for process restart. Resolves RecoveryRequired as a transient condition.

  3. W3 — Boot supervision: Replace silent graph exclusion on open failure with an observable quarantined registry state and a capped-backoff re-open loop, making recovery from transient substrate errors automatic.

Together these establish the invariant: Caller fate is not a protocol participant. An armed graph-write protocol completes or compensates under engine ownership regardless of caller cancellation, and resolving any recovery residual requires only exclusive gate acquisition — never process restart.

Backing issue / RFC

  • Implements / is an accepted RFC: docs/rfcs/0029-write-lifetime-and-recovery-barriers.md

Motivation

A validated production incident showed the chain of failures: client timeout interrupted a multi-statement delete mid-protocol, leaving an Armed sidecar; the live-handle heal is roll-forward-only and parks rollback-class residuals; the next write hit a stale manifest version; boot recovery hit a transient S3 error and silently quarantined the graph until redeploy. Every link was validated against current code (Lance 9.0.0-rc.1 at pinned rev).

The RFC documents this incident in detail (§1) and validates the mechanism at each step. The long-run liability: cancel-safety is maintained by audit today, and every write-path change must re-reason about cancellation at every await. W1 replaces that with a structural property.

Design summary

W1 (Stage 1 — server boundary): Apply the existing spawn-and-clone idiom from server_export to every _as writer handler. The HTTP response contract is unchanged; on client disconnect, the spawned task runs the protocol to terminal state (success or error) while the request future drops.

W1 (Stage 2 — engine level): Move the shield into the engine via Arc<Self> receivers or an Arc<OmnigraphInner> split (shape TBD in review), so the property holds for embedded SDK callers.

W2: When the write-entry heal encounters a rollback-class sidecar, escalate it to RecoveryMode::Full under exclusive admission + existing schema → branch → table gates, instead of parking it. The heal re-reads the sidecar under those gates (existing behavior), dispatches with Full mode (new), and the triggering write proceeds from a fresh capture.

W3: A graph whose open fails enters Quarantined { since, attempts, last_error, retry_at } in the registry. One supervision task per quarantined graph retries the full open_single_graph with capped exponential backoff (proposed: 5 s initial, ×2, 10 min cap). GET /graphs gains a per-graph status field (serving | quarantined) with quarantine detail.

Safety argument

  • W1: The spawned task holds the Arc and the admission permit; caller cancellation releases neither. The per-actor admission bound is preserved (admission bounds concurrent work, and the work genuinely continues).
  • W2: Restore needs exclusive authority over affected tables while restore-and-publish completes. In-process writers are excluded by exclusive admission + table gates (stronger than open-time). In-process readers are safe by construction: reads pin manifest-published versions; restore appends a new version; read caches are keyed by version/e_tag. Foreign processes are exactly as (un)protected as at open time (documented single-writer-process boundary unchanged).
  • W3: The retry unit is the whole idempotent open; recovery

https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5


Note

High Risk
Changes multi-graph boot, all mutating HTTP paths, recovery/reopen behavior, and public GET /graphs contract; incorrect supervision or registry swaps could affect availability or write semantics.

Overview
Implements RFC-029 at the HTTP server: armed writes survive client disconnect, failed graph opens become visible and retryable, and RecoveryRequired can trigger an in-process reopen instead of leaving the graph wedged until restart.

W1 (cancellation shield): Mutating handlers (mutate, load, schema apply, branch ops) run engine work through shielded_write — a tokio::spawn that keeps the RFC-022 commit protocol running if the request future is dropped; admission slots release when the protocol finishes, not on disconnect.

W3 + W2(b) (supervision): Boot retains per-graph GraphStartupConfig, records open failures as quarantined registry entries (not silent omission), and starts a GraphSupervisor loop before serve() with capped backoff. The loop re-runs open_single_graph and **publish**es healed handles; shielded_write calls request_reopen when a write returns RecoveryRequired. Routing returns 503 for quarantined graphs; GET /graphs adds status (serving | quarantined) and optional quarantine detail (OpenAPI updated).

Supporting changes: GraphRegistry::publish / set_quarantined, engine probe rfc029_probe.rs, server failpoints feature + integration tests, docs/RFC-029, and CI running omnigraph-server failpoint tests. Engine-level W1 Stage 2 and write-entry W2(a) remain future work per the RFC.

Reviewed by Cursor Bugbot for commit dad1cf4. Bugbot is set up for automated code reviews on this repo. Configure here.

Greptile Summary

This PR strengthens write recovery and graph availability at the server boundary. The main changes are:

  • Detached execution for mutating HTTP handlers.
  • Quarantined registry state for graphs that fail to open.
  • Supervised reopen with capped retry backoff.
  • Serving and quarantine status in graph-list responses.
  • Fault-injection and convergence coverage in CI.

Confidence Score: 5/5

No additional blocking issue qualifies for this follow-up.

  • The latest changes implement the previously missing supervision loop.
  • Detached primary write paths retain their admission guards until completion.
  • No new distinct production breakage remains to report in this round.

Important Files Changed

Filename Overview
crates/omnigraph-server/src/handlers.rs Moves write protocols into detached tasks and requests supervised reopen after recovery-required responses.
crates/omnigraph-server/src/supervisor.rs Adds graph reopen scheduling, retry backoff, and recovered-handle publication.
crates/omnigraph-server/src/registry.rs Adds quarantined graph state and serialized serving-handle replacement.
crates/omnigraph-server/src/lib.rs Retains graph startup configuration and starts supervision from the production server path.

Reviews (15): Last reviewed commit: "docs: quarantine supervision, transient ..." | Re-trigger Greptile

Context used:

  • Context used - CLAUDE.md (source)
  • Context used - AGENTS.md (source)

…n barriers

Proposes three coordinated, independently landable changes closing the
class behind the interrupted-mutation availability incident: engine-owned
write-protocol lifetime (cancellation shielding, staged server-first),
full recovery escalation at the write-entry heal under exclusive gates
(restart barrier -> gate barrier), and server boot supervision that makes
quarantine an observable, converging state. The RFC-022 commit protocol,
Armed/EffectsConfirmed classification, fail-closed ambiguity, and the
single-writer-process boundary are explicitly unchanged.

Every motivating claim is validated against current code and Lance
upstream source at the pinned rev, including decoding the incident's
_versions/18446744073709551611.manifest as an ordinary V2-named commit
of table version 4 and locating Lance's verbatim orphaned-transaction-
file warning.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
Comment thread docs/rfcs/0029-write-lifetime-and-recovery-barriers.md
Comment thread docs/rfcs/0029-write-lifetime-and-recovery-barriers.md
Comment thread docs/rfcs/0029-write-lifetime-and-recovery-barriers.md

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb55888a10

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +168 to +170
let task = tokio::spawn(async move {
engine.mutate_as(&branch, &query, &name, &params, actor_id.as_deref()).await
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Move admission guard into the spawned task

In the current run_mutate path the per-actor permit is acquired before this call (handlers.rs:686-689). If Stage 1 follows this snippet, the spawned task owns only the engine and inputs; _admission stays in the handler and is dropped when the client disconnects, releasing the slot while the write continues. That violates this RFC's admission invariant and lets disconnecting clients bypass per-actor concurrency limits, so the spawned future needs to own the guard or the whole admitted operation must be spawned.

Useful? React with 👍 / 👎.

Comment on lines +472 to +473
1. **PR-1 (W1 Stage 1):** server-boundary shield + the two W1 tests + stale
comment fix. Smallest change, kills the incident's trigger.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Move engine-cancellation test to Stage 2

PR-1 is scoped to the HTTP server-boundary shield, but requiring both W1 tests here also includes the failpoint test above that drops the engine caller future directly. That case remains cancellable until PR-4's engine-level Arc/inner-handle restructure, so PR-1 cannot turn its required red test green. Split the plan so PR-1 only tests a dropped HTTP client and reserve the direct future-drop test for Stage 2.

Useful? React with 👍 / 👎.

Comment thread docs/rfcs/0029-write-lifetime-and-recovery-barriers.md
Feature-gated failpoint test that pins the three behaviors RFC-029's
design rests on, at the engine boundary: (1) dropping a mutation future
between sidecar arm and confirmation leaks the Armed recovery sidecar
(no Drop guard compensates); (2) a subsequent write on the still-live
handle returns RecoveryRequired because the write-entry heal is
roll-forward-only; (3) an in-process read-write Omnigraph::open runs the
Full sweep under the shared root-scoped write queue, resolves the
residual, and the previously wedged handle becomes writable again
without a process restart.

These are assertions of current behavior (green today), recorded as the
baseline the RFC's W1/W2/W3 changes will deliberately move: after W1,
cell (1) inverts — cancellation can no longer create the residual.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
Comment thread crates/omnigraph/tests/rfc029_probe.rs Outdated
// effect is committed, the sidecar is armed, confirmation has not been
// durably recorded. Cancelling here manufactures the exact Armed residual
// a client disconnect produces.
let rv = Rendezvous::park_first(omnigraph::failpoints::names::RECOVERY_SIDECAR_CONFIRM);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing FailScenario setup

Medium Severity

The RFC-029 probe registers a Rendezvous on RECOVERY_SIDECAR_CONFIRM but never calls FailScenario::setup(), unlike other failpoint integration tests. With the fail crate, injected failpoints stay disabled in tests without that setup, so the mutation likely never parks and the probe cannot reliably reproduce cancellation mid-confirmation.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0793712. Configure here.

Code investigation strengthened both bug claims and upgraded the design:

- W1 has an exact in-engine precedent: the RFC-026 B1 stream-fold writer
  already detaches its whole protocol via Arc<Self> +
  spawn_with_query_io_probes, explicitly so cancellation cannot abandon
  it, and that helper already solves task-local probe propagation across
  the spawn. Stage 2 is rescoped to bringing the established writers up
  to that existing standard.

- W2 gains a zero-engine-change implementation, W2(b): a supervised
  in-process reopen + registry swap. The open-time Full sweep runs under
  the same root-scoped write queue live handles share and takes
  exclusive recovery admission, so it serializes against live traffic by
  construction; the open path's own comment ('only rollback-eligible
  sidecars wait for this open-time sweep') names it as the designed
  resolution. W2(b) and W3 unify into one supervision mechanism with two
  triggers; the heal escalation W2(a) becomes an optional end state and
  the only part of the RFC that relaxes a stated rule.

- Evidence: reference the checked-in tests/rfc029_probe.rs baseline pin
  and rework the rollout into PR-1 (shield), PR-2 (unified supervision),
  PR-3 (engine shield), PR-4 (optional heal escalation).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
Comment thread crates/omnigraph/tests/rfc029_probe.rs Outdated
claude added 2 commits July 25, 2026 17:31
…idual class

The first probe run refuted the naive Bug-2 reproduction: a mutation
cancelled AT the confirmation failpoint on local FS still produced a
durably-confirmed sidecar, because storage puts run on spawn_blocking
and an already-spawned blocking write completes after the awaiting
future is dropped — the residual self-healed on the next write's
roll-forward-only heal. On a network object store the in-flight PUT dies
with the future, leaving the Armed class that wedges; consistent with
the motivating incident occurring on S3. RFC-029 now records this: the
cancellation residual's class is a storage-backend-dependent race, and
W1 removes the roulette rather than tuning it.

The probe is reworked into three deterministic phases, all green:
(1) cancellation leaks the sidecar (class-agnostic assert);
(2) a failed confirmation write — the failpoint's documented crash
model — manufactures Armed deterministically, and the live handle then
returns RecoveryRequired with the documented reopen guidance;
(3) an in-process read-write open clears __recovery/ and the wedged
handle writes again without a process restart, proving the W2(b)
supervised-reopen mechanism end to end.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
RFC-029 PR-1 prep: expose the engine's failpoint registry to
omnigraph-server's feature-gated integration tests, mirroring the
omnigraph-cluster passthrough but without dep:fail since the server
defines no failpoint hooks of its own. No behavior change; the feature
is never enabled in production builds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
@ragnorc ragnorc changed the title RFC-029: Write-protocol lifetime ownership and recovery barriers write-protocol lifetime ownership and recovery barriers Jul 25, 2026
claude added 3 commits July 25, 2026 20:51
…idual (RFC-029 W1, red)

HTTP-boundary cancellation cell: park the engine at the
sidecar-confirmation failpoint, abort the task driving the oneshot
request (the exact disconnect model — tower drives the handler future
inline, so the abort drops it as hyper does on client disconnect), then
poll ONLY the filesystem: __recovery/ must empty with no further
requests and no reopen. Sidecar deletion happens strictly after the
manifest publish, so an empty __recovery/ proves the whole protocol
completed under engine ownership. The poll deliberately sends nothing:
a follow-up write would run the write-entry heal and mask an
unshielded server.

RED as predicted: 'dropped mutation abandoned its armed protocol:
__recovery/ still holds [<ulid>.json] after the deadline with no
further requests'. Green arrives with the next commit's shield.

Also: CI runs the new suite in the failpoints step, and the
failpoint-names source guard now walks omnigraph-server so the
compile-checked-catalog rule covers the new call sites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
…FC-029 W1 Stage 1)

One shielded_write helper + six site conversions (mutate, load, schema
apply, branch create/delete/merge, and the merge's follow-up source
delete). Cedar authorization, request validation, and per-actor
admission stay in the handler in their existing order; the admission
guard and owned inputs then move into a spawned task that runs the
engine protocol to its own terminal state regardless of the request
future's fate. The handler still awaits the result, so the HTTP
contract is unchanged — only abandonment semantics change: a client
disconnect no longer cancels the commit protocol mid-flight, and the
admission slot releases at protocol completion rather than at
disconnect. Panic in the protocol surfaces as JoinError -> 500.

Turns the previous commit's red HTTP-boundary cancellation cell green;
full server suite green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
…ation claims

- Correct writes.rs::cancelled_mutation_future_leaves_no_state's stale
  doc comment: 'only orphaned Lance fragments can remain' predates the
  RFC-022 sidecar protocol, and the test's final open runs the Full
  sweep before its assertions, so it cannot observe post-arm residue;
  point at rfc029_probe.rs as the residue-lifecycle pin.
- Qualify writes.md's cancellation paragraph the same way: the
  no-Lance-write guarantee holds during op execution, while post-arm
  drops leave recovery-covered residue; the HTTP boundary now shields
  cancellation entirely (RFC-029 W1 Stage 1).
- server.md: document disconnect semantics (writes complete server-side;
  outcome ambiguous to the disconnected client — verify by read) and
  admission-slot-until-completion.
- testing.md: register the new server failpoints suite and its
  passthrough feature; names-guard coverage note.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
Comment thread crates/omnigraph-server/src/handlers.rs
.await
.map_err(ApiError::from_omni)
})
.await?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Merge delete drops admission early

Medium Severity

For delete_branch: true, the per-actor admission guard now lives only inside the merge shielded_write task and is released when the merge finishes. The follow-on branch_delete_as runs with no admission slot, so one merge request can briefly run two armed write protocols while consuming one slot, and other writes can admit during the post-merge delete.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit bd2a043. Configure here.

…W3 prep)

QuarantineInfo + a parallel quarantined map on RegistrySnapshot (a key
is never in both maps; disjointness debug-asserted), a
RegistryLookup::Quarantined variant, and two production writer methods
serialized on the now-ungated mutate mutex — its original doc comment
named this exact consumer as the ungating condition:

- publish(handle): RCU install for the handle's key, clearing any
  quarantine entry and replacing a prior serving handle (the
  supervised-reopen swap; duplicate-URI check skips the replaced key).
- set_quarantined(key, info): records/updates a quarantine entry, no-op
  while the key is serving so a failed supervised reopen of a healthy
  graph cannot take it down.

A quarantined graph whose config declares a per-graph policy keeps
any_per_graph_policy true, so bearer auth cannot flap off while the
graph heals (strictly safer than today, where the graph vanishes).

Nothing populates quarantine state yet: from_handles delegates to
from_boot with no entries, and the routing middleware temporarily maps
Quarantined to the existing 404 until the lookup-semantics change lands
behind its red test. Zero behavior change; unit tests cover
publish-replaces-and-clears, noop-while-serving, the auth-flap closure,
and concurrent-publish serialization.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
@ragnorc
ragnorc force-pushed the claude/interrupted-mutations-graph-drift-njvits branch from bd2a043 to d7ae7b4 Compare July 25, 2026 21:13
…r 503 (RFC-029 W3, red)

Flip cluster_boot_quarantines_graph_open_failures' lenient-mode
assertions to the RFC-029 contract: a failed boot open is observable,
configured state, not silent absence. GET /graphs must list both graphs
with a status discriminator (serving|quarantined) and quarantine detail
(last_error, attempts); routes under the quarantined graph answer 503
(configured-but-healing, retry later); a never-configured id stays 404,
pinning Quarantined-vs-Gone. Strict-mode assertions unchanged.

RED as predicted: GET /graphs lists only 'good' and the broken graph
routes 404. Green arrives with the boot-wiring commit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
self.snapshot.store(Arc::new(RegistrySnapshot::with_quarantined(
new_graphs,
current.quarantined.clone(),
)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test insert breaks quarantine invariant

Low Severity

The test-only insert path copies the existing quarantined map unchanged when adding a serving handle. If the key exists only under quarantine, the snapshot can hold the same key in both graphs and quarantined, unlike publish, which clears quarantine for that key.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 57fb5d0. Configure here.

…on GET /graphs (RFC-029 W3)

Turns the previous commit's red test green:

- open_multi_graph_state keeps each failed graph's startup config,
  builds a QuarantineInfo entry (attempts=1, the open error verbatim,
  policy_configured from the config), and retains clones of EVERY
  configured graph's startup config on GraphRouting.startup_configs for
  the upcoming supervision loop (healthy graphs included — a supervised
  reopen needs them too). Strict --require-all-graphs still bails before
  any quarantine state exists; the no-healthy-graphs bail is unchanged.
- New AppState::new_multi_with_boot carries the supervision state;
  new_multi delegates with empty state so its six existing callers are
  untouched.
- resolve_graph_handle maps Quarantined to a new
  ApiError::service_unavailable (503, no closed ErrorCode — mirroring
  the recovery-required 503 shape): configured-but-healing is 'retry
  later', not 'no such resource'. Never-configured ids stay 404.
- GET /graphs merges quarantined graphs into the listing with an
  additive status discriminator (serde-defaulted to serving for wire
  compat) and a quarantine detail object (Unix-second timestamps, no
  new dependency); openapi.json regenerated.

Full server suite (failpoints included) and api-types suite green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
Comment thread crates/omnigraph-server/src/handlers.rs
…int (RFC-029 prep)

Behavior-free skeleton so the upcoming red convergence tests compile:

- src/supervisor.rs: SupervisorConfig (production 5s/x2/10min/10% jitter;
  fast_for_tests at ms scale) and GraphSupervisor with a drain-only run()
  (receives and discards reopen requests; no reopen yet).
- AppState::spawn_supervision(config): idempotent one-shot channel setup
  + task spawn — the entry point both serve() and in-process tests use.
  Shutdown is free: the only sender lives in AppState, so when the state
  drops, recv() yields None and the loop exits.
- shielded_write gains the (state, key) chokepoint: a shielded writer
  whose result carries recovery_required notifies request_reopen, arming
  the W2(b) supervised reopen for every write surface at one site. The
  merge's follow-up source delete threads state through.

Full server suite green (scaffolding drains; nothing converges yet).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
Comment thread crates/omnigraph-server/src/lib.rs
…pervised reopen (RFC-029 W3+W2(b), red)

Two bounded-poll convergence cells against the drain-only supervision
skeleton:

- multi_graph::boot_quarantined_graph_converges_to_serving_without_restart:
  a graph whose root is missing at boot quarantines (visible in GET
  /graphs with the verbatim open error), the root is initialized while
  the server runs (the deterministic transient-fault-clears model), and
  the listing must converge to serving without a restart, after which
  the graph's routes answer 200. RED: 'quarantined graph never converged
  to serving without a restart' at the 10s deadline.

- failpoints::recovery_required_write_triggers_supervised_reopen_and_heals:
  the HTTP twin of rfc029_probe phases 2-3. A confirm-failure-injected
  mutation leaves an Armed sidecar; the next write surfaces 503
  recovery_required (arming trigger B through the shielded-write
  chokepoint); polling the same write must converge to 200 and
  __recovery/ must empty. RED: 'RecoveryRequired never healed without a
  restart; last status: 503' at the 15s deadline.

Green arrives with the next commit's supervision loop.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 5 total unresolved issues (including 4 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 36c900d. Configure here.

{
state.request_reopen(key);
}
result

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reopen signal dropped on disconnect

Medium Severity

request_reopen runs only after the handler awaits the spawned write task. If the client disconnects first, the detached protocol may still finish with RecoveryRequired, but the handler never reaches that hook, so W2(b) reopen is not triggered for that outcome unless another client writes later.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 36c900d. Configure here.

Comment thread crates/omnigraph-server/src/supervisor.rs
claude added 2 commits July 25, 2026 22:20
… W3+W2(b))

The real supervision loop replaces the drain-only skeleton, turning
both red convergence cells green:

- Seeds per-graph retry state from the registry's boot quarantine
  entries (trigger A) and selects over reopen notifications from the
  shielded-write chokepoint (trigger B — retried immediately, deduped
  while pending) and the earliest scheduled retry.
- A due graph gets one full open_single_graph; success RCU-publishes
  the healed handle (clearing quarantine, or swapping a serving handle
  after a W2(b) reopen — in-flight requests finish on their own Arc);
  failure reschedules with capped exponential backoff plus jitter and
  refreshes the quarantine record, which no-ops while the graph still
  serves so a failed reopen never takes a healthy graph down.
- One loop, not one task per graph: dedups notification storms and
  guarantees at most one in-flight open per root (two concurrent
  read-write opens would run redundant Full sweeps). Deliberate
  refinement of RFC-029 §5.1's per-graph wording; observable contract
  identical.
- serve() starts supervision with production pacing before binding the
  listener; shutdown is free (the sender drops with the state).

Full server suite green, failpoints included.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
- server.md: quarantine as a converging observable state — supervision
  backoff constants, GET /graphs status schema, 503-vs-404 semantics,
  and the RecoveryRequired self-heal on the write path.
- errors.md: RecoveryRequired guidance rewritten — against the HTTP
  server the residual now heals automatically via the supervised
  reopen; embedded SDK callers keep the read-write reopen remedy.
- invariants.md: note under the in-process recovery boundary gap that
  RFC-029 narrows its operational cost without relaxing any recovery
  rule (a reopen IS the documented resolution barrier).
- testing.md: register the new W3/W2(b) coverage cells; AGENTS.md
  GET /graphs bullet mentions per-graph status.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EqR7rPVfAT7UMiy85pG9b5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants