mirror of https://github.com/garrytan/gstack.git
fix(codex): BSD mktemp templates broke /codex on every macOS install (#2091)
macOS mktemp requires the X's to end the template; the five "codex-*-XXXXXX.txt" sites failed with "mkstemp failed ... File exists" before Codex ever ran (reproduced live on this machine). Same class fixed in claude/SKILL.md.tmpl's three sites. bin/gstack-paths now strips macOS's trailing slash from TMPDIR so TMP_ROOT-built paths stop carrying "//". Static tripwire scans every tracked .tmpl for characters after the X-run in a mktemp template (longer X-runs stay valid), plus a live portability check of the emitted shape. Co-authored-by: Sina Matian <sina@time-attack.dev> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
6e8a1ae87f
commit
d2e6925638
|
|
@ -55,6 +55,12 @@ elif [ -n "${TMP:-}" ]; then
|
|||
else
|
||||
_tmp_root=".gstack/tmp"
|
||||
fi
|
||||
# macOS exports TMPDIR with a trailing slash; mktemp templates built as
|
||||
# "$TMP_ROOT/name-XXXXXX" would then carry "//", and any consumer comparing
|
||||
# paths gets a spurious mismatch. Strip it (never strips a bare "/"). #2091
|
||||
case "$_tmp_root" in
|
||||
*/) [ "$_tmp_root" != "/" ] && _tmp_root="${_tmp_root%/}" ;;
|
||||
esac
|
||||
|
||||
# Best-effort mkdir; if it fails (read-only fs, permission denied), the caller
|
||||
# will discover that on their own write attempt. Don't fail the eval here.
|
||||
|
|
|
|||
|
|
@ -95,8 +95,8 @@ Create temp files:
|
|||
|
||||
```bash
|
||||
PROMPT_FILE=$(mktemp /tmp/gstack-claude-prompt-XXXXXX)
|
||||
RESP_FILE=$(mktemp /tmp/gstack-claude-response-XXXXXX.json)
|
||||
ERR_FILE=$(mktemp /tmp/gstack-claude-error-XXXXXX.txt)
|
||||
RESP_FILE=$(mktemp /tmp/gstack-claude-response-XXXXXX)
|
||||
ERR_FILE=$(mktemp /tmp/gstack-claude-error-XXXXXX)
|
||||
```
|
||||
|
||||
Cleanup at the end of every mode:
|
||||
|
|
@ -151,7 +151,7 @@ Review the current branch diff with nested Claude in tool-less mode.
|
|||
```bash
|
||||
_REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; }
|
||||
cd "$_REPO_ROOT"
|
||||
DIFF_FILE=$(mktemp /tmp/gstack-claude-diff-XXXXXX.patch)
|
||||
DIFF_FILE=$(mktemp /tmp/gstack-claude-diff-XXXXXX)
|
||||
git fetch origin <base> --quiet 2>/dev/null || true
|
||||
git diff "origin/<base>" > "$DIFF_FILE" 2>/dev/null || git diff "<base>" > "$DIFF_FILE"
|
||||
```
|
||||
|
|
|
|||
|
|
@ -158,7 +158,7 @@ Run Codex code review against the current branch diff.
|
|||
|
||||
1. Create temp files for output capture:
|
||||
```bash
|
||||
TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX.txt")
|
||||
TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX")
|
||||
```
|
||||
|
||||
2. Run the review (5-minute timeout). **Codex CLI ≥ 0.130.0 rejects passing a
|
||||
|
|
@ -206,7 +206,7 @@ when the diff content is adversarial:
|
|||
_REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; }
|
||||
cd "$_REPO_ROOT"
|
||||
_USER_INSTRUCTIONS="<everything after '/codex review ' in user input>"
|
||||
_PROMPT_FILE=$(mktemp "$TMP_ROOT/codex-prompt-XXXXXX.txt")
|
||||
_PROMPT_FILE=$(mktemp "$TMP_ROOT/codex-prompt-XXXXXX")
|
||||
{
|
||||
printf '%s\n' "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. Do NOT modify agents/openai.yaml. Stay focused on repository code only."
|
||||
printf '\nCustom focus: %s\n\n' "$_USER_INSTRUCTIONS"
|
||||
|
|
@ -335,7 +335,7 @@ if [ -z "$PYTHON_CMD" ]; then
|
|||
fi
|
||||
# Fix 1+2: wrap with timeout (gtimeout/timeout fallback chain via probe helper),
|
||||
# capture stderr to $TMPERR for auth error detection (was: 2>/dev/null).
|
||||
TMPERR=${TMPERR:-$(mktemp "$TMP_ROOT/codex-err-XXXXXX.txt")}
|
||||
TMPERR=${TMPERR:-$(mktemp "$TMP_ROOT/codex-err-XXXXXX")}
|
||||
_gstack_codex_timeout_wrapper 600 codex exec "<prompt>" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 "$PYTHON_CMD" -u -c "
|
||||
import sys, json
|
||||
turn_completed_count = 0
|
||||
|
|
@ -434,8 +434,8 @@ B) Start a new conversation
|
|||
|
||||
2. Create temp files:
|
||||
```bash
|
||||
TMPRESP=$(mktemp "$TMP_ROOT/codex-resp-XXXXXX.txt")
|
||||
TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX.txt")
|
||||
TMPRESP=$(mktemp "$TMP_ROOT/codex-resp-XXXXXX")
|
||||
TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX")
|
||||
```
|
||||
|
||||
3. **Plan review auto-detection:** If the user's prompt is about reviewing a plan,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,57 @@
|
|||
/**
|
||||
* Regression pin for #2091: /codex was broken on every macOS install because
|
||||
* its mktemp templates carried a suffix after the X's ("codex-err-XXXXXX.txt").
|
||||
* BSD mktemp (macOS) requires the X's to be the trailing characters of the
|
||||
* template; with a suffix it fails ("mkstemp failed ... File exists"), the
|
||||
* temp file never exists, and the skill dies before Codex ever runs.
|
||||
*
|
||||
* GNU mktemp accepts a --suffix flag but ALSO rejects inline suffixes in the
|
||||
* template argument on BusyBox, so the portable form is: X's last, no suffix.
|
||||
*
|
||||
* This scans every .tmpl (the sources of truth — generated SKILL.md files
|
||||
* follow at regen time) for the broken shape.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from "bun:test";
|
||||
import { execFileSync } from "child_process";
|
||||
import { readFileSync } from "fs";
|
||||
import { join } from "path";
|
||||
|
||||
const ROOT = join(import.meta.dir, "..");
|
||||
|
||||
function trackedTmplFiles(): string[] {
|
||||
const out = execFileSync("git", ["ls-files", "*.tmpl", "**/*.tmpl"], {
|
||||
cwd: ROOT,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
return out.split("\n").filter(Boolean);
|
||||
}
|
||||
|
||||
describe("mktemp portability (#2091)", () => {
|
||||
it("no .tmpl file uses a mktemp template with characters after XXXXXX", () => {
|
||||
const offenders: string[] = [];
|
||||
for (const rel of trackedTmplFiles()) {
|
||||
const src = readFileSync(join(ROOT, rel), "utf-8");
|
||||
src.split("\n").forEach((line, i) => {
|
||||
// Broken shape: the X-run followed by a non-quote, non-whitespace,
|
||||
// non-closing character inside a mktemp invocation. X{6,} is greedy,
|
||||
// so longer X-runs (spec's XXXXXXXX) stay valid — only a genuine
|
||||
// suffix after the final X trips it.
|
||||
if (/mktemp[^\n]*X{6,}[^"'\s)X]/.test(line)) {
|
||||
offenders.push(`${rel}:${i + 1}: ${line.trim()}`);
|
||||
}
|
||||
});
|
||||
}
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
|
||||
it("BSD-portable form actually works on this platform", () => {
|
||||
// Live sanity: the exact template shape the skills now emit.
|
||||
const tmp = process.env.TMPDIR || "/tmp";
|
||||
const created = execFileSync("mktemp", [`${tmp.replace(/\/$/, "")}/gstack-portability-XXXXXX`], {
|
||||
encoding: "utf-8",
|
||||
}).trim();
|
||||
expect(created.length).toBeGreaterThan(0);
|
||||
execFileSync("rm", ["-f", created]);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue