feat: launcher override chain for Claude plugins#126
Conversation
Plan to adopt the cc-thingz override-chain pattern for the two
Claude-side launcher scripts (launch-revdiff.sh and
launch-plan-review.sh) so users can drop their own launcher at
.claude/<plugin-namespace>/scripts/ or ${CLAUDE_PLUGIN_DATA}/scripts/
without forking the plugin. Resolves action item from discussion #121.
…code execution
The revdiff-planning hook fires automatically on every ExitPlanMode in
any repo. A project-layer executable override would let an untrusted
repo run arbitrary code on routine Claude actions without per-repo
opt-in. Drop the project layer from both resolvers (the diff-review
skill keeps the same shape for symmetry, even though it's user-invoked)
and reduce the chain to user (${CLAUDE_PLUGIN_DATA}/scripts/) +
bundled. Flagged by codex external review.
The -x test alone matches directories (search permission), so a user-data directory named launch-revdiff.sh / launch-plan-review.sh would win over the bundled launcher and then fail at exec. Combine with -f to require a regular file. Also append a scope-change addendum to the archived plan documenting the project-layer removal and this fix. Flagged by codex.
- hooks.json: quote ${CLAUDE_PLUGIN_ROOT} interpolation in the python3 command
string (pre-existing bug — spaced paths broke the hook)
- archived plan: add post-merge superseded notice at the top pointing to the
addendum, fix stale 'commit pending' note that should reference 4777d26
Flagged by codex external review iteration 3.
There was a problem hiding this comment.
Pull request overview
Adds a consistent “user → bundled” launcher-override mechanism for the two Claude plugins (diff-review skill + plan-review hook), allowing per-user launcher customization via ${CLAUDE_PLUGIN_DATA} without forking, with docs updated to reflect the security-driven omission of any project-level override.
Changes:
- Added
resolve-launcher.shresolvers (one per plugin) to select the first executable launcher fromuser → bundled. - Updated
revdiff-planninghook to resolvelaunch-plan-review.shvia the resolver (and pinned resolvercwdtoCLAUDE_PROJECT_DIR). - Bumped plugin versions and updated docs across README/CLAUDE/site docs and plugin-specific references.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| site/docs.html | Documents the new override chain and clarifies pi/Codex asymmetry. |
| plugins/revdiff-planning/scripts/resolve-launcher.sh | New resolver script for plan-review launcher (user → bundled). |
| plugins/revdiff-planning/scripts/plan-review-hook.py | Uses resolver to locate launcher before invoking revdiff overlay. |
| plugins/revdiff-planning/hooks/hooks.json | Quotes ${CLAUDE_PLUGIN_ROOT} path in the hook command. |
| plugins/revdiff-planning/README.md | New plugin README including override instructions and rationale. |
| plugins/revdiff-planning/.claude-plugin/plugin.json | Version bump for revdiff-planning. |
| plugins/codex/skills/revdiff/SKILL.md | Notes that Claude-style overrides don’t apply to Codex skills. |
| docs/plans/completed/20260419-launcher-override-chain.md | Archived plan documenting final (post-security-review) two-layer design. |
| README.md | Adds “Custom launchers” section + pi runtime clarification. |
| CLAUDE.md | Documents override chain + local testing commands. |
| .gitignore | Ignores Python bytecode artifacts. |
| .claude-plugin/skills/revdiff/scripts/resolve-launcher.sh | New resolver script for diff-review launcher (user → bundled). |
| .claude-plugin/skills/revdiff/references/install.md | Documents override paths + guidance for safe launcher customization. |
| .claude-plugin/skills/revdiff/SKILL.md | Switches launcher invocation to resolver-based command substitution. |
| .claude-plugin/plugin.json | Version bump for revdiff Claude plugin. |
| .claude-plugin/marketplace.json | Updates marketplace versions (and syncs planning drift). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def resolve_launcher(plugin_root: str, name: str) -> Path | None: | ||
| """resolve launcher path through override chain. |
There was a problem hiding this comment.
This finding is incorrect — from __future__ import annotations (PEP 563) does make Path | None work on Python 3.9.
Mechanism: PEP 563 (available since 3.7) stores all annotations as string literals at compile time and never evaluates them. Path | None is syntactically valid Python regardless of version (| is a normal expression operator), and with the future import the runtime semantic of type | type is never invoked. PEP 604 (3.10+) only adds the runtime meaning — the string is just a string until something explicitly calls typing.get_type_hints(), which this hook does not.
Verified empirically on macOS system Python 3.9.6:
$ /usr/bin/python3 --version
Python 3.9.6
$ /usr/bin/python3 -c "
import importlib.util
spec = importlib.util.spec_from_file_location('h', 'plugins/revdiff-planning/scripts/plan-review-hook.py')
m = importlib.util.module_from_spec(spec)
spec.loader.exec_module(m)
print('annotation type:', type(m.resolve_launcher.__annotations__['return']).__name__)
print('annotation value:', repr(m.resolve_launcher.__annotations__['return']))
"
annotation type: str
annotation value: 'Path | None'
Module imports clean, no SyntaxError, annotation lives as the string 'Path | None' exactly per PEP 563.
Keeping the current code.
Adds a two-layer override chain (
user → bundled) for the launcher scripts shipped by both Claude plugins, so users can drop a custom launcher at${CLAUDE_PLUGIN_DATA}/scripts/<launcher>.shwithout forking the plugin.Resolves the action item from discussion #121.
What's new
.claude-plugin/skills/revdiff/scripts/resolve-launcher.sh— walks the override chain, returns the path of the first executable launcherplugins/revdiff-planning/scripts/resolve-launcher.sh— same shape for the plan-review hookplan-review-hook.pyresolves the launcher through the same chain (withcwd=CLAUDE_PROJECT_DIRpinning for hygiene)revdiff0.7.6 → 0.8.0,revdiff-planning0.2.2 → 0.3.0(also resyncs themarketplace.jsondrift onrevdiff-planning)README.md,CLAUDE.md,site/docs.html,install.md, the newplugins/revdiff-planning/README.md, and a Claude-only note inplugins/codex/skills/revdiff/SKILL.mdWhy no project layer
The original plan included a third
.claude/<plugin-namespace>/scripts/project layer matching cc-thingz. Codex external review flagged it as a security risk: therevdiff-planninghook fires automatically on everyExitPlanModein any repo Claude opens, so a repo-controlled executable at that path would auto-execute on routine actions without per-repo opt-in. Project layer was dropped from both resolvers (commit5e58e8c); diff-review skill keeps the same shape for symmetry. cc-thingz's project layer is fine because it resolves prompt files (read by Claude, not executed) — different threat model.Notable hardening from the review loop
from __future__ import annotationsso the hook stays Python 3.9 compatible (macOS system Python). PEP 604Path | Nonewould otherwise crash at module load.[ -f path ] && [ -x path ]in both resolvers so a directory at the override path doesn't win over the bundled launcher.${CLAUDE_PLUGIN_DATA}and${CLAUDE_SKILL_DIR}in the resolver invocation so paths with spaces work.${CLAUDE_PLUGIN_ROOT}inhooks.json(pre-existing bug, fixed per the no-pre-existing-issues policy).__pycache__/and*.pycadded to.gitignore.Backward compatibility
Zero behavior change for users with no overrides — bundled launcher is still the default.
Out of scope (already covered in CLAUDE.md / docs)
~/.codex/skills/and edits launchers directly there~/.config/opencode/, user-ownedCLAUDE_PLUGIN_DATAin Pi runtime; documented asymmetry