feat(context): PR-B — mm context install skill + lockfile + concurrency by memtomem · Pull Request #623 · memtomem/memtomem

memtomem · 2026-04-30T14:42:23Z

Summary

Implements ADR-0008 PR-B: install a wiki asset into a project tree with a recorded commit pin and concurrency-safe lockfile. Builds on PR-A (#622) which scaffolded ~/.memtomem-wiki/, mm wiki init, and mm wiki list.

CLI: mm context install skill <name> — copytree snapshot of ~/.memtomem-wiki/skills/<name>/ into <project>/.memtomem/skills/<name>/, pinned to the wiki's HEAD commit.
Lockfile: <project>/.memtomem/lock.json (version 1) records wiki_commit (full 40-char SHA) and microsecond-precision installed_at. Reads round-trip through dict so unknown forward-compat fields survive untouched (ADR-0008 §Lockfile schema).
Concurrency: lock window covers only the lockfile JSON load→mutate→atomic_write; the slow copytree runs unlocked. The wiki commit is pinned before copy so a concurrent git pull cannot drift the recorded wiki_commit from the bytes that were copied.

Scope

PR-B is skill-only by design. Agents and commands land in PR-C alongside override resolution — without override-aware extraction the canonical snapshot exists on disk but does not flow through fan-out, which would surprise users into thinking install is broken. The Choice(['skill']) widens to Choice(['skill', 'agent', 'command']) in one line when PR-C lands; the internal _install_asset(asset_type, …) already takes the wider parameter.

Out of scope per ADR-0008 PR breakdown:

PR-C — override resolution, OVERRIDE_FORMATS, mm wiki <type> override
PR-D — mm context update, --force, .bak, mm context status, install --all, mtime/dirty detection
v2 — --commit <ref> flag, directory-level os.replace atomic snapshot

Refusal posture (forward-protects Invariant 2)

AlreadyInstalledError fires on either existing-lockfile-entry OR existing-dest-dir, not AND. Catches the four cases:

lockfile entry	dest dir	behavior
no	no	proceed
yes	yes	refuse — normal "already installed"
yes	no	refuse — user wiped dest, would silently re-pin
no	yes	refuse — dest exists outside lockfile, would clobber

The error message classifies which side is present so the user knows exactly what to remove.

Refactors carried in this PR

copy_tree_atomic hoisted from context/skills.py to context/_atomic.py as a public helper with defensive .git skip (single-file ripple verified by git grep).
_file_lock + _lock_path_for hoisted from context/projects.py to context/_atomic.py so KnownProjectsStore and Lockfile share the PR-feat(index): mm index --debounce-window + --flush + --status (closes #536 documented gap) #548 sidecar pattern instead of cross-importing private symbols.
git_identity / wiki_root fixtures moved to tests/_wiki_fixtures.py and re-exported from conftest.py — test_wiki_store.py, test_wiki_cmd.py, and test_context_install.py share one definition.

Decisions worth flagging for review

Question	Decision	Rationale
Lockfile dict vs dataclass	plain `dict[str, Any]`	ADR-0008:152-155 requires unknown-field round-trip
`version != 1` handling	`LockfileVersionError` in strict (write) paths; `load(strict=False)` returns raw dict	future `mm context status` (PR-D) needs to render unknown versions diagnostically without forking the loader
`source_path` lockfile field	dropped	redundant — derivable from `(asset_type, name)`; ADR schema doesn't include it
`installed_at` precision	microseconds	concurrency tests in same second need ordering signal
PR-B asset scope	skill-only	agent/command install without PR-C override-aware fan-out is misleading; smaller PR matches single-change-per-PR

User-facing docs (README, getting-started) for mm context install deferred until the wiki series is feature-complete after PR-D — matches how PR-A shipped ADR-only.

Test plan

uv run pytest tests/test_lockfile.py tests/test_context_install.py — 26 new tests pass
uv run pytest tests/test_wiki_store.py tests/test_wiki_cmd.py — 28 PR-A regression tests still pass after fixture refactor
uv run pytest tests/test_context_projects.py tests/test_context_skills.py — 56 regression tests still pass after _file_lock and copy_tree_atomic hoists
uv run pytest -m "not ollama" — full suite, only the pre-existing test_calibrate_portfolio live-corpus check fails (unrelated; same on origin/main)
uv run ruff check + uv run ruff format --check on every changed file — clean
uv run mypy (advisory) on the four new/modified source files — 0 issues
End-to-end smoke in isolated HOME=/tmp/...: mm wiki init → seed skill → mm context install skill hello (succeeds, lock.json + dest tree correct) → re-run (refuses with classified error)

🤖 Generated with Claude Code

Implements ADR-0008 PR-B: install a wiki asset into a project tree with a recorded commit pin and concurrency-safe lockfile. Surface - mm context install skill <name> — copytree snapshot of ~/.memtomem-wiki/skills/<name>/ into <project>/.memtomem/skills/<name>/ pinned to wiki HEAD. PR-B is skill-only; agents/commands land in PR-C alongside override resolution. The Choice widens to one line then. - <project>/.memtomem/lock.json — version 1, full 40-char wiki_commit, microsecond-precision installed_at. Reads preserve unknown fields per ADR-0008 (round-trip through plain dict; no strict schema strip). Concurrency - Lock window covers only the lockfile JSON load → mutate → atomic_write triple. The slow copytree runs unlocked. The wiki commit is pinned at the start of install so a concurrent git pull cannot drift the recorded wiki_commit from the bytes copied. - _file_lock + _lock_path_for hoisted from projects.py to _atomic.py so both KnownProjectsStore and Lockfile share the PR-#548 sidecar pattern. Refusal posture - AlreadyInstalledError fires on (lockfile_entry OR dest_exists), not AND. Catches "user wiped dest only" and "dest exists, lockfile damaged" silent-overwrite holes. Forward-protects ADR Invariant 2 without depending on PR-D's mtime/dirty detection. - LockfileVersionError raises in strict (write) paths; load(strict=False) returns the raw dict so PR-D's mm context status can render "version 99 (unknown), this tool supports v1". Refactors - copy_tree_atomic hoisted from skills.py to _atomic.py as a public helper, with defensive .git skip and a returned file count. - git_identity / wiki_root fixtures moved to tests/_wiki_fixtures.py and re-exported from conftest.py so test_wiki_store.py, test_wiki_cmd.py, and test_context_install.py share one definition. Tests - test_lockfile.py — load recovery (missing/invalid/unknown-version), upsert round-trip with unknown-field preservation, 8-process spawn concurrency. - test_context_install.py — copies tree, records lockfile, .git skip, Invariant 3 message, asset-missing, four AlreadyInstalled cases (AND case + both OR-only cases), invalid name, two-skill spawn concurrency, full CLI surface incl. classified error messages. Out of scope - PR-C: agent/command install, override resolution, OVERRIDE_FORMATS. - PR-D: mm context update, --force, .bak, mm context status, install --all, mtime/dirty detection. - v2: --commit <ref>, directory-level os.replace atomic snapshot. Co-Authored-By: Claude <[email protected]>

Correctness - Replace `asset_type.rstrip('s')` with `removesuffix('s')` — rstrip strips characters in a set, not a suffix; lucky for the three current asset types but PR-C `agents`/`commands` widening is the surfacing trigger. - `copy_tree_atomic` skips symlinks with a logger.warning rather than silently dereferencing them. `Path.is_file()` returns True for symlinks-to-files, and the original `read_bytes()` would have copied the link target's bytes (e.g. `/etc/passwd`) into the project. The helper's contract is "byte-for-byte tree mirror", which symlink follow-through violated. UX / API - `copy_tree_atomic` gains `mode=0o644` parameter (was inheriting `atomic_write_bytes`'s 0o600 default). Skill content is meant to be read by other tools (Claude Code, Gemini CLI, Codex) so 0o600 propagation would have made fan-out scripts unreadable on multi-user systems. `Lockfile` keeps the 0o600 default for state files via its own `atomic_write_bytes` call. - `_install_asset` raises `FileNotFoundError` when `project_root` does not exist instead of silently scaffolding `.memtomem/` inside a fresh directory. A typo'd `/Users/me/projetc` errors loudly now. - `COPY_SKIP_NAMES = {".git", ".DS_Store", "__pycache__"}` extends the skip list — Finder side-effects on macOS and Python bytecode caches in wikis no longer leak into `<project>/.memtomem/`. Type tightening - `Lockfile.__init__(path: Path | str)` annotation widens to match the runtime `Path(path).expanduser()` body. - `InstallResult.asset_type: Literal["skills"]` for the public dataclass in PR-B; `_install_asset` keeps a wider parameter type so PR-C's agent/command widening stays a one-line tweak. Construction site uses `cast` with a comment explaining the boundary. Concurrency contract - `_install_asset` docstring documents same-asset race semantics (last-write-wins on identical bytes) so reviewers don't confuse it with the distinct-asset concurrency the existing test exercises. Explicit same-asset test deferred — the design contract is now in the docstring and PR-C will exercise it via widening. Tests - 4 new tests: `test_install_skips_dsstore_and_pycache`, `test_install_skips_symlinks_in_source`, `test_install_files_written_with_default_mode`, `test_install_project_root_missing`. - All pre-existing tests updated to use `tmp_path` directly as project root (always exists), keeping the new is_dir() check happy. Skipped (per review reasoning) - Same-skill concurrent-install test (covered by docstring; PR-C will exercise widening). - `os.fspath()` for cross-platform CLI path display (no cross-platform test surface to validate). Verification - `uv run ruff check` + `uv run ruff format --check` on every changed file: clean. - `uv run pytest tests/test_lockfile.py tests/test_context_install.py tests/test_wiki_*.py tests/test_context_projects.py tests/test_context_skills.py`: 114 passed (PR-B + PR-A regression + projects/skills regression). - `uv run mypy` advisory on the three modified source files: 0 issues. Co-Authored-By: Claude <[email protected]>

pandas-studio and others added 2 commits April 30, 2026 23:41

memtomem merged commit 5cd88d3 into main Apr 30, 2026
7 checks passed

memtomem deleted the feat/pr-b-context-install branch April 30, 2026 14:57

github-actions Bot locked and limited conversation to collaborators Apr 30, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(context): PR-B — mm context install skill + lockfile + concurrency#623

feat(context): PR-B — mm context install skill + lockfile + concurrency#623
memtomem merged 2 commits intomainfrom
feat/pr-b-context-install

memtomem commented Apr 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

memtomem commented Apr 30, 2026

Summary

Scope

Refusal posture (forward-protects Invariant 2)

Refactors carried in this PR

Decisions worth flagging for review

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants