From 1b2282f89c086f9e1663aae01d844f4fdd033545 Mon Sep 17 00:00:00 2001 From: spbkgw-beep Date: Fri, 11 Sep 2026 21:06:48 -0700 Subject: [PATCH] feat: agent-memory v0.1.1 Adds `event write`: it publishes one org-memory event into the home's events store, failure-atomically, and never replaces a record that is already published. Fixes five sync and doctor behaviours. The publication allowlist now anchors `.gitignore` and `.oacp-memory-repo` to the home root, so a file of either name deeper in the tree no longer blocks `pull`. `enable` retires the lines a superseded managed block carried and names them in its receipt. `doctor` scopes its working-tree row to the allowlist. `push` and `doctor` keep both paths of a rename git detects in the working tree, where the source path was previously misread. And `push` names a credential helper the sandbox blocked as its own failure, with the remedy, instead of reporting it as a rejected push. Raises the pinned artifact actions in the release workflow and the development ruff floor. --- .github/workflows/release.yml | 6 +- CHANGELOG.md | 15 + README.md | 2 + docs/commands.md | 43 +- pyproject.toml | 2 +- src/agent_memory/__init__.py | 2 +- src/agent_memory/cli.py | 113 +++- src/agent_memory/doctor.py | 44 +- src/agent_memory/events.py | 409 ++++++++++++ src/agent_memory/layout.py | 18 +- src/agent_memory/sync.py | 132 +++- tests/golden/canonical_memory_gitignore.txt | 4 +- tests/golden/event_record.md | 14 + tests/test_cli.py | 5 +- tests/test_doctor.py | 68 ++ tests/test_events.py | 706 ++++++++++++++++++++ tests/test_layout.py | 18 +- tests/test_sync.py | 176 +++++ 18 files changed, 1728 insertions(+), 49 deletions(-) create mode 100644 src/agent_memory/events.py create mode 100644 tests/golden/event_record.md create mode 100644 tests/test_events.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 84b66a9..7ddfbe4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -40,7 +40,7 @@ jobs: test "${RELEASE_TAG}" = "v${PACKAGE_VERSION}" - name: Upload release artifacts - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: python-package-distributions path: dist/ @@ -57,7 +57,7 @@ jobs: steps: - name: Download release artifacts - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: name: python-package-distributions path: dist/ @@ -78,7 +78,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Download release artifacts - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: name: python-package-distributions path: dist/ diff --git a/CHANGELOG.md b/CHANGELOG.md index e7e1f50..686de2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ 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.1 - 2026-09-12 + +### Added + +- `event write` publishes one org-memory event under `org-memory/events/`, never replacing a published record ([event](docs/commands.md#event-write)). +- A `v*` tag attaches the built sdist and wheel to the GitHub Release ([releases](https://github.com/kiloloop/agent-memory/releases)). + +### Fixed + +- A nested `.gitignore` or `.oacp-memory-repo` no longer blocks `pull`; the allowlist anchors both names to the home root ([sync](docs/commands.md#sync)). +- `enable` retires superseded lines when it updates a home's `.gitignore`, and the receipt names them ([sync](docs/commands.md#sync)). +- `doctor` scopes its working-tree row to the allowlist: memory changes read DIRTY, and changes elsewhere are named as outside it ([doctor](docs/commands.md#doctor)). +- `push` and `doctor` keep both paths of a rename git detects in the working tree; the source path was previously misread ([sync](docs/commands.md#sync)). +- `push` names a credential helper a sandbox blocked as its own failure, with the remedy, instead of reporting it as a rejected push ([sync](docs/commands.md#sync)). + ## 0.1.0 - 2026-09-06 ### Added diff --git a/README.md b/README.md index 17153e7..008362e 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,7 @@ similarity queries, synthesis, or indexing. Recall reads a fixed file set. | [`capture`][capture-and-recall] | Record a decision. | | [`recall`][capture-and-recall] | Read bounded context. | | [`debrief write`][debrief-write] | Publish a session summary. | +| [`event write`][event-write] | Publish an org-memory event. | [status]: https://github.com/kiloloop/agent-memory/blob/main/docs/commands.md#status [doctor]: https://github.com/kiloloop/agent-memory/blob/main/docs/commands.md#doctor @@ -157,6 +158,7 @@ similarity queries, synthesis, or indexing. Recall reads a fixed file set. [startup]: https://github.com/kiloloop/agent-memory/blob/main/docs/commands.md#startup [capture-and-recall]: https://github.com/kiloloop/agent-memory/blob/main/docs/commands.md#capture-and-recall [debrief-write]: https://github.com/kiloloop/agent-memory/blob/main/docs/commands.md#debrief-write +[event-write]: https://github.com/kiloloop/agent-memory/blob/main/docs/commands.md#event-write ## Examples diff --git a/docs/commands.md b/docs/commands.md index 7d04fb4..4e1e0a4 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -21,7 +21,8 @@ the tree is dirty or diverged (ahead and behind are reported, not failed). layout (presence, canonical paths, staging leftovers, symlinks; it never opens a record, and a traversal it cannot finish is an error row, never a pass) and the sync repository (marker, allowlist, tracked and untracked memory files, -tree, upstream, remote, last-commit age, per-instance state, overlay ignores). +the tree with memory changes told apart from changes outside the allowlist, +upstream, remote, last-commit age, per-instance state, overlay ignores). It reads no memory content, repairs nothing, exits 1 only on an error row, and points at `memory-lint` when that is installed; `--json` emits the same rows as data. @@ -41,12 +42,16 @@ 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 -the head of its `.gitignore` as a managed block (existing lines are kept, and -the write comes with a before/after receipt), drops the sync marker, and makes +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 one commit. With `--remote`, `enable` also pushes that initial commit. `push` commits only what the allowlist selects, as a partial commit, so anything else staged in the index stays staged and uncommitted; it refuses a home that is behind or diverged, and a push the remote rejects -leaves the local commit in place and says so. `pull` fast-forwards only when +leaves the local commit in place and says so. A credential helper a sandbox +blocked is a distinct failure from a rejection -- git prompts for a username +and has no terminal to read it from -- and `push` names it as such; the remedy +is to rerun with the sandbox off. `pull` fast-forwards only when the tree is clean, not ahead, not diverged, and has an upstream. `clone` brings a memory repository down; `disable` removes the marker. The network verbs time out after 30 seconds. The engine never reads memory content, @@ -158,3 +163,33 @@ record reports as a collision. That narrower contract rests on the store directory not being writable by other users, its default mode, so the swap needs the owner's own uid; it is a platform limitation. Either way the writer restages and retries, and a swap that persists is reported with the canonical path absent. + +## Event write + +`event write` publishes one org-memory event into the home's events store, +at `org-memory/events/--.md`: the mechanical writer +the layout spec names, ported from the kernel's `write-event` script and +byte-identical to it for the same inputs. The record is a frontmatter block +of plain scalars (`created_at_utc`, `date`, `agent`, `project`, `type`, then +`source_ref`, `related` and `supersedes` when given), a blank line, the body +with its trailing newlines dropped, and one closing newline. Because the +scalars are plain, a value a YAML reader would not hand back verbatim -- a +control character, a leading indicator, `: ` or ` #` inside, or a string it +re-types such as `true`, `12` or `2026-03-21` -- is refused (exit 1) rather +than written. The body comes from `--body`, from `--body-file ` or +`--body-file -` (stdin), or from piped stdin when neither is given. A body +file is read as text, as the script reads it, so its CRLF and CR line endings +become LF and a Windows-authored file lands the same record as its LF twin; +stdin and `--body` are taken as given. `--type` is one of `decision`, +`event`, `rule`; the slug is lowercase alphanumerics and hyphens, +alphanumeric at both ends, no dots. Publication is the writer +contract of `debrief write`, through the same `agent_memory.publication` +module: staged in a private file, verified through the descriptor that wrote +it by a check that parses the frontmatter back, published with an atomic +no-replace link, and read back. An identical record already at the name is +idempotent (exit 0, the file untouched), a differing one is a collision (exit +2, a published record is never replaced), and a failure before the link +leaves `events/` with no partial record and no staging debris. `--dry-run` +composes and prints the record and touches nothing; `--json` reports the +path, status, stamp and identity. The verb appends one event and reads +nothing; synthesis stays with the caller. diff --git a/pyproject.toml b/pyproject.toml index ce92632..1865f50 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,7 +38,7 @@ Changelog = "https://github.com/kiloloop/agent-memory/blob/main/CHANGELOG.md" dev = [ "build>=1.2,<2", "pytest>=8,<10", - "ruff>=0.11,<1", + "ruff>=0.16,<0.17", "twine>=7,<8", ] diff --git a/src/agent_memory/__init__.py b/src/agent_memory/__init__.py index 5ad3b6c..023dc31 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.0" +__version__ = "0.1.1" __all__ = ["HomeError", "HomeResolution", "__version__", "resolve_home"] diff --git a/src/agent_memory/cli.py b/src/agent_memory/cli.py index eaa92e6..7c4c79a 100644 --- a/src/agent_memory/cli.py +++ b/src/agent_memory/cli.py @@ -10,7 +10,7 @@ from pathlib import Path from typing import Callable, List, Optional, Sequence, Tuple -from . import __version__, archive, debrief, doctor, org, setup, startup, status, sync, workflow +from . import __version__, archive, debrief, doctor, events, org, setup, startup, status, sync, workflow from .home import BINDING_FILE, HomeError, HomeResolution, find_project, resolve_home EXIT_OK = 0 @@ -189,6 +189,38 @@ def add(name: str, handler: Callable[[argparse.Namespace], int], help_text: str, write.add_argument("--oacp-dir", dest="home", metavar="PATH", help=argparse.SUPPRESS) write.add_argument("--dry-run", action="store_true", help="validate and compose the record, print it, and write nothing") write.add_argument("--json", dest="json_output", action="store_true", help="emit a machine-readable result") + event_ = commands.add_parser("event", help="Org-memory event commands.", description="Org-memory event commands.") + event_commands = event_.add_subparsers(dest="event_command", metavar="") + event_help = ( + "Publish one org-memory event into the home's events store, failure-atomically: the canonical path only ever " + "holds a complete, verified record, byte-identical to the kernel's write-event script for the same inputs. " + "Exit 0 published (or an identical record was already there), 1 on a validation error, 2 on a publication " + "failure." + ) + event_write = event_commands.add_parser("write", parents=[common], help=event_help, description=event_help) + event_write.set_defaults(handler=_event_write) + event_write.add_argument("--agent", required=True, help="agent creating the event") + event_write.add_argument("--project", required=True, help="originating project") + event_write.add_argument( + "--type", dest="event_type", required=True, choices=sorted(events.ALLOWED_TYPES), help="event type" + ) + event_write.add_argument( + "--slug", required=True, help="short slug for the filename: lowercase alphanumerics and hyphens, no dots" + ) + event_body = event_write.add_mutually_exclusive_group() + event_body.add_argument("--body", help="event body, inline") + event_body.add_argument("--body-file", help="path to the event body in Markdown, or '-' to read stdin") + event_write.add_argument("--source-ref", help="provenance id, e.g. the debrief stem the event was folded from") + event_write.add_argument( + "--related", help="cross-references: comma-separated or a JSON array (e.g. 'PR #43,issue #10')" + ) + event_write.add_argument("--supersedes", help="path of the event this entry overrides") + # The kernel script's home flag; accepted, unadvertised, through v0.1.x. + event_write.add_argument("--oacp-dir", dest="home", metavar="PATH", help=argparse.SUPPRESS) + event_write.add_argument( + "--dry-run", action="store_true", help="validate and compose the record, print it, and write nothing" + ) + event_write.add_argument("--json", dest="json_output", action="store_true", help="emit a machine-readable result") return parser @@ -414,6 +446,85 @@ def _debrief_write(args: argparse.Namespace) -> int: return EXIT_OK +def _event_write(args: argparse.Namespace) -> int: + home = resolve_home(args.home).path + try: + body = _event_body(args) + except OSError as exc: + print(f"ERROR: cannot read body: {exc}", file=sys.stderr) + return EXIT_FAILED + except events.WriterError as exc: + print(f"ERROR: {exc}", file=sys.stderr) + return exc.code + + try: + result = events.write_event( + home=home, + agent=args.agent, + project=args.project, + event_type=args.event_type, + slug=args.slug, + body=body, + source_ref=args.source_ref, + related=events.normalize_related(args.related) if args.related else None, + supersedes=args.supersedes, + dry_run=args.dry_run, + ) + except events.WriterError as exc: + print(f"ERROR: {exc}", file=sys.stderr) + return exc.code + except OSError as exc: + print(f"ERROR: publication failed: {exc}", file=sys.stderr) + return 2 + + if args.json_output: + print( + json.dumps( + { + "path": str(result.path), + "status": result.status, + "created_at_utc": result.created_at_utc, + "date": result.created_at_utc[:10], + "agent": args.agent, + "project": args.project, + "type": args.event_type, + }, + indent=2, + ) + ) + else: + print(f"{result.status}: {result.path}") + print(f"created_at_utc: {result.created_at_utc}") + + if args.dry_run: + print("--- record preview (nothing was written) ---", file=sys.stderr) + sys.stderr.flush() + sys.stderr.buffer.write(result.record) + sys.stderr.buffer.flush() + return EXIT_OK + + +def _event_body(args: argparse.Namespace) -> bytes: + """The body bytes: ``--body-file`` (a path, or ``-`` for stdin), ``--body``, else piped stdin. + + Trailing newlines are dropped, as the kernel script drops them; the record + closes the body with exactly one. A body file is read as text, so its + CRLF and CR line endings become LF (:func:`events.body_from_file`); stdin + and the inline body are taken as given, both as the script does. + """ + if args.body_file is not None and args.body_file != "-": + return events.body_from_file(Path(args.body_file).expanduser().read_bytes()) + if args.body_file is not None: + raw = sys.stdin.buffer.read() + elif args.body is not None: + raw = args.body.encode("utf-8") + elif not sys.stdin.isatty(): + raw = sys.stdin.buffer.read() + else: + raise events.WriterError("no body provided: use --body, --body-file , or pipe the body to stdin", 1) + return raw.rstrip(b"\n") + + def _report(outcome: sync.Outcome) -> int: if outcome.lines: print("\n".join(outcome.lines), file=sys.stdout if outcome.ok else sys.stderr) diff --git a/src/agent_memory/doctor.py b/src/agent_memory/doctor.py index 9a69eae..24abd0e 100644 --- a/src/agent_memory/doctor.py +++ b/src/agent_memory/doctor.py @@ -444,15 +444,7 @@ def check_memory_sync( state = None if state is not None: - if state.dirty: - cat.add( - "working-tree", - Severity.warn, - "working tree — DIRTY memory changes present", - "Run `agent-memory push` or resolve changes manually", - ) - else: - cat.add("working-tree", Severity.ok, "working tree — clean") + _add_working_tree(cat, home, runner, state) text = f"sync state — {sync_state_text(state)}" if not state.has_remote: @@ -534,6 +526,40 @@ def check_memory_sync( return cat +def _add_working_tree(cat: Category, home: Path, runner: Optional[GitRunner], state: GitState) -> None: + """The tree row, scoped like the counts above it. + + Memory changes inside the allowlist are what ``push`` would publish and + read DIRTY. Changes anywhere else are named as outside the allowlist: not + memory content, but ``pull`` still refuses a tree that carries them. + """ + if not state.dirty: + cat.add("working-tree", Severity.ok, "working tree — clean") + return + try: + changed = sync.changed_paths(home, runner) + except sync.SyncError as exc: + cat.add("working-tree", Severity.warn, f"working tree — DIRTY; the change readout failed: {exc}") + return + memory = [path for path in changed if layout.is_allowed_memory_path(path)] + elsewhere = [path for path in changed if not layout.is_allowed_memory_path(path)] + if memory: + text = f"working tree — DIRTY {len(memory)} memory change(s): {_summarize(memory)}" + if elsewhere: + text += f"; {len(elsewhere)} change(s) outside the memory allowlist" + cat.add("working-tree", Severity.warn, text, "Run `agent-memory push` or resolve changes manually") + elif elsewhere: + cat.add( + "working-tree", + Severity.warn, + f"working tree — {len(elsewhere)} change(s) outside the memory allowlist: {_summarize(elsewhere)}; " + "no memory change is pending, but memory pull refuses a dirty tree", + "Commit, stash or ignore them outside the memory tiers", + ) + else: + cat.add("working-tree", Severity.ok, "working tree — clean") + + def enclosing_repository(home: Path, runner: Optional[GitRunner] = None) -> Optional[Path]: """The worktree root when ``home`` sits inside a repository that is not its own; ``None`` when it is the root.""" root = sync.worktree_root(home, runner) diff --git a/src/agent_memory/events.py b/src/agent_memory/events.py new file mode 100644 index 0000000..3b04994 --- /dev/null +++ b/src/agent_memory/events.py @@ -0,0 +1,409 @@ +# SPDX-FileCopyrightText: 2026 Kiloloop +# SPDX-License-Identifier: Apache-2.0 +"""Publish one org-memory event: ``agent-memory event write``. + +The mechanical event writer of the memory layout spec (the kernel's +``docs/protocol/org_memory.md`` -> "Events"), ported from the kernel's +``scripts/write_event.py``. For the same inputs and the same clock the record +is byte-identical to the script's; what the port adds is the writer contract +of :mod:`agent_memory.publication`: the record is staged in a private file, +verified through the descriptor that wrote it, published with an atomic +no-replace link and read back, so the canonical path only ever holds a +complete record. The verb appends one event and reads nothing: synthesis +never enters here. + +Canonical path:: + + /org-memory/events/--.md + +Exit codes of the verb: + 0 published (or idempotent re-publish of a byte-identical record) + 1 usage / validation error + 2 publication failure (collision, read-back mismatch, hostile target) +""" + +from __future__ import annotations + +import datetime as dt +import json +import re +from pathlib import Path +from typing import Dict, List, NamedTuple, Optional, Sequence, Tuple + +from . import layout +from .debrief import AGENT_RE, CONTROL_CHARS_RE, split_record, valid_project_segment +from .publication import WriterError, publish + +ALLOWED_TYPES = ("decision", "event", "rule") + +#: Lowercase alphanumerics and hyphens, 1-64 characters, alphanumeric at both +#: ends. No dots: a version segment is spelled ``0-2-3``, not ``v0.2.3``. +SLUG_RE = re.compile(r"^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$|^[a-z0-9]$") + +EVENTS_DIR = "events" +FRONTMATTER_DELIM = b"---\n" +TIMESTAMP_FORMAT = "%Y-%m-%dT%H:%M:%SZ" + +#: The frontmatter, in the order the kernel script emits it: the five fields +#: every event carries, then the optional provenance fields that are present. +REQUIRED_FRONTMATTER_ORDER = ("created_at_utc", "date", "agent", "project", "type") +OPTIONAL_FRONTMATTER_ORDER = ("source_ref", "related", "supersedes") + +STATUS_DRY_RUN = "dry-run" + +# The frontmatter is emitted as plain (unquoted) YAML scalars, which is the +# byte contract with the kernel script. A plain scalar cannot carry an +# arbitrary string: a reader hands some values back re-typed (``true`` a +# boolean, ``12`` an integer, ``2026-03-21`` a date), reads ``: `` and `` #`` +# as structure, and refuses a leading indicator outright. The writer cannot +# quote without breaking the contract, so it refuses what it cannot carry. +_PLAIN_INDICATORS = "-?:,[]{}#&*!|>'\"%@`" +_RETYPED_KEYWORD_RE = re.compile(r"^(?:true|false|yes|no|on|off|y|n|null|~)$", re.IGNORECASE) +_RETYPED_NUMBER_RE = re.compile( + r"^[-+]?(?:" + r"[0-9_]+(?:\.[0-9_]*)?(?:[eE][-+]?[0-9]+)?" # 12, 1.0, 1e3, 1_000 + r"|\.[0-9_]+(?:[eE][-+]?[0-9]+)?" # .5 + r"|0x[0-9a-fA-F_]+|0o[0-7_]+|0b[01_]+" # 0x1f, 0o7, 0b1 + r"|[0-9]+(?::[0-5]?[0-9])+(?:\.[0-9_]*)?" # 1:30 (YAML 1.1 sexagesimal) + r"|\.(?:inf|nan)" + r")$", + re.IGNORECASE, +) +_RETYPED_TIMESTAMP_RE = re.compile(r"^[0-9]{4}-[0-9]{1,2}-[0-9]{1,2}(?:[Tt ].*)?$") + +#: An upper bound on the provenance scalars, so a record stays readable. +MAX_SCALAR_CHARS = 200 + + +class EventResult(NamedTuple): + """Outcome of one event write.""" + + path: Path + status: str + created_at_utc: str + record: bytes + + +# -------------------------------------------------------------------------- +# validation +# -------------------------------------------------------------------------- + + +def normalize_related(raw: str) -> List[str]: + """Parse a ``--related`` value into a list of strings. + + Accepts a comma-separated string (``PR #43, PR #44``) or a pre-encoded + JSON array (``["PR #43", "PR #44"]``); a value that starts with ``[`` but + is not JSON falls back to the comma split. Items are stripped and empty + ones dropped. Ported verbatim from the kernel script. + """ + raw = raw.strip() + if raw.startswith("["): + try: + parsed = json.loads(raw) + return [str(item).strip() for item in parsed if str(item).strip()] + except (json.JSONDecodeError, TypeError): + pass # fall through to the comma split + return [item.strip() for item in raw.split(",") if item.strip()] + + +def validate_slug(slug: str) -> None: + if not SLUG_RE.match(slug): + raise WriterError( + f"invalid slug {slug!r}: lowercase alphanumerics and hyphens, 1-64 characters, " + "alphanumeric at both ends (no dots: spell v0.2.3 as 0-2-3)", + 1, + ) + + +def validate_type(event_type: str) -> None: + if event_type not in ALLOWED_TYPES: + raise WriterError(f"invalid type {event_type!r}: must be one of {', '.join(ALLOWED_TYPES)}", 1) + + +def validate_plain_scalar(label: str, value: str) -> None: + """Refuse a value the plain-scalar frontmatter cannot carry verbatim.""" + if not value or value != value.strip(): + raise WriterError(f"{label} must be a non-empty string with no leading or trailing whitespace: {value!r}", 1) + if CONTROL_CHARS_RE.search(value): + raise WriterError(f"{label} must not contain control characters: {value!r}", 1) + if len(value) > MAX_SCALAR_CHARS: + raise WriterError(f"{label} is longer than {MAX_SCALAR_CHARS} characters", 1) + if value[0] in _PLAIN_INDICATORS or ": " in value or " #" in value or value.endswith(":"): + raise WriterError( + f"{label} is not representable as a plain YAML scalar (leading indicator, ': ', ' #' or trailing ':'): " + f"{value!r}", + 1, + ) + if _RETYPED_KEYWORD_RE.match(value) or _RETYPED_NUMBER_RE.match(value) or _RETYPED_TIMESTAMP_RE.match(value): + raise WriterError( + f"{label} would be read back as a boolean, null, number or date rather than a string: {value!r}", 1 + ) + + +def validate_identity(agent: str, project: str) -> None: + if not AGENT_RE.match(agent): + raise WriterError(f"agent {agent!r} does not match the protocol agent-name rule {AGENT_RE.pattern}", 1) + validate_plain_scalar("agent", agent) + if not valid_project_segment(project): + raise WriterError( + f"project {project!r} is not a valid workspace name (must not start with '.' or contain '/' or '\\')", + 1, + ) + validate_plain_scalar("project", project) + + +def validate_related_item(item: str) -> None: + """A related item is emitted inside double quotes with no escaping.""" + if not item or item != item.strip(): + raise WriterError(f"related item must be a non-empty trimmed string: {item!r}", 1) + if CONTROL_CHARS_RE.search(item) or '"' in item or "\\" in item: + raise WriterError(f"related item must not contain control characters, double quotes or backslashes: {item!r}", 1) + if len(item) > MAX_SCALAR_CHARS: + raise WriterError(f"related item is longer than {MAX_SCALAR_CHARS} characters", 1) + + +def validate_body(body: bytes) -> None: + """The record is a Markdown file, so the body must be non-empty, valid UTF-8.""" + if not body: + raise WriterError("refusing to publish an event with an empty body", 1) + try: + body.decode("utf-8") + except UnicodeDecodeError as exc: + raise WriterError(f"event body is not valid UTF-8 at byte {exc.start}: {exc.reason}", 1) from exc + + +def body_from_file(raw: bytes) -> bytes: + """The body a body *file* contributes: read as text, the way the kernel script reads it. + + The script opens the file in text mode, so universal newlines apply -- + ``\r\n`` and a lone ``\r`` both become ``\n`` -- and then trailing + newlines are dropped. A Windows-authored body file therefore produces the + same record bytes as its LF twin. Stdin and ``--body`` are not files: the + script takes those as given (trailing ``\n`` dropped, ``\r`` kept), and so + does the verb. A file that is not UTF-8 is refused here, before anything + is composed. + """ + try: + text = raw.decode("utf-8") + except UnicodeDecodeError as exc: + raise WriterError(f"event body is not valid UTF-8 at byte {exc.start}: {exc.reason}", 1) from exc + return text.replace("\r\n", "\n").replace("\r", "\n").rstrip("\n").encode("utf-8") + + +def _as_utc(now: Optional[dt.datetime]) -> dt.datetime: + if now is None: + return dt.datetime.now(dt.timezone.utc) + if now.tzinfo is None: + return now.replace(tzinfo=dt.timezone.utc) + return now.astimezone(dt.timezone.utc) + + +# -------------------------------------------------------------------------- +# record composition +# -------------------------------------------------------------------------- + + +def related_scalar(related: Sequence[str]) -> str: + """``["a", "b"]``: the JSON-array shape the kernel script emits, unescaped.""" + return "[" + ", ".join(f'"{item}"' for item in related) + "]" + + +def frontmatter_fields( + *, + agent: str, + project: str, + event_type: str, + created_at: dt.datetime, + source_ref: Optional[str] = None, + related: Optional[Sequence[str]] = None, + supersedes: Optional[str] = None, +) -> Dict[str, str]: + """The frontmatter mapping, in emission order, every value already a scalar string.""" + fields: Dict[str, str] = { + "created_at_utc": created_at.strftime(TIMESTAMP_FORMAT), + "date": created_at.strftime("%Y-%m-%d"), + "agent": agent, + "project": project, + "type": event_type, + } + if source_ref: + fields["source_ref"] = source_ref + if related: + fields["related"] = related_scalar(related) + if supersedes: + fields["supersedes"] = supersedes + return fields + + +def compose_record( + *, + agent: str, + project: str, + event_type: str, + created_at: dt.datetime, + body: bytes, + source_ref: Optional[str] = None, + related: Optional[Sequence[str]] = None, + supersedes: Optional[str] = None, +) -> bytes: + """Build the full record: plain-scalar frontmatter, a blank line, the body, a newline. + + Byte-identical to the kernel script's ``build_event`` for the same inputs. + """ + fields = frontmatter_fields( + agent=agent, + project=project, + event_type=event_type, + created_at=created_at, + source_ref=source_ref, + related=related, + supersedes=supersedes, + ) + lines = [f"{key}: {value}" for key, value in fields.items()] + head = FRONTMATTER_DELIM + ("\n".join(lines) + "\n").encode("utf-8") + FRONTMATTER_DELIM + record = head + b"\n" + body + b"\n" + _assert_record_roundtrips(record, fields, body) + return record + + +def _assert_record_roundtrips(record: bytes, fields: Dict[str, str], body: bytes) -> None: + """Re-parse the composed record and assert it says what it was asked to say. + + Composition is the one step that can silently change a record's identity: + the read-back after publication compares the file against these same + bytes, so the writer closes the loop itself, before the store is touched. + """ + parsed, parsed_body = parse_record(record) + if list(parsed) != list(fields): + raise WriterError(f"composed frontmatter does not carry exactly the composed fields in order: {list(parsed)}") + for key, want in fields.items(): + if parsed[key] != want: + raise WriterError(f"composed frontmatter field {key!r} did not round-trip: {parsed[key]!r} != {want!r}") + if parsed_body != body: + raise WriterError("composed record body did not round-trip byte-for-byte") + + +def parse_record(raw: bytes) -> Tuple[Dict[str, str], bytes]: + """Split a stored event into (frontmatter mapping, body bytes) and check its shape. + + The body is what the writer was handed: the bytes between the blank line + that follows the frontmatter and the newline the record ends with. The + frontmatter must carry the required fields first, in order, then any + optional fields in their order, and every value must read back as the + string that was written -- the check the verifier runs over the staged + bytes and over the read-back. + """ + frontmatter, rest = split_record(raw) + keys = list(frontmatter) + if keys[: len(REQUIRED_FRONTMATTER_ORDER)] != list(REQUIRED_FRONTMATTER_ORDER): + raise WriterError(f"event frontmatter does not open with the required fields in order: {keys}") + optional = keys[len(REQUIRED_FRONTMATTER_ORDER) :] + expected_optional = [key for key in OPTIONAL_FRONTMATTER_ORDER if key in optional] + if optional != expected_optional or len(set(optional)) != len(optional): + raise WriterError(f"event frontmatter carries unknown, duplicate or misordered optional fields: {optional}") + + try: + created = dt.datetime.strptime(frontmatter["created_at_utc"], TIMESTAMP_FORMAT) + except ValueError as exc: + raise WriterError(f"created_at_utc is not a UTC timestamp: {frontmatter['created_at_utc']!r}") from exc + if frontmatter["date"] != created.strftime("%Y-%m-%d"): + raise WriterError(f"date {frontmatter['date']!r} does not match created_at_utc {frontmatter['created_at_utc']!r}") + try: + validate_identity(frontmatter["agent"], frontmatter["project"]) + validate_type(frontmatter["type"]) + if "source_ref" in frontmatter: + validate_plain_scalar("source_ref", frontmatter["source_ref"]) + if "supersedes" in frontmatter: + validate_plain_scalar("supersedes", frontmatter["supersedes"]) + if "related" in frontmatter: + items = _parse_related_scalar(frontmatter["related"]) + for item in items: + validate_related_item(item) + except WriterError as exc: + raise WriterError(f"event frontmatter does not read back as written: {exc}") from exc + + if not rest.startswith(b"\n") or not rest.endswith(b"\n") or len(rest) < 3: + raise WriterError("event body is not a blank line, the body and a closing newline") + return frontmatter, rest[1:-1] + + +def _parse_related_scalar(value: str) -> List[str]: + try: + items = json.loads(value) + except json.JSONDecodeError as exc: + raise WriterError(f"related is not a JSON array of strings: {value!r}") from exc + if not isinstance(items, list) or not items or not all(isinstance(item, str) for item in items): + raise WriterError(f"related is not a non-empty JSON array of strings: {value!r}") + if related_scalar(items) != value: + raise WriterError(f"related does not re-serialize to the bytes written: {value!r}") + return items + + +def verify_record(raw: bytes) -> None: + """The consistency check publication runs over staged and read-back bytes.""" + parse_record(raw) + + +def canonical_name(created_at: dt.datetime, slug: str) -> str: + return f"{created_at.strftime('%Y%m%d-%H%M%S')}-{slug}.md" + + +def canonical_path(home: Path, created_at: dt.datetime, slug: str) -> Path: + return layout.org_memory_dir(home) / EVENTS_DIR / canonical_name(created_at, slug) + + +# -------------------------------------------------------------------------- +# the verb +# -------------------------------------------------------------------------- + + +def write_event( + *, + home: Path, + agent: str, + project: str, + event_type: str, + slug: str, + body: bytes, + source_ref: Optional[str] = None, + related: Optional[Sequence[str]] = None, + supersedes: Optional[str] = None, + now: Optional[dt.datetime] = None, + dry_run: bool = False, +) -> EventResult: + """Validate, compose and publish one event. + + ``now`` names the clock the record is stamped and named with; it defaults + to the current UTC time. With ``dry_run`` the record is validated and + composed exactly as it would be published, and the store is not touched -- + no directories created, no files written. The status is then ``dry-run``. + """ + validate_slug(slug) + validate_type(event_type) + validate_identity(agent, project) + if source_ref: + validate_plain_scalar("source_ref", source_ref) + if supersedes: + validate_plain_scalar("supersedes", supersedes) + for item in related or (): + validate_related_item(item) + validate_body(body) + + created_at = _as_utc(now) + record = compose_record( + agent=agent, + project=project, + event_type=event_type, + created_at=created_at, + body=body, + source_ref=source_ref, + related=related, + supersedes=supersedes, + ) + target = canonical_path(home, created_at, slug) + created_at_utc = created_at.strftime(TIMESTAMP_FORMAT) + if dry_run: + return EventResult(target, STATUS_DRY_RUN, created_at_utc, record) + status = publish(target, record, verify=verify_record) + return EventResult(target, status, created_at_utc, record) diff --git a/src/agent_memory/layout.py b/src/agent_memory/layout.py index ebf16f8..0b69a31 100644 --- a/src/agent_memory/layout.py +++ b/src/agent_memory/layout.py @@ -39,6 +39,19 @@ NEVER_SYNCED_DIRS: Tuple[str, ...] = ("keys",) _NEVER_SYNCED_COMMENT = "# never sync private key material — explicit deny, wins over any future allowlist widening" +#: Lines earlier versions of the managed block carried and the current one does +#: not. :func:`agent_memory.sync.ensure_gitignore` retires them when it brings a +#: home's block up to date. Left in place they would follow the block as custom +#: rules and, git reading an ignore file last-match-wins, keep their old effect. +#: A block edit that changes or drops a line appends the old line here; never +#: remove an entry, a home somewhere may still carry it. +SUPERSEDED_GITIGNORE_LINES: Tuple[str, ...] = ( + # Unanchored through 0.1.0: re-admitted a .gitignore or a marker at any depth, + # so an excluded tree's own ignore file read as an untracked change and blocked pull. + f"!{GITIGNORE_FILE}", + f"!{MARKER_FILE}", +) + @dataclass(frozen=True) class Tier: @@ -79,7 +92,10 @@ def parts(self) -> Tuple[str, ...]: def gitignore_text() -> str: """The canonical sync allowlist for a home's ``.gitignore``, byte for byte.""" - lines = [WILDCARD, f"!{WILDCARD}/", f"!{GITIGNORE_FILE}", f"!{MARKER_FILE}"] + # The ignore file and the marker are root-only: anchored, so neither entry + # re-admits a same-named file deeper in the tree. One that sits inside a tier + # is still ordinary tier content; anywhere else it stays ignored. + lines = [WILDCARD, f"!{WILDCARD}/", f"!/{GITIGNORE_FILE}", f"!/{MARKER_FILE}"] lines.extend(f"!{tier.pattern}/**" for tier in TIERS) lines.extend(f"{tier.pattern}/{sub}/" for tier in TIERS for sub in tier.unsynced) lines.append(_NEVER_SYNCED_COMMENT) diff --git a/src/agent_memory/sync.py b/src/agent_memory/sync.py index 0ca006c..a76f78d 100644 --- a/src/agent_memory/sync.py +++ b/src/agent_memory/sync.py @@ -20,10 +20,11 @@ reported. The home must be the root of its own git worktree; a home nested inside another repository is refused before anything is written. -``.gitignore`` is never overwritten. The canonical allowlist is a managed -block that ``init`` puts at the head of the file when it is missing and -leaves alone when it is present, keeping every other line, and every write -comes back with a before/after receipt. +``.gitignore`` is never replaced wholesale. The canonical allowlist is a +managed block that ``init`` puts at the head of the file when it is missing +or out of date and leaves alone when it is current, keeping every other line +except the ones an earlier block carried and the current one retired, and +every write comes back with a before/after receipt that names them. The network verbs (fetch, pull, push, clone) run under a 30 s timeout. ``pull`` fast-forwards only when the tree is clean, not ahead, not diverged @@ -57,6 +58,22 @@ _AGENT_FALLBACK_ENV = ("AGENT_NAME", "USER") UNKNOWN_AGENT = "unknown" +#: git's own words when no credential helper produced credentials and it fell back +#: to prompting. On its own this is also what an unconfigured remote looks like. +_CREDENTIAL_PROMPT_FAILURES = ("could not read username for", "could not read password for") +#: What the helper itself left behind when a sandbox blocked its `op` call. Every +#: entry must be helper-origin: paired with a prompt failure above, this is the +#: signature -- and not a rejected push. Text git emits on its own never belongs +#: here, however sandbox-flavoured it reads. "Device not configured" is the trap: +#: it is macOS's errno for git's own terminal fallback, so it accompanies *every* +#: prompt failure on the platform and identifies nothing. +_SANDBOXED_HELPER_TELLS = ("1password document", "osstatus") +_HELPER_BLOCKED_LINE = ( + "memory push: the git credential helper could not run under the sandbox; its `op` call was " + "blocked, so git was left without credentials for the remote. Rerun with the sandbox off. " + "The commit remains local." +) + class SyncError(Exception): """A precondition failed; nothing was changed.""" @@ -90,6 +107,8 @@ class GitignoreReceipt: action: str before: Optional[str] after: str + #: Lines of an earlier managed block that the update dropped, in file order. + retired: Tuple[str, ...] = () @property def changed(self) -> bool: @@ -104,6 +123,12 @@ def lines(self) -> List[str]: out = [ f"{GITIGNORE_FILE}: managed block added at the top; " f"{len(kept.splitlines())} existing line(s) kept after it.", + ] + if self.retired: + out.append( + f"{GITIGNORE_FILE}: {len(self.retired)} superseded line(s) retired: {', '.join(self.retired)}" + ) + out += [ f"{GITIGNORE_FILE} before:", *_indent(self.before.splitlines() or ["(empty)"]), f"{GITIGNORE_FILE} after:", @@ -147,35 +172,44 @@ def write_marker(home: Path) -> bool: def ensure_gitignore(home: Path) -> GitignoreReceipt: - """Put the managed block at the head of ``.gitignore`` unless it is already there. + """Put the managed block at the head of ``.gitignore`` unless it is already current. The block is the canonical allowlist from :func:`layout.gitignore_text`. A missing file is created with the block alone, so a fresh home's file is byte-identical to the canonical text. A file that already contains the - block, contiguous and on line boundaries, is left untouched wherever the - block sits. Otherwise the block goes first and every line of the existing - file that is not itself a block line follows it verbatim: custom rules - survive, and a file carrying an older version of the block is brought up - to date without duplicating its lines. + block, contiguous and on line boundaries, and none of the lines an earlier + block carried (:data:`layout.SUPERSEDED_GITIGNORE_LINES`) is left untouched + wherever the block sits. Otherwise the block goes first and every line of + the existing file that is neither a block line nor a superseded one follows + it verbatim: custom rules survive, a file carrying an older version of the + block is brought up to date without duplicating its lines, and the lines + that version retired go with it -- git reads an ignore file last-match-wins, + so a retired rule left after the block would keep its old effect. The + receipt names what was retired. """ path = home / GITIGNORE_FILE block = layout.gitignore_text() + superseded = set(layout.SUPERSEDED_GITIGNORE_LINES) try: before: Optional[str] = path.read_text(encoding="utf-8") except FileNotFoundError: before = None + retired: Tuple[str, ...] = () if before is None: after, action = block, "created" - elif _contains_block(before, block): - after, action = before, "unchanged" else: - managed = set(block.splitlines()) - kept = [line for line in before.replace("\r\n", "\n").splitlines() if line not in managed] - after = block + ("\n".join(kept) + "\n" if kept else "") - action = "updated" + lines = before.replace("\r\n", "\n").splitlines() + retired = tuple(dict.fromkeys(line for line in lines if line in superseded)) + if _contains_block(before, block) and not retired: + after, action = before, "unchanged" + else: + managed = set(block.splitlines()) + kept = [line for line in lines if line not in managed and line not in superseded] + after = block + ("\n".join(kept) + "\n" if kept else "") + action = "updated" if after != before: path.write_text(after, encoding="utf-8") - return GitignoreReceipt(path, action, before, after) + return GitignoreReceipt(path, action, before, after, retired) def _contains_block(text: str, block: str) -> bool: @@ -363,7 +397,8 @@ def push( Silent on a home that is not configured for sync. A home that is behind or diverged is refused before anything is committed. A commit that could not be delivered -- no remote, an unreachable one, a rejected push -- is - reported as such, distinctly from delivery. + reported as such, distinctly from delivery; a credential helper a sandbox + blocked is named as its own cause, not as a rejection. """ home = _home(home) if not is_configured(home): @@ -409,15 +444,21 @@ def select_paths(home: Path, runner: Optional[GitRunner] = None) -> Tuple[List[s so a widened ignore file cannot smuggle a tier's unsynced subdirectory in. """ pathspecs = [GITIGNORE_FILE, MARKER_FILE, *(f":(glob){tier.pattern}/**" for tier in layout.TIERS)] - result = _git(home, ["status", "--porcelain=v1", "-z", "--untracked-files=all", "--", *pathspecs], runner) - if not result.ok: - raise SyncError(f"git status failed: {result.output}") - paths = _parse_status_z(result.stdout) + paths = _status_paths(home, runner, pathspecs) selected = sorted(path for path in paths if layout.is_allowed_memory_path(path)) outside = sorted(path for path in paths if not layout.is_allowed_memory_path(path)) return selected, outside +def changed_paths(home: Path, runner: Optional[GitRunner] = None) -> List[str]: + """Every path ``git status`` reports anywhere in the tree, untracked files one by one. + + The whole-tree readout behind :attr:`GitState.dirty`, as paths: what ``pull`` + refuses to fast-forward over, whether or not the allowlist selects it. + """ + return _status_paths(home, runner) + + def staged_paths(home: Path, runner: Optional[GitRunner] = None) -> List[str]: """Every path the index differs in from HEAD (or from the empty tree before the first commit).""" result = _git(home, ["diff", "--cached", "--name-only", "-z"], runner) @@ -506,6 +547,8 @@ def _deliver( receipt: Optional[GitignoreReceipt] = None, ) -> Outcome: if state.fetch_failed: + if _credential_helper_blocked(state.fetch_output): + 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) @@ -514,6 +557,8 @@ def _deliver( return Outcome("up_to_date", True, tuple(lines), (), published.preserved, receipt) result = _push_remote(home, state, runner) if not result.ok: + if _credential_helper_blocked(result.output): + return _helper_blocked(lines, published, receipt) 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" @@ -530,6 +575,30 @@ def _push_remote(home: Path, state: GitState, runner: Optional[GitRunner]) -> Gi return _git(home, ["push", "--quiet", "-u", remote, branch], runner, timeout=NETWORK_TIMEOUT_SECONDS) +def _credential_helper_blocked(output: str) -> bool: + """Does this git failure carry the blocked-credential-helper signature? + + git falls back to prompting when a helper hands it nothing, and under a + sandbox that prompt has no terminal to read from either. Both halves are + required, and the second must come from the helper: a prompt failure on + its own is what any host without usable credentials looks like, down to + the errno text macOS appends to it. + """ + lowered = output.lower() + if not any(prompt in lowered for prompt in _CREDENTIAL_PROMPT_FAILURES): + return False + return any(tell in lowered for tell in _SANDBOXED_HELPER_TELLS) + + +def _helper_blocked( + lines: List[str], published: Outcome, receipt: Optional[GitignoreReceipt] +) -> Outcome: + lines.append(_HELPER_BLOCKED_LINE) + return Outcome( + "credential_helper_blocked", False, tuple(lines), published.committed, published.preserved, receipt + ) + + def _fetch_failure(state: GitState, verb: str) -> Outcome: if state.fetch_timed_out: return Outcome("fetch_timed_out", False, (f"{verb}: the fetch timed out; {state.fetch_output}".rstrip(),)) @@ -566,6 +635,16 @@ def _require_root(home: Path, runner: Optional[GitRunner]) -> None: ) +def _status_paths(home: Path, runner: Optional[GitRunner], pathspecs: Sequence[str] = ()) -> List[str]: + args = ["status", "--porcelain=v1", "-z", "--untracked-files=all"] + if pathspecs: + args += ["--", *pathspecs] + result = _git(home, args, runner) + if not result.ok: + raise SyncError(f"git status failed: {result.output}") + return _parse_status_z(result.stdout) + + def _status_porcelain(home: Path, runner: Optional[GitRunner]) -> str: result = _git(home, ["status", "--porcelain"], runner) if not result.ok: @@ -599,7 +678,12 @@ def _is_non_empty(path: Path) -> bool: def _parse_status_z(data: str) -> List[str]: - """Paths from ``git status --porcelain=v1 -z``; a rename or copy contributes both of its paths.""" + """Paths from ``git status --porcelain=v1 -z``; a rename or copy contributes both of its paths. + + The ``R``/``C`` marker sits in the index column for a staged move and in the + worktree column for one git detects in the tree (an intent-to-add move); either + way the record carries the source path as the next NUL-separated field. + """ paths: List[str] = [] fields = data.split("\0") index = 0 @@ -609,7 +693,7 @@ def _parse_status_z(data: str) -> List[str]: if len(entry) < 4: continue paths.append(entry[3:]) - if entry[0] in "RC" and index < len(fields) and fields[index]: + if (entry[0] in "RC" or entry[1] in "RC") and index < len(fields) and fields[index]: paths.append(fields[index]) index += 1 return paths diff --git a/tests/golden/canonical_memory_gitignore.txt b/tests/golden/canonical_memory_gitignore.txt index d4bde11..9242a63 100644 --- a/tests/golden/canonical_memory_gitignore.txt +++ b/tests/golden/canonical_memory_gitignore.txt @@ -1,7 +1,7 @@ * !*/ -!.gitignore -!.oacp-memory-repo +!/.gitignore +!/.oacp-memory-repo !org-memory/** !projects/*/memory/** projects/*/memory/.cache/ diff --git a/tests/golden/event_record.md b/tests/golden/event_record.md new file mode 100644 index 0000000..bd96cd7 --- /dev/null +++ b/tests/golden/event_record.md @@ -0,0 +1,14 @@ +--- +created_at_utc: 2026-03-21T12:00:00Z +date: 2026-03-21 +agent: alice +project: demo-project +type: decision +source_ref: 20260321-alice-1f3a9c2b +related: ["PR #43", "issue #10"] +supersedes: events/20260316-120000-old-convention.md +--- + +# Decision + +Use the thing. diff --git a/tests/test_cli.py b/tests/test_cli.py index 61fd97f..9f3cb57 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -156,7 +156,8 @@ def test_distribution_version_matches_the_package() -> None: def test_source_tree_names_the_kernel_only_where_allowed() -> None: # The only permitted mentions: the compat env var line(s), the sync marker filename, # setup/legacy.py (which retires the kernel's hooks by exact match and so must spell them), - # and the debrief writer's hidden --oacp-dir alias (the flag it carried before it became a verb). + # and the hidden --oacp-dir alias of the two writer verbs, debrief write and event write (the flag + # each carried before it became a verb). src = CHECKOUT / "src" / "agent_memory" if not src.is_dir(): pytest.skip("no source checkout beside the tests") @@ -176,7 +177,7 @@ def test_source_tree_names_the_kernel_only_where_allowed() -> None: continue offending.append(f"{path.relative_to(CHECKOUT)}:{number}: {line.strip()}") assert offending == [] - assert alias_lines == 1 + assert alias_lines == 2 def test_import_resolves_to_the_installed_wheel_when_asked() -> None: diff --git a/tests/test_doctor.py b/tests/test_doctor.py index e8c5ba7..7f3e543 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -680,6 +680,74 @@ def test_an_unpublished_memory_file_is_a_warning_with_the_push_hint(tmp_path: Pa assert row.fix_hint == "Run: agent-memory push" +def test_a_dirty_memory_tree_names_the_changes(tmp_path: Path, git_env: None) -> None: + home, _ = synced_home(tmp_path) + write(home / "projects" / "demo" / "memory" / "open_threads.md", "unpublished\n") + row = _by_name(check_memory_sync(home))["working-tree"] + assert row.severity is Severity.warn + assert row.message == "working tree — DIRTY 1 memory change(s): projects/demo/memory/open_threads.md" + assert row.fix_hint == "Run `agent-memory push` or resolve changes manually" + + +def test_changes_outside_the_allowlist_are_not_dirty_memory(tmp_path: Path, git_env: None) -> None: + # The 0.1.0 block re-admitted a nested .gitignore at any depth. On such a home the counts + # are allowlist-scoped while the tree is whole-tree dirty; the row names the scope instead + # of reading DIRTY beside "untracked memory files — none". + home, _ = synced_home(tmp_path) + old_block = layout.gitignore_text().replace(f"!/{layout.GITIGNORE_FILE}\n", f"!{layout.GITIGNORE_FILE}\n") + write(home / layout.GITIGNORE_FILE, old_block) + git("commit", "--quiet", "-am", "the 0.1.0 block", cwd=home) + nested = "projects/demo/evidence/tree/.gitignore" + write(home / nested, "build/\n") + + rows = _by_name(check_memory_sync(home)) + + assert rows["untracked-memory"].message == "untracked memory files — none" + assert rows["working-tree"].severity is Severity.warn + assert "DIRTY" not in rows["working-tree"].message + assert rows["working-tree"].message == ( + f"working tree — 1 change(s) outside the memory allowlist: {nested}; " + "no memory change is pending, but memory pull refuses a dirty tree" + ) + assert rows["working-tree"].fix_hint == "Commit, stash or ignore them outside the memory tiers" + + +def test_a_worktree_rename_keeps_its_memory_source_pending(tmp_path: Path, git_env: None) -> None: + # An intent-to-add move reports as " R new\0old": the R sits in the worktree column, and the + # parser once read only the index column, so the source path came back sliced ("-memory/…") + # and the row claimed no memory change was pending while the note had left its tier. + home, _ = synced_home(tmp_path) + write(home / "org-memory" / "note.md", "note\n") + git("add", "org-memory/note.md", cwd=home) + git("commit", "--quiet", "-m", "a note", cwd=home) + (home / "org-memory" / "note.md").rename(home / "elsewhere.md") + git("add", "-N", "-f", "elsewhere.md", cwd=home) + status = git("status", "--porcelain=v2", cwd=home) # ".R": index untouched, rename in the worktree column + assert status.startswith("2 .R ") and status.endswith("R100 elsewhere.md\torg-memory/note.md") + + assert sync.changed_paths(home) == ["elsewhere.md", "org-memory/note.md"] + row = _by_name(check_memory_sync(home))["working-tree"] + assert row.severity is Severity.warn + assert row.message == ( + "working tree — DIRTY 1 memory change(s): org-memory/note.md; 1 change(s) outside the memory allowlist" + ) + assert row.fix_hint == "Run `agent-memory push` or resolve changes manually" + + +def test_a_failed_change_readout_on_a_dirty_tree_is_a_warning(tmp_path: Path) -> None: + home = _scripted_home(tmp_path) + runner = ScriptedRunner( + { + **_clean(home), + ("status", "--porcelain"): (0, "?? x"), + ("status", "--porcelain=v1", "-z", "--untracked-files=all"): (128, "fatal: index locked"), + } + ) + row = _by_name(check_memory_sync(home, runner=runner))["working-tree"] + assert row.severity is Severity.warn + assert "index locked" in row.message + + # --- the report and the command line ---------------------------------------- diff --git a/tests/test_events.py b/tests/test_events.py new file mode 100644 index 0000000..9fde5d1 --- /dev/null +++ b/tests/test_events.py @@ -0,0 +1,706 @@ +# SPDX-FileCopyrightText: 2026 Kiloloop +# SPDX-License-Identifier: Apache-2.0 +"""``agent-memory event write``: the port of the kernel's ``write_event.py`` and the writer contract on top. + +The first section carries the kernel's ``tests/test_write_event.py`` cases, +one for one under their original names (14: nine for ``normalize_related``, +three for the ``related`` frontmatter line, two dry runs through the CLI). +The rest is what the port adds: byte parity with the script, the publication +contract (idempotent retry, refused collision, clean failure), the plain-scalar +grammar the frontmatter needs, and the verb's CLI surface. +""" + +from __future__ import annotations + +import datetime as dt +import io +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from agent_memory import events as ev +from agent_memory import publication +from agent_memory.cli import main +from agent_memory.home import ENV_COMPAT_HOME, ENV_HOME + +GOLDEN = Path(__file__).resolve().parent / "golden" / "event_record.md" + +NOW = dt.datetime(2026, 3, 21, 12, 0, 0, tzinfo=dt.timezone.utc) +BODY = b"# Decision\n\nUse the thing." + +# What the kernel script composes for the minimal inputs the CLI tests use. +MINIMAL = b"---\ncreated_at_utc: 2026-03-21T12:00:00Z\ndate: 2026-03-21\nagent: claude\nproject: test\ntype: event\n---\n\nhello\n" + + +def call(home: Path, *, body: bytes = BODY, slug: str = "api-convention", **kw): + params = dict( + home=home, + agent="alice", + project="demo-project", + event_type="decision", + slug=slug, + body=body, + source_ref="20260321-alice-1f3a9c2b", + related=["PR #43", "issue #10"], + supersedes="events/20260316-120000-old-convention.md", + now=NOW, + ) + params.update(kw) + return ev.write_event(**params) + + +def expected_path(home: Path, slug: str = "api-convention") -> Path: + return home / "org-memory" / "events" / f"20260321-120000-{slug}.md" + + +def cli_args(home: Path, *extra: str) -> list: + return [ + "event", "write", + "--agent", "claude", "--project", "test", "--type", "event", "--slug", "test-slug", + "--home", str(home), *extra, + ] + + +def minimal_path(home: Path) -> Path: + return home / "org-memory" / "events" / "20260321-120000-test-slug.md" + + +@pytest.fixture +def fixed_clock(monkeypatch): + """Pin the verb's clock so a CLI run names and stamps the record deterministically.""" + monkeypatch.setattr(ev, "_as_utc", lambda now: NOW) + + +# ============================================================ ported: kernel tests/test_write_event.py === + +# --- TestNormalizeRelated (9) --- + + +def test_comma_separated(): + assert ev.normalize_related("PR #43, event/20260316-foo") == ["PR #43", "event/20260316-foo"] + + +def test_single_item(): + assert ev.normalize_related("PR #43") == ["PR #43"] + + +def test_json_array(): + assert ev.normalize_related('["PR #13", "PR #14"]') == ["PR #13", "PR #14"] + + +def test_json_single_item_array(): + assert ev.normalize_related('["PR #13"]') == ["PR #13"] + + +def test_json_empty_array(): + assert ev.normalize_related("[]") == [] + + +def test_malformed_json_falls_back_to_comma_split(): + # Starts with '[' but is not valid JSON: falls back to the comma split. + assert ev.normalize_related("[broken, json") == ["[broken", "json"] + + +def test_strips_whitespace(): + assert ev.normalize_related(" PR #1 , PR #2 ") == ["PR #1", "PR #2"] + + +def test_json_with_leading_whitespace(): + assert ev.normalize_related(' ["PR #1"] ') == ["PR #1"] + + +def test_skips_empty_items(): + assert ev.normalize_related("PR #1,,, PR #2,") == ["PR #1", "PR #2"] + + +# --- TestBuildEventRelated (3) --- + + +def _build(related=None) -> str: + return ev.compose_record( + agent="claude", project="test", event_type="event", created_at=NOW, body=b"test body", related=related + ).decode("utf-8") + + +def test_no_related_field(): + assert "related:" not in _build(related=None) + + +def test_plain_items(): + assert 'related: ["PR #43", "issue #10"]' in _build(related=["PR #43", "issue #10"]) + + +def test_no_double_quoting(): + """Pre-parsed JSON items are not quoted a second time.""" + content = _build(related=["PR #13"]) + assert 'related: ["PR #13"]' in content + assert '["[' not in content + + +# --- TestBuildEventDryRun (2): end to end through the CLI; the preview goes to stderr in this package --- + + +def test_dry_run_json_related(tmp_path, capsys): + assert main(cli_args(tmp_path, "--body", "hello", "--related", '["PR #13", "PR #14"]', "--dry-run")) == 0 + preview = capsys.readouterr().err + assert 'related: ["PR #13", "PR #14"]' in preview + assert '["[' not in preview + + +def test_dry_run_comma_related(tmp_path, capsys): + assert main(cli_args(tmp_path, "--body", "hello", "--related", "PR #13, PR #14", "--dry-run")) == 0 + assert 'related: ["PR #13", "PR #14"]' in capsys.readouterr().err + + +# ================================================================================ byte parity === + + +def test_record_is_byte_identical_to_the_script_golden(tmp_path): + """The record the kernel script composes for these inputs, captured once; the port must match it.""" + golden = GOLDEN.read_bytes() + assert call(tmp_path, dry_run=True).record == golden + result = call(tmp_path) + assert result.status == "published" + assert result.path.read_bytes() == golden + + +def test_minimal_record_matches_the_script_bytes(tmp_path): + result = ev.write_event( + home=tmp_path, agent="claude", project="test", event_type="event", slug="test-slug", body=b"hello", now=NOW + ) + assert result.record == MINIMAL + assert result.path == minimal_path(tmp_path) + assert result.path.read_bytes() == MINIMAL + + +def test_publishes_at_the_canonical_path(tmp_path): + target, status, created_at_utc, _ = call(tmp_path) + assert status == "published" + assert target == expected_path(tmp_path) + assert target.is_file() + assert created_at_utc == "2026-03-21T12:00:00Z" + + +def test_a_naive_clock_is_taken_as_utc_and_an_aware_one_is_converted(tmp_path): + naive = call(tmp_path, dry_run=True, now=dt.datetime(2026, 3, 21, 12, 0, 0)) + eastward = call(tmp_path, dry_run=True, now=NOW.astimezone(dt.timezone(dt.timedelta(hours=9)))) + assert naive.record == eastward.record == GOLDEN.read_bytes() + + +# ============================================================================ frontmatter parity === + + +def test_frontmatter_parses_back_in_canonical_order(tmp_path): + parsed, body = ev.parse_record(call(tmp_path, dry_run=True).record) + assert list(parsed) == [ + "created_at_utc", "date", "agent", "project", "type", "source_ref", "related", "supersedes", + ] + assert parsed["created_at_utc"] == "2026-03-21T12:00:00Z" + assert parsed["date"] == "2026-03-21" + assert parsed["agent"] == "alice" + assert parsed["project"] == "demo-project" + assert parsed["type"] == "decision" + assert parsed["source_ref"] == "20260321-alice-1f3a9c2b" + assert parsed["related"] == '["PR #43", "issue #10"]' + assert parsed["supersedes"] == "events/20260316-120000-old-convention.md" + assert body == BODY + + +def test_optional_fields_are_omitted_when_absent(): + parsed, body = ev.parse_record(MINIMAL) + assert list(parsed) == list(ev.REQUIRED_FRONTMATTER_ORDER) + assert body == b"hello" + + +@pytest.mark.parametrize( + "tamper", + [ + (b"date: 2026-03-21\n", b"date: 2026-03-22\n"), # date disagrees with the stamp + (b"type: decision\n", b""), # a required field missing + (b"type: decision\n", b"type: memo\n"), # a type outside the enum + (b'related: ["PR #43", "issue #10"]\n', b"related: [1, 2]\n"), # not an array of strings + (b'related: ["PR #43", "issue #10"]\n', b'related: ["PR #43","issue #10"]\n'), # not the bytes written + (b"supersedes: events/", b"related: x\nsupersedes: events/"), # duplicate / misordered optional + (b"---\n\n# Decision", b"---\n# Decision"), # the blank line before the body missing + ], +) +def test_verify_refuses_a_record_whose_frontmatter_does_not_read_back(tmp_path, tamper): + record = call(tmp_path, dry_run=True).record + old, new = tamper + assert old in record + with pytest.raises(ev.WriterError): + ev.verify_record(record.replace(old, new, 1)) + + +def test_identity_values_survive_a_real_yaml_parser(tmp_path): + yaml = pytest.importorskip("yaml") + record = call(tmp_path, dry_run=True).record.decode("utf-8") + head = record.split("---\n")[1] + loaded = yaml.safe_load(head) + assert loaded["agent"] == "alice" + assert loaded["project"] == "demo-project" + assert loaded["type"] == "decision" + assert loaded["source_ref"] == "20260321-alice-1f3a9c2b" + assert loaded["related"] == ["PR #43", "issue #10"] + assert loaded["supersedes"] == "events/20260316-120000-old-convention.md" + + +# ============================================================================= write contract === + + +def test_retry_after_success_is_idempotent(tmp_path): + first_target, first_status, _, _ = call(tmp_path) + assert first_status == "published" + stamp = first_target.stat().st_mtime_ns + + second_target, second_status, _, _ = call(tmp_path) + assert second_status == "idempotent" + assert second_target == first_target + assert second_target.stat().st_mtime_ns == stamp, "record must not be rewritten" + assert list(first_target.parent.glob(".stage.*")) == [] + + +def test_differing_record_at_the_same_name_is_refused(tmp_path): + target, _, _, _ = call(tmp_path) + original = target.read_bytes() + + with pytest.raises(ev.WriterError, match="different record") as excinfo: + call(tmp_path, body=b"# Decision\n\nUse the other thing.") + assert excinfo.value.code == 2 + + assert target.read_bytes() == original, "a published event is never replaced" + assert sorted(path.name for path in target.parent.iterdir()) == [target.name] + + +def test_failure_before_the_link_leaves_events_clean(tmp_path, monkeypatch): + def boom(*_a, **_kw): + raise OSError("disk on fire") + + monkeypatch.setattr(os, "link", boom) + with pytest.raises(OSError): + call(tmp_path) + + target = expected_path(tmp_path) + assert not target.exists() + assert list(target.parent.iterdir()) == [], "events/ holds no partial record and no staging debris" + + +def test_short_write_before_publish_leaves_events_clean(tmp_path, monkeypatch): + real_write = os.write + + def truncating_write(fd, data): + if isinstance(data, bytes) and data.startswith(b"---\n") and len(data) > 100: + return real_write(fd, data[: len(data) // 2]) + return real_write(fd, data) + + monkeypatch.setattr(os, "write", truncating_write) + with pytest.raises(ev.WriterError): + call(tmp_path) + + target = expected_path(tmp_path) + assert not target.exists() + assert list(target.parent.iterdir()) == [] + + +def test_publication_runs_the_frontmatter_verifier(tmp_path, monkeypatch): + seen = [] + + def spy(raw: bytes) -> None: + seen.append(raw) + ev.parse_record(raw) + + monkeypatch.setattr(ev, "verify_record", spy) + result = call(tmp_path) + assert result.status == "published" + assert seen and all(raw == result.record for raw in seen), "staged bytes and read-back both pass through it" + + +def test_a_symlink_at_the_canonical_path_is_refused(tmp_path): + target = expected_path(tmp_path) + target.parent.mkdir(parents=True) + decoy = tmp_path / "elsewhere.md" + decoy.write_bytes(b"not an event\n") + target.symlink_to(decoy) + + with pytest.raises(publication.WriterError, match="symlink"): + call(tmp_path) + assert decoy.read_bytes() == b"not an event\n" + assert target.is_symlink() + + +def test_dry_run_composes_the_real_record_and_writes_nothing(tmp_path): + result = call(tmp_path, dry_run=True) + assert result.status == "dry-run" + assert result.path == expected_path(tmp_path) + assert result.record == GOLDEN.read_bytes() + assert not (tmp_path / "org-memory").exists(), "the store is not even created" + + +def test_dry_run_then_publish_lands_the_previewed_bytes(tmp_path): + previewed = call(tmp_path, dry_run=True).record + published = call(tmp_path) + assert published.status == "published" + assert published.path.read_bytes() == previewed + + +# ================================================================================= validation === + + +@pytest.mark.parametrize("slug", ["", "-a", "a-", "A", "v0.2.3", "a b", "a_b", "a" * 65]) +def test_invalid_slug_is_rejected_before_anything_is_written(tmp_path, slug): + with pytest.raises(ev.WriterError, match="slug") as excinfo: + call(tmp_path, slug=slug) + assert excinfo.value.code == 1 + assert not (tmp_path / "org-memory").exists() + + +@pytest.mark.parametrize("slug", ["a", "a-b", "0-2-3", "x" * 64]) +def test_slug_grammar_boundaries_are_accepted(tmp_path, slug): + assert call(tmp_path, slug=slug, dry_run=True).path.name == f"20260321-120000-{slug}.md" + + +def test_invalid_type_is_rejected(tmp_path): + with pytest.raises(ev.WriterError, match="type") as excinfo: + call(tmp_path, event_type="memo") + assert excinfo.value.code == 1 + assert not (tmp_path / "org-memory").exists() + + +@pytest.mark.parametrize( + "field, value", + [ + ("agent", "al\nice"), + ("project", "de\x00mo"), + ("source_ref", "a\tb"), + ("supersedes", "x\nsupersedes: y"), + ], +) +def test_control_characters_cannot_inject_frontmatter(tmp_path, field, value): + with pytest.raises(ev.WriterError) as excinfo: + call(tmp_path, **{field: value}) + assert excinfo.value.code == 1 + assert not (tmp_path / "org-memory").exists() + + +@pytest.mark.parametrize("value", ["true", "Null", "y", "12", "1.0", "1e3", "0x1f", "0o7", "1_000", "1:30", "2026-03-21"]) +def test_a_value_a_yaml_reader_would_retype_is_refused(tmp_path, value): + """The frontmatter is plain scalars by contract, so it refuses what it cannot carry as a string.""" + with pytest.raises(ev.WriterError, match="read back as") as excinfo: + call(tmp_path, source_ref=value) + assert excinfo.value.code == 1 + + +@pytest.mark.parametrize("value", ["- item", "[a]", "{a}", "a: b", "a #b", "trailing:", '"quoted"', "'quoted'", "*ref"]) +def test_a_value_a_yaml_reader_would_read_as_structure_is_refused(tmp_path, value): + with pytest.raises(ev.WriterError, match="plain YAML scalar") as excinfo: + call(tmp_path, supersedes=value) + assert excinfo.value.code == 1 + + +@pytest.mark.parametrize("value", [" padded", "padded ", ""]) +def test_an_untrimmed_or_empty_scalar_is_refused(tmp_path, value): + with pytest.raises(ev.WriterError) as excinfo: + call(tmp_path, agent=value) + assert excinfo.value.code == 1 + + +def test_a_scalar_past_the_length_cap_is_refused(tmp_path): + with pytest.raises(ev.WriterError, match="longer than"): + call(tmp_path, source_ref="x" * (ev.MAX_SCALAR_CHARS + 1)) + + +@pytest.mark.parametrize("item", ['PR "43"', "back\\slash", "a\nb"]) +def test_a_related_item_that_breaks_the_array_is_refused(tmp_path, item): + with pytest.raises(ev.WriterError, match="related item") as excinfo: + call(tmp_path, related=[item]) + assert excinfo.value.code == 1 + + +@pytest.mark.parametrize("agent", ["-lead", "a/b", "alice bob"]) +def test_agent_grammar_violations_are_rejected(tmp_path, agent): + with pytest.raises(ev.WriterError, match="agent") as excinfo: + call(tmp_path, agent=agent) + assert excinfo.value.code == 1 + + +@pytest.mark.parametrize("project", [".hidden", "a/b", "a\\b"]) +def test_project_grammar_violations_are_rejected(tmp_path, project): + with pytest.raises(ev.WriterError, match="project") as excinfo: + call(tmp_path, project=project) + assert excinfo.value.code == 1 + + +def test_empty_body_is_rejected(tmp_path): + with pytest.raises(ev.WriterError, match="empty body") as excinfo: + call(tmp_path, body=b"") + assert excinfo.value.code == 1 + assert not (tmp_path / "org-memory").exists() + + +def test_a_body_that_is_not_utf8_is_rejected(tmp_path): + with pytest.raises(ev.WriterError, match="UTF-8") as excinfo: + call(tmp_path, body=b"\xff\xfe") + assert excinfo.value.code == 1 + assert not (tmp_path / "org-memory").exists() + + +def test_multibyte_utf8_bodies_round_trip_byte_for_byte(tmp_path): + body = "# Sesión\n\nDécision — 決定 🎉".encode("utf-8") + result = call(tmp_path, body=body) + assert result.path.read_bytes().endswith(b"\n" + body + b"\n") + assert ev.parse_record(result.path.read_bytes())[1] == body + + +# ======================================================================================== CLI === + + +def test_cli_publishes_and_reports_json(tmp_path, capsys, fixed_clock): + assert main(cli_args(tmp_path, "--body", "hello", "--json")) == 0 + payload = json.loads(capsys.readouterr().out) + assert list(payload) == ["path", "status", "created_at_utc", "date", "agent", "project", "type"] + assert payload["status"] == "published" + assert payload["created_at_utc"] == "2026-03-21T12:00:00Z" + assert payload["date"] == "2026-03-21" + assert (payload["agent"], payload["project"], payload["type"]) == ("claude", "test", "event") + assert Path(payload["path"]) == minimal_path(tmp_path) + assert minimal_path(tmp_path).read_bytes() == MINIMAL + + +def test_cli_text_output_names_status_path_and_stamp(tmp_path, capsys, fixed_clock): + assert main(cli_args(tmp_path, "--body", "hello")) == 0 + assert capsys.readouterr().out == f"published: {minimal_path(tmp_path)}\ncreated_at_utc: 2026-03-21T12:00:00Z\n" + + +def test_cli_stamps_the_current_clock(tmp_path, capsys): + before = dt.datetime.now(dt.timezone.utc).replace(microsecond=0) + assert main(cli_args(tmp_path, "--body", "hello", "--json")) == 0 + payload = json.loads(capsys.readouterr().out) + stamped = dt.datetime.strptime(payload["created_at_utc"], ev.TIMESTAMP_FORMAT).replace(tzinfo=dt.timezone.utc) + assert before <= stamped <= dt.datetime.now(dt.timezone.utc) + assert Path(payload["path"]).name == f"{stamped.strftime('%Y%m%d-%H%M%S')}-test-slug.md" + + +def test_cli_reads_the_body_from_a_file_and_drops_trailing_newlines(tmp_path, capsys, fixed_clock): + body_file = tmp_path / "body.md" + body_file.write_bytes(b"hello\n\n") + assert main(cli_args(tmp_path, "--body-file", str(body_file))) == 0 + assert minimal_path(tmp_path).read_bytes() == MINIMAL + + +# The kernel script reads a body FILE as text (universal newlines: CRLF and CR +# become LF) and takes stdin and --body as given; probed against the script at +# oacp-dev 9fe2f76 on 2026-09-10: file CRLF -> 'First line\nSecond line', stdin +# CRLF -> 'A\r\nB\r\n\r', inline CRLF -> 'a\r\nb\r'. Round-1 F-001 (codex). +KERNEL_CRLF_FILE = b"First line\r\nSecond line\r\n\r\n" +KERNEL_CRLF_FILE_BODY = b"First line\nSecond line" + + +@pytest.mark.parametrize( + "raw, body", + [ + (KERNEL_CRLF_FILE, KERNEL_CRLF_FILE_BODY), + (b"One\rTwo\r", b"One\nTwo"), + (b"a\r\r\nb\n", b"a\n\nb"), + (b"lf only\n\n", b"lf only"), + (b"no newline", b"no newline"), + ("bom \ufeffkept".encode("utf-8"), "bom \ufeffkept".encode("utf-8")), + ], +) +def test_body_from_file_reads_like_the_kernel_text_read(raw, body): + assert ev.body_from_file(raw) == body + + +def test_body_from_file_refuses_a_file_that_is_not_utf8(): + with pytest.raises(ev.WriterError, match="UTF-8") as excinfo: + ev.body_from_file(b"First\r\n\xff\xfe") + assert excinfo.value.code == 1 + + +def test_cli_crlf_body_file_produces_the_kernel_record_bytes(tmp_path, capsys, fixed_clock): + body_file = tmp_path / "windows-body.md" + body_file.write_bytes(KERNEL_CRLF_FILE) + assert main(cli_args(tmp_path, "--body-file", str(body_file))) == 0 + assert minimal_path(tmp_path).read_bytes() == MINIMAL.replace(b"\nhello\n", b"\n" + KERNEL_CRLF_FILE_BODY + b"\n") + assert b"\r" not in minimal_path(tmp_path).read_bytes() + + +def test_cli_cr_body_file_produces_the_kernel_record_bytes(tmp_path, capsys, fixed_clock): + body_file = tmp_path / "classic-mac-body.md" + body_file.write_bytes(b"hello\r") + assert main(cli_args(tmp_path, "--body-file", str(body_file))) == 0 + assert minimal_path(tmp_path).read_bytes() == MINIMAL + + +def test_cli_crlf_body_file_and_its_lf_twin_land_the_same_record(tmp_path, capsys, fixed_clock): + crlf = tmp_path / "crlf.md" + crlf.write_bytes(b"hello\r\n") + lf = tmp_path / "lf.md" + lf.write_bytes(b"hello\n") + assert main(cli_args(tmp_path, "--body-file", str(crlf))) == 0 + assert capsys.readouterr().out.startswith("published:") + assert main(cli_args(tmp_path, "--body-file", str(lf))) == 0 + assert capsys.readouterr().out.startswith("idempotent:"), "the LF twin is the same record, not a collision" + + +def test_cli_non_utf8_body_file_is_refused_before_the_store_is_touched(tmp_path, capsys): + body_file = tmp_path / "latin1.md" + body_file.write_bytes(b"caf\xe9\r\n") + assert main(cli_args(tmp_path, "--body-file", str(body_file))) == 1 + assert "UTF-8" in capsys.readouterr().err + assert not (tmp_path / "org-memory").exists() + + +def test_cli_stdin_keeps_carriage_returns_like_the_kernel(tmp_path, capsys, monkeypatch, fixed_clock): + monkeypatch.setattr(sys, "stdin", io.TextIOWrapper(io.BytesIO(b"A\r\nB\r\n\r\n"))) + assert main(cli_args(tmp_path, "--body-file", "-")) == 0 + assert ev.parse_record(minimal_path(tmp_path).read_bytes())[1] == b"A\r\nB\r\n\r" + + +def test_cli_inline_body_keeps_carriage_returns_like_the_kernel(tmp_path, capsys, fixed_clock): + assert main(cli_args(tmp_path, "--body", "a\r\nb\r\n")) == 0 + assert ev.parse_record(minimal_path(tmp_path).read_bytes())[1] == b"a\r\nb\r" + + +def test_cli_reads_the_body_from_stdin_with_a_dash(tmp_path, capsys, monkeypatch, fixed_clock): + monkeypatch.setattr(sys, "stdin", io.TextIOWrapper(io.BytesIO(b"hello\n"))) + assert main(cli_args(tmp_path, "--body-file", "-")) == 0 + assert minimal_path(tmp_path).read_bytes() == MINIMAL + + +def test_cli_reads_piped_stdin_when_no_body_flag_is_given(tmp_path, capsys, monkeypatch, fixed_clock): + monkeypatch.setattr(sys, "stdin", io.TextIOWrapper(io.BytesIO(b"hello\n"))) + assert main(cli_args(tmp_path)) == 0 + assert minimal_path(tmp_path).read_bytes() == MINIMAL + + +def test_cli_without_a_body_is_a_validation_error(tmp_path, capsys, monkeypatch): + class Terminal: + def isatty(self) -> bool: + return True + + monkeypatch.setattr(sys, "stdin", Terminal()) + assert main(cli_args(tmp_path)) == 1 + assert "no body provided" in capsys.readouterr().err + assert not (tmp_path / "org-memory").exists() + + +def test_cli_body_and_body_file_are_mutually_exclusive(tmp_path, capsys): + with pytest.raises(SystemExit) as exit_info: + main(cli_args(tmp_path, "--body", "a", "--body-file", "b.md")) + assert exit_info.value.code == 2 + assert "not allowed with" in capsys.readouterr().err + + +def test_cli_returns_one_on_an_unreadable_body_file(tmp_path, capsys): + assert main(cli_args(tmp_path, "--body-file", str(tmp_path / "missing.md"))) == 1 + assert "cannot read body" in capsys.readouterr().err + assert not (tmp_path / "org-memory").exists() + + +def test_cli_returns_one_on_a_validation_error(tmp_path, capsys): + args = cli_args(tmp_path, "--body", "hello") + args[args.index("test-slug")] = "v0.2.3" + assert main(args) == 1 + assert "slug" in capsys.readouterr().err + assert not (tmp_path / "org-memory").exists() + + +def test_cli_rejects_a_type_outside_the_enum(tmp_path, capsys): + args = cli_args(tmp_path, "--body", "hello") + args[args.index("event")] = "memo" + with pytest.raises(SystemExit) as exit_info: + main(args) + assert exit_info.value.code == 2 + assert "invalid choice" in capsys.readouterr().err + + +def test_cli_identical_rerun_exits_zero_and_leaves_the_record_unchanged(tmp_path, capsys, fixed_clock): + assert main(cli_args(tmp_path, "--body", "hello")) == 0 + stamp = minimal_path(tmp_path).stat().st_mtime_ns + assert main(cli_args(tmp_path, "--body", "hello")) == 0 + assert capsys.readouterr().out.endswith(f"idempotent: {minimal_path(tmp_path)}\ncreated_at_utc: 2026-03-21T12:00:00Z\n") + assert minimal_path(tmp_path).stat().st_mtime_ns == stamp + assert minimal_path(tmp_path).read_bytes() == MINIMAL + + +def test_cli_returns_two_on_a_differing_record_at_the_same_name(tmp_path, capsys, fixed_clock): + assert main(cli_args(tmp_path, "--body", "hello")) == 0 + assert main(cli_args(tmp_path, "--body", "goodbye")) == 2 + assert "different record" in capsys.readouterr().err + assert minimal_path(tmp_path).read_bytes() == MINIMAL + + +def test_cli_renders_every_optional_field(tmp_path, capsys, fixed_clock): + args = [ + "event", "write", "--agent", "alice", "--project", "demo-project", "--type", "decision", + "--slug", "api-convention", "--body", BODY.decode("utf-8"), + "--source-ref", "20260321-alice-1f3a9c2b", "--related", "PR #43, issue #10", + "--supersedes", "events/20260316-120000-old-convention.md", "--home", str(tmp_path), + ] + assert main(args) == 0 + assert expected_path(tmp_path).read_bytes() == GOLDEN.read_bytes() + + +def test_cli_accepts_the_hidden_oacp_dir_alias(tmp_path, capsys, fixed_clock): + store = tmp_path / "store" + args = cli_args(store, "--body", "hello", "--json") + args[args.index("--home")] = "--oacp-dir" + assert main(args) == 0 + assert Path(json.loads(capsys.readouterr().out)["path"]) == minimal_path(store) + with pytest.raises(SystemExit) as exit_info: + main(["event", "write", "--help"]) + assert exit_info.value.code == 0 + help_text = capsys.readouterr().out + assert "--home" in help_text and "--oacp-dir" not in help_text + + +def test_cli_resolves_the_home_like_every_other_verb(tmp_path, capsys, monkeypatch, fixed_clock): + store = tmp_path / "env-store" + monkeypatch.setenv(ENV_HOME, str(store)) + monkeypatch.delenv(ENV_COMPAT_HOME, raising=False) + args = cli_args(store, "--body", "hello", "--json") + del args[args.index("--home") : args.index("--home") + 2] + assert main(args) == 0 + assert Path(json.loads(capsys.readouterr().out)["path"]) == minimal_path(store) + + +def test_cli_dry_run_writes_nothing_and_reports_the_status(tmp_path, capsys, fixed_clock): + assert main(cli_args(tmp_path, "--body", "hello", "--dry-run", "--json")) == 0 + captured = capsys.readouterr() + assert json.loads(captured.out)["status"] == "dry-run" + assert captured.err.endswith(MINIMAL.decode("utf-8")), "the composed record is shown for review" + assert not (tmp_path / "org-memory").exists() + + +def test_event_without_a_subcommand_shows_help(capsys): + assert main(["event"]) == 2 + assert "usage: agent-memory" in capsys.readouterr().err + + +# ------------------------------------------------------------ installed verb --- + + +def test_the_verb_runs_from_an_unrelated_working_directory(tmp_path): + """The console script, from a working directory unrelated to the home, with no kernel and no env var.""" + project = tmp_path / "elsewhere" + project.mkdir() + body_file = project / "body.md" + body_file.write_bytes(b"hello\n") + store = tmp_path / "store" + env = {key: value for key, value in os.environ.items() if key not in (ENV_HOME, ENV_COMPAT_HOME)} + args = [sys.executable, "-m", "agent_memory", *cli_args(store, "--body-file", str(body_file), "--json")] + + dry = subprocess.run(args + ["--dry-run"], cwd=project, capture_output=True, text=True, env=env) + assert dry.returncode == 0, dry.stderr + assert '"status": "dry-run"' in dry.stdout + assert not (store / "org-memory").exists() + + real = subprocess.run(args, cwd=project, capture_output=True, text=True, env=env) + assert real.returncode == 0, real.stderr + payload = json.loads(real.stdout) + assert payload["status"] == "published" + assert Path(payload["path"]).read_bytes().endswith(b"---\n\nhello\n") diff --git a/tests/test_layout.py b/tests/test_layout.py index 01d963f..4344030 100644 --- a/tests/test_layout.py +++ b/tests/test_layout.py @@ -20,7 +20,8 @@ def _digest(root: Path) -> Dict[str, str]: def test_gitignore_text_matches_the_golden_bytes() -> None: - # tests/golden/canonical_memory_gitignore.txt is a byte copy of the 0.4.5 kernel's canonical text. + # tests/golden/canonical_memory_gitignore.txt is the canonical text byte for byte. It is deliberately + # ahead of the 0.4.5 kernel's fixture (the root entries are anchored); the kernel's copy follows separately. assert layout.gitignore_text().encode("utf-8") == GOLDEN.read_bytes() @@ -32,6 +33,21 @@ def test_gitignore_denies_keystore_last() -> None: assert lines.index("keys/") > max(index for index, line in enumerate(lines) if line.startswith("!")) +def test_the_ignore_file_and_the_marker_are_root_only() -> None: + # Anchored: a .gitignore or a marker deeper in the tree is ignored like anything else + # the allowlist leaves out, instead of reading as an untracked memory change. + lines = layout.gitignore_text().splitlines() + assert f"!/{layout.GITIGNORE_FILE}" in lines and f"!/{layout.MARKER_FILE}" in lines + assert f"!{layout.GITIGNORE_FILE}" not in lines and f"!{layout.MARKER_FILE}" not in lines + + +def test_superseded_lines_are_the_unanchored_pair_and_never_current() -> None: + # The retired set stays disjoint from the block: a line in both would make every + # ensure_gitignore run report an update that changes nothing. + assert layout.SUPERSEDED_GITIGNORE_LINES == (f"!{layout.GITIGNORE_FILE}", f"!{layout.MARKER_FILE}") + assert not set(layout.SUPERSEDED_GITIGNORE_LINES) & set(layout.gitignore_text().splitlines()) + + def test_marker_and_gitignore_names_are_the_fleet_contract() -> None: assert layout.MARKER_FILE == ".oacp-memory-repo" assert layout.GITIGNORE_FILE == ".gitignore" diff --git a/tests/test_sync.py b/tests/test_sync.py index 19586be..baf9449 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -129,6 +129,29 @@ def __call__(self, args: Sequence[str], *, cwd: Path, timeout: Optional[float] = return run_git(args, cwd=cwd, timeout=timeout) +class FetchBlockedByCredentialHelper: + """The real runner, except that the fetch fails the way a sandboxed `op` call does. + + The shape is the 2026-09-09 AM-09 acceptance transcript's -- the helper's own + TLS failure, then git's fallback prompt failing for want of a terminal -- with + the vault and item names replaced by placeholders. The predicate keys off + "1password document" and "osstatus", so no assertion here depends on what the + blocked secret was called, and this file ships to a public repository. + """ + + OUTPUT = ( + "Error: could not read 1Password document 'example-signing-key' from vault " + "'example': [ERROR] failed to request.DoUnencrypted: Post \"/api/v3/auth/start\": " + "tls: failed to verify certificate: x509: OSStatus -26276\n" + "fatal: could not read Username for 'https://github.com': Device not configured" + ) + + def __call__(self, args: Sequence[str], *, cwd: Path, timeout: Optional[float] = None) -> GitResult: + if args and args[0] == "fetch": + return GitResult(128, "", self.OUTPUT) + return run_git(args, cwd=cwd, timeout=timeout) + + # --- the network verbs carry the timeout ----------------------------------- @@ -193,6 +216,71 @@ def test_fetch_timeout_on_push_keeps_the_commit_local_and_skips_the_push(tmp_pat assert not _remote_has(remote, "org-memory/local.md") +def test_a_sandboxed_credential_helper_is_named_as_its_own_failure(tmp_path: Path) -> None: + remote, _ = _create_remote(tmp_path) + home = tmp_path / "home" + sync.clone(home, str(remote)) + _write(home / "org-memory" / "local.md", "local update\n") + + outcome = sync.push(home, runner=FetchBlockedByCredentialHelper()) + + assert outcome.status == "credential_helper_blocked" + assert not outcome.ok + assert outcome.committed == ("org-memory/local.md",) + said = outcome.lines[-1] + assert "credential helper" in said and "sandbox off" in said + assert "The commit remains local." in said + assert "was rejected" not in said and "the fetch failed" not in said + assert _head_paths(home) == ["org-memory/local.md"] + assert not _remote_has(remote, "org-memory/local.md") + + +#: git's prompt failure with no helper in play, as each platform spells the errno. +#: macOS's "Device not configured" is git's own terminal fallback failing, not a +#: helper tell -- it rides along with every prompt failure on the platform. +PROMPT_ONLY_FAILURES = { + "macos": "fatal: could not read Username for 'http://127.0.0.1:62548': Device not configured", + "linux": "fatal: could not read Username for 'https://github.com': No such device or address", +} + + +@pytest.mark.parametrize("platform", sorted(PROMPT_ONLY_FAILURES)) +def test_a_prompt_failure_without_helper_evidence_is_not_a_blocked_helper(platform: str) -> None: + """Both halves are required, and the second one has to come from the helper.""" + assert not sync._credential_helper_blocked(PROMPT_ONLY_FAILURES[platform]) + + +def test_helper_evidence_alongside_the_prompt_failure_is_the_signature() -> None: + assert sync._credential_helper_blocked(FetchBlockedByCredentialHelper.OUTPUT) + + +@pytest.mark.parametrize("platform", sorted(PROMPT_ONLY_FAILURES)) +def test_a_prompt_failure_keeps_the_generic_diagnostic_through_delivery( + tmp_path: Path, platform: str +) -> None: + """The whole path, not just the predicate: no helper evidence, no sandbox claim.""" + output = PROMPT_ONLY_FAILURES[platform] + + class FetchFailsWithoutAHelper: + def __call__(self, args: Sequence[str], *, cwd: Path, timeout: Optional[float] = None) -> GitResult: + if args and args[0] == "fetch": + return GitResult(128, "", output) + return run_git(args, cwd=cwd, timeout=timeout) + + remote, _ = _create_remote(tmp_path) + home = tmp_path / "home" + sync.clone(home, str(remote)) + _write(home / "org-memory" / "local.md", "local update\n") + + outcome = sync.push(home, runner=FetchFailsWithoutAHelper()) + + assert outcome.status == "fetch_failed" + assert not outcome.ok + said = outcome.lines[-1] + assert output in said and "The commit remains local." in said + assert "sandbox" not in said and "credential helper" not in said + + # --- the selected root must be the worktree root --------------------------- @@ -286,6 +374,65 @@ def test_init_upgrades_an_older_block_without_duplicating_its_lines(tmp_path: Pa assert (home / ".gitignore").read_text(encoding="utf-8") == CANONICAL + "private-notes/\n" +#: The block 0.1.0 wrote: the root entries unanchored, so git re-admitted a file of either +#: name at any depth and an excluded tree's own ignore file read as an untracked change. +OLD_BLOCK = CANONICAL.replace(f"!/{layout.GITIGNORE_FILE}\n", f"!{layout.GITIGNORE_FILE}\n").replace( + f"!/{MARKER}\n", f"!{MARKER}\n" +) +SUPERSEDED = layout.SUPERSEDED_GITIGNORE_LINES +NESTED_IGNORE = "projects/demo/evidence/tree/.gitignore" + + +def test_the_old_block_differs_from_canonical_by_the_superseded_pair() -> None: + assert OLD_BLOCK != CANONICAL + assert [line for line in OLD_BLOCK.splitlines() if line not in CANONICAL.splitlines()] == list(SUPERSEDED) + + +def test_init_retires_the_unanchored_root_entries_of_the_old_block(tmp_path: Path) -> None: + # A home carrying the 0.1.0 block ends with the anchored lines and no unanchored survivor, + # and the nested ignore file that read as an untracked change is ignored again. + home = _local_home(tmp_path) + _write(home / ".gitignore", OLD_BLOCK) + _write(home / NESTED_IGNORE, "build/\n") + assert f"?? {NESTED_IGNORE}" in _git("status", "--porcelain", "--untracked-files=all", cwd=home) + + outcome = sync.init(home) + + receipt = outcome.receipt + assert outcome.ok and receipt is not None and receipt.action == "updated" + assert receipt.retired == SUPERSEDED + after = (home / ".gitignore").read_text(encoding="utf-8") + assert after == CANONICAL + assert not any(line in SUPERSEDED for line in after.splitlines()) + assert f".gitignore: 2 superseded line(s) retired: {', '.join(SUPERSEDED)}" in outcome.lines + assert _git("status", "--porcelain", "--untracked-files=all", cwd=home) == "" + + +def test_init_keeps_a_genuine_custom_negation_rule_while_retiring_the_old_pair(tmp_path: Path) -> None: + # Retirement is by exact line, so a user's own negation rule survives the migration. + home = _local_home(tmp_path) + custom = "# mine\n!notes/**\n" + _write(home / ".gitignore", OLD_BLOCK + custom) + + outcome = sync.init(home) + + assert outcome.receipt is not None and outcome.receipt.retired == SUPERSEDED + assert (home / ".gitignore").read_text(encoding="utf-8") == CANONICAL + custom + + +def test_init_retires_a_stale_pair_left_after_a_current_block(tmp_path: Path) -> None: + # The shape a block-only fix would have left behind: the current block with the old pair + # surviving after it as "custom rules", still re-admitting nested files last-match-wins. + home = _local_home(tmp_path) + _write(home / ".gitignore", CANONICAL + "\n".join(SUPERSEDED) + "\n") + + outcome = sync.init(home) + + assert outcome.receipt is not None and outcome.receipt.action == "updated" + assert outcome.receipt.retired == SUPERSEDED + assert (home / ".gitignore").read_text(encoding="utf-8") == CANONICAL + + def test_init_keeps_the_bytes_of_an_existing_marker(tmp_path: Path) -> None: home = _local_home(tmp_path) _write(home / MARKER, "legacy marker text\n") @@ -852,3 +999,32 @@ def test_status_parser_takes_both_paths_of_a_rename() -> None: data = "R new.md\0old.md\0 M other.md\0?? fresh.md\0" assert sync._parse_status_z(data) == ["new.md", "old.md", "other.md", "fresh.md"] assert sync._parse_status_z("") == [] + + +@pytest.mark.parametrize("xy", ["R ", " R", "RM", "C ", " C"]) +def test_status_parser_reads_the_rename_marker_in_either_column(xy: str) -> None: + # Staged moves carry R/C in the index column; a move git detects in the tree (intent-to-add) + # carries it in the worktree column. Both records are followed by the source path. + assert sync._parse_status_z(f"{xy} new.md\0old.md\0 M other.md\0") == ["new.md", "old.md", "other.md"] + + +def test_push_commits_a_rename_git_detects_in_the_working_tree(tmp_path: Path) -> None: + # The source of an intent-to-add move once came back sliced ("-memory/a.md") and push + # refused it as a path outside the allowlist. + remote, _ = _create_remote(tmp_path) + home = tmp_path / "home" + sync.clone(home, str(remote)) + _write(home / "org-memory" / "a.md", "a\n") + assert sync.push(home).ok + (home / "org-memory" / "a.md").rename(home / "org-memory" / "b.md") + _git("add", "-N", "-f", "org-memory/b.md", cwd=home) + status = _git("status", "--porcelain=v2", cwd=home) # ".R": index untouched, rename in the worktree column + assert status.startswith("2 .R ") and status.endswith("R100 org-memory/b.md\torg-memory/a.md") + + assert sync.select_paths(home) == (["org-memory/a.md", "org-memory/b.md"], []) + outcome = sync.push(home) + + assert outcome.status == "pushed" and outcome.ok, outcome.lines + assert outcome.committed == ("org-memory/b.md",) # HEAD reports a rename by its destination + assert _git("status", "--porcelain", cwd=home) == "" + assert _git("show", "--name-status", "--format=", "HEAD", cwd=home) == "R100\torg-memory/a.md\torg-memory/b.md"