mirror of https://github.com/garrytan/gstack.git
fix(gstack-paths): add --get <key> so /codex skips eval-tilde footgun
- Pattern 2 in #1329: `eval "$(~/.claude/skills/gstack/bin/gstack-paths)"` trips Claude Code's bash AST parser ("Unhandled node type: string") because of tilde inside $() inside double-quoted eval arg. Forces manual approval on every /codex. - `gstack-paths --get state-root|plan-root|tmp-root` prints one path, no `KEY=` prefix. Callers use `PLAN_ROOT=$(gstack-paths --get plan-root)`. - Bare invocation unchanged (KEY=VALUE form), so unmigrated skills keep working. - `codex/SKILL.md.tmpl` Step 0.6 switched + regenerated SKILL.md. - Patterns 1/3/4 + other skills using the eval form out of scope; one-at-a-time migration. Refs #1329
This commit is contained in:
parent
386fe518f9
commit
b30d7e3653
|
|
@ -1,7 +1,15 @@
|
|||
#!/usr/bin/env bash
|
||||
# gstack-paths — output portable state-root paths for skill bash blocks
|
||||
# Usage: eval "$(gstack-paths)" → sets GSTACK_STATE_ROOT, PLAN_ROOT, TMP_ROOT
|
||||
# Or: gstack-paths → prints GSTACK_STATE_ROOT=... etc.
|
||||
# Usage: eval "$(gstack-paths)" → sets GSTACK_STATE_ROOT, PLAN_ROOT, TMP_ROOT
|
||||
# gstack-paths → prints GSTACK_STATE_ROOT=... etc.
|
||||
# gstack-paths --get <key> → prints a single resolved path (no KEY= prefix)
|
||||
# where <key> is state-root | plan-root | tmp-root
|
||||
#
|
||||
# The --get form was added so callers can use `PLAN_ROOT=$(gstack-paths --get plan-root)`
|
||||
# instead of `eval "$(gstack-paths)"`. The eval form trips Claude Code's PreToolUse
|
||||
# bash AST parser (tilde inside $() inside double-quoted eval arg = "Unhandled node
|
||||
# type: string"), forcing manual approval on every skill that needs portable roots.
|
||||
# See issue #1329 Pattern 2.
|
||||
#
|
||||
# Resolves three roots with explicit fallback chains so skills work the same
|
||||
# whether installed as a Claude Code plugin (CLAUDE_PLUGIN_DATA / CLAUDE_PLANS_DIR
|
||||
|
|
@ -18,6 +26,27 @@
|
|||
# expansions ("$GSTACK_STATE_ROOT", not $GSTACK_STATE_ROOT).
|
||||
set -u
|
||||
|
||||
# Parse optional --get <key> selector.
|
||||
_get_key=""
|
||||
case "${1:-}" in
|
||||
--help|-h)
|
||||
sed -n '2,18p' "$0" | sed 's/^# \{0,1\}//'
|
||||
exit 0
|
||||
;;
|
||||
--get)
|
||||
if [ -z "${2:-}" ]; then
|
||||
echo "gstack-paths: --get requires a key (state-root|plan-root|tmp-root)" >&2
|
||||
exit 1
|
||||
fi
|
||||
_get_key="$2"
|
||||
;;
|
||||
"" ) ;; # bare invocation — backward-compatible default output
|
||||
*)
|
||||
echo "gstack-paths: unknown argument '$1' (expected --get <key>)" >&2
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
|
||||
# State root: where gstack writes projects/, sessions/, analytics/.
|
||||
if [ -n "${GSTACK_HOME:-}" ]; then
|
||||
_state_root="$GSTACK_HOME"
|
||||
|
|
@ -56,6 +85,18 @@ fi
|
|||
# will discover that on their own write attempt. Don't fail the eval here.
|
||||
mkdir -p "$_tmp_root" 2>/dev/null || true
|
||||
|
||||
echo "GSTACK_STATE_ROOT=$_state_root"
|
||||
echo "PLAN_ROOT=$_plan_root"
|
||||
echo "TMP_ROOT=$_tmp_root"
|
||||
if [ -n "$_get_key" ]; then
|
||||
case "$_get_key" in
|
||||
state-root) echo "$_state_root" ;;
|
||||
plan-root) echo "$_plan_root" ;;
|
||||
tmp-root) echo "$_tmp_root" ;;
|
||||
*)
|
||||
echo "gstack-paths: unknown key '$_get_key' (expected state-root|plan-root|tmp-root)" >&2
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
else
|
||||
echo "GSTACK_STATE_ROOT=$_state_root"
|
||||
echo "PLAN_ROOT=$_plan_root"
|
||||
echo "TMP_ROOT=$_tmp_root"
|
||||
fi
|
||||
|
|
|
|||
|
|
@ -872,8 +872,13 @@ This keeps the skill working whether installed as a Claude Code plugin
|
|||
(`CLAUDE_PLANS_DIR` set), a global `~/.claude/skills/gstack/` install, or a CI
|
||||
container where `HOME` may be unset and `/tmp` may be read-only.
|
||||
|
||||
`--get <key>` is preferred over `eval "$(...)"` here: the eval form trips Claude
|
||||
Code's bash AST parser (tilde inside `$()` inside double-quoted eval arg) and
|
||||
forces a manual confirmation on every run. Issue #1329 Pattern 2.
|
||||
|
||||
```bash
|
||||
eval "$(~/.claude/skills/gstack/bin/gstack-paths)"
|
||||
PLAN_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get plan-root)
|
||||
TMP_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get tmp-root)
|
||||
```
|
||||
|
||||
After this, every subsequent bash block in this skill uses `"$PLAN_ROOT"` and
|
||||
|
|
|
|||
|
|
@ -98,8 +98,13 @@ This keeps the skill working whether installed as a Claude Code plugin
|
|||
(`CLAUDE_PLANS_DIR` set), a global `~/.claude/skills/gstack/` install, or a CI
|
||||
container where `HOME` may be unset and `/tmp` may be read-only.
|
||||
|
||||
`--get <key>` is preferred over `eval "$(...)"` here: the eval form trips Claude
|
||||
Code's bash AST parser (tilde inside `$()` inside double-quoted eval arg) and
|
||||
forces a manual confirmation on every run. Issue #1329 Pattern 2.
|
||||
|
||||
```bash
|
||||
eval "$(~/.claude/skills/gstack/bin/gstack-paths)"
|
||||
PLAN_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get plan-root)
|
||||
TMP_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get tmp-root)
|
||||
```
|
||||
|
||||
After this, every subsequent bash block in this skill uses `"$PLAN_ROOT"` and
|
||||
|
|
|
|||
|
|
@ -98,4 +98,61 @@ describe('gstack-paths', () => {
|
|||
expect(line).toMatch(/^[A-Z_]+=.*/);
|
||||
}
|
||||
});
|
||||
|
||||
// --- --get <key> CLI form (issue #1329 Pattern 2) ---
|
||||
|
||||
function runGet(env: Record<string, string | undefined>, args: string[]): { stdout: string; stderr: string; status: number } {
|
||||
const r = spawnSync('bash', [BIN, ...args], {
|
||||
env: { PATH: process.env.PATH, USERPROFILE: '', ...env } as Record<string, string>,
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
return { stdout: r.stdout || '', stderr: r.stderr || '', status: r.status ?? -1 };
|
||||
}
|
||||
|
||||
describe('--get <key>', () => {
|
||||
test('--get state-root prints just the resolved state root (no KEY= prefix)', () => {
|
||||
const r = runGet({ HOME: '/tmp/myhome' }, ['--get', 'state-root']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout.trim()).toBe('/tmp/myhome/.gstack');
|
||||
expect(r.stdout).not.toContain('=');
|
||||
});
|
||||
|
||||
test('--get plan-root respects the GSTACK_PLAN_DIR override', () => {
|
||||
const r = runGet({ GSTACK_PLAN_DIR: '/tmp/explicit', HOME: '/h' }, ['--get', 'plan-root']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout.trim()).toBe('/tmp/explicit');
|
||||
});
|
||||
|
||||
test('--get tmp-root honors TMPDIR', () => {
|
||||
const r = runGet({ TMPDIR: '/tmp/x', HOME: '/h' }, ['--get', 'tmp-root']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout.trim()).toBe('/tmp/x');
|
||||
});
|
||||
|
||||
test('--get with unknown key exits 1 and explains', () => {
|
||||
const r = runGet({ HOME: '/h' }, ['--get', 'bogus']);
|
||||
expect(r.status).toBe(1);
|
||||
expect(r.stderr).toContain("unknown key 'bogus'");
|
||||
});
|
||||
|
||||
test('--get without a key exits 1', () => {
|
||||
const r = runGet({ HOME: '/h' }, ['--get']);
|
||||
expect(r.status).toBe(1);
|
||||
expect(r.stderr).toContain('--get requires a key');
|
||||
});
|
||||
|
||||
test('unknown top-level flag exits 1', () => {
|
||||
const r = runGet({ HOME: '/h' }, ['--bogus']);
|
||||
expect(r.status).toBe(1);
|
||||
expect(r.stderr).toContain('unknown argument');
|
||||
});
|
||||
|
||||
test('bare invocation stays backward-compatible (KEY=VALUE form)', () => {
|
||||
const r = runGet({ HOME: '/tmp/h' }, []);
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toContain('GSTACK_STATE_ROOT=');
|
||||
expect(r.stdout).toContain('PLAN_ROOT=');
|
||||
expect(r.stdout).toContain('TMP_ROOT=');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue