Skip to content

fix(codex): guard mktemp failures in codex mode templates - #2966

Closed
SomSamantray wants to merge 1 commit into
garrytan:mainfrom
SomSamantray:fix/2881-guard-mktemp-codex-templates
Closed

SomSamantray wants to merge 1 commit into
garrytan:mainfrom
SomSamantray:fix/2881-guard-mktemp-codex-templates

Conversation

@SomSamantray

@SomSamantray SomSamantray commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Why

Every codex mode (/codex review, /codex consult, /codex challenge) creates its stderr/response/prompt temp files with mktemp "$TMP_ROOT/..." and never checks whether that call actually succeeded. bin/gstack-paths resolves $TMP_ROOT with a deliberately best-effort mkdir -p ... || true, explicitly punting failure discovery to "the caller's own write attempt." When $TMP_ROOT turns out to be unwritable (no permission, disk full, a read-only sandbox), mktemp fails silently, the variable is empty, the next command redirects stderr to 2>"", and the user gets a codex-attributed error instead of the real cause. This guards every mktemp assignment in the three codex mode templates with a loud, correctly-attributed exit, matching the guard shape the repo already uses at five other call sites (redact-doc.ts, ship/pr-body.md.tmpl, gstack-upgrade/SKILL.md.tmpl, build-app.sh, verify-rls.sh).

Live evidence

$ bun test test/regression-pr1169-mktemp-fallbacks.test.ts   # before the fix (templates unguarded)
error: expect(received).toMatch(expected)
Expected substring or pattern: /TMPERR=\$\(mktemp "\$TMP_ROOT\/codex-err-XXXXXX"\)\s*\|\|\s*\{[^}]*exit 1/
Received: "## Step 2A: Review Mode\n\n...\nTMPERR=$(mktemp \"$TMP_ROOT/codex-err-XXXXXX\")\n..."
(3 of 4 new #2881 tests fail — confirmed red before the fix)

$ bun test test/regression-pr1169-mktemp-fallbacks.test.ts   # after guarding + bun run gen:skill-docs --host all
bun test v1.4.2 (744846f84)
 18 pass
 0 fail
 31 expect() calls
Ran 18 tests across 1 file. [240.00ms]

$ bash -c 'unset TMPERR; TMPERR=${TMPERR:-$(mktemp --tmpdir=/nonexistent/dir codex-err-XXXXXX 2>/dev/null)} || { echo "guard fired, exit=$?"; }'
guard fired, exit=1
# confirms the ${TMPERR:-$(mktemp ...)} form used in challenge-mode.md.tmpl propagates the inner
# command substitution's failure through the parameter-expansion default correctly

$ bun test test/skill-validation.test.ts test/mktemp-portability.test.ts test/regression-issue2091-bsd-mktemp.test.ts
 365 pass
 0 fail

$ bun run gen:skill-docs --host all && bun run skill:check
Checked 667 generated artifacts; 109 tracked outputs compared.   # no drift beyond the 3 .tmpl + 3 generated .md files this PR touches

bun run test (full free suite) also ran; it reports 37 pre-existing failures in 9 files, all in unrelated subsystems (CSO/Docker watchdog, gbrain MCP timeouts, macOS codesigning, PTY setup). Confirmed via git stash that the same failures occur on unmodified upstream/main before this branch's changes — not introduced by this PR.

Scope

  • Changed: codex/sections/{review,consult,challenge}-mode.md.tmpl (source templates, hand-edited) + their generated codex/sections/{review,consult,challenge}-mode.md counterparts (regenerated via bun run gen:skill-docs --host all), plus a new #2881 test block in test/regression-pr1169-mktemp-fallbacks.test.ts.
  • Verified live by: running the targeted regression test red-before/green-after, direct bash verification of the ${TMPERR:-$(mktemp ...)} guard composition, the adjacent static-validation suites (skill-validation, mktemp-portability, regression-issue2091), bun run skill:check (no template/generated drift), and the full bun run test suite cross-checked against upstream/main for pre-existing failures.
  • Did NOT test: an actual live /codex review|consult|challenge invocation against a real unwritable $TMP_ROOT end-to-end (would need a live Codex CLI session + a deliberately broken sandbox); the static + runtime-guard tests cover the failure-propagation contract instead.

Liveness proof (required for external contributors)

Attached as a PR comment — GSTACK PR typed live in my terminal.

Checklist

Every mktemp assignment in the three codex mode templates
(review-mode, consult-mode, challenge-mode) was unguarded.
bin/gstack-paths deliberately best-effort-creates $TMP_ROOT and
defers failure discovery to the caller's own write attempt, so an
unwritable $TMP_ROOT made these mktemp calls silently yield an
empty variable. The next command then redirected stderr to 2>"",
which failed before codex even ran, misattributing the failure to
codex instead of the missing temp file.

Guard every mktemp assignment with a loud exit, matching the
existing repo convention (redact-doc.ts, ship/pr-body.md.tmpl,
gstack-upgrade/SKILL.md.tmpl, supabase/verify-rls.sh,
build-app.sh). challenge-mode's ${TMPERR:-$(mktemp ...)} wrapper
is preserved unchanged; verified live that the guard composes
correctly with the parameter-expansion default.

Regenerated codex/sections/*.md from the updated .tmpl sources
per the repo's SKILL.md generation workflow.

Fixes garrytan#2881
@trunk-io

trunk-io Bot commented Sep 24, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@SomSamantray

Copy link
Copy Markdown
Contributor Author
Screenshot 2026-09-24 at 8 00 15 PM

Liveness proof for the checklist — GSTACK PR typed live in my terminal.

@capy-ai

capy-ai Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Thank you for this. It's superseded on main: every mktemp in codex/sections/* and the /review adversarial pass is now guarded with a loud exit (#3014, #3025, #3057). Closing.

@capy-ai capy-ai Bot closed this Oct 8, 2026
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.

1 participant