mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix: sourced helpers locate themselves under zsh (#3024)
Skill blocks source bin/gstack-codex-probe and bin/gstack-egress-lib.sh into
the host's shell. Under zsh (macOS default) BASH_SOURCE is empty, so Codex
model selection resolved /../scripts/... and every outside pass reported
CODEX_MODEL: invalid; the egress lib resolved to the caller's cwd. Resolve
the file once at source time (BASH_SOURCE under bash, %x under zsh), and
replace the bash-only ${!name} indirection in the auth probe. A free test
derives every sourced helper from the generated skills and sources each
under bash and zsh.
Co-authored-by: marcosdetry <marcosdetry@users.noreply.github.com>
Co-authored-by: akshaykalia <akshaykalia@users.noreply.github.com>
Co-authored-by: scottgigante-hubflow <scottgigante-hubflow@users.noreply.github.com>
Co-authored-by: woodsonl <woodsonl@users.noreply.github.com>
Co-authored-by: bezdekt <bezdekt@users.noreply.github.com>
This commit is contained in:
1 parent
5885157a90
commit
f0531a229f
3 files changed
+182
-10
No files matched your search
+30
-7
@@ -17,7 +17,27 @@
|
||||
# - Never change -e / -u / traps / IFS / PATH in the caller shell.
|
||||
# - All internal vars prefix with _GSTACK_CODEX_.
|
||||
# - All functions prefix with _gstack_codex_.
|
||||
# - No command execution at source time (only function defs).
|
||||
# - No command execution at source time (only function defs and the one
|
||||
# self-location assignment below, which is parameter expansion only).
|
||||
|
||||
# --- Self-location (bash and zsh) -------------------------------------------
|
||||
# Template blocks source this file into whatever shell the host runs; on macOS
|
||||
# that is zsh (Claude Code's Bash tool included). BASH_SOURCE is bash-only, so
|
||||
# under zsh every "${BASH_SOURCE[0]%/*}/..." path became "/..." and Codex
|
||||
# model selection failed (#3024). zsh names the sourced file with %x; the eval
|
||||
# keeps bash from parsing zsh-only syntax. Resolved once, at source time,
|
||||
# because inside a function zsh's %x names the function, not the file.
|
||||
_GSTACK_CODEX_BIN=""
|
||||
if [ -n "${BASH_VERSION:-}" ]; then
|
||||
_GSTACK_CODEX_BIN="${BASH_SOURCE[0]}"
|
||||
elif [ -n "${ZSH_VERSION:-}" ]; then
|
||||
eval '_GSTACK_CODEX_BIN="${(%):-%x}"'
|
||||
fi
|
||||
case "$_GSTACK_CODEX_BIN" in
|
||||
*/*) _GSTACK_CODEX_BIN="${_GSTACK_CODEX_BIN%/*}" ;;
|
||||
?*) _GSTACK_CODEX_BIN="$PWD" ;;
|
||||
*) _GSTACK_CODEX_BIN="$HOME/.claude/skills/gstack/bin" ;;
|
||||
esac
|
||||
|
||||
# --- Auth probe -------------------------------------------------------------
|
||||
|
||||
@@ -43,7 +63,10 @@ _gstack_codex_auth_probe() {
|
||||
if [ -f "$_codex_home/config.toml" ]; then
|
||||
while IFS= read -r _env_key; do
|
||||
case "$_env_key" in ''|[0-9]*|*[!A-Za-z0-9_]*) continue ;; esac
|
||||
_resolved=$(printf '%s' "${!_env_key:-}" | tr -d '[:space:]')
|
||||
# Indirect read through eval, not ${!name}: zsh rejects bash's
|
||||
# indirection, and _env_key is a validated identifier by this point.
|
||||
eval "_resolved=\${$_env_key:-}"
|
||||
_resolved=$(printf '%s' "$_resolved" | tr -d '[:space:]')
|
||||
if [ -n "$_resolved" ]; then
|
||||
echo "AUTH_OK"
|
||||
return 0
|
||||
@@ -72,9 +95,9 @@ _gstack_codex_select_model() {
|
||||
return 1
|
||||
fi
|
||||
if [ -n "$_explicit" ]; then
|
||||
_line=$(bun run "${BASH_SOURCE[0]%/*}/../scripts/resolve-codex-generation-model.ts" --runtime "$_kind" --explicit "$_explicit")
|
||||
_line=$(bun run "$_GSTACK_CODEX_BIN/../scripts/resolve-codex-generation-model.ts" --runtime "$_kind" --explicit "$_explicit")
|
||||
else
|
||||
_line=$(bun run "${BASH_SOURCE[0]%/*}/../scripts/resolve-codex-generation-model.ts" --runtime "$_kind")
|
||||
_line=$(bun run "$_GSTACK_CODEX_BIN/../scripts/resolve-codex-generation-model.ts" --runtime "$_kind")
|
||||
fi
|
||||
_rc=$?
|
||||
_GSTACK_CODEX_SEL="${_line%%$'\t'*}"
|
||||
@@ -135,7 +158,7 @@ _gstack_codex_first_use_notice() {
|
||||
# and how to turn reviews off. Shown once per state root; never blocks and
|
||||
# never prints a credential, only where it comes from.
|
||||
local _root _marker _codex_home="${CODEX_HOME:-$HOME/.codex}" _account _provider
|
||||
_root="$(. "${BASH_SOURCE[0]%/*}/gstack-state-root.sh" 2>/dev/null && gstack_state_root)" || return 0
|
||||
_root="$(. "$_GSTACK_CODEX_BIN/gstack-state-root.sh" 2>/dev/null && gstack_state_root)" || return 0
|
||||
[ -n "$_root" ] || return 0
|
||||
_marker="$_root/.codex-review-notice-shown"
|
||||
[ -e "$_marker" ] && return 0
|
||||
@@ -194,7 +217,7 @@ _gstack_codex_model_probe() {
|
||||
# State root from the shared twin (sourced in a subshell, so the caller's
|
||||
# shell is untouched). A missing twin only disables the cache.
|
||||
local _gstack_home _cache=""
|
||||
_gstack_home="$(. "${BASH_SOURCE[0]%/*}/gstack-state-root.sh" 2>/dev/null && gstack_state_root)" || _gstack_home=""
|
||||
_gstack_home="$(. "$_GSTACK_CODEX_BIN/gstack-state-root.sh" 2>/dev/null && gstack_state_root)" || _gstack_home=""
|
||||
[ -n "$_gstack_home" ] && _cache="$_gstack_home/.codex-model-probe"
|
||||
# Cache signature (#2787): everything that can change the round trip's
|
||||
# outcome without the hour passing — the model, the Codex home and its
|
||||
@@ -401,7 +424,7 @@ _gstack_codex_log_event() {
|
||||
local _duration="${2:-0}"
|
||||
[ "${_TEL:-off}" = "off" ] && return 0
|
||||
local _root
|
||||
_root="$(. "${BASH_SOURCE[0]%/*}/gstack-state-root.sh" 2>/dev/null && gstack_state_root)" || return 0
|
||||
_root="$(. "$_GSTACK_CODEX_BIN/gstack-state-root.sh" 2>/dev/null && gstack_state_root)" || return 0
|
||||
[ -n "$_root" ] || return 0
|
||||
mkdir -p "$_root/analytics" 2>/dev/null || return 0
|
||||
local _ts
|
||||
|
||||
@@ -37,11 +37,21 @@
|
||||
# the caller's. All temp handling is immediate, per call.
|
||||
|
||||
# Self-locate without dirname (builtins only), so the lib works even under
|
||||
# a stripped test PATH.
|
||||
case "${BASH_SOURCE[0]}" in
|
||||
*/*) _gstack_egress_lib_dir="$(cd "${BASH_SOURCE[0]%/*}" && pwd)" ;;
|
||||
# a stripped test PATH. BASH_SOURCE is bash-only; under zsh (the macOS
|
||||
# default, sourced from skill blocks) %x names this file, read through eval
|
||||
# so bash never parses it. Without it the lib resolved to the caller's cwd
|
||||
# and looked for its siblings in the user's project (#3024).
|
||||
_gstack_egress_self=""
|
||||
if [ -n "${BASH_VERSION:-}" ]; then
|
||||
_gstack_egress_self="${BASH_SOURCE[0]}"
|
||||
elif [ -n "${ZSH_VERSION:-}" ]; then
|
||||
eval '_gstack_egress_self="${(%):-%x}"'
|
||||
fi
|
||||
case "$_gstack_egress_self" in
|
||||
*/*) _gstack_egress_lib_dir="$(cd "${_gstack_egress_self%/*}" && pwd)" ;;
|
||||
*) _gstack_egress_lib_dir="$(pwd)" ;;
|
||||
esac
|
||||
unset _gstack_egress_self
|
||||
|
||||
# State root from the shared twin (bin/gstack-state-root.sh, builtins only),
|
||||
# sourced lazily on first use.
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
/**
|
||||
* #3024: skill blocks `source` gstack helpers into whatever shell the host
|
||||
* runs. On macOS that is zsh (Claude Code's Bash tool included), where
|
||||
* BASH_SOURCE is empty and bash-only expansions such as ${!name} are errors.
|
||||
* Since v1.91.16.0 every Codex outside pass from zsh reported
|
||||
* `CODEX_MODEL: invalid` because the probe resolved its resolver at
|
||||
* "/../scripts/...".
|
||||
*
|
||||
* Class guard: the helper list is derived from the generated skills and
|
||||
* resolvers (not a hand list), and each helper is sourced under bash and zsh
|
||||
* from an unrelated cwd. CI installs zsh (free-tests.yml), so a missing zsh
|
||||
* there fails instead of skipping.
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { spawnSync } from 'child_process';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const BIN = path.join(ROOT, 'bin');
|
||||
const HAS_ZSH = Boolean(Bun.which('zsh'));
|
||||
const ZSH_REQUIRED = Boolean(process.env.CI) && process.platform !== 'win32';
|
||||
|
||||
function walk(dir: string, keep: (file: string) => boolean, out: string[] = []): string[] {
|
||||
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
||||
if (entry.name === 'node_modules' || entry.name.startsWith('.')) continue;
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) walk(full, keep, out);
|
||||
else if (keep(full)) out.push(full);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Every bin/ helper a generated skill or resolver loads with `source` or `.`. */
|
||||
function sourcedHelpers(): string[] {
|
||||
const files = [
|
||||
...walk(path.join(ROOT, 'scripts', 'resolvers'), f => f.endsWith('.ts')),
|
||||
...fs.readdirSync(ROOT, { withFileTypes: true })
|
||||
.filter(e => e.isDirectory() && !e.name.startsWith('.') && e.name !== 'node_modules' && e.name !== 'test')
|
||||
.flatMap(e => walk(path.join(ROOT, e.name), f => /(?:SKILL\.md|\.md\.tmpl)$/.test(f))),
|
||||
];
|
||||
const names = new Set<string>();
|
||||
const direct = /(?:^|[\s;&|({]|then\s)(?:source|\.)\s+"?[^"\s;)<]*\bbin\/([A-Za-z0-9._-]+)/g;
|
||||
const viaVar = /\/(gstack-[A-Za-z0-9._-]+\.sh)"; \[ -r "\$[A-Z_]+" \] && \. "\$[A-Z_]+"/g;
|
||||
for (const file of files) {
|
||||
const text = fs.readFileSync(file, 'utf8');
|
||||
for (const re of [direct, viaVar]) {
|
||||
for (const m of text.matchAll(re)) {
|
||||
if (fs.existsSync(path.join(BIN, m[1]!))) names.add(m[1]!);
|
||||
}
|
||||
}
|
||||
}
|
||||
return [...names].sort();
|
||||
}
|
||||
|
||||
function run(shell: 'bash' | 'zsh', script: string, env: Record<string, string> = {}) {
|
||||
const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'zsh-helpers-cwd-'));
|
||||
const args = shell === 'zsh' ? ['-f', '-c', script] : ['--noprofile', '--norc', '-c', script];
|
||||
const r = spawnSync(shell, args, {
|
||||
cwd,
|
||||
env: { PATH: process.env.PATH ?? '', HOME: cwd, ...env },
|
||||
encoding: 'utf8',
|
||||
timeout: 20_000,
|
||||
});
|
||||
fs.rmSync(cwd, { recursive: true, force: true });
|
||||
return { code: r.status, out: `${r.stdout ?? ''}${r.stderr ?? ''}` };
|
||||
}
|
||||
|
||||
const SHELLS = ['bash', 'zsh'] as const;
|
||||
const skipShell = (shell: string) => shell === 'zsh' && !HAS_ZSH && !ZSH_REQUIRED;
|
||||
|
||||
describe('#3024: sourced helpers work under bash and zsh', () => {
|
||||
const helpers = sourcedHelpers();
|
||||
|
||||
test('zsh is present wherever CI runs this suite', () => {
|
||||
if (ZSH_REQUIRED) expect(HAS_ZSH).toBe(true);
|
||||
});
|
||||
|
||||
test('the derived helper list covers the known sourced helpers', () => {
|
||||
for (const known of ['gstack-codex-probe', 'gstack-egress-lib.sh', 'gstack-gbrain-lib.sh']) {
|
||||
expect(helpers).toContain(known);
|
||||
}
|
||||
});
|
||||
|
||||
test('no sourced helper uses bash-only self-location or indirection without a zsh branch', () => {
|
||||
for (const name of helpers) {
|
||||
const text = fs.readFileSync(path.join(BIN, name), 'utf8')
|
||||
.split('\n').filter(line => !/^\s*#/.test(line)).join('\n');
|
||||
if (text.includes('BASH_SOURCE')) {
|
||||
expect({ name, zshBranch: text.includes('ZSH_VERSION') }).toEqual({ name, zshBranch: true });
|
||||
expect({ name, rawUse: /\$\{BASH_SOURCE\[0\]%/.test(text) }).toEqual({ name, rawUse: false });
|
||||
}
|
||||
expect({ name, indirect: /\$\{![A-Za-z_]/.test(text) }).toEqual({ name, indirect: false });
|
||||
}
|
||||
});
|
||||
|
||||
for (const shell of SHELLS) {
|
||||
test.skipIf(skipShell(shell))(`every sourced helper parses and sources cleanly (${shell})`, () => {
|
||||
for (const name of helpers) {
|
||||
const file = path.join(BIN, name);
|
||||
const r = run(shell, `${shell} -n '${file}' && . '${file}' && echo SOURCED`);
|
||||
expect({ name, out: r.out.trim().split('\n').pop() }).toEqual({ name, out: 'SOURCED' });
|
||||
}
|
||||
});
|
||||
|
||||
test.skipIf(skipShell(shell))(`codex probe resolves its own directory and selects the model (${shell})`, () => {
|
||||
const codexHome = fs.mkdtempSync(path.join(os.tmpdir(), 'zsh-helpers-codex-'));
|
||||
fs.writeFileSync(path.join(codexHome, 'config.toml'), 'model = "gpt-test-model"\n');
|
||||
const r = run(
|
||||
shell,
|
||||
`. '${path.join(BIN, 'gstack-codex-probe')}' && echo "BIN=$_GSTACK_CODEX_BIN" && _gstack_codex_select_model exec`,
|
||||
{ CODEX_HOME: codexHome },
|
||||
);
|
||||
fs.rmSync(codexHome, { recursive: true, force: true });
|
||||
expect(r.out).toContain(`BIN=${BIN}\n`);
|
||||
expect(r.out).toContain('CODEX_MODEL: gpt-test-model (exec;');
|
||||
expect(r.out).not.toContain('Module not found');
|
||||
expect(r.code).toBe(0);
|
||||
});
|
||||
|
||||
test.skipIf(skipShell(shell))(`codex auth probe reads a custom provider's env_key (${shell})`, () => {
|
||||
const codexHome = fs.mkdtempSync(path.join(os.tmpdir(), 'zsh-helpers-auth-'));
|
||||
fs.writeFileSync(path.join(codexHome, 'config.toml'), '[model_providers.local]\nenv_key = "LOCAL_PROVIDER_KEY"\n');
|
||||
const r = run(shell, `. '${path.join(BIN, 'gstack-codex-probe')}' && _gstack_codex_auth_probe`, {
|
||||
CODEX_HOME: codexHome,
|
||||
LOCAL_PROVIDER_KEY: 'present',
|
||||
});
|
||||
fs.rmSync(codexHome, { recursive: true, force: true });
|
||||
expect(r.out.trim()).toBe('AUTH_OK');
|
||||
expect(r.code).toBe(0);
|
||||
});
|
||||
|
||||
test.skipIf(skipShell(shell))(`egress lib resolves its own directory, not the caller's cwd (${shell})`, () => {
|
||||
const r = run(shell, `. '${path.join(BIN, 'gstack-egress-lib.sh')}' && echo "DIR=$_gstack_egress_lib_dir"`);
|
||||
expect(r.out).toContain(`DIR=${BIN}\n`);
|
||||
});
|
||||
}
|
||||
});
|
||||
Reference in new issue
Block a user