From b58146cec3c655e487ebc649350766f4e72ee7cc Mon Sep 17 00:00:00 2001 From: Jeff Date: Sat, 19 Sep 2026 22:51:26 -0700 Subject: [PATCH] fix(acp): persist session cwd and git metadata to the sessions table ACP sessions stored their workspace only inside the model_config JSON blob (a correct choice when the cwd column did not exist yet), so Desktop, the Projects sidebar and hermes sessions list showed every editor session as unassigned. create_session now passes cwd, an existing row (the live path, since the agent flushes the transcript incrementally) gets the column promoted via update_session_cwd, update_cwd() moves it on reopen, and git branch/root are probed off the interactive path under the same generation contract tui_gateway/session_workdir.py uses. backfill_acp_session_cwd promotes model_config.cwd for rows minted before this change. Squashed from the five commits of #115707; the accidentally committed Windows cache files under %SystemDrive% are dropped. --- acp_adapter/session.py | 60 +++++++++++- hermes_state_sessions.py | 19 ++++ tests/acp_adapter/test_session_cwd_column.py | 91 +++++++++++++++++++ .../test_session_db_acp_cwd_backfill.py | 46 ++++++++++ 4 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 tests/acp_adapter/test_session_cwd_column.py create mode 100644 tests/hermes_state/test_session_db_acp_cwd_backfill.py diff --git a/acp_adapter/session.py b/acp_adapter/session.py index 0508ba0eed..56afcb46f5 100644 --- a/acp_adapter/session.py +++ b/acp_adapter/session.py @@ -252,6 +252,11 @@ class SessionManager: state.cwd = cwd _register_task_cwd(session_id, cwd) self._persist(state) + # Promote the authoritative column and claim an ordering generation, the + # same contract tui_gateway/session_workdir.py uses: a git probe may only + # publish while its generation is still current, so a slow probe for a + # previous workspace cannot overwrite a newer claim (A -> B -> A). + self._schedule_git_metadata(state, self._claim_cwd_generation(state)) return state def save_session(self, session_id: str) -> None: @@ -310,12 +315,21 @@ class SessionManager: # Empty editor probes stay ephemeral; copied fork history persists. return db.create_session(session_id=state.session_id, source="acp", model=model_str, - model_config=session_meta) + model_config=session_meta, cwd=state.cwd or None) else: try: db.update_session_meta(state.session_id, json.dumps(session_meta), model_str) except Exception: logger.debug("Failed to update ACP session metadata", exc_info=True) + # The create branch above is not the live path: an agent that owns + # persistence to this same DB flushes the transcript incrementally, + # so the row already exists by the time we get here. + # update_session_meta touches only model_config/model, so without + # this promotion the column stays NULL for the whole session and + # Desktop files it as unassigned. Claiming a generation keeps the + # A -> B -> A ordering contract shared with update_cwd(). + if state.cwd: + self._schedule_git_metadata(state, self._claim_cwd_generation(state)) # An agent that owns persistence to this same DB already flushed the transcript # incrementally (append_message) and keeps pre-compaction turns as archived @@ -341,6 +355,50 @@ class SessionManager: except Exception: logger.warning("Failed to persist ACP session %s", state.session_id, exc_info=True) + def _claim_cwd_generation(self, state: SessionState) -> Optional[int]: + """Write the cwd column and return its new git-metadata generation. + + Returns None when the row does not exist yet (a contentless session is + deliberately ephemeral until it has history) or the DB is unavailable. + """ + db = self._get_db() + if db is None or not state.cwd: + return None + try: + return db.update_session_cwd(state.session_id, state.cwd) + except Exception: + logger.debug("Failed to persist ACP session cwd column for %s", + state.session_id, exc_info=True) + return None + + def _schedule_git_metadata(self, state: SessionState, generation: Optional[int]) -> None: + """Probe git off the critical path and publish under ``generation``. + + ``session/new`` is on the editor's interactive path; ``git rev-parse`` + on a cold or networked filesystem is not something to put in front of + the user. The generation guard in ``publish_session_git_metadata`` + means a slow probe for a previous workspace is dropped rather than + applied to the new one. + """ + if not generation or not state.cwd: + return + + session_id, cwd = state.session_id, state.cwd + + def _run() -> None: + try: + from tui_gateway import git_probe + branch, root = git_probe.branch(cwd), git_probe.common_repo_root(cwd) + if not (branch or root): + return + db = self._get_db() + if db is not None: + db.publish_session_git_metadata(session_id, cwd, generation, branch, root) + except Exception: + logger.debug("Failed to publish ACP git metadata for %s", session_id, exc_info=True) + + threading.Thread(target=_run, name=f"acp-git-meta-{session_id[:8]}", daemon=True).start() + def _restore(self, session_id: str) -> Optional[SessionState]: """Load an ACP session from the database into memory, recreating the AIAgent.""" db = self._get_db() diff --git a/hermes_state_sessions.py b/hermes_state_sessions.py index bb55b120f2..ef5a9ed4b0 100644 --- a/hermes_state_sessions.py +++ b/hermes_state_sessions.py @@ -832,6 +832,25 @@ class SessionSessionsMixin: (stamp,), ) or 0) + def backfill_acp_session_cwd(self) -> int: + """Promote ``model_config.cwd`` into the cwd column for ACP rows lacking one. + + ACP sessions minted before the adapter populated the column still carry + their workspace inside ``model_config``, written by the same adapter that + knew the real directory — so this is a record being promoted, not a guess. + Only fills NULL/empty; an explicit column value always wins. Returns the + number of rows changed. + """ + return int(self._write_rowcount( + """UPDATE sessions + SET cwd = json_extract(model_config, '$.cwd') + WHERE source = 'acp' + AND COALESCE(cwd, '') = '' + AND json_valid(model_config) + AND COALESCE(json_extract(model_config, '$.cwd'), '') != ''""", + (), + ) or 0) + def _set_lineage_column(self, column: str, session_id: str, value: Any) -> bool: """Set one ``sessions`` column across a whole compression lineage: Desktop projects roots forward to their tip, so updating only the tip would let the root resurrect it on refresh.""" diff --git a/tests/acp_adapter/test_session_cwd_column.py b/tests/acp_adapter/test_session_cwd_column.py new file mode 100644 index 0000000000..43612d87e0 --- /dev/null +++ b/tests/acp_adapter/test_session_cwd_column.py @@ -0,0 +1,91 @@ +"""ACP sessions must populate the cwd COLUMN, not only model_config. + +Hermes Desktop's Projects sidebar, ``hermes sessions list``, and every +profile-keyed consumer group sessions off ``sessions.cwd``. The ACP adapter +recorded the workspace only inside the ``model_config`` JSON blob, so every +editor-created session (VS Code, Antigravity, Zed, JetBrains, Buzz) rendered +as unassigned -- "Workspace: --" -- even though its transcript was intact. + +``_insert_session_row`` already accepted ``cwd``/``git_repo_root``; the ACP +adapter simply never passed them. +""" +import json +from types import SimpleNamespace + +from acp_adapter.session import SessionManager +from hermes_state import SessionDB + + +def _manager(db): + return SessionManager(db=db, agent_factory=lambda: SimpleNamespace(model="fixture")) + + +def test_created_session_records_cwd_in_its_own_column(tmp_path): + db = SessionDB(tmp_path / "state.db") + workspace = tmp_path / "hs-wwd" + workspace.mkdir() + manager = _manager(db) + + state = manager.create_session(cwd=str(workspace)) + # An empty session stays ephemeral by design (test_empty_session_persistence); + # content is what mints the row. + state.history.append({"role": "user", "content": "hello"}) + manager.save_session(state.session_id) + + row = db.get_session(state.session_id) + assert row["cwd"] == state.cwd, "cwd column must carry the workspace" + # The JSON copy stays -- _restore() rebuilds the agent from it. + assert json.loads(row["model_config"])["cwd"] == state.cwd + db.close() + + +def test_cwd_is_promoted_when_the_row_already_exists(tmp_path): + """The create branch is not the live path. + + An agent that owns persistence to this same DB flushes the transcript + incrementally, so the sessions row is already there by the time the adapter + persists. ``_persist`` then takes its ``else`` branch, and + ``update_session_meta`` writes only ``model_config``/``model`` -- leaving the + column NULL for the entire life of a real editor session. + """ + db = SessionDB(tmp_path / "state.db") + workspace = tmp_path / "app-1" + workspace.mkdir() + manager = _manager(db) + + state = manager.create_session(cwd=str(workspace)) + state.history.append({"role": "user", "content": "hello"}) + # Stand in for the agent's own incremental flush: the row exists, and it + # knows nothing about the ACP workspace. + db.create_session(session_id=state.session_id, source="acp", model="fixture") + assert db.get_session(state.session_id)["cwd"] in (None, "") + + manager.save_session(state.session_id) + + row = db.get_session(state.session_id) + assert row["cwd"] == state.cwd, ( + "an existing row must still have its cwd column promoted" + ) + db.close() + + +def test_reopening_in_another_workspace_moves_the_cwd_column(tmp_path): + db = SessionDB(tmp_path / "state.db") + first, second = tmp_path / "old", tmp_path / "new" + first.mkdir() + second.mkdir() + manager = _manager(db) + + state = manager.create_session(cwd=str(first)) + state.history.append({"role": "user", "content": "hello"}) + manager.save_session(state.session_id) + + manager.update_cwd(state.session_id, str(second)) + + row = db.get_session(state.session_id) + assert row["cwd"] == state.cwd + assert str(second) in row["cwd"] + # A moved workspace must claim a fresh probe generation, so a slow git + # probe for the OLD cwd cannot publish onto the new one. + assert (row["git_metadata_generation"] or 0) >= 1 + db.close() diff --git a/tests/hermes_state/test_session_db_acp_cwd_backfill.py b/tests/hermes_state/test_session_db_acp_cwd_backfill.py new file mode 100644 index 0000000000..35e9b229fe --- /dev/null +++ b/tests/hermes_state/test_session_db_acp_cwd_backfill.py @@ -0,0 +1,46 @@ +"""Repair ACP rows whose workspace lives only in the model_config JSON. + +ACP sessions minted before the ``cwd`` column was populated still carry the +workspace inside ``model_config``, so promoting it is a lossless repair rather +than a guess. On a real install every ACP session predating the fix was +affected, and every one was recoverable this way. +""" +import pytest + +from hermes_state import SessionDB + + +@pytest.fixture +def db(tmp_path): + store = SessionDB(tmp_path / "state.db") + yield store + store.close() + + +def test_backfill_promotes_cwd_from_model_config(db): + """Promotes only rows that need it, and only once.""" + db.create_session(session_id="legacy", source="acp", model="m", + model_config={"cwd": "/work/hs-wwd"}) + # A non-ACP row is not this repair's business even if it has a JSON cwd, + # and an ACP row with no JSON cwd has nothing to promote. + db.create_session(session_id="cli", source="cli", model="m", + model_config={"cwd": "/somewhere"}) + db.create_session(session_id="bare", source="acp", model="m", + model_config={"provider": "anthropic"}) + + assert db.backfill_acp_session_cwd() == 1 + assert db.get_session("legacy")["cwd"] == "/work/hs-wwd" + assert db.get_session("cli")["cwd"] in (None, "") + assert db.get_session("bare")["cwd"] in (None, "") + + # Idempotent: a second run is a no-op, not a rewrite. + assert db.backfill_acp_session_cwd() == 0 + + +def test_backfill_never_overwrites_an_existing_cwd(db): + """An explicit column value always wins over the JSON copy.""" + db.create_session(session_id="fine", source="acp", model="m", + cwd="/real/path", model_config={"cwd": "/stale/json/path"}) + + assert db.backfill_acp_session_cwd() == 0 + assert db.get_session("fine")["cwd"] == "/real/path"