fix(sessions): scope every session-store refusal to sessions - #2648
Conversation
A session store whose LCM schema version, git correlation version, or workflow schema identity this binary refuses is now admitted untouched for its other authorities, the same way #2613 admits a store whose observation rows predate the unified identity. Project open, code intelligence, and MCP initialize/tools/list keep serving; every session feature answers the typed reset refusal naming `tracedecay wipe --stale --yes`; doctor counts the store as a pending operator action; and the scoped reset deletes exactly that store. When a later authority also fails, the earliest session refusal stays the store's hard verdict, so a legacy store keeps its LCM identity.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e06dc1927d
ℹ️ 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".
| .await? | ||
| { | ||
| RegisteredSchemaAttachmentV1::Admitted(convergence) => (Some(convergence), None), | ||
| RegisteredSchemaAttachmentV1::SessionsRefused(refused) => (None, Some(refused)), |
There was a problem hiding this comment.
When a project store hits one of the newly softened LCM/workflow/git refusals, this still constructs a normal mounted owner. mount_project_session_store then unconditionally registers its weak issuer as a remote-replay target, while issue_target_lease never checks reset_required() before dispatching RemoteObservationReplay. A remote capture arriving before the operator resets the store can therefore receive a committed receipt and be written into a database that wipe --stale subsequently deletes, even though ordinary session calls correctly refuse the same store. Keep refused owners out of the replay/fence target or enforce the refusal when issuing replay leases.
AGENTS.md reference: AGENTS.md:L209-L211
Useful? React with 👍 / 👎.
| let surface = |refused: Option<RefusedAuthorityV1>| { | ||
| move |error| refused.map_or(error, RefusedAuthorityV1::error) | ||
| }; |
There was a problem hiding this comment.
The surface closure replaces every later error whenever an earlier session refusal exists, rather than only choosing precedence among schema-reset verdicts. For example, with a stale LCM marker, a ConfigurationSchemaError::Storage from the freshness query is first converted into the correct database-operation error and then discarded here in favor of ProfileResetRequired; attach fails and the registry advertises the destructive stale-store reset even though the additional failure may be I/O or database corruption. Preserve operational/storage errors and apply the earlier-refusal precedence only to other typed schema incompatibilities.
AGENTS.md reference: AGENTS.md:L189-L191
Useful? React with 👍 / 👎.
1 participant
Follow-up to #2613. That PR scoped only one refusal kind: a session store whose observation rows predate the unified identity. A project sessions store refused for any other session authority still failed admission outright, so project open stalled and every code tool answered
reset_required.Root cause
classify_registered_schema_authoritiesreturnedErrfor an LCM schema version skew, a git correlation version skew, or a workflow schema identity mismatch. The store never mounted. #2613's scoping (RefusedAuthorityV1carried on the admitted owner and lease) only covered the observation check, which runs later, during installation.Change
Every kind now goes through #2613's mechanism. No second mechanism was added.
RefusedAuthorityV1is now an enum.Shape { authority, reason }covers observations and workflow.Version { component, found_version, required_version }covers LCM and git correlation, so the census keeps the typed versions.ensure_attached_registered_schemareturnsRegisteredSchemaAttachmentV1::{Admitted(plan), SessionsRefused(refusal)}, and the daemon path only schedules convergence for an admitted plan. Nothing fabricates a plan for a refused store.legacy_profile_requires_reset_without_carrying_session_content_forwardis unchanged and passes).tracedecay wipe --stale --yes, andrefuse_reset_requiredsession gating from fix(sessions)!: scope stale session-store refusals and reset #2613 already handle any refused authority, so they needed no change.refused_session_stores_serve_code_until_their_scoped_reset, parameterized by refusal. The test file got 129 lines shorter.Configuration, session temporal (it holds the code query cursor keys), remote-deletion, and registry refusals still refuse the whole store. Project open and code search read those authorities, so they are not session-only data.
Fails on master, passes here
stale_sessions_store_resetruns one journey per kind against a physically spawned daemon in an isolatedHOME. It ages only the project sessions store: LCMversion = 12, git correlationversion = 5, or a foreign workflowdefinition_digest. Then it checks:initializeandtools/listserve.search,callers, andfile_dependentsserve.lcm_statusrefuses withreset_requiredand names the authority andtracedecay wipe --stale --yes. So do the kind's own session tool (lcm_grep,sessions_for,workflow_list_definitions).1 pending operator action(s),no issues., and neverStalled.wipe --staleremoves only that store's files, and every other profile file (includinguser-sessions.db) stays byte-identical.On base 75926a1 with only the tests applied, all three new cases fail:
On this branch:
4 passed.The global-db tests that pinned "store refused whole" now assert the new contract: the store is admitted,
reset_required()returns the typed refusal, and the main database bytes are unchanged. These arean_older_git_correlation_schema_is_refused_without_mutation, the four workflow*_requires_reset_without_mutationtests, andstale_or_future_lcm_marker_requires_reset_without_rewriting_marker.Runtime journey (debug CLI from this branch, isolated HOME,
cargo-slotclone, daemon in aMemoryMax=6Gscope)The corpus was indexed (
{"outcome": "reached"} 1122symbols), the daemon stopped, andUPDATE session_schema_migrations SET version = 12 WHERE name = 'lcm'run on the project sessions store. Then the daemon was restarted:Verification (final tree)
tracedecay-global-dblib 385 passed;tracedecay-store-runtimelib 123 passed (1 pre-existing ignore);tracedecay-mcplib 387 passed;tracedecay-applicationlib 482 passed,application_suite66 passed.mcp_suite: 609 passed, 0 failed.transport_acceptance_suite: 18 passed, 0 failed.tracedecaylib: 770 passed, 2 failed, neither from this change:rmcp_wire_matrix_matches_raw_dispatch_initialize_tools_and_resourcesis red on master. It reproduces with this branch's sources reverted and is filed as rmcp_wire_matrix_matches_raw_dispatch_initialize_tools_and_resources red on master #2645.daemon_http_shutdown_releases_loopback_listenerfails under full-suite load (a port-reuse check) and passes when run alone.cargo clippy -p tracedecay-global-db -p tracedecay-store-runtime -p tracedecay-mcp -p tracedecay -p tracedecay-cli --all-targets --features tracedecay/test-transport,tracedecay/test-helpers,tracedecay-cli/test-transport -- -D warnings: clean.cargo fmt --all -- --check: clean.#2611 (
absent_query_port_fails_closed_for_every_awaiting_graph_handler) was a production ordering bug: the session-store requirement ran before request validation and graph admission. It landed independently as #2640 while this was in flight, so this PR carries no copy of that fix. The test passes on this tree.