diff --git a/emrg/agent.py b/emrg/agent.py new file mode 100644 index 00000000..3f0d6cf8 --- /dev/null +++ b/emrg/agent.py @@ -0,0 +1,82 @@ +"""The Agent: the one directory a session runs in. + +An `Agent` is a directory and nothing else. It is not a repository, a workspace or a +registry entry — those are facts *about* a directory that other layers keep, and +letting one word answer all of them is what made the older vocabulary ambiguous: a +name could mean the directory, the alias it is registered under, or a field of a +record. + +What the class owns is the layout derived from that directory: + + /.emrg emrg_dir + /.emrg/sessions sessions_dir + /.emrg/memory memory_dir + +Nothing else lives here on purpose. An Agent runs no model, keeps no history and knows +nothing about sandbox tiers; those belong to the session that runs in it and to the +daemon that schedules it, and a field added here would be a second place to keep a fact +that already has an owner. The class exists for one reason: a path spelled out at every +site that needs it is one convention per site, so a layout change becomes a search; one +object with one derivation makes it an edit. + +`base_dir` is stored **exactly as given** — no `resolve()`, no `expanduser()`, no +trailing-slash normalisation, and a `str` stays a `str`. The value is compared for +equality downstream against strings that are already recorded elsewhere (what a client +sent, what a registry entry kept, what `meta.json` stored). Normalising it here would +make two spellings of one directory disagree with every reader that normalised only its +own side. A symlink, a trailing slash and a `..` all survive unchanged. +""" + +from __future__ import annotations + +from pathlib import Path + + +class Agent: + """One directory a session runs in: holds it, and derives its layout.""" + + def __init__(self, base_dir: Path | str | Agent) -> None: + # Idempotent, so a caller holding an Agent cannot end up with a second wrapper + # around it. Wrapping twice would be harmless only while every derivation + # agreed with every other, which is the assumption this class exists to remove. + if isinstance(base_dir, Agent): + base_dir = base_dir.base_dir + self.base_dir = base_dir + + @property + def emrg_dir(self) -> Path: + """``/.emrg`` — everything this Agent keeps lives under here.""" + return Path(self.base_dir) / ".emrg" + + @property + def sessions_dir(self) -> Path: + """``/.emrg/sessions`` — one directory per session, named by its id.""" + return self.emrg_dir / "sessions" + + @property + def memory_dir(self) -> Path: + """``/.emrg/memory`` — memory shared by every session in this Agent.""" + return self.emrg_dir / "memory" + + def __eq__(self, other: object) -> bool: + # Value equality on the stored value, with no normalisation on either side, so + # `Agent("x")` and `Agent(Path("x"))` stay different objects — exactly as the + # two paths they carry are different values. + if not isinstance(other, Agent): + return NotImplemented + return self.base_dir == other.base_dir + + def __hash__(self) -> int: + return hash(self.base_dir) + + def __repr__(self) -> str: + return f"Agent({self.base_dir!r})" + + +def as_agent(value: Agent | Path | str) -> Agent: + """The Agent for a directory, whether the caller holds one or only its path. + + This is what lets every existing caller keep passing a path: the one place that + knows "a path here means an Agent" is this function, instead of each of them. + """ + return value if isinstance(value, Agent) else Agent(value) diff --git a/emrg/memory.py b/emrg/memory.py index ae5dbe61..e5349928 100644 --- a/emrg/memory.py +++ b/emrg/memory.py @@ -35,6 +35,7 @@ # restated: the boundary is one fact, and a second spelling of it is a second boundary # that can drift. `emrg.tools.base` imports only `emrg.server.tool_types`, so this adds # no cycle — measured by importing this module from a fresh interpreter. +from emrg.agent import as_agent from emrg.tools.base import non_regular_kind logger = logging.getLogger(__name__) @@ -1434,7 +1435,7 @@ class ProjectMemoryStore(MemoryStore): """Project-level memory: ``/.emrg/memory/`` — shared across all sessions.""" def __init__(self, cwd: Path): - super().__init__(cwd / ".emrg" / "memory", scope="project") + super().__init__(as_agent(cwd).memory_dir, scope="project") class SessionMemoryStore(MemoryStore): diff --git a/emrg/server/daemon.py b/emrg/server/daemon.py index 3b5522cd..d6021f66 100644 --- a/emrg/server/daemon.py +++ b/emrg/server/daemon.py @@ -2845,7 +2845,7 @@ def _collect_memory_data(self, session: Session) -> dict[str, Any] | None: """Read memory indexes, return structured data for template.""" data: dict[str, Any] = {"has_memories": False} - project_dir = session.cwd / ".emrg" / "memory" + project_dir = session.agent.memory_dir pindex_path = project_dir / "MEMORY.md" if pindex_path.exists(): data["project_memory_index"] = self._index_for_prompt(pindex_path) @@ -8387,7 +8387,7 @@ async def _reflect(): # de-duplication that existed only to keep it from being named twice # went with it. subjects = [ - session.cwd / ".emrg" / "memory" / "MEMORY.md", + session.agent.memory_dir / "MEMORY.md", store.index_path, ] hygiene_note += _memory_index_compaction_note(subjects) diff --git a/emrg/session.py b/emrg/session.py index f34da7cd..e70eeb2a 100644 --- a/emrg/session.py +++ b/emrg/session.py @@ -23,6 +23,7 @@ from pathlib import Path from typing import Iterable +from emrg.agent import Agent, as_agent from emrg.memory import NotARegularFile, SessionMemoryStore from emrg.sandbox.grants import GRANT_REGISTRY, RootGrant from emrg.sandbox.policy import SANDBOX_MODES @@ -53,7 +54,7 @@ def generate_session_id(cwd: Path) -> str: """ now = datetime.now() prefix = f"s_{now.strftime('%y%m%d_%H%M')}_" - sessions_dir = cwd / ".emrg" / "sessions" + sessions_dir = as_agent(cwd).sessions_dir for _ in range(100): suffix = secrets.token_hex(4) sid = prefix + suffix @@ -354,17 +355,28 @@ def session_dir(cwd: Path, session_id: str) -> Path: The id is not validated here — ``is_safe_session_id`` decides that, and its caller decides whether an unsafe id is refused or merely not a directory (see its docstring for what an unchecked id did). + + The expression itself belongs to ``Agent.sessions_dir``: this function is the + signature the callers already use, not a second convention beside the Agent's. """ - return Path(cwd) / ".emrg" / "sessions" / session_id + return as_agent(cwd).sessions_dir / session_id class Session: - """Manages a single conversation session on disk.""" + """Manages a single conversation session on disk. + + A session runs *in* an Agent — one directory. Every path it keeps is derived from + that Agent, so the session never spells a ``.emrg`` path itself: a second derivation + is a second convention, waiting to disagree with the first. + """ - def __init__(self, session_id: str, cwd: Path) -> None: + def __init__(self, session_id: str, agent: Agent | Path | str) -> None: self.session_id = session_id - self.cwd = cwd - self._dir = session_dir(cwd, session_id) + # A caller may still hand over the path — every factory below does — and the + # wrapping happens here rather than at each of them, so exactly one place knows + # that a path means an Agent. + self._agent = as_agent(agent) + self._dir = session_dir(self._agent, session_id) self._dir.mkdir(parents=True, exist_ok=True) # Initialize memory subdirectory @@ -470,6 +482,25 @@ def save_image(self, data: bytes, label: str = "") -> str: logger.info("image saved: %s (%d bytes)", filename, len(data)) return filename + # ── Identity ────────────────────────────────────────────── + + @property + def agent(self) -> Agent: + """The Agent — the directory — this session runs in.""" + return self._agent + + @property + def cwd(self) -> Path: + """The directory this session runs in: ``agent.base_dir``, read-only. + + A property rather than a field because the directory is decided once, when the + session is built: a second assignment could put a session's files and its + ``cwd`` in different places with nothing recording which one is right. The value + is answered verbatim — a caller that passed a string reads that string back — + so every existing reader of ``session.cwd`` is unchanged. + """ + return self._agent.base_dir + # ── Factory methods ─────────────────────────────────────── @classmethod @@ -1238,7 +1269,7 @@ def list_sessions(cwd: Path) -> list[dict]: session_id, created_at, updated_at, cwd, message_count, compact_count, last_compact_at, title (if set) """ - sessions_dir = cwd / ".emrg" / "sessions" + sessions_dir = as_agent(cwd).sessions_dir if not sessions_dir.exists(): return [] diff --git a/emrg/sessions_index.py b/emrg/sessions_index.py index 9b7da345..4c6a998f 100644 --- a/emrg/sessions_index.py +++ b/emrg/sessions_index.py @@ -22,6 +22,7 @@ import tempfile from pathlib import Path +from emrg.agent import as_agent from emrg.config import config_dir # The "is this path a regular file" rule is the tool layer's, reached at a path. Imported @@ -185,7 +186,7 @@ def _read_meta_session_id(meta_path: Path) -> str | None: def _iter_project_sessions(project_path: Path): """Yield (session_id, session_dir) from /.emrg/sessions/*/meta.json.""" - sessions_dir = project_path / ".emrg" / "sessions" + sessions_dir = as_agent(project_path).sessions_dir if not sessions_dir.is_dir(): return for entry in sorted(sessions_dir.iterdir()): diff --git a/tests/test_agent.py b/tests/test_agent.py new file mode 100644 index 00000000..cf4a331a --- /dev/null +++ b/tests/test_agent.py @@ -0,0 +1,131 @@ +"""The Agent is one directory, and it stores that directory exactly as given. + +Two properties these tests exist for: + +* the value is **never normalised** — ``resolve()``, ``expanduser()`` and a trailing + slash all change what a downstream reader compares against, so none of them may + happen here; +* the ``.emrg`` layout is derived in **one place** — the assertions below pin the + literal layout rather than comparing two things that both ask the Agent, because an + equality between two derivations stays green under a mutated derivation. That is what + makes an arm that changes the layout visible. +""" + +from __future__ import annotations + +from emrg.agent import Agent, as_agent +from emrg.memory import ProjectMemoryStore +from emrg.session import Session, generate_session_id, session_dir +from emrg.sessions_index import _iter_project_sessions + + +class TestBaseDirIsStoredAsGiven: + """No resolve, no expanduser, no tidying — the value a caller gave survives.""" + + def test_a_trailing_slash_survives(self): + assert Agent("a/b/").base_dir == "a/b/" + + def test_a_dotdot_survives(self): + """`resolve()` would collapse this to `b`; the string must not move.""" + assert Agent("a/../b").base_dir == "a/../b" + + def test_a_home_placeholder_is_not_expanded(self): + """`expanduser()` would answer a real home directory; the `~` must survive.""" + assert Agent("~/project").base_dir == "~/project" + + def test_the_type_is_part_of_the_value(self, tmp_path): + """A str stays a str and a Path stays a Path: two spellings are two values.""" + assert Agent(str(tmp_path)).base_dir == str(tmp_path) + assert Agent(tmp_path).base_dir == tmp_path + + def test_wrapping_an_agent_answers_the_same_agent(self, tmp_path): + """A second wrapper would be a second base_dir, which is the drift to avoid.""" + agent = Agent(tmp_path) + assert as_agent(agent) is agent + assert Agent(agent).base_dir == tmp_path + assert Agent(agent) == agent + + def test_equality_does_not_normalise_either_side(self, tmp_path): + assert Agent(tmp_path) == Agent(tmp_path) + assert Agent(tmp_path) != Agent(tmp_path / "other") + assert not Agent(tmp_path) == tmp_path + + +class TestTheLayoutIsDerivedInOnePlace: + def test_the_three_directories(self, tmp_path): + agent = Agent(tmp_path) + assert agent.emrg_dir == tmp_path / ".emrg" + assert agent.sessions_dir == tmp_path / ".emrg" / "sessions" + assert agent.memory_dir == tmp_path / ".emrg" / "memory" + + def test_the_derivation_carries_the_value_it_was_handed(self): + """A `~` and a `..` reach the filesystem as they were written.""" + sessions_dir = Agent("~/a/../b").sessions_dir + assert str(sessions_dir).startswith("~") + assert ".." in str(sessions_dir) + + +class TestASessionRunsInAnAgent: + def test_a_path_and_an_agent_build_the_same_session(self, tmp_path): + from_path = Session("s1", tmp_path) + from_agent = Session("s1", Agent(tmp_path)) + + assert from_path.agent == from_agent.agent + assert from_path._dir == from_agent._dir == tmp_path / ".emrg" / "sessions" / "s1" + assert from_path.cwd == from_agent.cwd == tmp_path + + def test_the_session_directory_is_the_agents_own_derivation(self, tmp_path): + session = Session("s1", tmp_path) + assert session._dir == Agent(tmp_path).sessions_dir / "s1" + + def test_cwd_is_read_only(self, tmp_path): + """A second assignment could move the files and the cwd apart, silently.""" + session = Session("s1", tmp_path) + try: + session.cwd = tmp_path / "elsewhere" + except AttributeError: + pass + else: + raise AssertionError("cwd accepted an assignment") + assert session.cwd == tmp_path + + def test_cwd_answers_the_value_it_was_given(self, tmp_path): + """A caller that handed over a string reads that string back, not a Path.""" + session = Session("s1", str(tmp_path)) + assert session.cwd == str(tmp_path) + assert session._dir == tmp_path / ".emrg" / "sessions" / "s1" + + +class TestTheCallersAskTheAgent: + """The four sites that used to spell `/.emrg/...` themselves.""" + + def test_session_dir_answers_the_literal_layout(self, tmp_path): + assert session_dir(tmp_path, "s1") == tmp_path / ".emrg" / "sessions" / "s1" + + def test_session_dir_takes_an_agent_as_well_as_a_path(self, tmp_path): + assert session_dir(Agent(tmp_path), "s1") == tmp_path / ".emrg" / "sessions" / "s1" + + def test_generate_session_id_reserves_an_id_it_does_not_create(self, tmp_path): + sid = generate_session_id(tmp_path) + assert sid.startswith("s_") + assert not (Agent(tmp_path).sessions_dir / sid).exists() + + def test_list_sessions_answers_the_same_rows_for_either_spelling(self, tmp_path): + Session.create_with_id("s1", tmp_path) + from_agent = Session.list_sessions(Agent(tmp_path)) + assert from_agent == Session.list_sessions(tmp_path) + assert [row["session_id"] for row in from_agent] == ["s1"] + + def test_the_project_memory_store_is_the_agents_memory_dir(self, tmp_path): + store = ProjectMemoryStore(tmp_path) + assert store.directory == Agent(tmp_path).memory_dir + assert store.directory == tmp_path / ".emrg" / "memory" + + def test_the_session_index_walk_reads_the_agents_sessions_dir(self, tmp_path): + session_dir_path = tmp_path / ".emrg" / "sessions" / "s1" + session_dir_path.mkdir(parents=True) + (session_dir_path / "meta.json").write_text( + '{"session_id": "s1"}', encoding="utf-8" + ) + + assert list(_iter_project_sessions(tmp_path)) == [("s1", str(session_dir_path))] diff --git a/tests/test_prompt_templates.py b/tests/test_prompt_templates.py index 45284e48..d04522f6 100644 --- a/tests/test_prompt_templates.py +++ b/tests/test_prompt_templates.py @@ -44,6 +44,7 @@ import pytest import yaml +from emrg.agent import Agent from emrg.protocol import InstanceIdentity from emrg.sandbox.policy import SandboxPolicy from emrg.sandbox.roots import canonical_path, writable_roots @@ -618,12 +619,19 @@ def test_prompt_memory_writes_land_where_the_daemon_loads_them(tmp_path, monkeyp # (3) That workspace's `.emrg/memory` is what the daemon embeds as project memory. memory_root.mkdir(parents=True, exist_ok=True) (memory_root / "MEMORY.md").write_text("# Index\n", encoding="utf-8") - # `_collect_memory_data` touches only `session.cwd` and `session.memory_dir`, and - # reaches the module-level `INDEX_SIZE_WARN` through a method — so an uninitialised - # instance is enough, and no daemon, socket or event loop is started to measure this. + # `_collect_memory_data` reads the session's Agent (for the project half) and + # `session.memory_dir` (for the session half), and reaches the module-level + # `INDEX_SIZE_WARN` through a method — so an uninitialised instance is enough, and no + # daemon, socket or event loop is started to measure this. The stand-in carries an + # `Agent` rather than a bare path because that is what a session holds: its `cwd` + # *is* the Agent's `base_dir`, so the two facts below cannot disagree. daemon = EmrgServer.__new__(EmrgServer) data = daemon._collect_memory_data( - SimpleNamespace(cwd=source_dir, memory_dir=tmp_path / "session-memory") + SimpleNamespace( + agent=Agent(source_dir), + cwd=source_dir, + memory_dir=tmp_path / "session-memory", + ) ) assert data, ( "the daemon embedded no project index for a session whose cwd carries one — " diff --git a/tests/test_session.py b/tests/test_session.py index 7348c1cb..06fb1565 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -152,17 +152,53 @@ def _spells_a_session_directory(tree: ast.AST) -> list[int]: return sorted(found) -def test_the_directory_a_session_lives_in_has_one_producer(): - """The rule is `session_dir`, and no other module may build the path itself. +def _spells_a_layout_leaf(tree: ast.AST) -> list[int]: + """Line numbers of expressions that build `/.emrg/{sessions,memory}`. + + The same walk as `_spells_a_session_directory`, with the second component widened to + the whole layout: a module that spells `/.emrg/sessions` or + `/.emrg/memory` is holding a copy of the Agent's derivation whether or not + it goes on to name a leaf inside it. + + A formatted string is not a path builder and is deliberately not walked here: a + prompt that *tells a reader* where memory lives spells the same two components, and + flagging it would report a clean tree as dirty. The spelling that hides the + components in a format string — `cwd / f".emrg/sessions/{sid}"` — is still caught, + because the division that builds the path is the node that carries them. That + exemption is measured, not assumed: see + `test_a_prompt_that_names_the_layout_is_not_a_producer`. + """ + found: set[int] = set() + for node in ast.walk(tree): + if not isinstance(node, (ast.BinOp, ast.Call)): + continue + parts = _path_components(node) + if parts is None: + continue + for i in range(len(parts) - 1): + if parts[i] == ".emrg" and parts[i + 1] in ("sessions", "memory"): + found.add(node.lineno) + break + return sorted(found) + - Dozens of call sites need this directory, and each one that spelled it out - was a copy that a change to the layout would have to find. They agreed, so - nothing was broken — which is exactly what makes a copy invisible to a - behavioural test: five of them survived a refactor that claimed to leave one - producer, because every copy still produced the right answer. +def test_the_directory_a_session_lives_in_has_one_producer(): + """The rule is `Agent.sessions_dir`; no module may build the layout itself. + + Dozens of call sites need a session's directory, and each one that spelled it out + was a copy that a change to the layout would have to find. They agreed, so nothing + was broken — which is exactly what makes a copy invisible to a behavioural test: + five of them survived a refactor that claimed to leave one producer, because every + copy still produced the right answer. + + The producer is an object now, so this guard's subject moved with it and its + assertion got wider rather than weaker: **no module may spell the layout at all** — + neither the session leaf nor either half of the derivation — because the one place + that does is `Agent`. That the Agent's own literals *are* the layout is pinned + behaviourally in `tests/test_agent.py`, which is the half no source scan can see. """ package_root = Path(__file__).resolve().parents[1] / "emrg" - producer, offenders = [], [] + spellings: list[str] = [] for path in sorted(package_root.rglob("*.py")): rel = path.relative_to(package_root).as_posix() # A module elsewhere in the package carries an invalid escape sequence in a @@ -172,15 +208,12 @@ def test_the_directory_a_session_lives_in_has_one_producer(): with warnings.catch_warnings(): warnings.simplefilter("ignore", SyntaxWarning) tree = ast.parse(path.read_bytes()) - for lineno in _spells_a_session_directory(tree): - (producer if rel == "session.py" else offenders).append(f"{rel}:{lineno}") + for lineno in _spells_a_layout_leaf(tree): + spellings.append(f"{rel}:{lineno}") - assert offenders == [], ( - "these build /.emrg/sessions/ themselves instead of asking " - f"session_dir(): {offenders} — a second producer is a second rule" - ) - assert len(producer) == 1, ( - f"expected exactly one producer in session.py, found {producer}" + assert spellings == [], ( + "these build a `/.emrg/{sessions,memory}` path themselves instead of " + f"asking the Agent: {spellings} — a second producer is a second rule" ) @@ -231,6 +264,37 @@ def test_the_sessions_root_is_not_a_session_directory(expression): assert _spells_a_session_directory(ast.parse(expression)) == [] +@pytest.mark.parametrize( + "expression", + [ + 'cwd / ".emrg" / "sessions"', + 'Path(cwd, ".emrg", "memory")', + 'os.path.join(cwd, ".emrg", "sessions")', + 'cwd / f".emrg/sessions/{sid}"', + 'session.cwd / ".emrg" / "memory" / "MEMORY.md"', + ], +) +def test_every_builder_of_the_layout_is_found(expression): + """Both halves of the derivation, in every joining syntax a caller uses.""" + assert _spells_a_layout_leaf(ast.parse(expression)) == [1] + + +@pytest.mark.parametrize( + "expression", + [ + # A prompt that tells a reader where memory lives is prose, not a producer. + 'f"Project: `{cwd}/.emrg/memory/` (project-scope, cross-session)"', + # The host's install root is a different fact from a directory's layout. + 'Path.home() / ".emrg" / "install"', + 'cwd / ".emrg" / "config.toml"', + 'agent.memory_dir / "MEMORY.md"', + ], +) +def test_a_prompt_that_names_the_layout_is_not_a_producer(expression): + """The exemption above, measured: naming the layout is not building it.""" + assert _spells_a_layout_leaf(ast.parse(expression)) == [] + + def _javascript_without_comments(source: str) -> str: """`source` with every comment blanked to spaces, newlines and strings kept.