Addresses two PR review findings empirically confirmed in a Claude Code
session: `printenv CLAUDE_SKILL_DIR` from the Bash tool returns
`NOT_SET`, proving CLAUDE_SKILL_DIR (and CLAUDE_PLUGIN_ROOT) are
SKILL.md content substitutions only, not environment variables exported
to Bash subprocesses.
ce-update probes (P1, Codex thread):
Previously, currently-loaded-version.sh and marketplace-name.sh read
`${CLAUDE_SKILL_DIR:-}` directly from the environment. Since the env
var is never set in subprocesses, both scripts always emitted
__CE_UPDATE_NOT_MARKETPLACE__ — meaning ce-update would never actually
perform version comparison even on real marketplace installs.
Fix: derive skill_dir from BASH_SOURCE[0] (the script's own location).
Adds regression tests that copy each script into a fake
marketplace-shaped path and run it with CLAUDE_SKILL_DIR explicitly
cleared from the env, asserting the correct version/marketplace
segments are extracted.
ce-setup platform check (P2, Codex thread):
The check at line 47 keyed off "non-empty AND not literal
${CLAUDE_PLUGIN_ROOT}", which incorrectly accepted unresolved command
strings like `echo "${CLAUDE_PLUGIN_ROOT}"` left in place by
non-Claude harnesses that don't process `!` pre-resolution. Tightened
to "starts with `/` and contains no `${`", which naturally rejects all
unresolved forms while accepting real absolute paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit moved probes from `!` pre-resolution to the runtime
Bash tool, which sidestepped Claude Code's load-time permission gate.
But two empirical issues remained:
1. Bare `bash scripts/<name>.sh` failed with "No such file or directory"
because the runtime Bash tool runs from the user's project CWD, not
the skill directory. The AGENTS.md guidance "all platforms resolve
script paths relative to the skill's directory" is aspirational, not
reality for the runtime Bash tool path.
2. Falling back to `bash <abs-path>` triggered a runtime permission
prompt for users without `Bash(bash:*)` allow rules (most users have
`Bash(bash -c:*)` at most).
Fix:
- Use `${CLAUDE_SKILL_DIR}/scripts/<name>.sh` in each runtime command.
Claude Code sets that env var to the active skill's directory at
runtime, so the path resolves correctly in both `--plugin-dir` and
marketplace-cached installs.
- Declare narrow `allowed-tools` patterns pinned to each script
filename. The skills docs explicitly state that `allowed-tools`
grants permission for runtime tool calls "while the skill is active,
so Claude can use them without prompting" — that coverage was murky
for pre-resolution but is documented for runtime.
Tests:
- New regression guard fails if any probe lacks the `${CLAUDE_SKILL_DIR}`
prefix (catching reverts to bare relative paths).
- New regression guard fails if `allowed-tools` is dropped or broadened
to `Bash(bash *)`.
- Existing guard against `!` pre-resolution stays.
AGENTS.md updated to document both pieces (path form + allow-listing)
that the runtime-Bash pattern requires, with a concrete code-block
example.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Empirical testing showed the previous approach was unreliable. With
`defaultMode: bypassPermissions` ON the `allowed-tools` frontmatter
appeared to work, but with bypass OFF (the configuration most users
have) the narrow `Bash(bash *<script>.sh)` patterns failed the
load-time permission check. The official docs are inconsistent on
whether `*` works as an internal-token glob, and `allowed-tools`
coverage of pre-resolution is undocumented. We can't ship a fix that
only works for users who have bypassPermissions on.
The reliable fix is to remove `!` pre-resolution entirely. Probes now
run from the skill body via the runtime Bash tool, which:
- Honors `defaultMode: bypassPermissions` (silent for those users)
- Falls back to a normal one-time approval prompt that Claude Code
remembers (acceptable UX for users without bypass)
Skill body restructured with a "Step 1: Probe versions" section that
instructs the agent to run all three scripts in parallel, then "Step
2: Apply decision logic" that reads the captured outputs.
Tests updated:
- Drop the now-obsolete `allowed-tools` and pre-resolution-section
assertions
- Add a regression guard that fails if `!`bash <path>`` pre-resolution
is reintroduced
- Add a guard that ensures each of the three probe-script invocations
appears in the skill body
- Refactor `runUpstreamCommand` -> `runUpstreamScript` since the test
no longer extracts a pre-resolution command from SKILL.md and just
runs the script directly
AGENTS.md updated: replace the now-incorrect "declare allowed-tools"
guidance with the correct pattern (invoke from skill body via runtime
Bash tool when the first token would be `bash <path>`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous fix used `Bash(bash *)` to grant pre-resolution permission for
the three extracted scripts, but that surface is broader than necessary
— it allows any `bash <anything>` command, not just the three scripts
ce-update actually invokes.
Narrows to per-script patterns: `Bash(bash *upstream-version.sh)` etc.
Per Claude Code's permission docs, `*` works at any position and quotes
are stripped before matching, so each pattern matches both the local
checkout path and the marketplace cache path without granting blanket
Bash access.
Updates the regression test to assert each script-specific pattern is
present and to fail if `Bash(bash *)` is reintroduced. Updates AGENTS.md
guidance to recommend script-pinned patterns with an example.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The prior fix (commit 8605e0f9) extracted ce-update's pre-resolution
logic into `bash "${CLAUDE_SKILL_DIR}/scripts/<name>.sh"` invocations
to clear Claude Code's safety check. That worked, but introduced a
new failure mode: the *permission* check rejects the resulting
`bash "/abs/path/script.sh"` form because pre-resolution `!` commands
do not honor `defaultMode: bypassPermissions`, and `Bash(bash:*)` is
not a rule most users allow-list (they have `Bash(bash -c:*)` at most).
Fix: declare `allowed-tools: Bash(bash *), Bash(echo *)` in ce-update's
frontmatter so the skill carries its own permission grant instead of
depending on user settings.
- Adds regression test in tests/skills/ce-update.test.ts that fails
if allowed-tools is dropped or stops covering Bash.
- Updates AGENTS.md with a "Permission gate on extracted scripts"
subsection so future skills using the same script-extraction pattern
declare allowed-tools upfront.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two safety-check rejections in `!` backtick pre-resolutions were
breaking skill-load in Claude Code:
- `[A] && B || C` shape ("ambiguous syntax with command separators",
issue #710): ce-setup, ce-update, ce-work-beta/codex-delegation-workflow.
- `$()` containing a double-quoted string ("Unhandled node type:
string", issue #709): ce-compound, ce-sessions, ce-update, ce-work-beta.
Replaces each with a safe shape: raw env-var emit, pure-pipe sed,
`${var%suffix}` parameter expansion, or an extracted script under the
skill's `scripts/`. ce-update gets three scripts (upstream-version,
currently-loaded-version, marketplace-name) since it's Claude-only
and has the most complex pre-resolution logic.
Adds regression tests in tests/skill-shell-safety.test.ts that flag
both antipatterns at PR time, and updates AGENTS.md to enumerate the
rejected shapes alongside the existing `case`/`esac` rule.
Closes#709, #710.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Trevin Chow