From a933c06016457ed826fa68a571baff3fce5a15c8 Mon Sep 17 00:00:00 2001 From: "kiloloop-release[bot]" <269344367+kiloloop-release[bot]@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:26:31 -0700 Subject: [PATCH] feat: agent-memory v0.1.4 Fixes six sync, startup and doctor defects present in 0.1.3: - `pull` and `push` sync one at a time per home and never abort a rebase they did not start. - `pull` with only unpushed commits no longer also reports "already synced". - `startup --pull` no longer publishes a debrief or event another agent wrote. - `pull` and `push` update past untracked files the incoming commits do not touch, so SessionStart keeps pulling. - `doctor` on Python 3.14 reports a debrief record it cannot inspect as unreadable, not as an irregular file. - `enable` starts a new memory repository on `main` even where git's `init.defaultBranch` is `master`. CI also runs on Python 3.14. --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 11 ++ docs/commands.md | 16 ++- src/agent_memory/__init__.py | 2 +- src/agent_memory/doctor.py | 16 +-- src/agent_memory/startup.py | 47 +++++++- src/agent_memory/sync.py | 221 ++++++++++++++++++++++++++++------- tests/test_doctor.py | 34 +++--- tests/test_startup.py | 78 +++++++++++++ tests/test_sync.py | 169 ++++++++++++++++++++++++++- 10 files changed, 521 insertions(+), 75 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2e9af2..faa0b58 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.10", "3.11", "3.12", "3.13"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] steps: - name: Check out repository diff --git a/CHANGELOG.md b/CHANGELOG.md index ff84f47..0e08ead 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ All notable changes to agent-memory are documented in this file. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## 0.1.4 - 2026-10-26 + +### Fixed + +- `pull` and `push` sync one at a time per home and never abort a rebase they did not start ([sync](docs/commands.md#sync)). +- `pull` with only unpushed commits no longer also reports "already synced" ([sync](docs/commands.md#sync)). +- `startup --pull` no longer publishes a debrief or event another agent wrote ([startup](docs/commands.md#startup)). +- `pull` and `push` update past untracked files the incoming commits do not touch, so SessionStart keeps pulling ([sync](docs/commands.md#sync)). +- `doctor` on Python 3.14 reports a debrief record it cannot inspect as unreadable, not as an irregular file ([doctor](docs/commands.md#doctor)). +- `enable` starts a new memory repository on `main` even where git's `init.defaultBranch` is `master` ([sync](docs/commands.md#sync)). + ## 0.1.3 - 2026-09-29 ### Fixed diff --git a/docs/commands.md b/docs/commands.md index e964343..4be9806 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -74,7 +74,9 @@ credentials. `org init` is the org tier alone, for a home that already exists. ## Sync -`enable` makes the home a git repository of its own, puts the sync allowlist at +`enable` makes the home a git repository of its own on branch `main`, whatever +git's `init.defaultBranch` says (a home that is already a repository keeps its +branch), puts the sync allowlist at the head of its `.gitignore` as a managed block (existing lines are kept, lines an earlier block carried are retired, and the write comes with a before/after receipt that names them), drops the sync marker, and makes @@ -84,15 +86,20 @@ one commit. With `--remote`, `enable` also pushes that initial commit. rebase local commits onto upstream when behind, then push. Anything else staged stays staged and uncommitted. If unrelated changes remain when an update is needed, the command reports the paths and stops without stashing -them. A rebase conflict aborts, names the conflicting paths, and retains the +them, except untracked files the incoming commits do not touch: those stay +in place through the update and are named. A rebase conflict aborts, names the conflicting paths, and retains the local commit; differing same-path additions are never automatically resolved. Identical additions already upstream need no duplicate commit after rebase. Local merge commits require manual integration so their resolutions cannot be lost. An existing Git operation must be finished before either verb can run. +One `pull` or `push` syncs a home at a time: a second waits up to two minutes +for the first to finish, and neither aborts a rebase it did not start. A plain `pull` updates a clean home and reports any commits still ahead; a dirty home requires `--agent NAME`. `startup --pull` supplies its runtime name as the agent, so SessionStart can also resolve dirty, behind homes. +It leaves out a debrief or event that names another agent as its author +([startup](#startup)). A failed fetch or rejected push leaves newly committed work local. A credential helper blocked by the sandbox is reported separately; rerun with the sandbox off. `clone` brings a memory repository down; `disable` removes the marker. @@ -152,7 +159,10 @@ edited, never written through a symlink, and recorded in the receipt. project's four active files, then the three curated org files, each with its readability, size and modification time, and where the sync stands; with `--pull` it commits and syncs the home under the runtime identity first, and the files are described as -the pull left them. `events/`, `debriefs/` and `archive/` are excluded. No +the pull left them. The pull leaves another agent's pending files for that +agent to publish: a debrief whose filename names a different agent, or an +event whose frontmatter `agent:` does. Files with no author (the curated +org files, the project tier) sync as before. `events/`, `debriefs/` and `archive/` are excluded. No content is included and no file is claimed as read (`content_injected: false`); the text, notice included, is cut at a character budget (`--max-chars`, at least 1). The default output is what diff --git a/src/agent_memory/__init__.py b/src/agent_memory/__init__.py index 39d8dd2..f7495c6 100644 --- a/src/agent_memory/__init__.py +++ b/src/agent_memory/__init__.py @@ -4,6 +4,6 @@ from .home import HomeError, HomeResolution, resolve_home -__version__ = "0.1.3" +__version__ = "0.1.4" __all__ = ["HomeError", "HomeResolution", "__version__", "resolve_home"] diff --git a/src/agent_memory/doctor.py b/src/agent_memory/doctor.py index f38cb2d..a2fd343 100644 --- a/src/agent_memory/doctor.py +++ b/src/agent_memory/doctor.py @@ -237,7 +237,7 @@ def _walk_error(exc: OSError) -> None: for name in sorted(dirnames): entry = current / name try: - is_link = entry.is_symlink() + is_link = stat.S_ISLNK(os.lstat(entry).st_mode) except OSError as exc: walk_errors.append(f"{_relative(entry)}: {exc.__class__.__name__}") continue @@ -258,16 +258,18 @@ def _walk_error(exc: OSError) -> None: staging.append(rel) continue # The namespace holds regular files reached without following links; - # a classification failure surfaces, never raises. + # a classification failure surfaces, never raises. lstat, not the + # pathlib predicates: since Python 3.14 those answer False on any + # OSError, so a record that could not be inspected would read as irregular. try: - if file_path.is_symlink(): - irregular.append(f"{rel} (symlink)") - continue - regular = file_path.is_file() + mode = os.lstat(file_path).st_mode except OSError as exc: walk_errors.append(f"{rel}: {exc.__class__.__name__}") continue - if not regular: + if stat.S_ISLNK(mode): + irregular.append(f"{rel} (symlink)") + continue + if not stat.S_ISREG(mode): irregular.append(f"{rel} (not a regular file)") continue total += 1 diff --git a/src/agent_memory/startup.py b/src/agent_memory/startup.py index 7e63724..4e2b6ee 100644 --- a/src/agent_memory/startup.py +++ b/src/agent_memory/startup.py @@ -14,6 +14,12 @@ ``archive/`` are excluded), and the rendered text is cut at a character budget with a notice, so a hook can never flood a session. +With ``--pull`` the session's runtime is the publishing identity, and a +path another agent wrote is not its to publish: a debrief names its agent in +its filename and an event in its frontmatter, so either one, when it names +someone else, is held for that agent's own session. Files that carry no +author (the curated org files, the project tier) publish as before. + Output shapes: ``--json`` is the manifest itself, ``schema_version`` first; the default is what the runtime's hook expects on stdout: plain text for claude, whose session start takes stdout as context, and the hook JSON @@ -28,8 +34,9 @@ from pathlib import Path from typing import Any, Dict, List, Optional, Sequence, Tuple -from . import layout, sync +from . import debrief, events, layout, sync from .git_runner import GitRunner, run_git +from .publication import WriterError SCHEMA_VERSION = 1 RUNTIME_CLAUDE = "claude" @@ -46,6 +53,8 @@ RESULT_DEGRADED = "degraded" PULL_NOT_REQUESTED = "not_requested" PULL_ERROR = "error" +#: The most of an event the author check reads; the frontmatter sits at its head. +_EVENT_HEAD_BYTES = 64 * 1024 def build_manifest( @@ -161,7 +170,7 @@ def _sync( pull_info: Dict[str, Any] = {"requested": pull, "status": PULL_NOT_REQUESTED, "ok": True, "lines": []} if pull: try: - outcome = sync.pull(home, agent=runtime, runner=runner) + outcome = sync.pull(home, agent=runtime, runner=runner, hold=lambda path: _held(home, path, runtime)) pull_info = {"requested": True, "status": outcome.status, "ok": outcome.ok, "lines": list(outcome.lines)} except (sync.SyncError, OSError) as exc: pull_info = {"requested": True, "status": PULL_ERROR, "ok": False, "lines": [f"memory pull: {exc}"]} @@ -176,6 +185,40 @@ def _sync( return info, pull_info +def _held(home: Path, relative: str, runtime: str) -> bool: + """Whether ``relative`` names an author other than ``runtime``; no author named means not held.""" + author = author_of(home, relative) + return author is not None and author != runtime + + +def author_of(home: Path, relative: str) -> Optional[str]: + """The agent a memory path names as its author, or ``None`` when it names none. + + A debrief, ``org-memory/debriefs////--.md``, + names it in the filename: the session id has no dash, so the last dash + splits the agent from it whatever dashes the agent's own name carries. An + event, ``org-memory/events/.md``, names it in the frontmatter + ``agent:`` field. Anything else, and anything that does not parse, names + no one. + """ + parts = relative.split("/") + if parts[:2] == [layout.ORG.pattern, "debriefs"] and len(parts) == 6 and parts[5].endswith(".md"): + date, _, rest = parts[5][: -len(".md")].partition("-") + agent, _, session = rest.rpartition("-") + if len(date) == 8 and date.isdigit() and debrief.SESSION_RE.match(session) and debrief.AGENT_RE.match(agent): + return agent + return None + if parts[:2] == [layout.ORG.pattern, events.EVENTS_DIR] and len(parts) == 3 and parts[2].endswith(".md"): + try: + with open(home / relative, "rb") as handle: + frontmatter, _ = debrief.split_record(handle.read(_EVENT_HEAD_BYTES)) + except (OSError, ValueError, WriterError): + return None + agent = frontmatter.get("agent", "") + return agent if debrief.AGENT_RE.match(agent) else None + return None + + # --- rendering -------------------------------------------------------------- diff --git a/src/agent_memory/sync.py b/src/agent_memory/sync.py index 297d7b8..7da1764 100644 --- a/src/agent_memory/sync.py +++ b/src/agent_memory/sync.py @@ -29,7 +29,17 @@ ``push`` and ``pull --agent`` commit allowlisted changes before fetching, rebase when behind, then deliver. A clean ``pull`` without an agent updates and reports unpushed commits. Conflicts abort the rebase and keep local -commits; rejected delivery also leaves them local. +commits; rejected delivery also leaves them local. ``pull`` takes a hold +predicate from its caller: a selected path it holds is left uncommitted for +whoever wrote it, which keeps the engine content-blind when the predicate +has to read a file to name its author. An update never moves a tracked +change, but an untracked path the incoming commits do not touch rides +through the fast-forward or rebase in place. + +``pull`` and ``push`` hold an exclusive lock under the git directory, so one +of them syncs a home at a time and a second waits for the first. A rebase +found in progress is never this verb's to abort: one is refused before +anything is written, and checked for again just before the verb's own rebase. """ from __future__ import annotations @@ -39,18 +49,34 @@ import shutil import socket import tempfile +import time +from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path -from typing import Iterable, List, Mapping, Optional, Sequence, Tuple +from typing import Callable, Iterable, Iterator, List, Mapping, Optional, Sequence, Tuple from . import layout from .git_runner import GitResult, GitRunner, run_git +try: + import fcntl +except ImportError: # pragma: no cover - no flock here; the in-progress checks still apply + fcntl = None # type: ignore[assignment] + MARKER_FILE = layout.MARKER_FILE GITIGNORE_FILE = layout.GITIGNORE_FILE MARKER_TEXT = "agent-memory sync repository. Remove this file to disable syncing locally.\n" NETWORK_TIMEOUT_SECONDS = 30 DEFAULT_REMOTE = "origin" +#: The branch a new memory repository starts on, whatever git's local ``init.defaultBranch`` says. +DEFAULT_BRANCH = "main" + +#: The sync lock, a file under the git directory that ``pull`` and ``push`` hold with flock. +LOCK_FILE = "agent-memory-sync.lock" +#: How long a verb waits for another sync of the same home: the longest a holder can +#: run is its four network-bounded steps (fetch, rebase, abort, push). +LOCK_WAIT_SECONDS: float = 4 * NETWORK_TIMEOUT_SECONDS +_LOCK_POLL_SECONDS = 0.1 #: The agent name a commit is published under, when the caller does not pass one. ENV_AGENT = "AGENT_MEMORY_AGENT" @@ -148,6 +174,8 @@ class Outcome: #: Paths that were staged outside the selection and were left exactly as found. preserved: Tuple[str, ...] = () receipt: Optional[GitignoreReceipt] = None + #: Selected paths the caller's hold predicate kept out of the commit, home-relative. + held: Tuple[str, ...] = () # --- marker and ignore file ------------------------------------------------- @@ -293,6 +321,10 @@ def init( result = _git(home, ["init", "--quiet"], runner) if not result.ok: raise SyncError(f"git init failed: {result.output}") + # Name the unborn branch here rather than with --initial-branch, which needs git 2.28. + result = _git(home, ["symbolic-ref", "HEAD", f"refs/heads/{DEFAULT_BRANCH}"], runner) + if not result.ok: + raise SyncError(f"git symbolic-ref failed: {result.output}") receipt = ensure_gitignore(home) lines = receipt.lines() if write_marker(home): @@ -355,37 +387,44 @@ def pull( agent: Optional[str] = None, runner: Optional[GitRunner] = None, env: Optional[Mapping[str, str]] = None, + hold: Optional[Callable[[str], bool]] = None, ) -> Outcome: """Update the home, publishing dirty memory only with an explicit identity. Clean pulls keep their read-mostly behavior: fast-forward, or rebase local commits and report ahead. An explicit agent permits dirty publication/delivery; - SessionStart supplies its runtime name through this same entry point. + SessionStart supplies its runtime name through this same entry point, with a + ``hold`` predicate that keeps another agent's pending paths out of the commit. """ home = _home(home) if not is_configured(home): return Outcome("not_configured", True) _require_repo(home, runner) - if _operation_in_progress(home, runner): - return Outcome("operation_in_progress", False, ("memory pull: a git operation is in progress; finish it first.",)) - if changed_paths(home, runner): - if agent: - return _sync_changes(home, agent=agent, runner=runner, env=env) - return Outcome("dirty", False, ("memory pull: uncommitted changes present; pass --agent NAME to commit and sync.",)) - state = git_state(home, runner=runner, fetch=True) - if not state.has_remote: - return Outcome("local_only", True, ("memory pull: local-only repository; no remote to pull from.",)) - if state.fetch_failed: - return _fetch_failure(state, "memory pull") - if not state.has_upstream: - return Outcome("no_upstream", True, ("memory pull: no upstream branch configured; skipping.",)) - updated = _update(home, state, runner) - if not updated.ok: + with _sync_lock(home, runner, "memory pull") as busy: + if busy: + return busy + if _operation_in_progress(home, runner): + return Outcome("operation_in_progress", False, ("memory pull: a git operation is in progress; finish it first.",)) + if changed_paths(home, runner): + if agent: + return _sync_changes(home, agent=agent, runner=runner, env=env, hold=hold) + return Outcome("dirty", False, ("memory pull: uncommitted changes present; pass --agent NAME to commit and sync.",)) + state = git_state(home, runner=runner, fetch=True) + if not state.has_remote: + return Outcome("local_only", True, ("memory pull: local-only repository; no remote to pull from.",)) + if state.fetch_failed: + return _fetch_failure(state, "memory pull") + if not state.has_upstream: + return Outcome("no_upstream", True, ("memory pull: no upstream branch configured; skipping.",)) + updated = _update(home, state, runner) + if not updated.ok: + return updated + current = git_state(home, runner=runner, fetch=False) if state.behind else state + if current.ahead: + # Nothing came down unless it was behind; "already synced" would contradict the count. + pulled = updated.lines if state.behind else () + return Outcome("ahead", True, (*pulled, f"memory pull: {current.ahead} unpushed commit(s); use push to deliver.")) return updated - current = git_state(home, runner=runner, fetch=False) if state.behind else state - if current.ahead: - return Outcome("ahead", True, (*updated.lines, f"memory pull: {current.ahead} unpushed commit(s); use push to deliver.")) - return updated def push( @@ -400,9 +439,49 @@ def push( if not is_configured(home): return Outcome("not_configured", True) _require_repo(home, runner) - if _operation_in_progress(home, runner): - return Outcome("operation_in_progress", False, ("memory push: a git operation is in progress; finish it first.",)) - return _sync_changes(home, agent=agent, runner=runner, env=env) + with _sync_lock(home, runner, "memory push") as busy: + if busy: + return busy + if _operation_in_progress(home, runner): + return Outcome("operation_in_progress", False, ("memory push: a git operation is in progress; finish it first.",)) + return _sync_changes(home, agent=agent, runner=runner, env=env) + + +@contextmanager +def _sync_lock(home: Path, runner: Optional[GitRunner], verb: str) -> Iterator[Optional[Outcome]]: + """Hold the home's sync lock; yield ``None``, or the outcome to return when another sync kept it. + + flock belongs to the open file, so the kernel releases it when the holder exits, + however it exits: a crashed sync never leaves the home locked. The file stays. + """ + if fcntl is None: # pragma: no cover + yield None + return + result = _git(home, ["rev-parse", "--git-path", LOCK_FILE], runner) + if not result.ok or not result.stdout.strip(): + raise SyncError(f"cannot locate the git directory for the sync lock: {result.output}") + fd = os.open(home / result.stdout.strip(), os.O_RDWR | os.O_CREAT, 0o644) + try: + if _flock_within(fd, LOCK_WAIT_SECONDS): + yield None + else: + yield Outcome("sync_in_progress", False, ( + f"{verb}: another agent-memory sync held this home for {LOCK_WAIT_SECONDS:g}s; nothing was changed.", + )) + finally: + os.close(fd) + + +def _flock_within(fd: int, seconds: float) -> bool: + deadline = time.monotonic() + seconds + while True: + try: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + return True + except BlockingIOError: + if time.monotonic() >= deadline: + return False + time.sleep(_LOCK_POLL_SECONDS) def _operation_in_progress(home: Path, runner: Optional[GitRunner]) -> bool: @@ -418,9 +497,14 @@ def _operation_in_progress(home: Path, runner: Optional[GitRunner]) -> bool: def _sync_changes( - home: Path, *, agent: Optional[str], runner: Optional[GitRunner], env: Optional[Mapping[str, str]] + home: Path, + *, + agent: Optional[str], + runner: Optional[GitRunner], + env: Optional[Mapping[str, str]], + hold: Optional[Callable[[str], bool]] = None, ) -> Outcome: - published = _publish(home, agent=agent, runner=runner, env=env) + published = _publish(home, agent=agent, runner=runner, env=env, hold=hold) if not published.ok: return published # Commit before fetch: even a network failure leaves a recoverable local commit. @@ -428,12 +512,14 @@ def _sync_changes( lines = list(published.lines) if not state.has_remote: lines.append("memory push: no remote configured; the commit remains local.") - return Outcome("local_only", True, tuple(lines), published.committed, published.preserved) + return Outcome("local_only", True, tuple(lines), published.committed, published.preserved, + held=published.held) if not state.fetch_failed and state.has_upstream and state.behind: updated = _update(home, state, runner) lines.extend(updated.lines) if not updated.ok: - return Outcome(updated.status, False, tuple(lines), published.committed, published.preserved) + return Outcome(updated.status, False, tuple(lines), published.committed, published.preserved, + held=published.held) state = git_state(home, runner=runner, fetch=False) return _deliver(home, published, state, lines, runner) @@ -442,9 +528,10 @@ def _update(home: Path, state: GitState, runner: Optional[GitRunner]) -> Outcome if not state.behind: return Outcome("up_to_date", True, ("memory pull: already synced.",)) remaining = changed_paths(home, runner) - if remaining: + blocking = _blocking_paths(home, state, remaining, runner) + if blocking: return Outcome("dirty", False, ( - "memory pull: cannot update with remaining uncommitted paths; left untouched: " + ", ".join(remaining), + "memory pull: cannot update with remaining uncommitted paths; left untouched: " + ", ".join(blocking), )) if not state.ahead: result = _git(home, ["pull", "--ff-only", "--quiet"], runner, timeout=NETWORK_TIMEOUT_SECONDS) @@ -452,20 +539,24 @@ def _update(home: Path, state: GitState, runner: Optional[GitRunner]) -> Outcome return Outcome("pull_timed_out", False, (f"memory pull: {result.output}",)) if not result.ok: return Outcome("pull_failed", False, (f"memory pull: fast-forward failed: {result.output}",)) - return Outcome("synced", True, (f"memory pull: synced {state.behind} commit(s).",)) + return Outcome("synced", True, (f"memory pull: synced {state.behind} commit(s).", *_kept_lines(remaining))) merges = _git(home, ["rev-list", "--merges", f"{state.upstream}..HEAD"], runner) if not merges.ok or merges.stdout.strip(): return Outcome("diverged", False, ( "memory pull: local merge commits cannot be safely rebased automatically; resolve manually." if merges.ok else f"memory pull: cannot inspect local history: {merges.output}", )) + # The sync lock keeps every other agent-memory verb out; this catches a rebase another + # tool began after the precheck, which a failed rebase of ours would otherwise abort. + if _operation_in_progress(home, runner): + return Outcome("operation_in_progress", False, ("memory pull: a git operation began during the sync; finish it first.",)) # Rebase exactly the fetched upstream. Do not stash unrelated edits or reuse # a recorded conflict resolution: a conflicting same-path add must surface. result = _git(home, ["-c", "rerere.enabled=false", "-c", "rebase.updateRefs=false", "rebase", "--no-autostash", state.upstream], runner, timeout=NETWORK_TIMEOUT_SECONDS) if result.ok: - return Outcome("synced", True, ("memory pull: rebased local commits onto upstream.",)) + return Outcome("synced", True, ("memory pull: rebased local commits onto upstream.", *_kept_lines(remaining))) conflicts = _git(home, ["diff", "--name-only", "--diff-filter=U", "-z"], runner) paths = [path for path in conflicts.stdout.split("\0") if path] if conflicts.ok else [] if _operation_in_progress(home, runner): @@ -482,6 +573,38 @@ def _update(home: Path, state: GitState, runner: Optional[GitRunner]) -> Outcome )) +def _blocking_paths(home: Path, state: GitState, remaining: Sequence[str], runner: Optional[GitRunner]) -> List[str]: + """The uncommitted paths an update cannot carry: every tracked change, and any untracked + path that an incoming commit adds, changes or removes, itself or as a parent directory.""" + if not remaining: + return [] + untracked = set(_untracked_paths(home, runner)) + if not untracked: + return list(remaining) + incoming = _git(home, ["diff", "--name-only", "-z", f"HEAD...{state.upstream}"], runner) + if not incoming.ok: + return list(remaining) + touched = [path for path in incoming.stdout.split("\0") if path] + return [path for path in remaining if path not in untracked or any(_overlaps(path, other) for other in touched)] + + +def _overlaps(path: str, other: str) -> bool: + return path == other or path.startswith(f"{other}/") or other.startswith(f"{path}/") + + +def _untracked_paths(home: Path, runner: Optional[GitRunner]) -> List[str]: + result = _git(home, ["ls-files", "--others", "--exclude-standard", "-z"], runner) + if not result.ok: + raise SyncError(f"git ls-files failed: {result.output}") + return [path for path in result.stdout.split("\0") if path] + + +def _kept_lines(remaining: Sequence[str]) -> Tuple[str, ...]: + if not remaining: + return () + return (f"memory pull: {len(remaining)} untracked path(s) left in place: {', '.join(remaining)}",) + + def disable(home: Path) -> Outcome: """Remove the marker; the repository and its history stay in place.""" home = _home(home) @@ -559,6 +682,7 @@ def _publish( agent: Optional[str], runner: Optional[GitRunner], env: Optional[Mapping[str, str]], + hold: Optional[Callable[[str], bool]] = None, ) -> Outcome: selected, outside = select_paths(home, runner) if outside: @@ -571,8 +695,14 @@ def _publish( f"{', '.join(outside)}", ), ) - preserved = tuple(sorted(set(staged_paths(home, runner)) - set(selected))) + held = tuple(path for path in selected if hold is not None and hold(path)) + selected = [path for path in selected if path not in held] + preserved = tuple(sorted(set(staged_paths(home, runner)) - set(selected) - set(held))) lines: List[str] = [] + if held: + lines.append( + f"memory publish: {len(held)} path(s) another agent wrote left for it to publish: {', '.join(held)}" + ) if preserved: lines.append( f"memory publish: {len(preserved)} staged path(s) outside the allowlist left staged and " @@ -580,22 +710,22 @@ def _publish( ) if not selected: lines.append("memory publish: no memory changes to commit.") - return Outcome("nothing_to_commit", True, tuple(lines), (), preserved) + return Outcome("nothing_to_commit", True, tuple(lines), (), preserved, held=held) message = commit_message(len(selected), resolve_agent(agent, env)) with _pathspec_file(selected) as pathspec: added = _git(home, ["--literal-pathspecs", "add", *_from_file(pathspec)], runner) if not added.ok: - return Outcome("commit_failed", False, (*lines, f"memory publish: git add failed: {added.output}")) + return Outcome("commit_failed", False, (*lines, f"memory publish: git add failed: {added.output}"), held=held) committed = _git(home, ["--literal-pathspecs", "commit", "--quiet", "-m", message, *_from_file(pathspec)], runner) if not committed.ok: - return Outcome("commit_failed", False, (*lines, f"memory publish: git commit failed: {committed.output}")) + return Outcome("commit_failed", False, (*lines, f"memory publish: git commit failed: {committed.output}"), held=held) published = committed_paths(home, runner) stray = sorted(path for path in published if not layout.is_allowed_memory_path(path)) if stray: raise SyncError(f"memory publish: HEAD commits paths outside the allowlist: {', '.join(stray)}") lines.append(f"memory publish: committed {len(published)} file(s).") - return Outcome("committed", True, tuple(lines), tuple(published), preserved) + return Outcome("committed", True, tuple(lines), tuple(published), preserved, held=held) def _deliver( @@ -611,10 +741,12 @@ def _deliver( return _helper_blocked(lines, published, receipt) failure = _fetch_failure(state, "memory push") lines.append(f"{failure.lines[0]} The commit remains local.") - return Outcome(failure.status, False, tuple(lines), published.committed, published.preserved, receipt) + return Outcome(failure.status, False, tuple(lines), published.committed, published.preserved, receipt, + held=published.held) if state.has_upstream and not state.ahead: lines.append("memory push: remote already up to date.") - return Outcome("up_to_date", True, tuple(lines), published.committed, published.preserved, receipt) + return Outcome("up_to_date", True, tuple(lines), published.committed, published.preserved, receipt, + held=published.held) result = _push_remote(home, state, runner) if not result.ok: if credential_helper_blocked(result.output): @@ -622,9 +754,11 @@ def _deliver( what = "timed out" if result.timed_out else "was rejected" lines.append(f"memory push: the push {what}; the commit remains local. {result.output}".rstrip()) status = "push_timed_out" if result.timed_out else "push_failed" - return Outcome(status, False, tuple(lines), published.committed, published.preserved, receipt) + return Outcome(status, False, tuple(lines), published.committed, published.preserved, receipt, + held=published.held) lines.append("memory push: delivered to the remote.") - return Outcome("pushed", True, tuple(lines), published.committed, published.preserved, receipt) + return Outcome("pushed", True, tuple(lines), published.committed, published.preserved, receipt, + held=published.held) def _push_remote(home: Path, state: GitState, runner: Optional[GitRunner]) -> GitResult: @@ -656,7 +790,8 @@ def _helper_blocked( ) -> Outcome: lines.append(_HELPER_BLOCKED_LINE) return Outcome( - "credential_helper_blocked", False, tuple(lines), published.committed, published.preserved, receipt + "credential_helper_blocked", False, tuple(lines), published.committed, published.preserved, receipt, + held=published.held, ) diff --git a/tests/test_doctor.py b/tests/test_doctor.py index d7a187e..a5da3a3 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -268,37 +268,37 @@ def test_a_store_that_cannot_be_inspected_is_an_error_not_uninitialized(tmp_path def test_directory_classification_failure_surfaces(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: - # An is_symlink failure on a directory entry is reported, never raised. + # An lstat failure on a directory entry is reported, never raised. root = tmp_path / "home" root.mkdir() _write_debrief(root) - real_is_symlink = Path.is_symlink + real_lstat = os.lstat - def flaky(self: Path) -> bool: - if self.name == "demo-project": - raise PermissionError(13, "Permission denied", str(self)) - return real_is_symlink(self) + def flaky(path: object, *args: object, **kwargs: object) -> os.stat_result: + if isinstance(path, (str, os.PathLike)) and Path(path).name == "demo-project": + raise PermissionError(13, "Permission denied", str(path)) + return real_lstat(path, *args, **kwargs) - monkeypatch.setattr(Path, "is_symlink", flaky) + monkeypatch.setattr(os, "lstat", flaky) cat = check_org_memory(root) assert ("debriefs-unreadable", Severity.error) in _rows(cat) assert not any(r.name == "debriefs-layout" and "empty store" in r.message for r in cat.results) def test_record_classification_failure_surfaces(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: - # A stat failure while classifying a record lands in the unreadable row; other records still pass. + # An lstat failure while classifying a record lands in the unreadable row; other records still pass. root = tmp_path / "home" root.mkdir() _write_debrief(root) victim = _write_debrief(root, "20260825-bob-77aa88bb.md") - real_stat = Path.stat + real_lstat = os.lstat - def flaky(self: Path, *args: object, **kwargs: object) -> os.stat_result: - if self.name == victim.name: - raise PermissionError(13, "Permission denied", str(self)) - return real_stat(self, *args, **kwargs) + def flaky(path: object, *args: object, **kwargs: object) -> os.stat_result: + if isinstance(path, (str, os.PathLike)) and Path(path).name == victim.name: + raise PermissionError(13, "Permission denied", str(path)) + return real_lstat(path, *args, **kwargs) - monkeypatch.setattr(Path, "stat", flaky) + monkeypatch.setattr(os, "lstat", flaky) rows = _org_rows(root) assert ("debriefs-unreadable", Severity.error) in rows assert ("debriefs-layout", Severity.ok) in rows @@ -836,7 +836,11 @@ def test_report_points_at_memory_lint_only_when_it_is_on_path(tmp_path: Path) -> assert doctor.to_json(categories, memory_lint="/opt/bin/memory-lint")["memory_lint"] == "/opt/bin/memory-lint" -def test_cli_exit_codes_follow_the_error_rows(tmp_path: Path, git_env: None, capsys: pytest.CaptureFixture[str]) -> None: +def test_cli_exit_codes_follow_the_error_rows( + tmp_path: Path, git_env: None, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + # The verdict is the last line only without the memory-lint pointer, so pin the lookup off the host's PATH. + monkeypatch.setattr(doctor, "find_memory_lint", lambda: None) home, _ = synced_home(tmp_path) assert main(["doctor", "--home", str(home)]) == 0 assert capsys.readouterr().out.rstrip("\n").endswith("No issues found.") diff --git a/tests/test_startup.py b/tests/test_startup.py index f0fba5d..13ae28c 100644 --- a/tests/test_startup.py +++ b/tests/test_startup.py @@ -25,6 +25,7 @@ MISSING, READABLE, UNREADABLE, + author_of, build_manifest, render_codex_hook, render_text, @@ -404,3 +405,80 @@ def test_session_start_resolves_dirty_behind_home(home: Path, tmp_path: Path, gi assert git("status", "--porcelain", cwd=home) == "" assert git("log", "-1", "--format=%s", cwd=home).startswith(f"memory: {runtime}@") assert git("--git-dir", str(remote), "show", "main:org-memory/events/local.md", cwd=tmp_path) == "local content" + + +@pytest.mark.parametrize( + ("relative", "author"), + [ + ("org-memory/debriefs/demo/2026/10/20261018-claude-ab12cd34.md", "claude"), + ("org-memory/debriefs/demo/2026/10/20261018-bob-ops-9f00aa11.md", "bob-ops"), + ("org-memory/debriefs/demo/2026/10/20261018-claude-AB12.md", None), + ("org-memory/debriefs/demo/20261018-claude-ab12cd34.md", None), + ("org-memory/events/20261018-120000-peer-rule.md", "codex"), + ("org-memory/events/20261018-120001-no-frontmatter.md", None), + ("org-memory/events/20261018-120002-missing.md", None), + ("org-memory/recent.md", None), + ("projects/demo/memory/open_threads.md", None), + ], +) +def test_author_of_reads_debrief_names_and_event_frontmatter(home: Path, relative: str, author: str) -> None: + from conftest import write + write(home / "org-memory/events/20261018-120000-peer-rule.md", "---\ncreated_at_utc: x\nagent: codex\n---\n\nbody\n") + write(home / "org-memory/events/20261018-120001-no-frontmatter.md", "just text\n") + assert author_of(home, relative) == author + + +@pytest.mark.parametrize("runtime", ["claude", "codex"]) +def test_session_start_holds_another_agents_paths_and_still_pulls( + home: Path, tmp_path: Path, git_env: None, runtime: str +) -> None: + # Synthetic files and identities only. + from conftest import git, write + peer_name = "codex" if runtime == "claude" else "claude" + remote = tmp_path / "remote.git" + git("init", "--bare", "--quiet", str(remote), cwd=tmp_path) + assert sync.init(home, remote=str(remote)).ok + peer = tmp_path / "peer" + git("clone", "--quiet", str(remote), str(peer), cwd=tmp_path) + write(peer / "org-memory/events/upstream.md", "upstream content\n") + git("add", "org-memory/events/upstream.md", cwd=peer) + git("commit", "-qm", "peer addition", cwd=peer) + git("push", cwd=peer) + + peer_debrief = f"org-memory/debriefs/demo/2026/10/20261018-{peer_name}-ab12cd34.md" + peer_event = "org-memory/events/20261018-120000-peer-rule.md" + own_debrief = f"org-memory/debriefs/demo/2026/10/20261018-{runtime}-ef56ab78.md" + write(home / peer_debrief, "peer debrief\n") + write(home / peer_event, f"---\ncreated_at_utc: x\nagent: {peer_name}\n---\n\nbody\n") + write(home / own_debrief, "own debrief\n") + write(home / "projects/demo/memory/open_threads.md", "unattributed edit\n") + + manifest = build_manifest(home, runtime=runtime, project="demo", pull=True) + assert manifest["pull"]["status"] == "pushed" and manifest["pull"]["ok"], manifest["pull"] + assert any("another agent wrote left for it to publish" in line for line in manifest["pull"]["lines"]) + assert (home / "org-memory/events/upstream.md").read_text() == "upstream content\n" + assert sorted(git("ls-files", "--others", "--exclude-standard", cwd=home).splitlines()) == sorted([peer_debrief, peer_event]) + published = git("--git-dir", str(remote), "ls-tree", "-r", "--name-only", "main", cwd=tmp_path).splitlines() + assert own_debrief in published and "projects/demo/memory/open_threads.md" in published + assert peer_debrief not in published and peer_event not in published + + +def test_session_start_with_only_held_paths_still_fast_forwards(home: Path, tmp_path: Path, git_env: None) -> None: + from conftest import git, write + remote = tmp_path / "remote.git" + git("init", "--bare", "--quiet", str(remote), cwd=tmp_path) + assert sync.init(home, remote=str(remote)).ok + peer = tmp_path / "peer" + git("clone", "--quiet", str(remote), str(peer), cwd=tmp_path) + write(peer / "org-memory/events/upstream.md", "upstream content\n") + git("add", "org-memory/events/upstream.md", cwd=peer) + git("commit", "-qm", "peer addition", cwd=peer) + git("push", cwd=peer) + held = "org-memory/debriefs/demo/2026/10/20261018-codex-ab12cd34.md" + write(home / held, "peer debrief\n") + head = git("rev-parse", "HEAD", cwd=home) + + manifest = build_manifest(home, runtime="claude", project="demo", pull=True) + assert manifest["pull"]["ok"] and manifest["result"] == "ok", manifest["pull"] + assert git("rev-parse", "HEAD", cwd=home) == git("--git-dir", str(remote), "rev-parse", "main", cwd=tmp_path) != head + assert git("status", "--porcelain", "--untracked-files=all", cwd=home) == f"?? {held}" diff --git a/tests/test_sync.py b/tests/test_sync.py index 428dcac..3291bcf 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -2,8 +2,11 @@ import os import subprocess +import threading +import time +from contextlib import contextmanager from pathlib import Path -from typing import Dict, List, Optional, Sequence, Tuple +from typing import Dict, Iterator, List, Optional, Sequence, Tuple import pytest @@ -136,11 +139,12 @@ def __call__(self, args: Sequence[str], *, cwd: Path, timeout: Optional[float] = def test_fetch_pull_push_and_clone_receive_network_timeouts(tmp_path: Path) -> None: root = tmp_path / "home" - root.mkdir() + (root / ".git").mkdir(parents=True) _write(root / MARKER, "memory sync enabled\n") common = { ("rev-parse", "--is-inside-work-tree"): "true", ("rev-parse", "--show-toplevel"): str(root), + ("rev-parse", "--git-path", sync.LOCK_FILE): f".git/{sync.LOCK_FILE}", ("remote",): "origin", ("rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"): "origin/main", } @@ -815,6 +819,10 @@ def test_pull_rebases_a_diverged_repository_and_reports_ahead(tmp_path: Path) -> outcome = sync.pull(home) assert outcome.status == "ahead" and outcome.ok + assert outcome.lines == ( + "memory pull: rebased local commits onto upstream.", + "memory pull: 1 unpushed commit(s); use push to deliver.", + ) assert (home / "org-memory" / "remote.md").exists() @@ -829,7 +837,8 @@ def test_pull_with_unpushed_commits_is_not_an_error(tmp_path: Path) -> None: outcome = sync.pull(home) assert outcome.status == "ahead" and outcome.ok - assert "1 unpushed commit(s)" in outcome.lines[-1] + # Nothing came down, so no "already synced" ahead of the unpushed count. + assert outcome.lines == ("memory pull: 1 unpushed commit(s); use push to deliver.",) def test_pull_when_already_synced(tmp_path: Path) -> None: @@ -926,6 +935,29 @@ def test_init_is_idempotent(tmp_path: Path) -> None: assert {path: path.read_bytes() for path in home.rglob("*") if path.is_file() and ".git" not in path.parts} == before +@pytest.mark.parametrize("local_default", ["master", "main"]) +def test_init_starts_a_new_repository_on_main_whatever_the_local_default( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, local_default: str +) -> None: + monkeypatch.setenv("GIT_CONFIG_VALUE_0", local_default) + home = _local_home(tmp_path) + assert _git("symbolic-ref", "--short", "HEAD", cwd=home) == "main" + assert _git("branch", "--format=%(refname:short)", cwd=home) == "main" + + +def test_init_keeps_the_branch_of_an_existing_repository(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GIT_CONFIG_VALUE_0", "master") + home = tmp_path / "home" + home.mkdir() + _git("init", "--quiet", cwd=home) + + assert sync.init(home).ok + assert sync.init(home).ok + + assert _git("branch", "--format=%(refname:short)", cwd=home) == "master" + assert _head_paths(home) == [".gitignore", MARKER] + + def test_init_adds_origin_when_another_remote_exists(tmp_path: Path) -> None: remote = tmp_path / "memory.git" other = tmp_path / "other.git" @@ -1094,6 +1126,87 @@ def test_existing_rebase_is_never_published_or_aborted(tmp_path: Path, verb) -> assert (home / ".git/rebase-merge").exists() +@pytest.mark.parametrize("verb", [sync.pull, sync.push]) +def test_a_rebase_begun_after_the_precheck_is_never_aborted(tmp_path: Path, verb) -> None: + # The in-progress precheck runs before the fetch. A rebase somebody else starts + # once the upstream has been read, just before ours, is theirs and must survive. + remote, seed = _create_remote(tmp_path) + home = tmp_path / "home" + sync.clone(home, str(remote)) + _write(home / "org-memory/local.md", "local\n") + _git("add", "org-memory/local.md", cwd=home) + _git("commit", "-qm", "local", cwd=home) + _commit_and_push(seed, "org-memory/remote.md", "remote\n") + + def runner(args, *, cwd, timeout=None): + result = run_git(args, cwd=cwd, timeout=timeout) + if tuple(args[:2]) == ("rev-list", "--merges"): + # Stops at the failing exec with a clean tree and its state directory in place. + assert not run_git(["rebase", "--exec", "false", "HEAD~1"], cwd=cwd).ok + assert (home / ".git/rebase-merge").exists() + return result + + outcome = verb(home, agent="test-agent", runner=runner) + assert outcome.status == "operation_in_progress" and not outcome.ok + assert (home / ".git/rebase-merge").exists() + assert not _remote_has(remote, "org-memory/local.md") + + +@contextmanager +def _held_sync_lock(home: Path) -> Iterator[None]: + """Hold the home's sync lock the way a concurrent agent-memory process would.""" + fcntl = pytest.importorskip("fcntl") + with open(home / ".git" / sync.LOCK_FILE, "a") as handle: + fcntl.flock(handle, fcntl.LOCK_EX) + yield + + +@pytest.mark.parametrize("verb", [sync.pull, sync.push]) +def test_a_sync_held_elsewhere_past_the_wait_changes_nothing( + tmp_path: Path, verb, monkeypatch: pytest.MonkeyPatch +) -> None: + remote, seed = _create_remote(tmp_path) + home = tmp_path / "home" + sync.clone(home, str(remote)) + _commit_and_push(seed, "org-memory/remote.md", "remote\n") + _write(home / "org-memory/local.md", "local\n") + monkeypatch.setattr(sync, "LOCK_WAIT_SECONDS", 0.2) + runner = FetchTimesOut() + + with _held_sync_lock(home): + outcome = verb(home, agent="test-agent", runner=runner) + + assert outcome.status == "sync_in_progress" and not outcome.ok + assert "another agent-memory sync" in outcome.lines[-1] + assert not any(call[0] in {"fetch", "add", "commit", "rebase"} for call in runner.calls) + assert not (home / "org-memory/remote.md").exists() + assert _git("status", "--porcelain", cwd=home) == "?? org-memory/local.md" + + +def test_a_second_sync_waits_for_the_first_to_finish(tmp_path: Path) -> None: + pytest.importorskip("fcntl") + remote, seed = _create_remote(tmp_path) + home = tmp_path / "home" + sync.clone(home, str(remote)) + _commit_and_push(seed, "org-memory/remote.md", "remote\n") + held = threading.Event() + + def first_sync() -> None: + with _held_sync_lock(home): + held.set() + time.sleep(0.3) + + holder = threading.Thread(target=first_sync) + holder.start() + assert held.wait(timeout=10), "the first sync never took the lock" + started = time.monotonic() + outcome = sync.pull(home) + holder.join() + + assert outcome.status == "synced" and outcome.ok + assert time.monotonic() - started >= 0.2, "the second sync ran alongside the first" + + @pytest.mark.parametrize("verb", [sync.pull, sync.push]) def test_dirty_fetch_timeout_keeps_commit_and_identity(tmp_path: Path, verb) -> None: remote, _ = _create_remote(tmp_path) @@ -1181,3 +1294,53 @@ def test_local_merge_content_is_not_dropped_by_rebase(tmp_path: Path, verb) -> N assert "merge commits" in outcome.lines[-1] assert _git("rev-parse", "HEAD", cwd=home) == before assert (home / "org-memory/merge-only.md").read_text() == "merge resolution\n" + + +# Real git shapes; all repository names, identities and contents are synthetic. +def test_pull_holds_paths_the_predicate_names_and_updates_past_them(tmp_path: Path) -> None: + remote, seed = _create_remote(tmp_path) + home = tmp_path / "home" + sync.clone(home, str(remote)) + _commit_and_push(seed, "org-memory/peer.md", "peer content\n") + _write(home / "org-memory/held.md", "someone else's\n") + _write(home / "org-memory/local.md", "local content\n") + outcome = sync.pull(home, agent="test-agent", hold=lambda path: path == "org-memory/held.md") + assert outcome.ok and outcome.status == "pushed", outcome.lines + assert outcome.held == ("org-memory/held.md",) + assert outcome.committed == ("org-memory/local.md",) + assert _git("status", "--porcelain", cwd=home) == "?? org-memory/held.md" + assert _remote_has(remote, "org-memory/local.md") and not _remote_has(remote, "org-memory/held.md") + assert (home / "org-memory/peer.md").read_text() == "peer content\n" + assert any("1 untracked path(s) left in place: org-memory/held.md" in line for line in outcome.lines) + + +@pytest.mark.parametrize( + ("held", "incoming"), + [("org-memory/held.md", "org-memory/held.md"), ("org-memory/held/inner.md", "org-memory/held")], +) +def test_an_untracked_path_the_upstream_touches_still_blocks_the_update(tmp_path: Path, held: str, incoming: str) -> None: + remote, seed = _create_remote(tmp_path) + home = tmp_path / "home" + sync.clone(home, str(remote)) + _commit_and_push(seed, incoming, "upstream\n") + _write(home / held, "local\n") + before = _git("rev-parse", "HEAD", cwd=home) + outcome = sync.pull(home, agent="test-agent", hold=lambda path: path == held) + assert outcome.status == "dirty" and not outcome.ok + assert outcome.lines[-1].endswith(f"left untouched: {held}") + assert _git("rev-parse", "HEAD", cwd=home) == before + assert (home / held).read_text() == "local\n" + + +def test_a_tracked_change_left_behind_still_blocks_the_update(tmp_path: Path) -> None: + remote, seed = _create_remote(tmp_path) + home = tmp_path / "home" + sync.clone(home, str(remote)) + _commit_and_push(seed, "org-memory/recent.md", "base\n") + assert sync.pull(home).ok + _commit_and_push(seed, "org-memory/peer.md", "peer content\n") + _write(home / "org-memory/recent.md", "edited by someone else\n") + outcome = sync.pull(home, agent="test-agent", hold=lambda path: path == "org-memory/recent.md") + assert outcome.status == "dirty" and not outcome.ok + assert "org-memory/recent.md" in outcome.lines[-1] + assert not (home / "org-memory/peer.md").exists()