From 369e54bfc454c5cf7569ea4fe1c0c491e37045eb Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:10:47 +0200 Subject: [PATCH 1/2] feat(harness): vendor runtime harness into consumer .claude/ (issue #128) An enabled plugin loads (and pays its ~18s load cost) on every session start regardless of how its hooks are wired, which blows claude remote-control's ~20-30s spawn-ack window for headless driver sessions in consumer repos. Make the vendored local .claude/ files the runtime path instead: scaffold.sh now copies the plugin's own agents/, commands/, hooks/, scripts/, and skills/ subtrees wholesale into the consumer repo, gated as one unit by a single .claude/.orchestrator-vendor marker reusing the existing managed-file marker ladder, and creates a settings.json (user-owned, create-if-absent) that wires the runtime hooks to $CLAUDE_PROJECT_DIR/.claude/scripts/... instead of the plugin's hooks/hooks.json. sync.sh mirrors the re-vendor on plugin upgrades with the same behind/conflict/never-downgrade semantics as every other managed row. .claude/scripts/arm-loop.sh is excluded from the whole-tree copy since it already has its own dedicated managed-file row with a different canonical template, avoiding two divergent sources of truth for the same destination. Once vendored, the plugin only needs to stay enabled to run /orchestrator:setup or /orchestrator:sync, not for everyday sessions. Updates MANIFEST.md, both skill docs, and the GETTING_STARTED/USAGE passages that described the old plugin-stays-loaded-at-runtime model. Co-Authored-By: Claude Sonnet 5 --- .claude/.orchestrator-vendor | 23 +++ .claude/scripts/vendor-runtime.test.sh | 156 +++++++++++++++++ .claude/skills/setup/SKILL.md | 33 +++- .claude/skills/setup/scaffold.sh | 83 +++++++++ .claude/skills/setup/templates/MANIFEST.md | 10 ++ .claude/skills/setup/templates/settings.json | 173 +++++++++++++++++++ .claude/skills/sync/SKILL.md | 20 ++- .claude/skills/sync/sync.sh | 109 ++++++++++++ docs/GETTING_STARTED.md | 24 ++- docs/USAGE.md | 17 +- 10 files changed, 634 insertions(+), 14 deletions(-) create mode 100644 .claude/.orchestrator-vendor create mode 100644 .claude/scripts/vendor-runtime.test.sh create mode 100644 .claude/skills/setup/templates/settings.json diff --git a/.claude/.orchestrator-vendor b/.claude/.orchestrator-vendor new file mode 100644 index 0000000..498068d --- /dev/null +++ b/.claude/.orchestrator-vendor @@ -0,0 +1,23 @@ +# @orchestrator-managed runtime-vendor v1 +# +# Version stamp for the vendored runtime harness (issue #128). `/orchestrator:setup`'s +# scaffold.sh copies this plugin's own `agents/`, `commands/`, `hooks/`, `scripts/`, and +# `skills/` subtrees wholesale into the consumer repo's `.claude/`, then writes this file +# alongside them at `.claude/.orchestrator-vendor` as the single top-level marker that +# governs the WHOLE vendored tree as one unit (not one marker per file). `/orchestrator:sync` +# re-vendors the tree the same way when this marker is behind what the plugin ships, using +# the same never-downgrade / flag-local-edits-as-conflict rules as every other managed file +# — see `.claude/skills/setup/templates/MANIFEST.md`. +# +# Once vendored, a consumer session reads agents/commands/hooks/scripts/skills straight out +# of the local `.claude/` tree — the plugin only needs to be ENABLED to run +# `/orchestrator:setup` or `/orchestrator:sync` (the install/update channel), not for +# everyday runtime sessions. That's what removes the plugin's session-start load cost from +# time-sensitive paths like headless `claude remote-control` spawns. +# +# EXCEPTION: `.claude/scripts/arm-loop.sh` is excluded from this wholesale copy. It already +# has its OWN dedicated managed-file row (canonical shipped copy: `templates/arm-loop.sh`, +# substituted with placeholders at `arm-loop.sh` ARM time) which can legitimately differ +# from this plugin repo's own live self-hosting copy of `.claude/scripts/arm-loop.sh`. +# Vendoring it a second time via this whole-tree copy would create two divergent sources of +# truth for the same destination path — see scaffold.sh/sync.sh's runtime-vendor sections. diff --git a/.claude/scripts/vendor-runtime.test.sh b/.claude/scripts/vendor-runtime.test.sh new file mode 100644 index 0000000..e01af39 --- /dev/null +++ b/.claude/scripts/vendor-runtime.test.sh @@ -0,0 +1,156 @@ +#!/usr/bin/env bash +# vendor-runtime.test.sh — offline smoke test for the runtime harness vendor +# (issue #128): scaffold.sh vendoring agents/commands/hooks/scripts/skills +# wholesale into a consumer .claude/, gated by the single top-level marker +# .claude/.orchestrator-vendor, and sync.sh's behind/up-to-date/conflict +# ladder for that same tree. Asserts: +# 1. fresh scaffold -> vendor tree created, marker stamped, arm-loop.sh +# NOT duplicated by the tree copy (still only the managed-file copy), +# settings.json created with no enabledPlugins/extraKnownMarketplaces. +# 2. re-running scaffold -> idempotent ("up to date" / "kept", no errors). +# 3. sync against an unchanged plugin root -> "up to date". +# 4. sync against a plugin root whose marker was bumped with NO other +# content change -> restamped (no false conflict). +# 5. sync against a plugin root whose marker was bumped WITH a real +# content change -> conflict (installed has no local edits, but the +# script can't tell a legitimate upstream change from a hand-edit — +# same conservative behavior as every other managed row). +# 6. sync with a genuine local edit to a vendored file (same version) -> +# conflict. +# 7. sync with a local edit ONLY to arm-loop.sh -> the runtime-vendor row +# stays "up to date" (excluded from its diff), while arm-loop.sh's OWN +# managed row reports the conflict — proves the two mechanisms don't +# double-manage the same file. +# +# Exit 0 on success, non-zero if any assertion fails. Runnable bare: +# bash .claude/scripts/vendor-runtime.test.sh +set -uo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +scaffold_sh="$repo_root/.claude/skills/setup/scaffold.sh" +sync_sh="$repo_root/.claude/skills/sync/sync.sh" + +work="$(mktemp -d "${TMPDIR:-/tmp}/vendor-runtime-test.XXXXXX")" +trap 'rm -rf "$work"' EXIT + +fail=0 +ok=0 +check() { + local desc="$1"; shift + if "$@"; then + ok=$((ok + 1)) + echo "ok - $desc" + else + fail=1 + echo "FAIL - $desc" + fi +} +check_output() { + # $1 = desc, $2 = haystack, $3 = needle (grep -F) + local desc="$1" haystack="$2" needle="$3" + if printf '%s' "$haystack" | grep -qF -- "$needle"; then + ok=$((ok + 1)) + echo "ok - $desc" + else + fail=1 + echo "FAIL - $desc (expected to find: $needle)" + fi +} + +# --------------------------------------------------------------------------- +# Scenario 1: fresh scaffold. +# --------------------------------------------------------------------------- +t1="$work/consumer1" +mkdir -p "$t1" +out1="$(bash "$scaffold_sh" "$t1" 2>&1)" +scaffold1_rc=$? +check "s1: scaffold exits 0" test "$scaffold1_rc" -eq 0 +check "s1: marker file created" test -f "$t1/.claude/.orchestrator-vendor" +check "s1: agents/ vendored" test -f "$t1/.claude/agents/orchestrator.md" +check "s1: commands/ vendored" test -d "$t1/.claude/commands" +check "s1: hooks/ vendored" test -f "$t1/.claude/hooks/hooks.json" +check "s1: skills/ vendored (includes scaffold.sh itself)" test -f "$t1/.claude/skills/setup/scaffold.sh" +check "s1: scripts/ vendored (e.g. gate.sh)" test -f "$t1/.claude/scripts/gate.sh" +check "s1: gate.sh executable bit preserved" test -x "$t1/.claude/scripts/gate.sh" +check "s1: arm-loop.sh present via its OWN managed row" test -f "$t1/.claude/scripts/arm-loop.sh" +check "s1: settings.json created" test -f "$t1/.claude/settings.json" +check "s1: settings.json has no enabledPlugins key" \ + node -e "process.exit('enabledPlugins' in require(process.argv[1]) ? 1 : 0)" "$t1/.claude/settings.json" +check "s1: settings.json has no extraKnownMarketplaces key" \ + node -e "process.exit('extraKnownMarketplaces' in require(process.argv[1]) ? 1 : 0)" "$t1/.claude/settings.json" +check_output "s1: vendor row reported created/up-to-date" "$out1" "runtime harness vendor" + +# --------------------------------------------------------------------------- +# Scenario 2: re-running scaffold is idempotent. +# --------------------------------------------------------------------------- +out2="$(bash "$scaffold_sh" "$t1" 2>&1)" +check_output "s2: re-run reports vendor up to date" "$out2" "up to date: runtime harness vendor" +check_output "s2: re-run keeps settings.json (user-owned)" "$out2" "kept: consumer runtime settings" + +# --------------------------------------------------------------------------- +# Scenario 3: sync against an unchanged plugin root -> up to date. +# --------------------------------------------------------------------------- +out3="$(bash "$sync_sh" "$t1" 2>&1)" +rc3=$? +check "s3: sync exits 0" test "$rc3" -eq 0 +check_output "s3: vendor row up to date" "$out3" "up to date: .claude/{agents,commands,hooks,scripts,skills}" + +# --------------------------------------------------------------------------- +# Scenario 4: plugin marker bumped, no other content change -> restamp. +# --------------------------------------------------------------------------- +plugin_v2="$work/plugin-v2" +cp -a "$repo_root/.claude" "$plugin_v2" +sed -i 's/@orchestrator-managed runtime-vendor v1/@orchestrator-managed runtime-vendor v2/' \ + "$plugin_v2/.orchestrator-vendor" +t4="$work/consumer4" +mkdir -p "$t4" +bash "$scaffold_sh" "$t4" >/dev/null 2>&1 +out4="$(bash "$plugin_v2/skills/sync/sync.sh" "$t4" 2>&1)" +check_output "s4: marker-only bump restamps cleanly" "$out4" "restamped: .claude/{agents,commands,hooks,scripts,skills} v1 -> v2" + +# --------------------------------------------------------------------------- +# Scenario 5: plugin marker bumped WITH a real content change -> conflict +# (can't distinguish a legitimate upstream change from a local hand-edit). +# --------------------------------------------------------------------------- +plugin_v3="$work/plugin-v3" +cp -a "$repo_root/.claude" "$plugin_v3" +sed -i 's/@orchestrator-managed runtime-vendor v1/@orchestrator-managed runtime-vendor v2/' \ + "$plugin_v3/.orchestrator-vendor" +echo "" >> "$plugin_v3/agents/orchestrator.md" +t5="$work/consumer5" +mkdir -p "$t5" +bash "$scaffold_sh" "$t5" >/dev/null 2>&1 +out5="$(bash "$plugin_v3/skills/sync/sync.sh" "$t5" 2>&1)" +check_output "s5: content-changing bump flags conflict, not a silent restamp" "$out5" \ + "conflict: .claude/{agents,commands,hooks,scripts,skills} is v1 (behind v2) AND has local edits" + +# --------------------------------------------------------------------------- +# Scenario 6: genuine local edit, same version -> conflict. +# --------------------------------------------------------------------------- +t6="$work/consumer6" +mkdir -p "$t6" +bash "$scaffold_sh" "$t6" >/dev/null 2>&1 +echo "" >> "$t6/.claude/agents/orchestrator.md" +out6="$(bash "$sync_sh" "$t6" 2>&1)" +check_output "s6: local edit at same version flags conflict" "$out6" \ + "conflict: .claude/{agents,commands,hooks,scripts,skills} is marked v1 but content diverges" + +# --------------------------------------------------------------------------- +# Scenario 7: local edit ONLY to arm-loop.sh doesn't leak into the vendor row. +# --------------------------------------------------------------------------- +t7="$work/consumer7" +mkdir -p "$t7" +bash "$scaffold_sh" "$t7" >/dev/null 2>&1 +echo "# hand edit" >> "$t7/.claude/scripts/arm-loop.sh" +out7="$(bash "$sync_sh" "$t7" 2>&1)" +check_output "s7: arm-loop.sh's OWN managed row flags the conflict" "$out7" \ + "conflict: .claude/scripts/arm-loop.sh is marked" +check_output "s7: vendor row unaffected by the arm-loop.sh edit" "$out7" \ + "up to date: .claude/{agents,commands,hooks,scripts,skills}" + +echo +if [ "$fail" -ne 0 ]; then + echo "vendor-runtime.test.sh: FAILED" + exit 1 +fi +echo "vendor-runtime.test.sh: all $ok checks passed" diff --git a/.claude/skills/setup/SKILL.md b/.claude/skills/setup/SKILL.md index 53ed893..20933fc 100644 --- a/.claude/skills/setup/SKILL.md +++ b/.claude/skills/setup/SKILL.md @@ -13,6 +13,14 @@ version-controlled inside the consumer's own repo: the project-specific adapter, workflow file, and GitHub Actions YAML. This skill's job is to scaffold exactly that non-distributable residue, on top of the interview below. +**Also (issue #128): this skill vendors the plugin's own runtime harness** — `agents/`, `commands/`, `hooks/`, +`scripts/`, `skills/` — wholesale into the consumer's local `.claude/`, plus a `.claude/settings.json` that +wires the runtime hooks locally. A plugin loads (and pays its load cost) on every session start while it's +enabled, no matter how its hooks are wired, so the only way to remove that cost from the runtime path is to +stop depending on the plugin being loaded at all once setup is done. **The `orchestrator` plugin only needs to +stay enabled to RUN `/orchestrator:setup`/`/orchestrator:sync`** (the install/update channel) — not for +everyday sessions. See step 4 below. + Be conversational but efficient. Use the `AskUserQuestion` tool for discrete choices; ask for free-text (names, paths, shell commands) in plain prose. **Never invent values** — if you don't know a command or path, ask. **Propose the final files and get an explicit "yes" before writing.** All `gh` runs through @@ -85,6 +93,24 @@ idempotently: the same way as `feature-fanout.js` (own `@orchestrator-managed vN` marker, re-stamped on upgrade). These carry `__WORKDIR__`/`__REPO_SLUG__`/etc. placeholders that `arm-loop.sh` substitutes at ARM time, not at scaffold time — scaffolding them here does NOT install or start anything. See step 9 below for arming. +- **`.claude/{agents,commands,hooks,scripts,skills}/`** (issue #128) — the runtime harness itself, vendored + wholesale from the plugin root, managed as ONE unit via a single top-level marker file + (`.claude/.orchestrator-vendor`, `@orchestrator-managed runtime-vendor vN`), restamped/re-vendored on the + same behind/never-downgrade ladder as every other managed row. **`.claude/scripts/arm-loop.sh` is excluded** + from this copy — it's already managed by its own row above with a different canonical source, so + double-vendoring it would create two disagreeing sources of truth for the same file. Once this tree is + vendored, the plugin's own `agents/commands/hooks/scripts/skills` are no longer on the runtime critical + path — a session reads the local `.claude/` copies instead, whether or not the plugin is enabled. +- **`.claude/settings.json`** — **user-owned, created only if absent.** Wires the runtime hooks + (`PostToolUse` lint + log-worker-tool, `Stop` test_affected, `PreToolUse` guard-git-add) to + `$CLAUDE_PROJECT_DIR/.claude/scripts/...`, plus baseline `permissions`/`sandbox`. Deliberately carries no + `enabledPlugins`/`extraKnownMarketplaces` — keep those only in a settings.json you maintain yourself while + installing/updating the plugin (e.g. the block from Step 1 of `docs/GETTING_STARTED.md`), not in the + runtime file, which must keep working with the plugin disabled. **If you already have a `settings.json`** + (likely, since you needed `enabledPlugins` to install the plugin in the first place), scaffold.sh reports it + "kept" and leaves it completely untouched — merge the four hooks above and the `permissions`/`sandbox` + blocks from `.claude/skills/setup/templates/settings.json` into your existing file by hand, then it's safe + to drop `enabledPlugins`/`extraKnownMarketplaces` from it once you don't need the plugin loaded anymore. - `.github/workflows/gates.yml` + `.github/actions/setup/action.yml` — the CI gate. Created if absent, left untouched if present. - `.gitignore` entries (append-if-missing, never duplicated): `.env`, `.env.*`, `!.env.example`, @@ -95,8 +121,11 @@ Report the script's per-file summary (created / kept / restamped / up to date / write the interview answers into `.claude/gates.json` (validate with `node -e "require('./.claude/gates.json')"`) and fill `CLAUDE.md` from its template sections (What this project is / Stack & layout mirroring the module map / Conventions / Merge policy mirroring `gates.json` / Don'ts) — propose both files and get an explicit "yes" -before writing. Do **not** touch `.claude/settings.json` or any generic agent/script — only the adapter and -`CLAUDE.md` are project-specific here. +before writing. Beyond that, **you (the interview) should not hand-edit `.claude/settings.json` or any vendored +agent/script/hook** — `scaffold.sh` already handled those mechanically (settings.json created-if-absent, +the runtime harness vendored); only the adapter and `CLAUDE.md` need YOUR project-specific answers written in. +If scaffold.sh reported settings.json "kept" because one already existed, tell the user to merge the runtime +hooks in by hand (see step 4 above) — don't do it for them silently. ## 5. Gitignore verification `scaffold.sh` already appended the required entries in step 4. Spot-check with `git check-ignore ` for diff --git a/.claude/skills/setup/scaffold.sh b/.claude/skills/setup/scaffold.sh index ae0c12f..17718d1 100755 --- a/.claude/skills/setup/scaffold.sh +++ b/.claude/skills/setup/scaffold.sh @@ -110,6 +110,89 @@ for entry in "${MANAGED_FILES[@]}"; do fi done +# --- 2b. runtime harness vendor: keep the consumer session-start critical path OFF the +# plugin (issue #128) ------------------------------------------------------- +# A Claude Code plugin loads (and pays its load cost, ~18s for this one) on EVERY session +# start if it's enabled, regardless of how its hooks are wired — so the only way to remove +# that cost from the consumer's runtime path is to stop depending on the plugin being loaded +# at all once setup is done. This copies the plugin's own runtime subtrees — `agents/`, +# `commands/`, `hooks/`, `scripts/`, `skills/` — wholesale from the plugin root into the +# consumer's local `.claude/`, so a session reads them straight off disk with the plugin +# disabled. The plugin is then only needed to RUN `/orchestrator:setup`/`/orchestrator:sync` +# (the install/update channel), not for everyday sessions. +# +# Managed as ONE unit (not one marker per file, since the whole tree moves together): a +# single top-level version stamp file, `.claude/.orchestrator-vendor`, gates the copy using +# the exact same managed_version_of marker-ladder as the MANAGED_FILES loop above. +# +# EXCEPTION: `.claude/scripts/arm-loop.sh` is skipped by this copy — it already has its own +# dedicated row in MANAGED_FILES above (canonical shipped copy: templates/arm-loop.sh, which +# gets `__WORKDIR__`-style placeholders substituted at ARM time and can legitimately differ +# from THIS plugin repo's own live self-hosting copy of arm-loop.sh). Vendoring it a second +# time here would create two divergent sources of truth for the same destination path. +VENDOR_DIRS=(agents commands hooks scripts skills) +VENDOR_MARKER_PREFIX="@orchestrator-managed runtime-vendor v" +plugin_root="$(cd "$script_dir/../.." && pwd)" +vendor_marker_src="$plugin_root/.orchestrator-vendor" +vendor_marker_dst="$target_root/.claude/.orchestrator-vendor" +vendor_label="runtime harness vendor (.claude/{agents,commands,hooks,scripts,skills})" + +copy_vendor_dirs() { + # Copies each VENDOR_DIRS subtree from the plugin root into target_root/.claude, + # preserving executable bits (cp -a), skipping arm-loop.sh (see EXCEPTION above), then + # stamps the marker file last so a failure mid-copy never leaves a stamped-but-partial tree. + local d entry base + for d in "${VENDOR_DIRS[@]}"; do + [ -d "$plugin_root/$d" ] || continue + mkdir -p "$target_root/.claude/$d" + for entry in "$plugin_root/$d"/* "$plugin_root/$d"/.[!.]*; do + [ -e "$entry" ] || continue + base="$(basename "$entry")" + if [ "$d" = "scripts" ] && [ "$base" = "arm-loop.sh" ]; then + continue + fi + cp -a "$entry" "$target_root/.claude/$d/$base" + done + done + cp "$vendor_marker_src" "$vendor_marker_dst" +} + +vendor_shipped_version="$(managed_version_of "$vendor_marker_src" "$VENDOR_MARKER_PREFIX")" +if [ -z "$vendor_shipped_version" ]; then + echo " error: $vendor_label — shipped marker $vendor_marker_src has no valid @orchestrator-managed marker; plugin install looks broken" >&2 +elif [ "$plugin_root" = "$target_root/.claude" ]; then + # Running the already-vendored copy of scaffold.sh directly (CLAUDE_PLUGIN_ROOT unset, so + # the ${CLAUDE_PLUGIN_ROOT:-.claude} fallback resolved to this repo's own .claude) — there + # is no distinct plugin root to vendor FROM, so there's nothing safe to do here. + echo " kept: $vendor_label — running from an already-vendored copy, no distinct plugin root to vendor from; enable the plugin and re-run to pull updates" +else + vendor_existing_version="$(managed_version_of "$vendor_marker_dst" "$VENDOR_MARKER_PREFIX")" + if [ ! -f "$vendor_marker_dst" ]; then + copy_vendor_dirs + echo " created: $vendor_label at v$vendor_shipped_version" + elif [ -z "$vendor_existing_version" ]; then + copy_vendor_dirs + echo " restamped: $vendor_label — no marker found, now v$vendor_shipped_version" + elif [ "$vendor_existing_version" -lt "$vendor_shipped_version" ]; then + copy_vendor_dirs + echo " restamped: $vendor_label v$vendor_existing_version -> v$vendor_shipped_version" + elif [ "$vendor_existing_version" -eq "$vendor_shipped_version" ]; then + echo " up to date: $vendor_label already v$vendor_shipped_version" + else + echo " kept: $vendor_label is v$vendor_existing_version, newer than this installer's v$vendor_shipped_version — left untouched" + fi +fi + +# --- 2c. consumer settings.json: user-owned from birth, create only if absent ------- +# Unlike gates.json/CLAUDE.md (also user-owned), this template must NEVER be silently +# considered "just another user-owned file" without comment: it carries the runtime hook +# wiring (PostToolUse lint + log-worker-tool, Stop test_affected, PreToolUse guard-git-add) +# that used to depend on the plugin's hooks/hooks.json (which only fires while the plugin is +# loaded). If the consumer already has a settings.json (likely — it may carry their own +# enabledPlugins/extraKnownMarketplaces while installing/updating), scaffold.sh leaves it +# completely untouched; SKILL.md instructs merging the runtime hooks in by hand in that case. +copy_if_absent "$templates_dir/settings.json" "$target_root/.claude/settings.json" "consumer runtime settings (.claude/settings.json)" + # --- 3. CI templates: create only if absent ---------------------------------------- copy_if_absent "$templates_dir/gates.yml" "$target_root/.github/workflows/gates.yml" "CI gate workflow (.github/workflows/gates.yml)" copy_if_absent "$templates_dir/action.yml" "$target_root/.github/actions/setup/action.yml" "CI setup action (.github/actions/setup/action.yml)" diff --git a/.claude/skills/setup/templates/MANIFEST.md b/.claude/skills/setup/templates/MANIFEST.md index 9450dd3..9aa5b1a 100644 --- a/.claude/skills/setup/templates/MANIFEST.md +++ b/.claude/skills/setup/templates/MANIFEST.md @@ -15,6 +15,9 @@ materializes them into the consumer repo on first run. | `arm-loop.sh` | `.claude/scripts/arm-loop.sh` | managed | (issue #102) installs both systemd units above (with placeholders substituted for THIS checkout) + `loginctl enable-linger` + starts the remote-control tmux session. MUST be run in a real terminal outside Claude Code (sandbox caveat — see `docs/HARDENING.md`). Marker `@orchestrator-managed arm-loop vN`; copied with the executable bit preserved. | | `gates.yml` | `.github/workflows/gates.yml` | ci | created only if absent; never overwritten | | `action.yml` | `.github/actions/setup/action.yml` | ci | created only if absent; never overwritten | +| *(live plugin tree, not a template file)* `agents/`, `commands/`, `hooks/`, `scripts/`, `skills/` | `.claude/agents/`, `.claude/commands/`, `.claude/hooks/`, `.claude/scripts/`, `.claude/skills/` | managed (whole-tree, issue #128) | Vendored wholesale from the plugin ROOT (not from `templates/` — this is the only vendored row that copies the plugin's own live directories) so consumer sessions read the runtime harness off local disk without the plugin loaded. Gated as ONE unit by the top-level marker `.claude/.orchestrator-vendor` (`@orchestrator-managed runtime-vendor vN`), re-stamped/re-vendored on the same behind/never-downgrade ladder as every other managed row. **Exception:** `.claude/scripts/arm-loop.sh` is skipped by this copy — it is already its own row below with a different canonical source (`templates/arm-loop.sh`), and double-managing the same destination path from two pristine sources would make the two mechanisms disagree about what "up to date" means for that one file. | +| `.orchestrator-vendor` | `.claude/.orchestrator-vendor` | managed (marker only) | The single version-stamp file that governs the whole-tree row above. Copied last, after the tree copy succeeds, so a failure mid-copy never leaves a stamped-but-partial vendor tree. | +| `settings.json` | `.claude/settings.json` | user | created only if absent; never overwritten. Wires the runtime hooks (`PostToolUse` lint + log-worker-tool, `Stop` test_affected, `PreToolUse` guard-git-add) to `$CLAUDE_PROJECT_DIR/.claude/scripts/...`, plus baseline `permissions`/`sandbox`. Deliberately carries **no** `enabledPlugins`/`extraKnownMarketplaces` — those belong only in a settings.json the user maintains themselves while installing/updating the plugin. If a settings.json already exists, setup leaves it untouched and the setup SKILL instructs merging the runtime hooks in by hand. | The three `managed` rows added by issue #102 (`pr-loop.service`, `claude-rc.service`, `arm-loop.sh`) follow exactly the same ownership class and marker convention as `feature-fanout.js` — `scaffold.sh` @@ -23,5 +26,12 @@ behind, `sync.sh` re-stamps them going forward, and neither ever touches a copy locally diverged from the last pristine version it was stamped from (that's a `conflict`, left for a human — see `.claude/skills/sync/SKILL.md`). +The runtime-vendor row added by issue #128 reuses this exact same marker ladder, just applied to a +directory-tree copy instead of a single `cp` — see `.claude/scripts/arm-loop.sh` above for why it's +carved out as an exception rather than folded into the tree copy. Once vendored, the `orchestrator` +plugin only needs to stay **enabled** to run `/orchestrator:setup`/`/orchestrator:sync` (the +install/update channel) — not for everyday sessions, which is what removes the plugin's session-start +load cost from time-sensitive paths like headless `claude remote-control` spawns. + See `.claude/skills/setup/scaffold.sh` for the implementation, and `.claude/skills/setup/SKILL.md` for the full onboarding flow this scaffold step is one part of. diff --git a/.claude/skills/setup/templates/settings.json b/.claude/skills/setup/templates/settings.json new file mode 100644 index 0000000..a2c62d2 --- /dev/null +++ b/.claude/skills/setup/templates/settings.json @@ -0,0 +1,173 @@ +{ + "_README": "Generic harness settings. Hooks call .claude/scripts/gate.sh (commands live in .claude/gates.json) — you usually edit gates.json, not this file. The 'allow' list pre-approves the agent command surface so PARALLEL workers never block on permission prompts (an unapproved command in one subagent stalls the fan-out) and so the harness never has to persist a grant into this tracked file mid-run. Build/lint/test are covered generically via the gate.sh wildcard; if your agents run package-manager/build tools DIRECTLY (not via gate.sh), add those prefixes too — e.g. 'Bash(pnpm -r:*)', 'Bash(forge test:*)', 'Bash(cargo test:*)', 'Bash(go test:*)'. NOTE: the harness OWNS this file at runtime — it may rewrite the working-tree copy with its own session grant list, so the COMMITTED version is the source of truth and hand-edits won't persist mid-session. To commit a clean version despite that rewrite, stage via the git index: `sha=$(git hash-object -w .claude/settings.json) && git update-index --cacheinfo 100644,$sha,.claude/settings.json`. See docs/USAGE.md and the project's notes on concurrent config writes (anthropics/claude-code#29217). NOTE (issue #128): this file is deliberately created ONLY if you don't already have one, and it deliberately does NOT set enabledPlugins/extraKnownMarketplaces — those belong only in a settings.json you maintain yourself while installing/updating the orchestrator plugin, never in the runtime file that must work with the plugin disabled.", + "hooks": { + "PostToolUse": [ + { + "matcher": "Edit|Write", + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/scripts/gate.sh\" lint" + } + ] + }, + { + "matcher": "Bash|Edit|Write", + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/scripts/log-worker-tool.sh\"" + } + ] + } + ], + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/scripts/gate.sh\" test_affected" + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "python3 \"$CLAUDE_PROJECT_DIR/.claude/scripts/guard-git-add.py\"" + } + ] + } + ] + }, + "permissions": { + "allow": [ + "Read(//**)", + "Bash(bash .claude/scripts/gate.sh:*)", + "Bash(bash .claude/scripts/notify-poll.sh:*)", + "Bash(bash .claude/scripts/pr-feedback.sh:*)", + "Bash(bash .claude/scripts/merge-ready.sh:*)", + "Bash(bash .claude/scripts/loop-tick.sh:*)", + "Bash(git status:*)", + "Bash(git diff:*)", + "Bash(git log:*)", + "Bash(git show:*)", + "Bash(git branch:*)", + "Bash(git checkout:*)", + "Bash(git switch:*)", + "Bash(git add:*)", + "Bash(git restore:*)", + "Bash(git stash:*)", + "Bash(git commit:*)", + "Bash(git merge:*)", + "Bash(git rebase:*)", + "Bash(git fetch:*)", + "Bash(git pull:*)", + "Bash(git push:*)", + "Bash(git worktree:*)", + "Bash(git remote:*)", + "Bash(git rev-parse:*)", + "Bash(git hash-object:*)", + "Bash(git update-index:*)", + "Bash(gh issue:*)", + "Bash(gh pr:*)", + "Bash(gh label:*)", + "Bash(gh repo view:*)", + "Bash(gh auth status)", + "Bash(ls:*)", + "Bash(pwd)", + "Bash(mkdir:*)", + "Bash(find:*)", + "Bash(grep:*)", + "Bash(rg:*)", + "Bash(wc:*)", + "Bash(chmod:*)" + ], + "deny": [ + "Bash(sudo:*)", + "Bash(doas:*)", + "Bash(su:*)", + "Bash(docker:*)", + "Bash(curl:*)", + "Bash(wget:*)", + "Bash(nc:*)", + "Bash(ncat:*)", + "Bash(telnet:*)", + "Bash(cmd.exe:*)", + "Bash(powershell.exe:*)", + "Bash(powershell:*)", + "Bash(pwsh:*)", + "Bash(explorer.exe:*)", + "Bash(wsl.exe:*)", + "Bash(/mnt:*)", + "Bash(npm publish:*)", + "Bash(pnpm publish:*)", + "Bash(yarn publish:*)", + "Bash(gh auth token:*)", + "Bash(gh auth token)", + "Bash(gh secret:*)", + "Bash(git push --force:*)", + "Bash(git push -f:*)", + "Bash(rm -rf:*)", + "Read(//**/.env)", + "Read(//**/.env.*)", + "Read(~/.ssh/**)", + "Read(~/.aws/**)", + "Read(~/.config/gcloud/**)", + "Read(~/.kube/**)", + "Read(~/.gnupg/**)", + "Read(~/.npmrc)", + "Read(~/.docker/config.json)", + "Read(//mnt/**)", + "Edit(//mnt/**)", + "Write(//mnt/**)", + "Edit(//etc/**)", + "Write(//etc/**)" + ] + }, + "sandbox": { + "_README": "OS sandbox (bubblewrap on Linux/WSL2) confines Bash writes to the repo + temp dirs and blocks reading Windows /mnt + credential dirs, so commands run contained. Needs `bwrap` (+ `socat`) installed; with allowUnsandboxedCommands the harness falls back to unsandboxed when bwrap is absent. Pairs with the permissions deny list (always enforced) as defense-in-depth. NOTE: .env is intentionally NOT denied at the shell level here — the loop scripts source it for GH_BOT_TOKEN; it is denied only to the Read tool in permissions above.", + "enabled": true, + "allowUnsandboxedCommands": true, + "filesystem": { + "denyRead": [ + "/mnt" + ] + }, + "credentials": { + "files": [ + { + "path": "~/.ssh", + "mode": "deny" + }, + { + "path": "~/.aws", + "mode": "deny" + }, + { + "path": "~/.config/gcloud", + "mode": "deny" + }, + { + "path": "~/.kube", + "mode": "deny" + }, + { + "path": "~/.gnupg", + "mode": "deny" + }, + { + "path": "~/.npmrc", + "mode": "deny" + }, + { + "path": "~/.docker/config.json", + "mode": "deny" + } + ] + } + } +} diff --git a/.claude/skills/sync/SKILL.md b/.claude/skills/sync/SKILL.md index e74b4af..91e0349 100644 --- a/.claude/skills/sync/SKILL.md +++ b/.claude/skills/sync/SKILL.md @@ -12,21 +12,33 @@ upgrade reach into repos that already onboarded, without a human re-running the ## Ownership model (reuse the setup MANIFEST — do not invent a new scheme) See `.claude/skills/setup/templates/MANIFEST.md` for the authoritative ownership classes. Sync only acts on -the **managed** rows — today: `feature-fanout.js` -> `.claude/workflows/feature-fanout.js`, and (issue #102) +the **managed** rows — today: `feature-fanout.js` -> `.claude/workflows/feature-fanout.js`, (issue #102) the cron-less loop daemon's systemd unit templates + installer: `pr-loop.service` -> `.claude/systemd/pr-loop.service`, `claude-rc.service` -> `.claude/systemd/claude-rc.service`, -and `arm-loop.sh` -> `.claude/scripts/arm-loop.sh`. All four are reconciled by the exact same marker-version -ladder below — the loop-daemon files are ordinary managed files, not a special case. It is designed so adding -a new managed file later is a one-line addition to `sync.sh`'s managed-file table, not a rewrite. +and `arm-loop.sh` -> `.claude/scripts/arm-loop.sh`, and (issue #128) **the vendored runtime harness tree**: +`agents/`, `commands/`, `hooks/`, `scripts/`, `skills/` -> `.claude/agents/`, `.claude/commands/`, +`.claude/hooks/`, `.claude/scripts/`, `.claude/skills/`, gated as ONE unit by a single top-level marker file, +`.claude/.orchestrator-vendor`. All of these are reconciled by the exact same marker-version ladder below — +the loop-daemon files and the runtime-vendor tree are ordinary managed rows, not a special case. It is +designed so adding a new managed file later is a one-line addition to `sync.sh`'s managed-file table, not a +rewrite. The one deliberate exception: `.claude/scripts/arm-loop.sh` is excluded from the runtime-vendor +tree's copy/diff because it's already its own row with a different canonical source — see the comment on +`VENDOR_DIRS` in `sync.sh`. Re-stamping the loop-daemon templates only updates the checked-in files in the repo — it never touches an already-installed unit under `~/.config/systemd/user/` or restarts a running daemon. Tell the user to re-run `bash .claude/scripts/arm-loop.sh` (in a real terminal, per the sandbox caveat) after a restamp if they want the installed units to pick up the change. +**The runtime-vendor tree is what lets the `orchestrator` plugin stay disabled between updates** (issue #128) +— once vendored, a session reads agents/commands/hooks/scripts/skills off local disk, so the plugin only +needs to be enabled to run this very skill (or `/orchestrator:setup`). Re-run this skill any time after +updating the plugin marketplace listing to pull the latest runtime harness in. + Sync **never** touches user-owned files, under any circumstance: - `.claude/gates.json` - `CLAUDE.md` +- `.claude/settings.json` - `.claude/settings.local.json` - `.claude/state/` diff --git a/.claude/skills/sync/sync.sh b/.claude/skills/sync/sync.sh index 8e92112..eb3dee5 100755 --- a/.claude/skills/sync/sync.sh +++ b/.claude/skills/sync/sync.sh @@ -70,10 +70,34 @@ MANAGED_FILES=( "arm-loop.sh|.claude/scripts/arm-loop.sh|@orchestrator-managed arm-loop v" ) +# --- runtime harness vendor: ONE managed unit, not a row in MANAGED_FILES (issue #128) -- +# Unlike everything in MANAGED_FILES above (a single template file -> single destination +# file), the runtime harness vendor is a whole-TREE copy — the plugin's own `agents/`, +# `commands/`, `hooks/`, `scripts/`, `skills/` subtrees, copied wholesale from the plugin +# root (not from templates_dir) into the consumer's `.claude/`. It is gated by ONE +# top-level marker file, `.claude/.orchestrator-vendor`, reusing the exact same +# managed_version_of ladder as MANAGED_FILES, just applied to a directory-tree copy +# instead of a single `cp`. See scaffold.sh's matching "runtime harness vendor" section +# and `.claude/skills/setup/templates/MANIFEST.md` for the full rationale. +# +# EXCEPTION: `.claude/scripts/arm-loop.sh` is excluded from this tree copy/diff — it is +# already a MANAGED_FILES row above with its own canonical template (`templates/arm-loop.sh`, +# placeholder-substituted at ARM time), which can legitimately differ from this plugin +# repo's own live self-hosting copy of arm-loop.sh. Double-managing the same destination +# path from two different pristine sources would make the two mechanisms disagree about +# what "up to date" even means for that one file. +VENDOR_DIRS=(agents commands hooks scripts skills) +VENDOR_MARKER_PREFIX="@orchestrator-managed runtime-vendor v" +plugin_root="$(cd "$script_dir/../.." && pwd)" +vendor_marker_src="$plugin_root/.orchestrator-vendor" +vendor_marker_dst="$target_root/.claude/.orchestrator-vendor" +vendor_dest_rel=".claude/{agents,commands,hooks,scripts,skills}" + # --- user-owned files: NEVER written by sync, only reported for visibility --------- USER_OWNED_FILES=( ".claude/gates.json" "CLAUDE.md" + ".claude/settings.json" ".claude/settings.local.json" ".claude/state/" ) @@ -132,6 +156,44 @@ has_local_edits() { [ "$a" != "$b" ] } +has_local_edits_vendor() { + # Directory-tree analog of has_local_edits, used only for the runtime harness vendor + # (issue #128): recursively diffs each VENDOR_DIRS subtree between the installed + # .claude/ and the plugin root's own .claude/, excluding arm-loop.sh (separately + # managed — see the EXCEPTION comment on VENDOR_DIRS above). Any residual difference — + # including an extra locally-added file — counts as a local edit, same conservative + # philosophy as has_local_edits. + local installed_claude="$1" plugin_claude="$2" d + for d in "${VENDOR_DIRS[@]}"; do + [ -d "$plugin_claude/$d" ] || continue + if ! diff -rq -x arm-loop.sh "$plugin_claude/$d" "$installed_claude/$d" >/dev/null 2>&1; then + return 0 + fi + done + return 1 +} + +copy_vendor_dirs() { + # Copies each VENDOR_DIRS subtree from the plugin root into $target_root/.claude, + # preserving executable bits (cp -a), skipping arm-loop.sh, then stamps the marker file + # last so a failure mid-copy never leaves a stamped-but-partial tree. Mirrors + # scaffold.sh's copy_vendor_dirs exactly. + local d entry base + for d in "${VENDOR_DIRS[@]}"; do + [ -d "$plugin_root/$d" ] || continue + mkdir -p "$target_root/.claude/$d" + for entry in "$plugin_root/$d"/* "$plugin_root/$d"/.[!.]*; do + [ -e "$entry" ] || continue + base="$(basename "$entry")" + if [ "$d" = "scripts" ] && [ "$base" = "arm-loop.sh" ]; then + continue + fi + cp -a "$entry" "$target_root/.claude/$d/$base" + done + done + cp "$vendor_marker_src" "$vendor_marker_dst" +} + # --- 1. managed files: compare marker version + content, act per the ladder below -- had_broken_install=0 @@ -200,6 +262,53 @@ for entry in "${MANAGED_FILES[@]}"; do fi done +# --- 1b. runtime harness vendor: same ladder, applied to a whole tree via ONE marker --- +if [ ! -d "$plugin_root/agents" ]; then + echo " error: $vendor_dest_rel — no shipped plugin root at $plugin_root; plugin install looks broken" >&2 + had_broken_install=1 +elif [ "$plugin_root" = "$target_root/.claude" ]; then + # Running the vendored copy of sync.sh directly (CLAUDE_PLUGIN_ROOT unset, so the + # ${CLAUDE_PLUGIN_ROOT:-.claude} fallback resolved to this repo's own .claude) — there is + # no distinct plugin root to re-vendor FROM. + echo " kept: $vendor_dest_rel — running from an already-vendored copy, no distinct plugin root to re-vendor from; enable the plugin and re-run to pull updates" +else + vendor_shipped_version="$(managed_version_of "$vendor_marker_src" "$VENDOR_MARKER_PREFIX")" + if ! is_sane_version "$vendor_shipped_version"; then + echo " error: $vendor_dest_rel — shipped marker $vendor_marker_src has no valid @orchestrator-managed marker (got \"$vendor_shipped_version\"); plugin install looks broken" >&2 + had_broken_install=1 + elif [ ! -f "$vendor_marker_dst" ]; then + echo " missing: $vendor_dest_rel — not present; run /orchestrator:setup to create it" + else + vendor_installed_version="$(managed_version_of "$vendor_marker_dst" "$VENDOR_MARKER_PREFIX")" + if [ -z "$vendor_installed_version" ]; then + vendor_installed_version=0 + elif ! is_sane_version "$vendor_installed_version"; then + echo " conflict: $vendor_dest_rel has a malformed or out-of-range version marker (\"$vendor_installed_version\") — needs-merge, left untouched" + vendor_installed_version="" + fi + + if [ -n "$vendor_installed_version" ]; then + if [ "$vendor_installed_version" -gt "$vendor_shipped_version" ]; then + echo " kept: $vendor_dest_rel is v$vendor_installed_version, newer than this plugin's v$vendor_shipped_version — left untouched" + elif [ "$vendor_installed_version" -eq "$vendor_shipped_version" ]; then + if has_local_edits_vendor "$target_root/.claude" "$plugin_root"; then + echo " conflict: $vendor_dest_rel is marked v$vendor_installed_version but content diverges from the pristine v$vendor_shipped_version tree — needs-merge, left untouched" + else + echo " up to date: $vendor_dest_rel already v$vendor_shipped_version" + fi + else + # vendor_installed_version < vendor_shipped_version + if has_local_edits_vendor "$target_root/.claude" "$plugin_root"; then + echo " conflict: $vendor_dest_rel is v$vendor_installed_version (behind v$vendor_shipped_version) AND has local edits — needs-merge, left untouched" + else + copy_vendor_dirs + echo " restamped: $vendor_dest_rel v$vendor_installed_version -> v$vendor_shipped_version" + fi + fi + fi + fi +fi + # --- 2. user-owned files: report only, never write ---------------------------------- for f in "${USER_OWNED_FILES[@]}"; do echo " user-owned — skipped by design: $f" diff --git a/docs/GETTING_STARTED.md b/docs/GETTING_STARTED.md index 83f8d0f..d2f4325 100644 --- a/docs/GETTING_STARTED.md +++ b/docs/GETTING_STARTED.md @@ -48,13 +48,25 @@ Then, in Claude Code, inside **your own project**: (`/plugin` alone opens an interactive picker if you'd rather browse marketplaces/plugins than type the commands above.) +Enabling the plugin here is what lets you run `/orchestrator:setup` in Step 2 — that step also **vendors the +plugin's own runtime harness into your repo's local `.claude/`**, so afterward the plugin only needs to stay +enabled to run `/orchestrator:setup`/`/orchestrator:sync` again later (the install/update channel), not for +day-to-day sessions. See Step 2. + ## Step 2 — Onboard: run `/orchestrator:setup` With the plugin enabled, run: ``` /orchestrator:setup ``` -It interviews you, then writes the files a plugin **cannot** carry into your repo — agents, commands, hooks, -and scripts ship *with* the plugin, so there's nothing to copy or wire by hand for those. What the interview +It interviews you, then writes the files a plugin **cannot** carry into your repo, and **vendors the plugin's +runtime harness — `agents/`, `commands/`, `hooks/`, `scripts/`, `skills/` — wholesale into your local +`.claude/`** so a session reads them off disk without the plugin loaded (issue #128: an enabled plugin loads, +and pays its ~18s load cost, on every session start regardless of how its hooks are wired — vendoring is what +removes that cost from anything time-sensitive, e.g. headless `claude remote-control` spawns). It also creates +a **`.claude/settings.json`** from a template *if you don't already have one*, wiring the runtime hooks to +`$CLAUDE_PROJECT_DIR/.claude/scripts/...` instead of the plugin's `hooks/hooks.json` (which only fires while +the plugin is loaded). **Once this is done, the plugin only needs to stay enabled to run +`/orchestrator:setup`/`/orchestrator:sync`** — feel free to disable it between updates. What the interview collects and scaffolds: - **`.claude/gates.json`** (the adapter — the *only* file that makes the generic agents work on YOUR stack). Set: @@ -97,9 +109,11 @@ Each should run the right command (or say "not configured — skipping"). > `Stop` hook and every agent's "done". ### Choosing `test_affected` per stack -`test_affected` runs on the `Stop` hook (shipped by the plugin's `hooks/hooks.json`, no `settings.json` edits -needed) after every change, so it should be *fast* — ideally only the tests touched by the diff. But "test -only what changed" isn't free in every stack. Sensible options: +`test_affected` runs on the `Stop` hook — wired by the `.claude/settings.json` `/orchestrator:setup` scaffolds +(Step 2 above; it calls `$CLAUDE_PROJECT_DIR/.claude/scripts/gate.sh test_affected`), not the plugin's +`hooks/hooks.json` (that only fires while the plugin is loaded) — after every change, so it should be *fast* +— ideally only the tests touched by the diff. But "test only what changed" isn't free in every stack. Sensible +options: | Stack | Cheap `test_affected` | Notes | |---|---|---| diff --git a/docs/USAGE.md b/docs/USAGE.md index 98b50ae..b849c5b 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -436,16 +436,27 @@ marketplace listing and let Claude Code update the installed plugin: /plugin marketplace update recode ``` Then re-stamp the files `/orchestrator:setup` scaffolded into **your** repo (`gates.json`, `CLAUDE.md`, the -fan-out workflow, the CI gate workflow) so they pick up any changes shipped in the update: +fan-out workflow, the CI gate workflow, and — issue #128 — the vendored runtime harness: `agents/`, +`commands/`, `hooks/`, `scripts/`, `skills/`) so they pick up any changes shipped in the update: ``` /orchestrator:sync ``` This compares the version markers `/orchestrator:setup` already scaffolded against what the current plugin ships and re-stamps anything behind — flagging local edits instead of clobbering them (see -`.claude/skills/sync/SKILL.md`) — so an update refreshes managed files (e.g. `feature-fanout.js`) without -re-running the whole interview, and never touches your own `gates.json`/`CLAUDE.md` (those are created once +`.claude/skills/sync/SKILL.md`) — so an update refreshes managed files (e.g. `feature-fanout.js`, or the whole +vendored runtime harness as one unit via its `.claude/.orchestrator-vendor` marker) without re-running the +whole interview, and never touches your own `gates.json`/`CLAUDE.md`/`settings.json` (those are created once and left alone on every re-run). +**The plugin only needs to be *enabled* to run `/orchestrator:setup`/`/orchestrator:sync`** — once the runtime +harness is vendored, everyday sessions read `agents/commands/hooks/scripts/skills` straight out of your local +`.claude/`, so the plugin doesn't need to load at session start at all. This matters because a loaded plugin +pays its load cost (~18s for this one) on **every** session start regardless of how its hooks are wired; for +anything time-sensitive — e.g. `claude remote-control` spawning a headless driver under a ~20-30s spawn-ack +window — that cost can be the difference between the parent seeing the child come up and declaring it dead +(see issue #128). Re-enable the plugin (or just leave it enabled — it's harmless, only slower) whenever you +next want to run setup/sync for an update. + > **Maintainer note: bump `plugin.json`'s `version` on every real change.** `/plugin marketplace update` only > re-fetches plugin content when the plugin's version string actually changes (`.claude/.claude-plugin/plugin.json` > and, for the local-clone method, `.claude/.claude-plugin/marketplace.json`'s matching entry). Merging a fix to From dd43dbc459d4448d86c5a9bcd130ca7a4fd73dbe Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:27:55 +0200 Subject: [PATCH 2/2] fix(harness): idempotent vendor copy, kill skills/*/skills/* nesting (issue #128) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit copy_vendor_dirs in scaffold.sh and sync.sh used `cp -a "$entry" "$dst/$base"` per top-level entry, which nests into an already-existing destination on any restamp — skills/ is the only vendored dir with subdirectories, so a real version bump produced duplicate nests (.claude/skills/setup/setup, .claude/skills/sync/sync) and left the real files stale, permanently diverging the tree and making every later /orchestrator:sync report a false conflict. Switch to prune-then-copy of directory CONTENTS (rm -rf dest, then cp -a src/. dest/) so restamps are idempotent and prune files removed upstream, while explicitly backing up and restoring .claude/scripts/arm-loop.sh around the scripts/ prune so its own MANAGED_FILES row/template stays the sole source of truth for that file. Strengthen vendor-runtime.test.sh: assert no nesting and genuine content refresh on restamp (fails against the old cp -a form, passes after the fix), add never-downgrade coverage for both scripts, prove conflict paths leave the tree untouched by grepping the injected local marker back out of the file (not just the log line), and trim plugin-root fixtures to only the entries the vendor step reads instead of copying the whole .claude/ tree. Co-Authored-By: Claude Sonnet 5 --- .claude/scripts/vendor-runtime.test.sh | 173 ++++++++++++++++++++----- .claude/skills/setup/scaffold.sh | 41 ++++-- .claude/skills/sync/sync.sh | 40 ++++-- 3 files changed, 198 insertions(+), 56 deletions(-) mode change 100644 => 100755 .claude/scripts/vendor-runtime.test.sh diff --git a/.claude/scripts/vendor-runtime.test.sh b/.claude/scripts/vendor-runtime.test.sh old mode 100644 new mode 100755 index e01af39..d2f01ae --- a/.claude/scripts/vendor-runtime.test.sh +++ b/.claude/scripts/vendor-runtime.test.sh @@ -9,18 +9,31 @@ # settings.json created with no enabledPlugins/extraKnownMarketplaces. # 2. re-running scaffold -> idempotent ("up to date" / "kept", no errors). # 3. sync against an unchanged plugin root -> "up to date". -# 4. sync against a plugin root whose marker was bumped with NO other -# content change -> restamped (no false conflict). -# 5. sync against a plugin root whose marker was bumped WITH a real +# 4. scaffold.sh restamp (v1 -> v2) with a REAL content change inside a +# vendored subdirectory (skills/setup/) -> proves the copy is idempotent +# AND refreshing: no duplicate nests (.claude/skills/setup/setup, +# .claude/skills/sync/sync — the reported bug) and the subdir file's +# content actually lands at the new version instead of staying stale. +# arm-loop.sh's own managed copy must also survive untouched. +# 5. sync.sh restamp (v1 -> v2), marker-only bump with no other content +# change -> restamped cleanly (no false conflict) AND no nesting. +# 6. sync against a plugin root whose marker was bumped WITH a real # content change -> conflict (installed has no local edits, but the # script can't tell a legitimate upstream change from a hand-edit — -# same conservative behavior as every other managed row). -# 6. sync with a genuine local edit to a vendored file (same version) -> -# conflict. -# 7. sync with a local edit ONLY to arm-loop.sh -> the runtime-vendor row +# same conservative behavior as every other managed row); the +# installed file must NOT have picked up the new upstream bytes. +# 7. sync with a genuine local edit to a vendored file (same version) -> +# conflict, and the local edit marker is still readable back out of +# the file afterward (proves it was genuinely left untouched, not +# just that the log line said so). +# 8. sync with a local edit ONLY to arm-loop.sh -> the runtime-vendor row # stays "up to date" (excluded from its diff), while arm-loop.sh's OWN # managed row reports the conflict — proves the two mechanisms don't -# double-manage the same file. +# double-manage the same file; the hand edit is still present in +# arm-loop.sh afterward. +# 9. never-downgrade: a target marker newer than the plugin's shipped +# version (v99) is left byte-for-byte untouched by scaffold.sh. +# 10. never-downgrade: same, for sync.sh. # # Exit 0 on success, non-zero if any assertion fails. Runnable bare: # bash .claude/scripts/vendor-runtime.test.sh @@ -56,6 +69,24 @@ check_output() { echo "FAIL - $desc (expected to find: $needle)" fi } +trees_identical() { + # $1, $2 = two directory trees. True iff a recursive diff finds no differences at all + # (used by the never-downgrade scenarios to prove a "kept, newer" verdict really left + # the whole vendored tree byte-for-byte alone, not just logged the right line). + diff -rq "$1" "$2" >/dev/null 2>&1 +} +build_plugin_fixture() { + # $1 = destination dir for a trimmed "plugin root" fixture. Copies only the entries + # scaffold.sh/sync.sh's vendor step actually reads from a plugin root — agents/, + # commands/, hooks/, scripts/, skills/, .orchestrator-vendor — instead of the whole live + # .claude/ tree, which would otherwise drag in .claude/worktrees/ (large), + # .claude/state/, and settings.local.json into every fixture. + local dest="$1" entry + mkdir -p "$dest" + for entry in agents commands hooks scripts skills .orchestrator-vendor; do + cp -a "$repo_root/.claude/$entry" "$dest/$entry" + done +} # --------------------------------------------------------------------------- # Scenario 1: fresh scaffold. @@ -86,6 +117,8 @@ check_output "s1: vendor row reported created/up-to-date" "$out1" "runtime harne out2="$(bash "$scaffold_sh" "$t1" 2>&1)" check_output "s2: re-run reports vendor up to date" "$out2" "up to date: runtime harness vendor" check_output "s2: re-run keeps settings.json (user-owned)" "$out2" "kept: consumer runtime settings" +check "s2: still no nested skills/setup/setup after idempotent re-run" test ! -e "$t1/.claude/skills/setup/setup" +check "s2: still no nested skills/sync/sync after idempotent re-run" test ! -e "$t1/.claude/skills/sync/sync" # --------------------------------------------------------------------------- # Scenario 3: sync against an unchanged plugin root -> up to date. @@ -96,57 +129,131 @@ check "s3: sync exits 0" test "$rc3" -eq 0 check_output "s3: vendor row up to date" "$out3" "up to date: .claude/{agents,commands,hooks,scripts,skills}" # --------------------------------------------------------------------------- -# Scenario 4: plugin marker bumped, no other content change -> restamp. +# Scenario 4: scaffold.sh restamp (v1 -> v2) with a REAL content change inside +# a vendored subdirectory -> no nesting, content genuinely refreshed. This is +# the direct regression test for the reported bug: the old `cp -a "$entry" +# "$dst/$base"` form nests the source dir INTO an already-existing destination +# (producing .claude/skills/setup/setup) instead of refreshing it in place, so +# the top-level file never actually picks up the new content. # --------------------------------------------------------------------------- -plugin_v2="$work/plugin-v2" -cp -a "$repo_root/.claude" "$plugin_v2" -sed -i 's/@orchestrator-managed runtime-vendor v1/@orchestrator-managed runtime-vendor v2/' \ - "$plugin_v2/.orchestrator-vendor" t4="$work/consumer4" mkdir -p "$t4" bash "$scaffold_sh" "$t4" >/dev/null 2>&1 -out4="$(bash "$plugin_v2/skills/sync/sync.sh" "$t4" 2>&1)" -check_output "s4: marker-only bump restamps cleanly" "$out4" "restamped: .claude/{agents,commands,hooks,scripts,skills} v1 -> v2" + +plugin_v2content="$work/plugin-v2-content" +build_plugin_fixture "$plugin_v2content" +sed -i 's/@orchestrator-managed runtime-vendor v1/@orchestrator-managed runtime-vendor v2/' \ + "$plugin_v2content/.orchestrator-vendor" +printf '\n\n' >> "$plugin_v2content/skills/setup/SKILL.md" + +out4="$(bash "$plugin_v2content/skills/setup/scaffold.sh" "$t4" 2>&1)" +check_output "s4: restamp reported v1 -> v2" "$out4" "restamped: runtime harness vendor" +check "s4: no nested .claude/skills/setup/setup (nesting bug)" test ! -e "$t4/.claude/skills/setup/setup" +check "s4: no nested .claude/skills/sync/sync (nesting bug)" test ! -e "$t4/.claude/skills/sync/sync" +check "s4: skills/setup/SKILL.md content actually refreshed to v2 (not stale)" \ + grep -q -- "v2 skill content" "$t4/.claude/skills/setup/SKILL.md" +check "s4: arm-loop.sh not vendored/overwritten by the tree copy" \ + grep -q -- "@orchestrator-managed arm-loop v" "$t4/.claude/scripts/arm-loop.sh" # --------------------------------------------------------------------------- -# Scenario 5: plugin marker bumped WITH a real content change -> conflict -# (can't distinguish a legitimate upstream change from a local hand-edit). +# Scenario 5: sync.sh restamp, marker-only bump with NO other content change +# -> restamps cleanly (no false conflict) AND without nesting. # --------------------------------------------------------------------------- -plugin_v3="$work/plugin-v3" -cp -a "$repo_root/.claude" "$plugin_v3" +plugin_v2="$work/plugin-v2" +build_plugin_fixture "$plugin_v2" sed -i 's/@orchestrator-managed runtime-vendor v1/@orchestrator-managed runtime-vendor v2/' \ - "$plugin_v3/.orchestrator-vendor" -echo "" >> "$plugin_v3/agents/orchestrator.md" + "$plugin_v2/.orchestrator-vendor" t5="$work/consumer5" mkdir -p "$t5" bash "$scaffold_sh" "$t5" >/dev/null 2>&1 -out5="$(bash "$plugin_v3/skills/sync/sync.sh" "$t5" 2>&1)" -check_output "s5: content-changing bump flags conflict, not a silent restamp" "$out5" \ - "conflict: .claude/{agents,commands,hooks,scripts,skills} is v1 (behind v2) AND has local edits" +out5="$(bash "$plugin_v2/skills/sync/sync.sh" "$t5" 2>&1)" +check_output "s5: marker-only bump restamps cleanly" "$out5" "restamped: .claude/{agents,commands,hooks,scripts,skills} v1 -> v2" +check "s5: no nested .claude/skills/setup/setup after sync restamp" test ! -e "$t5/.claude/skills/setup/setup" +check "s5: no nested .claude/skills/sync/sync after sync restamp" test ! -e "$t5/.claude/skills/sync/sync" # --------------------------------------------------------------------------- -# Scenario 6: genuine local edit, same version -> conflict. +# Scenario 6: plugin marker bumped WITH a real content change -> conflict +# (can't distinguish a legitimate upstream change from a local hand-edit). +# The installed file must NOT have picked up the new upstream bytes. # --------------------------------------------------------------------------- +plugin_v3="$work/plugin-v3" +build_plugin_fixture "$plugin_v3" +sed -i 's/@orchestrator-managed runtime-vendor v1/@orchestrator-managed runtime-vendor v2/' \ + "$plugin_v3/.orchestrator-vendor" +echo "" >> "$plugin_v3/agents/orchestrator.md" t6="$work/consumer6" mkdir -p "$t6" bash "$scaffold_sh" "$t6" >/dev/null 2>&1 -echo "" >> "$t6/.claude/agents/orchestrator.md" -out6="$(bash "$sync_sh" "$t6" 2>&1)" -check_output "s6: local edit at same version flags conflict" "$out6" \ - "conflict: .claude/{agents,commands,hooks,scripts,skills} is marked v1 but content diverges" +out6="$(bash "$plugin_v3/skills/sync/sync.sh" "$t6" 2>&1)" +check_output "s6: content-changing bump flags conflict, not a silent restamp" "$out6" \ + "conflict: .claude/{agents,commands,hooks,scripts,skills} is v1 (behind v2) AND has local edits" +check "s6: installed file did not silently pick up the new upstream bytes" \ + bash -c '! grep -qF -- "" "$1"' _ "$t6/.claude/agents/orchestrator.md" # --------------------------------------------------------------------------- -# Scenario 7: local edit ONLY to arm-loop.sh doesn't leak into the vendor row. +# Scenario 7: genuine local edit, same version -> conflict, and the edit is +# still readable back out of the file afterward (proves the tree was +# genuinely left untouched, not just that the log line said so). # --------------------------------------------------------------------------- t7="$work/consumer7" mkdir -p "$t7" bash "$scaffold_sh" "$t7" >/dev/null 2>&1 -echo "# hand edit" >> "$t7/.claude/scripts/arm-loop.sh" +echo "" >> "$t7/.claude/agents/orchestrator.md" out7="$(bash "$sync_sh" "$t7" 2>&1)" -check_output "s7: arm-loop.sh's OWN managed row flags the conflict" "$out7" \ +check_output "s7: local edit at same version flags conflict" "$out7" \ + "conflict: .claude/{agents,commands,hooks,scripts,skills} is marked v1 but content diverges" +check "s7: local marker persists — file genuinely untouched by sync" \ + grep -qF -- "" "$t7/.claude/agents/orchestrator.md" + +# --------------------------------------------------------------------------- +# Scenario 8: local edit ONLY to arm-loop.sh doesn't leak into the vendor row, +# and the hand edit is still present afterward (neither mechanism touched it). +# --------------------------------------------------------------------------- +t8="$work/consumer8" +mkdir -p "$t8" +bash "$scaffold_sh" "$t8" >/dev/null 2>&1 +echo "# hand edit" >> "$t8/.claude/scripts/arm-loop.sh" +out8="$(bash "$sync_sh" "$t8" 2>&1)" +check_output "s8: arm-loop.sh's OWN managed row flags the conflict" "$out8" \ "conflict: .claude/scripts/arm-loop.sh is marked" -check_output "s7: vendor row unaffected by the arm-loop.sh edit" "$out7" \ +check_output "s8: vendor row unaffected by the arm-loop.sh edit" "$out8" \ "up to date: .claude/{agents,commands,hooks,scripts,skills}" +check "s8: hand edit persists in arm-loop.sh — untouched by either mechanism" \ + grep -qF -- "# hand edit" "$t8/.claude/scripts/arm-loop.sh" + +# --------------------------------------------------------------------------- +# Scenario 9: never-downgrade — scaffold.sh must not touch a target whose +# vendor marker is newer (v99) than what this installer ships. +# --------------------------------------------------------------------------- +t9="$work/consumer9" +mkdir -p "$t9" +bash "$scaffold_sh" "$t9" >/dev/null 2>&1 +sed -i 's/@orchestrator-managed runtime-vendor v1/@orchestrator-managed runtime-vendor v99/' \ + "$t9/.claude/.orchestrator-vendor" +snapshot9="$work/snapshot9" +cp -a "$t9/.claude" "$snapshot9" +out9="$(bash "$scaffold_sh" "$t9" 2>&1)" +check_output "s9: scaffold keeps a newer-than-shipped marker" "$out9" \ + "is v99, newer than this installer's v1" +check "s9: vendored tree byte-identical after run (no downgrade)" \ + trees_identical "$snapshot9" "$t9/.claude" + +# --------------------------------------------------------------------------- +# Scenario 10: never-downgrade — sync.sh must not touch a target whose vendor +# marker is newer (v99) than what this plugin ships. +# --------------------------------------------------------------------------- +t10="$work/consumer10" +mkdir -p "$t10" +bash "$scaffold_sh" "$t10" >/dev/null 2>&1 +sed -i 's/@orchestrator-managed runtime-vendor v1/@orchestrator-managed runtime-vendor v99/' \ + "$t10/.claude/.orchestrator-vendor" +snapshot10="$work/snapshot10" +cp -a "$t10/.claude" "$snapshot10" +out10="$(bash "$sync_sh" "$t10" 2>&1)" +check_output "s10: sync keeps a newer-than-shipped marker" "$out10" \ + "is v99, newer than this plugin's v1" +check "s10: vendored tree byte-identical after run (no downgrade)" \ + trees_identical "$snapshot10" "$t10/.claude" echo if [ "$fail" -ne 0 ]; then diff --git a/.claude/skills/setup/scaffold.sh b/.claude/skills/setup/scaffold.sh index 17718d1..7dd8b6f 100755 --- a/.claude/skills/setup/scaffold.sh +++ b/.claude/skills/setup/scaffold.sh @@ -138,21 +138,40 @@ vendor_marker_dst="$target_root/.claude/.orchestrator-vendor" vendor_label="runtime harness vendor (.claude/{agents,commands,hooks,scripts,skills})" copy_vendor_dirs() { - # Copies each VENDOR_DIRS subtree from the plugin root into target_root/.claude, - # preserving executable bits (cp -a), skipping arm-loop.sh (see EXCEPTION above), then - # stamps the marker file last so a failure mid-copy never leaves a stamped-but-partial tree. - local d entry base + # Prune-then-copy of each VENDOR_DIRS subtree's CONTENTS (not the subtree itself) from + # the plugin root into target_root/.claude, preserving executable bits (cp -a). This must + # be idempotent AND pruning: a plain `cp -a src dst` when dst already EXISTS nests the + # source dir inside it (src becomes dst/src) instead of refreshing it in place — that's + # exactly the bug this replaced (a restamp/upgrade run used to leave duplicate nests like + # .claude/skills/setup/setup and never actually update changed files). Removing the + # destination subtree first and then copying the source's CONTENTS (`src/.` -> `dst/`) + # both fixes the nesting and prunes files removed upstream, so a later `diff -rq` never + # trips on stale leftovers. + # + # EXCEPTION: `.claude/scripts/arm-loop.sh` must survive this — it already has its own + # dedicated MANAGED_FILES row (canonical template: templates/arm-loop.sh) and must NOT be + # vendored/overwritten from this plugin repo's own live copy. Back it up before pruning + # `scripts/`, then restore it (or remove whatever the plugin copy dropped in its place if + # there was nothing to restore) after the copy. + local d arm_backup="" for d in "${VENDOR_DIRS[@]}"; do [ -d "$plugin_root/$d" ] || continue + if [ "$d" = "scripts" ] && [ -f "$target_root/.claude/scripts/arm-loop.sh" ]; then + arm_backup="$(mktemp "${TMPDIR:-/tmp}/arm-loop.sh.XXXXXX")" + cp -a "$target_root/.claude/scripts/arm-loop.sh" "$arm_backup" + fi + rm -rf "$target_root/.claude/$d" mkdir -p "$target_root/.claude/$d" - for entry in "$plugin_root/$d"/* "$plugin_root/$d"/.[!.]*; do - [ -e "$entry" ] || continue - base="$(basename "$entry")" - if [ "$d" = "scripts" ] && [ "$base" = "arm-loop.sh" ]; then - continue + cp -a "$plugin_root/$d/." "$target_root/.claude/$d/" + if [ "$d" = "scripts" ]; then + if [ -n "$arm_backup" ]; then + cp -a "$arm_backup" "$target_root/.claude/scripts/arm-loop.sh" + rm -f "$arm_backup" + arm_backup="" + else + rm -f "$target_root/.claude/scripts/arm-loop.sh" fi - cp -a "$entry" "$target_root/.claude/$d/$base" - done + fi done cp "$vendor_marker_src" "$vendor_marker_dst" } diff --git a/.claude/skills/sync/sync.sh b/.claude/skills/sync/sync.sh index eb3dee5..b6a74b5 100755 --- a/.claude/skills/sync/sync.sh +++ b/.claude/skills/sync/sync.sh @@ -174,22 +174,38 @@ has_local_edits_vendor() { } copy_vendor_dirs() { - # Copies each VENDOR_DIRS subtree from the plugin root into $target_root/.claude, - # preserving executable bits (cp -a), skipping arm-loop.sh, then stamps the marker file - # last so a failure mid-copy never leaves a stamped-but-partial tree. Mirrors - # scaffold.sh's copy_vendor_dirs exactly. - local d entry base + # Prune-then-copy of each VENDOR_DIRS subtree's CONTENTS (not the subtree itself) from + # the plugin root into $target_root/.claude, preserving executable bits (cp -a). Mirrors + # scaffold.sh's copy_vendor_dirs exactly — see that function's comment for why a plain + # `cp -a src dst` on an already-existing dst nests instead of refreshing (the bug this + # replaced: a restamp used to leave duplicate nests like .claude/skills/sync/sync and + # never actually update changed files), and why removing the destination subtree first + # then copying the source's CONTENTS (`src/.` -> `dst/`) both fixes the nesting and prunes + # files removed upstream. + # + # EXCEPTION: `.claude/scripts/arm-loop.sh` must survive this — it already has its own + # dedicated MANAGED_FILES row and must NOT be vendored/overwritten from this plugin + # repo's own live copy. Back it up before pruning `scripts/`, then restore it (or remove + # whatever the plugin copy dropped in its place if there was nothing to restore) after. + local d arm_backup="" for d in "${VENDOR_DIRS[@]}"; do [ -d "$plugin_root/$d" ] || continue + if [ "$d" = "scripts" ] && [ -f "$target_root/.claude/scripts/arm-loop.sh" ]; then + arm_backup="$(mktemp "${TMPDIR:-/tmp}/arm-loop.sh.XXXXXX")" + cp -a "$target_root/.claude/scripts/arm-loop.sh" "$arm_backup" + fi + rm -rf "$target_root/.claude/$d" mkdir -p "$target_root/.claude/$d" - for entry in "$plugin_root/$d"/* "$plugin_root/$d"/.[!.]*; do - [ -e "$entry" ] || continue - base="$(basename "$entry")" - if [ "$d" = "scripts" ] && [ "$base" = "arm-loop.sh" ]; then - continue + cp -a "$plugin_root/$d/." "$target_root/.claude/$d/" + if [ "$d" = "scripts" ]; then + if [ -n "$arm_backup" ]; then + cp -a "$arm_backup" "$target_root/.claude/scripts/arm-loop.sh" + rm -f "$arm_backup" + arm_backup="" + else + rm -f "$target_root/.claude/scripts/arm-loop.sh" fi - cp -a "$entry" "$target_root/.claude/$d/$base" - done + fi done cp "$vendor_marker_src" "$vendor_marker_dst" }