Repository navigation
fix(sessions): recover writer authority and verify publication joins - #4388
ScriptedAlchemy wants to merge 23 commits into
Conversation
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Pushed 6b9c4e2d96cea73b6d553b4e1fbd347396805755 to address the two concrete recovery blockers.
Both persisted-checkpoint reads use the async exact-SQL path. A refused physical attachment is drained, closed, and joined on the existing blocking executor before retry; close failures propagate.
For graph-first stores, a live graph publication's immutable binding cannot be replaced safely while owners or operation leases retain it. Recovery now retains the persisted epoch floor and fails once with typed WriterAuthorityBehindPersisted, preserving existing leases. It does not perform the same doomed physical open twice. After canonical owner/client release and graph close, reopening allocates a shared binding above the checkpoint and can commit. That follows #4372's explicit “adopts the persisted epoch, or fails once with a typed error” contract.
Added a real GraphDbRegistry owner/client + SQLite-checkpoint regression covering the refusal, continued validity of old leases, release/close, and successful reopen/write. Existing reopen coverage also checks native-worker wait telemetry. Pinned rustfmt and diff checks pass; native tests remain for CI.
The PR description now names //crates/tracedecay-cli:core_cli_suite for the isolated daemon journey. The previous blocked command selected a binary target, so it was not evidence that this journey had run.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| let frontier = access | ||
| .session_temporal_publication_result(session_id) | ||
| .await | ||
| .map(|publication| publication.observed_through()) | ||
| .map_err(|error| { | ||
| tracing::warn!(%session_id, %error, "session publication target unavailable"); | ||
| }) | ||
| .ok(); | ||
| entry.insert(frontier); | ||
| } | ||
| let Some(frontier) = frontiers.get(session_id).copied().flatten() else { | ||
| let _ = join.completion.send(false); | ||
| continue; |
There was a problem hiding this comment.
🟡 Transient reads fail session publication joins
When a publication read fails, capture_projection_targets rejects the named join before the worker projects the session. A transient read failure makes a successful Hermes ingest return temporal_refresh_unavailable.
Learn more
A named join first reads the session's output-producing observation frontier, then the worker projects that frontier. A failed snapshot or query becomes None, and the sender immediately resolves false; the worker never gets a chance to retry the read or project that join. The publication read in settle_projection_joins likewise converts a read failure into a negative result after the pass. The caller reports a retryable unavailable hook result despite a potentially successful write.
Example: A Hermes callback admits a message for session s, but a temporary database read error occurs while capturing s's frontier. Its join returns false immediately, even if the worker can read and publish s on its next pass.
Recommended fix: Retain named joins when their frontier or publication read fails transiently, requeue a bounded projection attempt, and resolve them only on proof, a definitive failure, cancellation, or the caller's deadline. Preserve an explicit distinction between read errors and a successful read with no receipt.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /// Join the worker's idle boundary, including background work. Callers | ||
| /// proving a rekey handoff still need that boundary; live transcript | ||
| /// publication uses the separate projection joins above. | ||
| pub async fn wake_and_wait_until_idle(&self, timeout: std::time::Duration) -> bool { |
There was a problem hiding this comment.
🟡 Idle join has no production caller
wake_and_wait_until_idle has no caller, including the rekey handoff it describes. This leaves a second, unused publication-wait contract in production.
Learn more
The repository requires production APIs to have real callers and forbids staging dead code. The new idle-boundary API is unused throughout the workspace; current production publication calls use the projection join instead. Unlike that join, this method also waits for background work, so retaining it creates a distinct, unexercised contract.
Example: Rekeying a project calls rekey_project, which joins retired supervisors directly; it never calls this idle join.
Recommended fix: Remove the unused method unless a real production handoff needs its idle-boundary semantics. If one does, wire that caller and test its behavior under pending background work.
Was this helpful? React with 👍 or 👎 to provide feedback.
Pull request was converted to draft
ScriptedAlchemy
changed the title
ScriptedAlchemy
changed the title
This branch has not been deployed
1 participant
Losing the daemon authority record could reopen a store with a writer epoch below its durable checkpoint, leaving observation admission retrying generic infrastructure failures. Read the exact shard/incarnation checkpoint before publication, retain its epoch floor, and retire the refused physical attachment on the blocking executor. Retry once when no live graph owner retains the binding; otherwise return typed
WriterAuthorityBehindPersistedwhile preserving live leases. Ledger fences remain typed and rejected writes roll back.Named Hermes inline transcript callbacks join their own pinned source frontier. Success requires an active temporal generation, an applied relation receipt, and its expected graph watermark. Pending resets, retired generations, and future frontiers cannot supply publication proof. Another session's failure cannot reject a proven publication; the requested session's failed or unproven projection remains unavailable. Source sweeps and unproven provider identities retain the aggregate publication contract.
Pending joins finish advancing bounded discovery pages before waiting for derived-summary admission or the next historical window. This fixes the complete-receipt/zero-backlog batch-join stall without changing deadlines or work budgets. Preserve the exact registry idle-completion assertions through rekey and retirement.
Validation:
-D warnings) passed for session-runtime, session-temporal-store, MCP, root unit, root session integration, and root MCP integration targets before the subsequent master integration.1b1764b4276be8c35396c68d6f8f9dd50095ede4and commitsd41885d4/a9324cea, and includes master throughcd9fde234d(including cache-output revalidation). The focused results above do not establish validation of this final combined tree.The idle-notification helper is compiled under the same test/test-helper configurations as its callers, addressing the prior strict-Clippy unused-function failure. Full hosted CI is explicitly dispatched on
810bed1e23ff7d3ec0f651e4d28bc765266d38db: CI run 39762409083. This PR remains a draft pending that validation; ordinary draft skip-green checks are insufficient.Fixes #4372