Stop vendoring the runtime harness into consumer repos (issue #134) - #156
Merged
Conversation
… (issue #134) scaffold.sh/sync.sh no longer copy agents/commands/hooks/scripts/skills wholesale into a consumer's local .claude/ (issue #128's model) — those resolve at runtime via ${CLAUDE_PLUGIN_ROOT} instead, so the plugin cache stays the single source of truth and can't be permanently shadowed by a stale, unmanaged local copy (resolve-roots.sh deliberately makes a repo-tracked .claude/scripts layout win, which is what let a vendored copy silently outlive the plugin cache in reDeploy). The vendor-version marker (.claude/.orchestrator-vendor) and its copy machinery are removed. sync.sh replaces the old restamp/re-vendor ladder with detect_stale_vendor_copies: it flags any local leftover of the old vendored tree (byte-identical to the plugin's shipped copy -> safe to delete; diverging -> possible deliberate override, needs a human look) but never deletes or restamps it — a legitimate local override must be surfaced, not silently clobbered, per the issue's decision. Replaces vendor-runtime.test.sh (tested the removed copy machinery) with sync-managed-files.test.sh, covering the MANAGED_FILES ladder end to end plus the new stale-vendor detect-and-warn behavior. Co-Authored-By: Claude Sonnet 5 <[email protected]>
…ugin cache
pr-loop.service (v1 -> v2): ExecStart now resolves loop-daemon.sh at unit-start time
instead of assuming a vendored repo-local copy — prefers a repo-tracked
.claude/scripts/loop-daemon.sh (self-hosting), else the newest
~/.claude/plugins/cache/*/orchestrator/*/scripts/loop-daemon.sh. arm-loop.sh (the only
place that could bake an exact plugin-cache path via a new placeholder) is a
separately-owned managed template, so this is a runtime resolver rather than an
ARM-time substitution — a follow-up could teach arm-loop.sh to bake the exact path
instead.
settings.json template + consumer CLAUDE.md template: hook commands now resolve via
${CLAUDE_PLUGIN_ROOT:-$CLAUDE_PROJECT_DIR/.claude}/scripts/... instead of a hardcoded
$CLAUDE_PROJECT_DIR/.claude/scripts/... path, and the plugin is now documented as
needing to stay ENABLED for everyday sessions (reversing the #128-era "keep it
disabled" guidance) since the runtime harness is no longer vendored locally.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
…tion (issue #134) Updates setup/sync SKILL.md, the setup MANIFEST, and docs/{GETTING_STARTED,USAGE, MIGRATION}.md to describe the reverted vendoring model: the plugin must stay enabled for everyday sessions, agents/commands/hooks/scripts/skills resolve via ${CLAUDE_PLUGIN_ROOT}, and /orchestrator:sync detects-and-warns about local leftovers from an old vendored install instead of restamping/deleting them. Also documents the migration caveat: a running pr-loop/claude-rc systemd unit holds its OLD script in memory until its unit restarts, so cleaning up a stale local copy on disk isn't enough on its own. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Co-Authored-By: Claude Sonnet 5 <[email protected]>
…start clean loop-census.test.sh's main_dirty section (issue #106) reuses fixture1 much later in the file, assuming it's otherwise clean — but the earlier ci_fix_prs section (issue #96) overwrites .claude/scripts/pr-ci-fix.sh in that same fixture without committing the change, leaving it permanently tracked-but-modified for the rest of the fixture's life. That untracked dirt isn't covered by any of the main_dirty exclusions (sandbox-mask phantom paths, .claude/agents/, .claude/skills/setup/ templates/), so every main_dirty=no assertion after it failed regardless of which exclusion was actually under test. Committing the swap in place restores the fixture-clean assumption. Pre-existing gap, unrelated to issue #134 — found while gating this branch. Co-Authored-By: Claude Sonnet 5 <[email protected]>
… stray marker without dirs (issue #134) Review found sync.sh's stale-vendor detection flagged the plugin's own agents/commands/hooks/scripts/skills as leftover vendor copies when run against reCode itself (self_hosting=1), telling the operator to reconcile/delete its own source tree and printing the systemd migration caveat. Short-circuit detect_stale_vendor_copies to report "none found" whenever plugin_root == target_root/.claude, and keep the call site silent in that mode even if a stray legacy marker is present. Also close a real gap: a consumer who deleted the vendored dirs by hand but left a stray .claude/.orchestrator-vendor marker previously got no warning at all, since the marker check only ran when a stale dir was also found. It now warns about the leftover marker on its own (still detect-only, never deletes), except in self-hosting mode. Added scenarios 9 (self-hosting silence, with/without a stray marker) and 10 (stray marker with no stale dirs warns) to sync-managed-files.test.sh.
Collaborator
Author
|
Stop vendoring the runtime harness into consumer repos (issue #134) (not yet reviewed) |
3 similar comments
Collaborator
Author
|
Stop vendoring the runtime harness into consumer repos (issue #134) (not yet reviewed) |
Collaborator
Author
|
Stop vendoring the runtime harness into consumer repos (issue #134) (not yet reviewed) |
Collaborator
Author
|
Stop vendoring the runtime harness into consumer repos (issue #134) (not yet reviewed) |
robercano
approved these changes
Jul 17, 2026
robercano
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the owner's decision on #134 (option 1 — stop vendoring):
agents/commands/hooks/scripts/skillsinto a consumer's local.claude/. Those now resolve at runtime via${CLAUDE_PLUGIN_ROOT}, so the plugin cache stays the single source of truth (fixes the shadowing bug:resolve-roots.shdeliberately makes a repo-tracked.claude/scriptslayout win over the plugin cache, which let a stale vendored copy permanently shadow a fresh plugin install — the reDeploy incident that prompted this issue). The.claude/.orchestrator-vendormarker and its copy machinery are removed. The 4 genuinely-managed files (feature-fanout.js,pr-loop.service,claude-rc.service,arm-loop.sh) are untouched by this change and keep working exactly as before./orchestrator:syncnow DETECTS AND WARNS about local leftovers of the old vendored tree instead of silently deleting/restamping them: byte-identical to the plugin's shipped copy → flaggedsafe to delete; diverging → flaggedconflict(possible deliberate local override, needs a human look). It never touches the directory itself. Seedetect_stale_vendor_copiesin.claude/skills/sync/sync.sh.pr-loop.service(v1 → v2):ExecStartnow resolvesloop-daemon.shat unit-start time — prefers a repo-tracked.claude/scripts/loop-daemon.sh(self-hosting), else the newest~/.claude/plugins/cache/*/orchestrator/*/scripts/loop-daemon.sh— instead of assuming a vendored repo-local copy..claude/settings.jsontemplate hooks now resolve via${CLAUDE_PLUGIN_ROOT:-$CLAUDE_PROJECT_DIR/.claude}/scripts/...instead of a hardcoded$CLAUDE_PROJECT_DIR/.claude/scripts/...path.GETTING_STARTED.md,USAGE.md,MIGRATION.md, both setup/syncSKILL.md,MANIFEST.md) updated: the plugin must now stay enabled for everyday sessions (reversing the Headless sessions in plugin-consumer repos always die: session-start plugin load breaches remote-control's spawn-ack timeout #128-era "keep it disabled" guidance), andMIGRATION.mdgets a short section for repos that onboarded during the Headless sessions in plugin-consumer repos always die: session-start plugin load breaches remote-control's spawn-ack timeout #128→Consumer repos carry stale vendored .claude/scripts that shadow the fresh plugin cache — manage them or stop vendoring #134 vendored window.plugin.json/marketplace.jsonto 0.2.2 + added aCHANGELOG.mdentry, per the "bump the version on every real change" maintainer note (otherwise cached installs never pick this up).vendor-runtime.test.sh(tested the removed copy machinery) withsync-managed-files.test.sh(37 checks: full MANAGED_FILES ladder + the new stale-vendor detect-and-warn behavior).loop-census.test.sh'smain_dirtysection reused a fixture that an earlier section left permanently dirty (an uncommittedpr-ci-fix.shswap), tripping everymain_dirty=noassertion regardless of which exclusion was under test. Committed the swap in the fixture so the assumption holds again.Self-hosting safety
This repo's own
.claude/self/self-hosting is unaffected:resolve-roots.sh's repo-tracked-layout precedence is untouched (verified — no changes toresolve-roots.sh), and every script invocation across agents/commands/skills already used the${CLAUDE_PLUGIN_ROOT:-<repo>/.claude}fallback idiom, which now also applies correctly to the settings.json hooks andpr-loop.service's resolver (both fall back to the repo-tracked copy whenCLAUDE_PLUGIN_ROOTis unset, which is always true in this repo's own self-hosted sessions)..claude/self/gates.json-driven gates (build/lint/test) all pass in-worktree.Migration caveat (from the issue)
A consumer's running
pr-loop/claude-rcsystemd unit holds its OLD script in memory until its unit restarts — refreshing/deleting files on disk is not enough.sync.sh's stale-vendor output and all updated docs explicitly tell the user to runsystemctl --user restart pr-loop-<repo-slug>.service claude-rc-<repo-slug>.serviceafter cleaning up a stale local copy (cf. the 2026-07-16 reCode deploy-lag incident, #131).Gate results (
.claude/self/gates.json, run from the worktree)Deferred / follow-ups
pr-loop.service's plugin-cache discovery is a runtime glob (~/.claude/plugins/cache/*/orchestrator/*/scripts/loop-daemon.sh, newest wins) rather than an exact path baked in at ARM time — teachingarm-loop.shto resolve+bake the precise plugin cache path via a new placeholder would be cleaner, butarm-loop.sh/claude-rc.serviceare off-limits to this PR (listed as separately-owned managed templates in the task scope). Flagging as a real follow-up, not a blocker — the glob fallback is tested manually and works against both the self-hosting and real-installed-plugin-cache cases on this machine.settings.json(Bash(bash .claude/scripts/gate.sh:*)etc.) are still relative-path prefixes; when${CLAUDE_PLUGIN_ROOT}resolves to an absolute plugin-cache path, those prefixes won't match the literal command text, which could reintroduce permission prompts for plugin-enabled sessions. This is a pre-existing gap (agent prompts already used the${CLAUDE_PLUGIN_ROOT:-.claude}idiom before this PR) — not introduced here, but now more likely to matter since the plugin must stay enabled. Worth a dedicated follow-up issue.Fixes #134
🤖 Generated with Claude Code