Skip to content

Fix subagent skill-path resolution; sync marketplace.json plugin version (closes #6)#7

Open
deinspanjer wants to merge 3 commits into
jcputney:mainfrom
deinspanjer:fix/skill-path-resolution
Open

Fix subagent skill-path resolution; sync marketplace.json plugin version (closes #6)#7
deinspanjer wants to merge 3 commits into
jcputney:mainfrom
deinspanjer:fix/skill-path-resolution

Conversation

@deinspanjer
Copy link
Copy Markdown

Summary

Two unrelated bugs in main:

  1. Subagent skill loading is broken. agents/codex-peer-reviewer.md instructs the subagent to Read skills/codex-peer-review/SKILL.md as a relative path, but the subagent's CWD is the user's project (or worktree), not the plugin install directory. The path almost never resolves. Symptoms range from a hard "file not found" failure to silent improvisation (subagent fakes the protocol from dispatcher hints), making behavior inconsistent and unsafe.
  2. marketplace.json plugin version is stale (marketplace.json plugin entry not bumped to 2.0.0 — clients can't auto-update from 1.x #6). The 2.0.0 release bumped plugin.json but left marketplace.json:plugins[0].version at 1.0.11. Clients use the marketplace number for update detection, so 1.x installs can't auto-update to 2.x and the catalog reports the wrong version.

This PR fixes both. Closes #6.

Why the relative path doesn't work

  • ${CLAUDE_PLUGIN_ROOT} substitution does not happen in markdown bodies (only in hooks/hooks.json command fields), so the agent file can't reference its own install path via that variable.
  • Subagents are not granted the Skill tool by tools: frontmatter — there's no precedent for it in any official or community plugin.
  • The skills: frontmatter field on the agent appeared aspirational; I couldn't find any plugin where the harness honors it as an auto-loader.

So the subagent has to discover the path itself.

Changes

  • plugins/codex-peer-review/agents/codex-peer-reviewer.md:
    • Rewrite "Your job" step 1: autodiscover $SKILL_ROOT via a single bash call with ||-chained ls globs — ~/.claude/plugins/cache/*/codex-peer-review/*/skills/... falling back to ~/.claude/plugins/marketplaces/*/plugins/codex-peer-review/skills/.... Single chained call (not two separate code blocks) so the subagent doesn't emit the globs as parallel tool calls and short-circuit the fallback.
    • Accept an explicit SKILL_ROOT=<absolute path> envelope as an escape hatch for non-standard installs (e.g., local-directory marketplace sources).
    • Replace relative-path references in the "Reference" section with $SKILL_ROOT/....
    • Remove the non-functional skills: frontmatter field.
  • plugins/codex-peer-review/.claude-plugin/plugin.json: bump 2.0.02.0.1.
  • .claude-plugin/marketplace.json:
    • Bump top-level version 1.0.121.0.13 (new marketplace release).
    • Bump plugins[0].version 1.0.112.0.1 (sync with plugin.json).

Three commits, kept separate for clarity — squash if you prefer.

Testing

  • Tested /codex-peer-review command locally with the patched plugin installed via github source pointing at the fork (deinspanjer/agent-peer-review).
  • SKILL_ROOT escape-hatch branch: dispatched the agent with SKILL_ROOT=<absolute path> in the prompt; subagent parsed it, cat'd all four protocol files (byte sizes 16974/8032/5926/8238 — matching disk), codex round-tripped (/tmp/codex_round{0,1}.{txt,jsonl} regenerated fresh), structured verdict returned.
  • Autodiscovery branch: dispatched without SKILL_ROOT= envelope; subagent reported running the ||-chained ls globs as a single Bash call, marketplaces/ glob matched on the fallback, all four protocol files loaded successfully, codex round-tripped, structured verdict returned.
  • Marketplace metadata: confirmed marketplace.json:plugins[0].version reports 2.0.1 after refetch from the fork — no version skew vs plugin.json.

Environment used to validate

  • macOS 26.4 (Darwin 25.4.0)
  • Claude Code 2.1.119
  • codex-cli 0.118.0
  • node v25.9.0

Checklist

  • Prompts remain language-agnostic
  • No breaking changes — autodiscovery is purely additive; explicit SKILL_ROOT envelope is opt-in
  • README — no update needed (skill loading is an implementation detail, not a user-facing contract)

Notes for the maintainer

  • The third commit (||-chain) exists because an earlier draft of this fix presented the two ls globs as separate code blocks ("run cache glob, then run marketplaces glob"). Live testing surfaced that the subagent emitted those as parallel tool calls and the harness cancelled the second when the first errored — short-circuiting the fallback. Collapsing into a single ||-chained bash call eliminates the parallelism question entirely. Worth keeping the lesson visible in the commit history, IMO, but feel free to squash.
  • Considered granting Skill to the subagent as a cleaner load mechanism. Rejected: no plugin precedent, undocumented for subagents, and the system-reminder skill registry is not documented to propagate to subagent contexts.
  • The removed skills: frontmatter was non-functional in my testing. If it's actively used by some Claude Code path I missed, restore at your discretion.

🤖 Generated with Claude Code

The agent file instructed the subagent to Read `skills/codex-peer-review/SKILL.md`
as a relative path. The subagent's CWD is the user's project (or worktree), not the
plugin install dir, so the path almost never resolves. Symptoms ranged from a hard
"file not found" failure to silent improvisation (subagent fakes a protocol from
dispatcher hints), making behavior inconsistent and unsafe.

Fix: rewrite the agent's "load the skill" step to autodiscover the install location
via `ls` glob against the standard plugin paths
(`~/.claude/plugins/cache/*/codex-peer-review/*/skills/...` and
`~/.claude/plugins/marketplaces/*/plugins/codex-peer-review/skills/...`), then
`cat` the four protocol files from the resolved `$SKILL_ROOT`. Also accept an
explicit `SKILL_ROOT=<absolute path>` envelope in the dispatch input as an escape
hatch for non-standard installs.

Also remove the `skills:` frontmatter field — there is no precedent in any plugin
or built-in agent for it being honored, and leaving it in the file falsely
suggested the harness was loading the skill automatically.

Tested against codex-cli 0.118.0 + Claude Code 2.1.119, plugin installed via
local-directory marketplace.

Bump plugin version 2.0.0 -> 2.0.1.
…putney#6)

The 2.0.0 release bumped plugin.json version but marketplace.json's plugin
entry stayed at 1.0.11. Clients use marketplace.json for update detection,
so 1.x users couldn't auto-update to 2.x and downloads served the wrong
version metadata.

Bump marketplace.json:
  - top-level version  1.0.12 -> 1.0.13 (new marketplace release)
  - plugins[0].version 1.0.11 -> 2.0.1  (matches plugin.json)

Closes upstream issue jcputney#6.
…spatch

The previous patch presented two separate `ls` glob commands as code blocks.
The subagent (Claude) interpreted them as independent tool calls and emitted
them in parallel — so when the first failed (e.g. cache/ empty pre-fetch), the
second was cancelled by the harness rather than running as a fallback.

Force sequential evaluation by collapsing the chain into a single bash call
with `||`. Stdout becomes the resolved SKILL.md path; nonzero exit means
neither location has the plugin.

Verified by dispatching with no SKILL_ROOT envelope: pre-fix dispatch errored
on the cache/ no-match and never tried marketplaces/. Post-fix the marketplaces/
fallback resolves correctly when cache/ is empty.
@deinspanjer
Copy link
Copy Markdown
Author

I've (the human) reviewed and tested this code and feel it is a good update for the upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

marketplace.json plugin entry not bumped to 2.0.0 — clients can't auto-update from 1.x

1 participant