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.
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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."""
|
||||
|
||||
91
tests/acp_adapter/test_session_cwd_column.py
Normal file
91
tests/acp_adapter/test_session_cwd_column.py
Normal file
@@ -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()
|
||||
46
tests/hermes_state/test_session_db_acp_cwd_backfill.py
Normal file
46
tests/hermes_state/test_session_db_acp_cwd_backfill.py
Normal file
@@ -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"
|
||||
Reference in New Issue
Block a user