From da1359ac180dccb70f0e4bc9eebf41c72f2b5a78 Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Tue, 14 Jul 2026 12:39:19 +0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(installer):=20.claude/agents=20is=20a?= =?UTF-8?q?=20shared=20namespace=20=E2=80=94=20never=20clean-replace=20it?= =?UTF-8?q?=20whole?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User-reported data loss: every `add` init/update swept ALL non-ADD subagents from .claude/agents. Root cause: the MANAGED table routed the agents tree through the whole-dir clean-replace (stage-then-swap), whose contract — "a file removed upstream leaves no orphan" — treats every undeclared file as an orphan. Correct for the ADD-owned trees (.add/tooling, .add/docs, .claude/skills/add, .add/personas-teacher); wrong for .claude/agents, the one destination other tools and the USER also write. - NEW shared-namespace lander in BOTH installers (npm bin/cli.js sharedFileReplace + pip _installer.py _shared_file_replace, byte-mirrored semantics): per shipped file, temp-sibling + atomic rename (crash never leaves a torn file); removal is EXPLICIT-tombstone-only (RETIRED_AGENTS/_RETIRED_AGENTS, empty today) — never a sweep, never a name heuristic (a user file named add-anything.md survives) - routed via SHARED={"agents"} in the one reconcile loop each installer has (init AND update funnel through it); GLOBAL_TREES excludes agents in both - roll-up shape {restored, refreshed} preserved for the reporting consumer - test_roster_shipped's superseded pin ("update must clean-replace agents/") STRENGTHENED to the new truth: undeclared files survive, shipped files still refresh (the sweep it demanded WAS the data-loss bug) Tests: NEW test_installer_shared_namespace.py (8: foreign-file survival through init+update x both installers · prefixed foreign file survives · fresh dir + roster lands · tombstone removal · non-shared docs tree still sweeps orphans · roll-up shape). Full fence 3562/3562 OK. Engine add.py untouched — no MD5 re-aim. refs: standalone fast task installer-shared-namespace-guard, gate PASS author: Tin Dang --- .add/state.json | 50 +++++- .../installer-shared-namespace-guard/TASK.md | 102 +++++++++++ add-method/bin/cli.js | 44 ++++- add-method/src/add_method/_installer.py | 45 ++++- .../test_installer_shared_namespace.py | 163 ++++++++++++++++++ add-method/tooling/test_roster_shipped.py | 20 ++- 6 files changed, 416 insertions(+), 8 deletions(-) create mode 100644 .add/tasks/installer-shared-namespace-guard/TASK.md create mode 100644 add-method/tooling/test_installer_shared_namespace.py diff --git a/.add/state.json b/.add/state.json index 6f9df1be..6411dcc9 100644 --- a/.add/state.json +++ b/.add/state.json @@ -9326,10 +9326,50 @@ "email": "tindang.ht97@gmail.com", "source": "git" } + }, + "installer-shared-namespace-guard": { + "title": "Installer: .claude/agents is a shared namespace \u2014 never clean-replace it whole", + "phase": "done", + "gate": "PASS", + "milestone": "six-phase-loop", + "depends_on": [], + "created": "2026-07-14T05:19:59+00:00", + "updated": "2026-07-14T05:38:57+00:00", + "fast": true, + "freeze": { + "version": "v1", + "frozen_at": "2026-07-14T05:21:55+00:00", + "approved_by": "Tin Dang", + "actor": { + "name": "Tin Dang", + "email": "tindang.ht97@gmail.com", + "source": "git" + } + }, + "flag_verified": true, + "tripwire": { + "contract_md5": "67f269a177c87328091886ba51da9ea4", + "tests": { + "add-method/tooling/test_installer_shared_namespace.py": "66d3417d56dad26dbf0de369c7dbd9c8" + } + }, + "scope": { + "declared": [ + "add-method/bin/cli.js", + "add-method/src/add_method/_installer.py", + "add-method/tooling/" + ], + "snapshot_md5": "98ebf94ca83d7cc9f2919d99ffe40f03" + }, + "gate_actor": { + "name": "Tin Dang", + "email": "tindang.ht97@gmail.com", + "source": "git" + } } }, "created": "2026-05-28T15:39:04+00:00", - "updated": "2026-07-14T04:33:38+00:00", + "updated": "2026-07-14T05:39:19+00:00", "milestones": { "v1-1": { "title": "v1.1 \u2014 adoption & ergonomics", @@ -10609,7 +10649,7 @@ "plan-legibility": "relations-surface", "ceremony-to-effort": "fold-draft-at-close", "call-floor": "skill-orient-split", - "six-phase-loop": "build-entry-spec-echo" + "six-phase-loop": "installer-shared-namespace-guard" }, "todos": [ { @@ -10809,6 +10849,12 @@ "text": "lock-reclaim TOCTOU race: test_concurrent_stale_reclaim_survives_scheduling_delay observed peak=2 holders twice on py3.10 CI (also the older test_project_scope_lock sibling) \u2014 the reclaim window is real on slow runners; harden the ticket-race path or gate the reclaim on a holder heartbeat", "created": "2026-07-13T11:37:30+00:00", "status": "open" + }, + { + "id": 34, + "text": "bench meter: the judge rubric's 'single float and nothing else' protocol caps spec_fidelity at ~0.98 (LLM hedging artifact \u2014 the SAME sonnet judge forced to itemize MET/UNMET with evidence scored sixphase-r1 1.0, all requirements MET); meter v3 candidate = itemize-then-float rubric, but changing it breaks comparability with every archived round", + "created": "2026-07-14T04:38:54+00:00", + "status": "open" } ] } diff --git a/.add/tasks/installer-shared-namespace-guard/TASK.md b/.add/tasks/installer-shared-namespace-guard/TASK.md new file mode 100644 index 00000000..4511d3a2 --- /dev/null +++ b/.add/tasks/installer-shared-namespace-guard/TASK.md @@ -0,0 +1,102 @@ +# TASK: Installer: .claude/agents is a shared namespace — never clean-replace it whole + +slug: installer-shared-namespace-guard · created: 2026-07-14 · stage: mvp +milestone: six-phase-loop +autonomy: auto +phase: done +fast: true + +> Fast lane — one small task, minimal sections, filled top-to-bottom. The trust floor still +> holds: a FROZEN §3 contract · ≥1 red test before build · a recorded §6 gate (security = HARD-STOP). +> The acceptance scenario collapses into §1 `Accept:`; the observe note is one optional line at the gate. + +--- + +## 1 · SPECIFY — the rules + +> Project the expectations from the milestone Ground + this request — light, not re-invented. +Feature: installer shared-namespace guard — .claude/agents holds the USER's own subagents; ADD's install/update may own only its shipped roster files there, never the directory (bug: clean-replace swept every non-ADD agent as an orphan) +Must: + - a foreign file in .claude/agents (any name not shipped and not tombstoned) survives init AND update byte-identical, in BOTH installers (npm bin/cli.js + pip _installer.py) + - shipped roster files still land/refresh (bytes == packaged agents/ copy), each via a per-file temp-sibling + rename (atomic overwrite; dest dir created when missing) + - upstream-retired roster names are removed ONLY via an explicit tombstone list (empty today) — no heuristic sweep, no prefix-based deletion + - every other managed tree keeps its whole-dir clean-replace semantics byte-unchanged (orphans there still swept) +Reject: + - clean-replacing .claude/agents as a whole directory -> "data_loss" (the reported bug — user agents deleted as orphans); never again + - deleting by name pattern (e.g. add-*.md heuristic) -> forbidden; a user file named add-anything.md survives unless explicitly tombstoned +Accept: Given a project with .claude/agents/my-custom.md and a modified add-verify.md, When cli.js update (and the pip twin) runs, Then my-custom.md is byte-identical, add-verify.md matches the shipped copy, and a docs-tree orphan is still swept. +Boundary: three destination states x two installers — fresh (no .claude/agents: dir created, roster lands) · populated with foreign files (all survive) · a tombstoned name present (removed) +Assumptions: ⚠ no OTHER code path clean-replaces .claude/agents outside the MANAGED-table loops (heal/reconcile/global restore route through the same helpers) — why: grep shows :690/:1466 (py) + dropFiles/update (js) as the call sites; if wrong: the fence names it, the same shared-branch applies there (cost: one more call site) + +--- + +## 3 · PLAN — the change plan: ground · contract · build-strategy + +### Grounding +Touches (files · symbols): add-method/bin/cli.js:MANAGED (:755 agents row) + cleanReplaceTree call sites (dropFiles + update loop) · add-method/src/add_method/_installer.py:MANAGED (:35) + _clean_replace call sites (:690, :1466) · NEW sharedFileReplace (js) / _shared_file_replace (py) +Context (working folder): npm<->pip parity comments bind the twins ("Mirror of _installer.py:_clean_replace"); this repo's own .claude/agents is dogfood evidence (roster + the user's 12+ own agents) +Honors (patterns / conventions): per-file temp-sibling + rename (the cleanReplaceTree staging idiom, applied per file) · design-for-failure (a crashed landing leaves dest valid) · explicit-tombstone-over-heuristic (the delta-evidence discipline) · Seams consulted: .add/SEAMS.md +Anchors the contract cites: MANAGED · cleanReplaceTree · _clean_replace +Ground SHA: e2cde5c — stamped by freeze + +### Contract + +``` +SHARED namespace set (both installers; today exactly {"agents"}): + a MANAGED tree whose destination is a directory OTHER TOOLS also write. + install/update routes it to the per-file lander instead of the dir swap: + +js: sharedFileReplace(src, dest) -> {restored, refreshed} +py: _shared_file_replace(src, dest) -> {"restored": n, "refreshed": n} + - mkdir dest if missing + - per shipped file: copy to dest/.add-tmp-* then rename onto + dest/ (atomic overwrite; refreshed if it existed, restored if not) + - remove ONLY names in RETIRED_AGENTS / _RETIRED_AGENTS (explicit list, + empty today); every other destination file is never opened +non-shared MANAGED trees: cleanReplaceTree/_clean_replace UNCHANGED +``` + +`Least-sure flag surfaced at freeze:` [contract] the update path may report per-tree file counts a fixed harness test pins (restored/refreshed roll-up shape) — why: cleanReplaceTree's return is consumed by reporting code I haven't fully traced; if wrong: the shared lander returns the same {restored, refreshed} shape so the consumer is agnostic (cost: small re-read, no shape change) +Status: FROZEN @ v1 — approved by Tin Dang +### Build-strategy +Scope (may touch): `add-method/bin/cli.js` `add-method/src/add_method/_installer.py` `add-method/tooling/` +Strategy & known-problem fixes: 1) red test_installer_shared_namespace.py (subprocess fixtures mirroring test_installer_handoff) 2) py _shared_file_replace + route :690/:1466 3) js sharedFileReplace + route dropFiles/update 4) parity comments updated 5) fence; traps: BOTH init and update paths must route (two call sites in py) · the .claude/skills/add dest is already namespaced (NOT shared — do not touch) · keep {restored, refreshed} roll-up shape for the reporting consumer +Approach (domain strategy): ownership-scoped writes in shared namespaces — per-file atomic rename, explicit tombstones over heuristics, correctness-first + +### AI-verify record (required when gate_mode: ai-plan-verify) +- [ ] §3 PLAN grounding anchors resolve in the current tree +- [ ] §1 every Must + every Reject present, each Reject paired with an error code +- [ ] §3 Contract shape is concrete (no template placeholder text remains) +- [ ] Lowest-confidence flag surfaced and substantive (mirrors unflagged_freeze's own bar) +Verified by: · at: + +--- + +## 4 · TESTS — failing-first (red) + +Plan: test_installer_shared_namespace.py — foreign-file survival through init+update x both installers (subprocess, scrubbed env) · roster files land/refresh to shipped bytes · fresh dir created · tombstone mechanism removes a listed name (py unit-level with an injected list) · a non-shared tree (docs) still sweeps an orphan · user file named add-custom.md survives (no prefix heuristic). +Tests live in: `add-method/tooling/test_installer_shared_namespace.py` · MUST run red before Build. + +--- + +## 5 · BUILD — AI writes the code (execution) + +> The change plan was frozen in §3 PLAN. Build to it: honor the §3 Build-strategy Scope; improve on the strategy if the code teaches you better. +Strategy actually used: as planned — _SHARED/_RETIRED_AGENTS + _shared_file_replace routed in _reconcile (py; init AND update both funnel through it), SHARED/RETIRED_AGENTS + sharedFileReplace routed in reconcile (js); GLOBAL_TREES excludes agents in both so no other call site; roll-up shape kept +Code lives in: `./src/` · Constraints: change no test, no frozen contract; stay inside the §3 Build-strategy Scope; allow-list packages only. + +--- + +## 6 · VERIFY — evidence + gate + +- [x] all tests pass · coverage held · no test or contract altered during build +- [x] green was EARNED — no overfit / vacuous asserts / stubbed-away logic +- [x] input dialect held — tests speak the spec's example formats (spec-dialect floor) +- [x] no exposed secrets, injection openings, or unexpected dependencies (security = HARD-STOP) + +Build expectations (from §1 Accept + §3 CONTRACT): after update, .claude/agents contains the user's files byte-identical + the shipped roster refreshed, while .add/docs orphans are still swept — confirmed by test_installer_shared_namespace.py + the full fence + +### GATE RECORD +Outcome: PASS +Reviewed by: Tin Dang · date: 2026-07-14 + diff --git a/add-method/bin/cli.js b/add-method/bin/cli.js index 45f1562f..c94c0449 100755 --- a/add-method/bin/cli.js +++ b/add-method/bin/cli.js @@ -763,6 +763,16 @@ const MANAGED = [ // _installer.py:OPTIONAL. Design-for-failure. `agents` joins here (roster-install-drift): the // phase-agent roster is a spawn-acceleration enhancement, not core runtime. const OPTIONAL = new Set(["personas-teacher", "agents"]); +// SHARED-namespace managed trees (installer-shared-namespace-guard): destinations OTHER +// TOOLS also write — `.claude/agents` holds the user's own Claude Code subagents. A +// whole-dir clean-replace there sweeps the user's files as "orphans" (the reported +// data-loss bug), so these trees route to sharedFileReplace: per-file atomic landings of +// the shipped files + removal of ONLY the explicit tombstones below. Twin of +// _installer.py:_SHARED / _RETIRED_AGENTS. +const SHARED = new Set(["agents"]); +// Roster names retired upstream — the ONLY names the shared lander may remove (never a +// pattern/prefix heuristic: a USER file named add-anything.md must survive). Empty today. +const RETIRED_AGENTS = []; const STAMP_FILE = ".add-version"; const LOCK_FILE = ".update.lock"; // the `update --global` home lock (never user-data) const LOCK_STALE_DEFAULT = 600; // seconds (10 min); ADD_LOCK_STALE_SECONDS env-overridable @@ -962,6 +972,36 @@ function managedStatus(target) { return status; } +// installer-shared-namespace-guard: land a SHARED-namespace managed tree per FILE. +// `.claude/agents` belongs to the user as much as to ADD — only the shipped files are +// written (temp-sibling + atomic rename each, so a crash never leaves a torn file) and +// only the explicit RETIRED_AGENTS tombstones are removed; every other destination file +// is never opened. Returns the same { restored, refreshed } roll-up shape as +// cleanReplaceTree so the reconcile reporting is agnostic. Mirror of +// _installer.py:_shared_file_replace. +function sharedFileReplace(src, dest) { + fs.mkdirSync(dest, { recursive: true }); + let restored = 0, refreshed = 0; + const entries = fs.readdirSync(src).filter((n) => fs.statSync(path.join(src, n)).isFile()).sort(); + for (const name of entries) { + const target = path.join(dest, name); + const existed = fs.existsSync(target); + const tmp = path.join(dest, name + ".add-tmp-" + Math.random().toString(36).slice(2, 10)); + try { + fs.copyFileSync(path.join(src, name), tmp); + fs.renameSync(tmp, target); // atomic overwrite (same directory) + } catch (e) { + fs.rmSync(tmp, { force: true }); // dest entry untouched or already whole + throw e; + } + if (existed) refreshed++; else restored++; + } + for (const name of RETIRED_AGENTS) { + fs.rmSync(path.join(dest, name), { force: true }); // absent tombstone — never fatal + } + return { restored: restored, refreshed: refreshed }; +} + // reconcile: restore-missing + refresh-present (sweep orphans) across the managed trees, // reporting per-tree status. Honors --no-skill (the plugin provides the skill). Touches // ONLY managed trees — never user data. Prechecks ALL sources first (design-for-failure: @@ -979,7 +1019,9 @@ function reconcile(args, target, srcRoot) { const status = managedStatus(target); let restored = 0, refreshed = 0; for (const [sub, destParts, stripTests] of trees) { - const roll = cleanReplaceTree(path.join(srcRoot, sub), path.join(target, ...destParts), stripTests); + const roll = SHARED.has(sub) + ? sharedFileReplace(path.join(srcRoot, sub), path.join(target, ...destParts)) + : cleanReplaceTree(path.join(srcRoot, sub), path.join(target, ...destParts), stripTests); restored += roll.restored; refreshed += roll.refreshed; const dest = destParts.join("/"); diff --git a/add-method/src/add_method/_installer.py b/add-method/src/add_method/_installer.py index 3e795414..3fcbc306 100644 --- a/add-method/src/add_method/_installer.py +++ b/add-method/src/add_method/_installer.py @@ -45,6 +45,15 @@ # enhancement — the CLI+skill loop is fully usable without it, and an older/malformed package # predating this fix must still install its core cleanly. OPTIONAL = frozenset({"personas-teacher", "agents"}) +# SHARED-namespace managed trees (installer-shared-namespace-guard): destinations OTHER +# TOOLS also write — `.claude/agents` holds the user's own Claude Code subagents. A +# whole-dir clean-replace there sweeps the user's files as "orphans" (the reported +# data-loss bug), so these trees route to _shared_file_replace: per-file atomic landings +# of the shipped files + removal of ONLY the explicit tombstones below. Mirrored in cli.js. +_SHARED = frozenset({"agents"}) +# Roster names retired upstream — the ONLY names the shared lander may remove (never a +# pattern/prefix heuristic: a USER file named add-anything.md must survive). Empty today. +_RETIRED_AGENTS = () STAMP_FILE = ".add-version" # records the materialized version, under .add/ # Forward-only, idempotent state migrations keyed by the version that introduces them. # Empty today — the framework exists so the NEXT schema change is an in-place update, @@ -1440,6 +1449,37 @@ def _clean_replace(src: Path, dest: Path, *, strip_tests: bool = False) -> dict: "personas-teacher": "personas"} +def _shared_file_replace(src: Path, dest: Path) -> dict: + """installer-shared-namespace-guard: land a SHARED-namespace managed tree per FILE. + `.claude/agents` belongs to the user as much as to ADD — only the shipped files are + written (temp-sibling + atomic os.replace each, so a crash never leaves a torn file) + and only the explicit _RETIRED_AGENTS tombstones are removed; every other destination + file is never opened. Returns the same {"restored", "refreshed"} roll-up shape as + _clean_replace so the _reconcile reporting is agnostic. Mirror of cli.js:sharedFileReplace.""" + dest.mkdir(parents=True, exist_ok=True) + restored = refreshed = 0 + for f in sorted(p for p in src.iterdir() if p.is_file()): + target = dest / f.name + existed = target.exists() + tmp = dest / f"{f.name}.add-tmp-{uuid.uuid4().hex[:8]}" + try: + shutil.copyfile(f, tmp) + os.replace(str(tmp), str(target)) # atomic on POSIX + Windows (same filesystem) + except BaseException: + tmp.unlink(missing_ok=True) # dest entry untouched or already whole + raise + if existed: + refreshed += 1 + else: + restored += 1 + for name in _RETIRED_AGENTS: + try: + (dest / name).unlink() + except OSError: + pass # absent (or unremovable) tombstone — never fatal + return {"restored": restored, "refreshed": refreshed} + + def _managed_status(target_path: Path) -> dict: """Per managed tree: 'missing' (dest absent OR empty) or 'present'.""" status = {} @@ -1463,7 +1503,10 @@ def _reconcile(target_path: Path, bundled_root: Path) -> dict: for sub, dest_rel, strip in MANAGED: if sub in OPTIONAL and not (bundled_root / sub).exists(): continue # optional enhancement absent from this package — nothing to materialize - roll = _clean_replace(bundled_root / sub, target_path / dest_rel, strip_tests=strip) + if sub in _SHARED: + roll = _shared_file_replace(bundled_root / sub, target_path / dest_rel) + else: + roll = _clean_replace(bundled_root / sub, target_path / dest_rel, strip_tests=strip) restored += roll["restored"] refreshed += roll["refreshed"] if status[sub] == "missing": diff --git a/add-method/tooling/test_installer_shared_namespace.py b/add-method/tooling/test_installer_shared_namespace.py new file mode 100644 index 00000000..1871132f --- /dev/null +++ b/add-method/tooling/test_installer_shared_namespace.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +"""Red/green tests for installer-shared-namespace-guard (fast lane, frozen v1): +`.claude/agents` is a SHARED namespace — the user's own Claude Code subagents +live there. ADD's install/update may own only its shipped roster files (per-file +atomic landings + an explicit tombstone list, empty today), never the directory. +The reported bug: the whole-dir clean-replace swept every non-ADD agent as an +orphan. Every other managed tree keeps whole-dir clean-replace unchanged. + +Run: python3 -m unittest test_installer_shared_namespace -v +""" +import os +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +PKG_ROOT = Path(__file__).resolve().parent.parent # add-method/ +CLI_JS = PKG_ROOT / "bin" / "cli.js" +SRC_DIR = PKG_ROOT / "src" +AGENTS_SRC = PKG_ROOT / "agents" +NODE = shutil.which("node") + +FOREIGN = "my-custom-agent.md" +FOREIGN_BODY = "# my agent\nuser-owned; ADD must never touch this file.\n" +PREFIXED_FOREIGN = "add-custom-of-mine.md" # tests the no-prefix-heuristic rule + + +def _baseline_env(): + env = dict(os.environ) + for k in list(env): + if k in ("CLAUDECODE", "CLAUDE_CODE_ENTRYPOINT") or k.startswith(("CODEX_", "OPENCODE")): + env.pop(k, None) + return env + + +def _run_node(args, cwd): + return subprocess.run([NODE, str(CLI_JS), *args], cwd=cwd, capture_output=True, + text=True, timeout=120, env=_baseline_env()) + + +def _run_pip(args, cwd): + env = _baseline_env() + env["PYTHONPATH"] = str(SRC_DIR) + os.pathsep + env.get("PYTHONPATH", "") + code = ("import sys; from add_method._cli import main; " + "sys.exit(main(sys.argv[1:]))") + return subprocess.run([sys.executable, "-c", code, *args], cwd=cwd, + capture_output=True, text=True, timeout=120, env=env) + + +class _Harness(unittest.TestCase): + def setUp(self): + self.tmp = Path(tempfile.mkdtemp(prefix="add-isn-")).resolve() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + + def _seed_foreign(self): + agents = self.tmp / ".claude" / "agents" + agents.mkdir(parents=True, exist_ok=True) + (agents / FOREIGN).write_text(FOREIGN_BODY, encoding="utf-8") + (agents / PREFIXED_FOREIGN).write_text(FOREIGN_BODY, encoding="utf-8") + return agents + + def _assert_shared_semantics(self, agents: Path, out: str): + self.assertEqual((agents / FOREIGN).read_text(encoding="utf-8"), FOREIGN_BODY, + f"user agent must survive byte-identical: {out}") + self.assertEqual((agents / PREFIXED_FOREIGN).read_text(encoding="utf-8"), + FOREIGN_BODY, + "an add- prefixed USER file survives too — removal is " + "tombstone-only, never a name heuristic") + for f in sorted(AGENTS_SRC.glob("*.md")): + self.assertEqual((agents / f.name).read_bytes(), f.read_bytes(), + f"shipped roster file must land/refresh: {f.name}") + + +@unittest.skipUnless(NODE, "node not on PATH — npm-side check skipped (honest skip)") +class NodeInstallerSharedNamespaceTest(_Harness): + def test_init_preserves_foreign_agents(self): # M1 + M2 + Accept + agents = self._seed_foreign() + r = _run_node(["init"], cwd=self.tmp) + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + self._assert_shared_semantics(agents, r.stdout + r.stderr) + + def test_update_preserves_foreign_and_refreshes_roster(self): # M1 + M2 + Accept + agents = self._seed_foreign() + r = _run_node(["init"], cwd=self.tmp) + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + (agents / "add-verify.md").write_text("stale local edit", encoding="utf-8") + r2 = _run_node(["update"], cwd=self.tmp) + self.assertEqual(r2.returncode, 0, r2.stdout + r2.stderr) + self._assert_shared_semantics(agents, r2.stdout + r2.stderr) + + def test_fresh_dir_created_with_roster(self): # Boundary (fresh) + r = _run_node(["init"], cwd=self.tmp) + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + agents = self.tmp / ".claude" / "agents" + for f in sorted(AGENTS_SRC.glob("*.md")): + self.assertTrue((agents / f.name).exists(), f"roster missing: {f.name}") + + def test_non_shared_tree_still_sweeps_orphans(self): # M4 (docs keeps clean-replace) + r = _run_node(["init"], cwd=self.tmp) + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + orphan = self.tmp / ".add" / "docs" / "zz-orphan.md" + orphan.write_text("upstream-removed leftover", encoding="utf-8") + r2 = _run_node(["update"], cwd=self.tmp) + self.assertEqual(r2.returncode, 0, r2.stdout + r2.stderr) + self.assertFalse(orphan.exists(), + "non-shared managed trees must keep whole-dir clean-replace") + + +class PipInstallerSharedNamespaceTest(_Harness): + def test_init_preserves_foreign_agents(self): # M1 + M2 + Accept + agents = self._seed_foreign() + r = _run_pip(["init"], cwd=self.tmp) + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + self._assert_shared_semantics(agents, r.stdout + r.stderr) + + def test_update_preserves_foreign_and_refreshes_roster(self): # M1 + M2 + Accept + agents = self._seed_foreign() + r = _run_pip(["init"], cwd=self.tmp) + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + (agents / "add-verify.md").write_text("stale local edit", encoding="utf-8") + r2 = _run_pip(["update"], cwd=self.tmp) + self.assertEqual(r2.returncode, 0, r2.stdout + r2.stderr) + self._assert_shared_semantics(agents, r2.stdout + r2.stderr) + + def test_tombstoned_name_is_removed(self): # M3 (explicit list, unit seam) + sys.path.insert(0, str(SRC_DIR)) + try: + from add_method import _installer + finally: + sys.path.pop(0) + self.assertEqual(getattr(_installer, "_RETIRED_AGENTS", None), (), + "_RETIRED_AGENTS must exist as an (empty today) explicit tuple") + agents = self._seed_foreign() + (agents / "add-observe.md").write_text("retired roster leftover", encoding="utf-8") + original = _installer._RETIRED_AGENTS + _installer._RETIRED_AGENTS = ("add-observe.md",) + try: + _installer._shared_file_replace(AGENTS_SRC, agents) + finally: + _installer._RETIRED_AGENTS = original + self.assertFalse((agents / "add-observe.md").exists(), + "a tombstoned name must be removed") + self.assertEqual((agents / FOREIGN).read_text(encoding="utf-8"), FOREIGN_BODY, + "tombstone removal must not touch foreign files") + + def test_shared_lander_reports_rollup_shape(self): # contract: {restored, refreshed} + sys.path.insert(0, str(SRC_DIR)) + try: + from add_method import _installer + finally: + sys.path.pop(0) + agents = self.tmp / ".claude" / "agents" # fresh: all restored + roll = _installer._shared_file_replace(AGENTS_SRC, agents) + n = len(list(AGENTS_SRC.glob("*.md"))) + self.assertEqual(roll, {"restored": n, "refreshed": 0}) + roll2 = _installer._shared_file_replace(AGENTS_SRC, agents) + self.assertEqual(roll2, {"restored": 0, "refreshed": n}) + + +if __name__ == "__main__": + unittest.main() diff --git a/add-method/tooling/test_roster_shipped.py b/add-method/tooling/test_roster_shipped.py index d455ae49..6f063667 100644 --- a/add-method/tooling/test_roster_shipped.py +++ b/add-method/tooling/test_roster_shipped.py @@ -136,19 +136,31 @@ def test_install_materializes_agents_into_dot_claude(self): self.assertTrue(installed.is_file(), "install() must materialize agents/ -> .claude/agents") self.assertEqual(installed.read_text(), "---\nname: add-build\n---\nbuild agent v-new\n") - def test_update_clean_replaces_agents(self): + def test_update_preserves_undeclared_agents(self): + # SUPERSEDED PIN (installer-shared-namespace-guard, frozen v1): .claude/agents is a + # SHARED namespace — the user's own subagents live there, so update lands the shipped + # files per-file and removes ONLY explicit _RETIRED_AGENTS tombstones. The old pin + # ("update must clean-replace agents/, sweep orphans") demanded the data-loss bug: + # any undeclared file — the user's included — was deleted. Strengthened, not weakened: + # survival is asserted for BOTH an add- prefixed undeclared file (no name heuristic) + # and the shipped files' refresh, and the tombstone path is pinned separately in + # test_installer_shared_namespace.py. bundled = _make_bundled_with_agents(self.tmp / "pkg") proj = self.tmp / "proj" proj.mkdir() with contextlib.redirect_stdout(io.StringIO()): _installer.install(target=str(proj), bundled=str(bundled), yes=True, non_interactive=True) - # upstream renames/removes an agent file; update must clean-replace, no orphan survives stale = proj / ".claude" / "agents" / "add-stale.md" - stale.write_text("# removed upstream — must be swept\n") + stale.write_text("# undeclared — must SURVIVE (tombstone-only removal)\n") with contextlib.redirect_stdout(io.StringIO()): code = _installer.update(target=str(proj), bundled=str(bundled), version="2.0.0") self.assertEqual(code, 0) - self.assertFalse(stale.exists(), "update must clean-replace agents/ (sweep orphans)") + self.assertTrue(stale.exists(), + "an undeclared file in the shared namespace survives update — " + "removal is tombstone-only, never a sweep or a name heuristic") + self.assertEqual((proj / ".claude" / "agents" / "add-build.md").read_text(), + "---\nname: add-build\n---\nbuild agent v-new\n", + "shipped roster files still refresh") def test_missing_agents_in_bundled_soft_skips(self): """A bundled source predating this fix (no agents/ at all) must NOT abort install — From e1d5459c2bdb647d30493c9d753d7788700da0c0 Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Tue, 14 Jul 2026 13:14:53 +0700 Subject: [PATCH 2/2] =?UTF-8?q?docs(task):=20installer-shared-namespace-gu?= =?UTF-8?q?ard=20=C2=A77=20observe=20=E2=80=94=20global-mirror=20roster-re?= =?UTF-8?q?fresh=20gap=20(user=20agents=20safe,=20verified=20live)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit author: Tin Dang --- .add/tasks/installer-shared-namespace-guard/TASK.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.add/tasks/installer-shared-namespace-guard/TASK.md b/.add/tasks/installer-shared-namespace-guard/TASK.md index 4511d3a2..25462b7e 100644 --- a/.add/tasks/installer-shared-namespace-guard/TASK.md +++ b/.add/tasks/installer-shared-namespace-guard/TASK.md @@ -99,4 +99,5 @@ Build expectations (from §1 Accept + §3 CONTRACT): after update, .claude/agent ### GATE RECORD Outcome: PASS Reviewed by: Tin Dang · date: 2026-07-14 +[SPEC · open] `update --global` propagation never refreshes the roster: agents are absent from _GLOBAL_TREES/GLOBAL_TREES, so the home mirror soft-skips them — user agents SAFE (verified live: my-precious.md survived, stale add-verify.md kept its local edit), but a roster refresh needs a project-level `update`. Candidate follow-up: add agents to the global mirror, routed through the shared lander. Also observed: the opt-in --global-data snapshot excludes .claude/agents (a MANAGED dest), so user agents are not backed up by it. (evidence: global-audit live run 2026-07-14)