fix(state): inherit cwd/git_repo_root on parent_session_id children - #64731
fix(state): inherit cwd/git_repo_root on parent_session_id children#64731wesleysimplicio wants to merge 2 commits into
Conversation
_insert_session_row never copied cwd/git_repo_root from a parent row when parent_session_id was set, and git_repo_root wasn't even in the INSERT's column list. The compression-fork path (and delegate/subagent spawns, branch continuations) creates a child session without passing cwd/ git_repo_root at all, so the child's tip is born NULL — and since the Desktop project sidebar groups sessions by cwd, the whole project silently drops out of the sidebar every time a long conversation compresses. A lineage that compresses repeatedly compounds this across generations. Add git_repo_root to _insert_session_row's INSERT/COALESCE-on-conflict column set, and backfill both cwd and git_repo_root from the immediate parent row (single non-recursive hop, matching the existing COALESCE "never overwrite an explicit value" contract) inside the same write transaction whenever parent_session_id is set. A multi-generation chain resolves correctly because each generation's own create_session call already backfills from its (already-resolved) immediate parent. Fixes NousResearch#64709
Resolves conflict in hermes_state.py's _insert_session_row: main added profile_name, this branch adds git_repo_root - kept both parameters, both in the INSERT column list/VALUES/ON CONFLICT upsert, and both in the bound-values tuple, in the same order. Also marks a pre-existing test fixture in tests/gateway/test_multiplex_adapter_registry.py (a fake Discord token used only to test credential fingerprinting) with the repo's allowlist convention so the secret scanner does not flag it on merge.
|
Thanks for the focused state-layer fix. The premise remains valid on current main: the rotation path in The proposed centralized, NULL-only parent backfill fits the existing non-clobbering upsert convention and also covers the other direct parent-linked child creators, including gateway branch creation ( Automated hermes-sweeper review. |
…n children Follow-ups on top of #64731's cwd/git_repo_root inheritance: - git_branch joins the parent-row backfill (same NULL-only COALESCE hop): the Desktop sidebar branch chip otherwise vanishes at every compaction boundary even though the workspace didn't change. - Belt-and-suspenders for #59527: compression forks (parent already ended with end_reason='compression') also inherit the gateway origin columns (user_id/session_key/chat_id/chat_type/thread_id/display_name/ origin_json) at DB-level child creation. The gateway re-records the peer after rotation (d5b4879), but a hard crash in the window between child creation and that write left the child unrecoverable by find_latest_gateway_session_for_peer. Scoped to compression forks only — delegate/subagent children (parent still live) must NOT inherit routing keys, or peer recovery could repoint gateway traffic into a subagent's session. - Behavioral test driving the real _compress_context rotation path, asserting the child row carries cwd/git_repo_root/git_branch and the origin columns.
…n children Follow-ups on top of NousResearch#64731's cwd/git_repo_root inheritance: - git_branch joins the parent-row backfill (same NULL-only COALESCE hop): the Desktop sidebar branch chip otherwise vanishes at every compaction boundary even though the workspace didn't change. - Belt-and-suspenders for NousResearch#59527: compression forks (parent already ended with end_reason='compression') also inherit the gateway origin columns (user_id/session_key/chat_id/chat_type/thread_id/display_name/ origin_json) at DB-level child creation. The gateway re-records the peer after rotation (fef540d), but a hard crash in the window between child creation and that write left the child unrecoverable by find_latest_gateway_session_for_peer. Scoped to compression forks only — delegate/subagent children (parent still live) must NOT inherit routing keys, or peer recovery could repoint gateway traffic into a subagent's session. - Behavioral test driving the real _compress_context rotation path, asserting the child row carries cwd/git_repo_root/git_branch and the origin columns.
What does this PR do?
When a long conversation auto-compresses, a new child session row is created (
parent_session_idset, parent gets
end_reason='compression') to continue the lineage.SessionDB._insert_session_rownever copied
cwd/git_repo_rootfrom the parent onto that child — worse,git_repo_rootwasn'teven in the
INSERT's column list at all, so it could only ever be set via a later,separate
update_session_cwd()call that the compression-fork path never makes. The same gapapplies to any other caller that creates a session with
parent_session_idset (delegate/subagentspawns, branch continuations) without explicitly passing
cwd/git_repo_root.The practical symptom: the Desktop project sidebar groups sessions into a Project by matching
sessions.cwdagainst the project's folder path, and only shows the tip of a lineage. Once acompression fork lands with
cwd = NULL, the project's entry silently disappears from thesidebar — and since compression can fire repeatedly on a long-lived conversation, each generation
compounds the loss (a lineage that compresses 9 times ships 9 opportunities for something to
finally re-populate
cwd, and none of them do, until a user manually patches the database).The fix adds
git_repo_rootto_insert_session_row'sINSERT/ON CONFLICT ... COALESCEcolumnset (matching the existing pattern already used for
cwd,model, etc. — only fill a still-NULLcolumn, never clobber a value an earlier writer set), and adds a single-hop backfill from the
immediate parent row, inside the same write transaction, whenever
parent_session_idis set. Amulti-generation chain still resolves correctly because each generation's own
create_sessioncallalready backfills from its own (by-then-already-resolved) immediate parent — no recursion needed.
Scope note on
git_repo_roottiming: gateway/Desktop sessions resolvegit_repo_rootasynchronously (a background git probe, separate from the synchronous
cwdset), so a childcreated in the narrow window before that probe finishes will inherit the parent's
cwdcorrectlybut not yet its
git_repo_root(parent doesn't have it yet either at that instant). This is not anew gap this PR introduces — the existing, independent
SessionDB.backfill_repo_roots()sweep(called wherever the project list resolves git roots) is keyed by
cwdvalue, not by lineage,so it already picks up any session sharing that
cwd— including a child that inherited it fromthis fix — once the probe resolves. No new code needed there.
How it works
flowchart TD A["Compression fires:\ncreate_session(parent_session_id=old_id)\n(no cwd/git_repo_root passed)"] --> B["_insert_session_row INSERT\n(git_repo_root now in column list)"] B --> C{"parent_session_id set?"} C -->|"before this PR"| D["child.cwd = NULL, child.git_repo_root = NULL\n→ project vanishes from Desktop sidebar"] C -->|"after this PR"| E["Same-transaction backfill UPDATE:\nchild.cwd/git_repo_root ← parent row\n(only fills NULLs, never overwrites)"] E --> F["child inherits parent's cwd/git_repo_root\n→ sidebar entry survives compression"]Related Issue
Fixes #64709
Type of Change
Changes Made
hermes_state.py:SessionDB._insert_session_rowgains agit_repo_rootparameter, adds it to theINSERT/ON CONFLICTcolumn set (COALESCEd, same pattern as every other backfill-only column here), and adds a same-transactionUPDATE ... WHERE id = ? AND parent_session_id IS NOT NULLthat backfillscwd/git_repo_rootfrom the parent row via a correlated subquery, only when they're still NULL.tests/test_hermes_state.py: three new tests inTestSessionLifecycle— direct parent→child inheritance, explicit childcwd/git_repo_rootis never overwritten, and a 3-generation chain (root → gen1 → gen2) all resolving to the root'scwd.Step-by-step
agent/conversation_compression.py's compression-forkcreate_session(...)call: it passesparent_session_id=old_session_idbut nocwd._insert_session_rowitself) over patching the one call site, per the issue's own suggested alternative — this protects every current and future caller that creates aparent_session_idchild without explicitcwd, not just the compression path.git_repo_rootto the column list/COALESCEset, then a same-transaction backfillUPDATEkeyed on the correlated parent lookup.SessionDBagainst a temp sqlite file./simplicio-review, 1 reviewer). It independently built a standalone in-memory sqlite3 repro of the exact correlated-subqueryUPDATEpattern to confirm it isn't a SQLite footgun, confirmed atomicity via_execute_write's single-transaction_doclosure, and traced delegate/subagent call sites to confirm inheritingcwdthere is desired (subagents run in-process, no isolated-directory concept to protect). It flagged one narrow edge case (the asyncgit_repo_rootprobe timing, described above) — I traced it to the existingbackfill_repo_roots()safety net and confirmed it's already covered, documented above rather than adding new code for it.Acceptance Criteria
cwd/git_repo_rootset, when a child session is created withparent_session_idpointing at it and nocwd/git_repo_rootof its own, then the child inherits both from the parent.cwd/git_repo_root, when it also hasparent_session_idset, then its own values are kept — never overwritten by the parent's.cwdset explicitly, when each generation is created in turn, then every generation ends up with root'scwd.create_session/cwd-related tests still pass.How to Test
main: create a session, set itscwdviaupdate_session_cwd, then create a second session withparent_session_idpointing at the first and nocwd— the second session'scwdisNULL.cwd(andgit_repo_root, if set).Tests Performed
python -m pytest tests/test_hermes_state.py -k "inherit or create_session or cwd" -q16 passedpython -m pytest tests/test_hermes_state.py -q369 passed in 43.70spython -m pytest tests/agent/test_compression_concurrent_fork.py -q23 passedpython -m pytest tests/test_lazy_session_regressions.py tests/hermes_state/test_session_archiving.py tests/hermes_state/test_resolve_resume_session_id.py tests/hermes_state/test_session_md_export.py -q36 passedpython scripts/check-windows-footguns.py hermes_state.py tests/test_hermes_state.pyNo Windows footguns found (2 file(s) scanned).python -m ruff check hermes_state.py tests/test_hermes_state.pyAll checks passed!assert None == '/work/repo'); all pass after the fixChecklist
Code
Documentation & Housekeeping
cli-config.yaml.exampleupdated if config keys changed — N/ACONTRIBUTING.md/AGENTS.mdupdated if architecture/workflow changed — N/AOut of scope (intentionally, flagged for a maintainer to decide)
The issue also suggests a one-time migration to backfill
cwd/git_repo_rooton already-affectedlineages (existing NULL-
cwdtips from before this fix, requiring a recursive/fixpoint sweep acrosspotentially many generations). That's a data-repair concern distinct from this code fix (which
prevents the problem going forward) — left out to keep this PR to the smallest change that resolves
the reported bug; happy to follow up with a migration if maintainers want one.
Screenshots / Logs