fix(sessions): harden user routing and daemon shutdown#448
Merged
Conversation
🦋 Changeset detectedLatest commit: 497500b The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d635d1337e
ℹ️ 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".
…arch-storage-scope # Conflicts: # src/sessions/mod.rs
…scope' into fix/user-message-search-storage-scope
# Conflicts: # src/sessions/mod.rs
…scope' into fix/pr448-macos # Conflicts: # src/sessions/mod.rs
…scope' into fix/user-message-search-storage-scope
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.
Summary
This change closes the remaining correctness, lifecycle, and recovery gaps found while dogfooding TraceDecay as the sole memory/context integration across Hermes, Codex, Cursor, Claude, and projectless chat sessions.
The core invariant is now explicit: code and project knowledge is routed to the active registered project shard, while genuinely projectless conversation is routed to the profile-owned user session/memory stores. Provider identity remains part of every session key. No mutating memory operation is allowed to cross a project boundary implicitly, and uncertain legacy evidence fails closed instead of being guessed into user memory.
Why this was needed
The earlier Hermes routing work correctly replaced process
cwdwith Hermes's per-session runtime workspace, but live logs and historical transcripts exposed adjacent gaps outside that single plugin path:storage_scope=userfortracedecay_message_search, even though the runtime handler already supported user-session search;session_meta.cwdas authoritative for the entire file;session_id, allowing two providers with the same ID to be merged into one response;~/.hermesas a code project, preserving projectless sessions in a blocked profile shard;doctorreported stale legacy storage-registry rows thatmigrate registry-gcdid not inspect;SIGKILL;These issues were corroborated by Hermes/TraceDecay journals, current registry state, historical session stores, installed integration manifests, and direct command failures. In particular, the live compression failure occurred while the TraceDecay daemon was down after a shutdown timeout; restoring the daemon immediately restored the socket, and the process-lifecycle fixes in this PR address the underlying hang rather than only restarting the service.
User and project session routing
User-scoped message search
tracedecay_message_searchnow advertises and accepts the samestorage_scopeselector as the LCM tools. The CLI and direct MCP paths both acceptprojectoruser, so a host can search profile-owned projectless sessions from/,$HOME, or another directory with no TraceDecay project.This does not make code-index tools projectless. Source outline, body, impact, search, and mutation operations still require a real project identity. User scope is specifically for session/LCM and user-memory operations.
Codex per-turn attribution
Codex ingestion now evaluates the latest
turn_context.cwdfor each record instead of rejecting or accepting an entire rollout from the firstsession_meta.cwd.That supports all expected transitions without copying evidence across shards:
Only records in the requested scope are emitted. Pending structured records are annotated with the latest in-scope context. Tests prove that a marker from project B never appears in project A and that project evidence is excluded from the profile user session store.
Codex final-turn ingestion
The Codex integration now installs a native
Stophook. It has a five-second host timeout and a three-second internal ingestion budget. For projectless sessions it ingests the completed rollout after the assistant response has landed, then schedules a user-session review only when new rows were actually added.The existing prompt hook still ingests before recall, but it no longer launches a prompt-only reflection. This gives one completed turn one review. Existing byte cursors make repeated
Stopreceipts idempotent, and a project-scopedStopreceipt is explicitly refused for user-store writes.Cursor collision safety
Cursor user-session ingestion now prefers hook-provided workspace identity in this order: exact event
cwd, event file path, thenworkspace_roots. The candidate is resolved through normal project-root discovery and compared against registered roots.The lossy transcript-directory slug is used only when the event has no workspace identity. A collision in that fallback path fails closed. This prevents
/work/foo-barand/work/foo/barfrom being treated as the same workspace while still protecting user memory when the host supplies insufficient evidence.Provider-qualified dashboard sessions
The dashboard session endpoint now detects when one
session_idexists under multiple providers across raw messages or summary nodes. It returns HTTP 409 instead of merging unrelated providers. A second guard after reads closes the concurrent-insert race.This preserves the storage model: session identity is
(provider, session_id), notsession_idalone.Legacy Hermes recovery
The migration recognizes the exact standard Hermes profile as projectless evidence and can move verified legacy sessions into the profile
user-sessions.db. Durable nested Git repositories remain project-scoped, including a repository physically located under~/.hermes. Missing or otherwise unresolved descendants fail closed and remain in their source store.For the short-lived profile-shard layout, candidates retain the manifest project ID. After a successful or already-verified migration, registry identity metadata is removed only when the current registry row's canonical or display root still points to the legacy Hermes profile. A corrected or reused project ID is preserved.
The source databases, manifests, and row evidence are never deleted. Tests verify row parity, migration provenance, idempotency, real-project routing, unresolved-path preservation, source preservation, positive cleanup, and reassigned-identity preservation.
Registry diagnostics and cleanup
doctorandmigrate registry-gcnow use the same storage-aware definition of stale metadata. GC scans both moderncode_projectsidentities and legacyprojectsstorage rows, filters by optional path prefixes, and classifies storage through registered aliases and profile roots.Preview remains the default because this is an operator repair command.
--applydeletes registry metadata only; it never deletes a project directory, profile shard, manifest, database, quarantine entry, or blocked store.JSON output separates logical projects from metadata rows:
candidate_countis the deduplicated logical project count;metadata_candidate_countis the number of rows across both tables;The text preview is also deduplicated. The regression fixture puts one stale project in both registries, proves it counts as one logical project and two metadata rows, verifies both rows are removed on apply, preserves a live initialized project, and preserves a blocked manifest byte-for-byte.
Daemon and automation shutdown
Shutdown now enters draining mode, closes and unlinks the socket, holds a global auxiliary-process creation guard, aborts all automation schedulers, and waits for the entire scheduler set under one aggregate deadline. It then drains clients under a bounded deadline. Timed-out client joins are also bounded.
Codex app-server children run in dedicated process groups. The shutdown guard terminates every active group and rejects new spawns while shutdown is in progress. The guard remains alive across scheduler and client draining so a killed app-server retry cannot respawn during teardown. The Unix regression test forces a real shell descendant and verifies that both the leader and descendant exit.
Scheduled pre-run commands now use
kill_on_drop, so aborting the scheduler future cannot intentionally leave the command attached to a daemon that is trying to exit.Profile-wide user transcript catch-up is now single-flight per profile with a short startup cooldown. The claim releases immediately if a sweep is cancelled and records the cooldown only after successful completion. Project-specific catch-up still runs independently for every server, and live hooks bypass the startup cooldown so a completed turn is never hidden. Test profile guards also isolate host-home discovery, preventing tests from scanning live Codex/Hermes/Cursor histories after restoring process environment.
These changes address the observed sequence where
daemon_shutdownwas logged, scheduler activity continued, systemd waited through its stop timeout, and then killed the daemon plus Codex/node descendants. They also prevent shutdown duration from scaling asnumber_of_projects * per_scheduler_timeout.Autonomous curation and memory mutation
This PR does not turn autonomous TraceDecay curation into a terminal dry run. The effective automation policy remains autonomous: the curator proposes operations, validates them deterministically, and applies accepted add/update/delete/merge operations in the same run. Agents can mutate and remove stale memories through the supported fact operations, subject to evidence, audit, pinning, and project-scope protections.
The internal validation report may describe a
dry_run_firstphase because validation is performed before mutation. That is a safety gate inside one autonomous run, not a requirement for Hermes or a person to rerun curation.Explicit migration, retention, registry repair, and destructive operator commands retain preview/apply controls. Those commands can affect historical storage and should not silently mutate it merely because the memory curator is autonomous.
Hermes and TraceDecay responsibilities
Hermes remains the agent host: it owns sessions, transport surfaces such as Telegram/Desktop/TUI, the authoritative per-session
runtime_cwd, and its progressive tool/skill discovery behavior.TraceDecay remains the memory/context provider: it resolves that session workspace to a registered project before opening a shard, ingests provider-qualified session evidence, stores project and user memory in explicit scopes, performs reflection/curation, and exports TraceDecay-owned managed skills through the Hermes plugin overlay.
Managed Hermes skills stay inside the TraceDecay-owned plugin package. TraceDecay does not write into host-owned user skill directories. Plugin registration discovers bundled and generated skills so Hermes's normal progressive discovery can expose them without creating a competing discovery system.
Concurrency model
Parallel agents are supported. Session and memory identity includes provider/session/project scope, database writes use the existing transactional store paths, and multiple agents in the same project converge on the same project shard without relying on one global mutable
active project. Agents in separate worktrees resolve through registered Git/worktree identity. Cross-project writes remain rejected.This does not make concurrent source edits conflict-free; Git/worktree discipline still governs code files. The storage changes ensure that independent agent transcripts and memory operations do not overwrite one another merely because they run concurrently.
Validation
Focused coverage includes:
The branch is also checked with formatting, diff whitespace validation, full workspace compilation/tests, and Clippy with warnings denied. Live dogfood after release will repeat projectless and project-scoped ingestion/search, registry preview/apply, daemon stop/start timing, installed hook/plugin discovery, and log checks against the released binary.
Final integration after master advanced
The branch was merged with the latest
mastercatch-up and recovery work instead of force-pushing over concurrent development. The conflict resolution preserves both halves of the intended design: profile-wide user history is coalesced and bulk-routed, while daemon startup uses the per-profile single-flight/cooldown path and project stores continue through the optimized project-only ingestion function. This avoids restoring the original startup herd while retaining the newer shared-source scan.The Windows/macOS follow-up gates Unix-only daemon shutdown constants and canonicalizes the expected Hermes project path in the migration regression, accounting for macOS
/varto/private/varaliases without weakening production path resolution.Automated review also identified that
storage_scope=usermessage search accepted the defaultcatch_up=truecontract but only openeduser-sessions.dbread-only. The final handler now performs a provider-bounded, single-flight catch-up first, waits on an identical in-flight catch-up when necessary, and then opens the selected user store read-only. Importantly, ingestion receives the selected profile root directly rather than falling back to process-global profile state, so a daemon serving multiple client profiles cannot catch up one user into another profile. The CLI regression intentionally omitscatch_up, proves the default path runs, and assertscatch_up_performed=true.Final merged-tree verification: formatting clean; Clippy passed for all targets and all features with warnings denied; all 162 session unit tests passed; the user-scope CLI regression passed; and full nextest executed 4,644 tests. Four local failures were isolated: two immediately passed alone and two are artifacts of running from a nested
.worktrees/checkout because their fixtures deliberately create sibling directories and therefore inherit the outer repository identity. The hosted checkout does not have that nested topology; CI remains the merge authority for those platform/topology cases.Final Windows lifecycle and coverage hardening
The Windows follow-up no longer treats owner-sidecar text as authority for cross-process exclusive-lease inheritance. Windows update/upgrade keeps the lifecycle lease throughout binary replacement, releases it immediately before invoking the newly installed
post-updateprocess, and requires that process to acquire a fresh OS lock. If another lifecycle operation wins that narrow handoff race,post-updatefails closed instead of mutating concurrently. Non-Windows inheritance additionally validates that the recorded owner PID is live and, when present, matches the recorded process start identity, preventing dead-owner and PID-reuse token acceptance. Regressions cover dead owners, reused PIDs, platform handoff behavior, and matching Windows sidecar tokens.The temporary broad Windows test exclusions were removed. All 35 platform-neutral consolidation tests and all four post-update health E2E tests are compiled again on Windows. Only the directory-symlink identity fixture remains gated because Windows symlink creation depends on host privilege/developer-mode policy; Unix-only permission assertions remain individually gated inside otherwise cross-platform tests. A real
x86_64-pc-windows-gnucargo check --testsvalidates the restored Windows compilation surface.