Skip to content

fix: clean temporary Codex output after failed runs - #138

Open
sylvesterkaczmarek wants to merge 3 commits into
openai:mainfrom
sylvesterkaczmarek:fix/cleanup-failed-output
Open

fix: clean temporary Codex output after failed runs#138
sylvesterkaczmarek wants to merge 3 commits into
openai:mainfrom
sylvesterkaczmarek:fix/cleanup-failed-output

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Summary

Make the composite action own the implicit --output-last-message file lifecycle so temporary output is removed even when codex exec fails.

Fixes #137.

Problem

When the user does not set output-file, the bundled run-codex-exec helper creates a temporary directory internally.

That directory is cleaned from finalizeExecution(), but finalizeExecution() is only reached after a zero exit code. A non-zero Codex exit rejects immediately, leaving the action-owned temporary directory behind.

On ephemeral hosted runners this is mostly noise. On long-lived self-hosted runners, repeated failed runs can accumulate abandoned temporary directories.

Fix

Move ownership of the implicit output path to the composite-action boundary:

  1. When a prompt is being run and the caller did not provide output-file, create a managed temporary directory before invoking Codex.
  2. Pass its output.md path through the helper's existing explicit --output-file option.
  3. Add an always() cleanup step after Run codex exec.

Because the helper sees an explicit output path, it no longer creates its own untracked temporary output directory. The action can therefore clean the managed directory after success, validation errors, process failures, or non-zero Codex exits.

User-provided output-file paths are not managed or deleted.

Privilege handling

The helper preserves the existing safety-strategy boundary:

  • ordinary strategies use Node's mkdtemp / recursive rm under the current runner identity;
  • unprivileged-user creates the directory through sudo -u <codex-user> -- mktemp -d, so Codex can write the final message;
  • cleanup for that path uses sudo rm -rf because the directory is owned by the unprivileged account.

drop-sudo remains safe because its managed directory is runner-owned and cleanup does not require sudo after privilege removal.

Regression coverage

Added dependency-free Node tests covering:

  • prepare + cleanup for a runner-owned temporary output path;
  • exact sudo -u <user> preparation and privileged cleanup for unprivileged-user;
  • fail-fast validation when unprivileged-user is selected without codex-user.

The unprivileged test uses a fake sudo executable and a real temporary directory, so both command construction and actual cleanup are exercised without elevated test privileges.

Validation

  • all 3/3 focused behavioural tests pass locally with Node's built-in test runner;
  • branch is based directly on current upstream main (c385816875cc2fc8e033ed9d1cba96f8c331210e);
  • branch is 0 commits behind upstream;
  • no files under src/ change, so the checked-in dist/main.js remains valid;
  • production manifest diff is 29 additions / 1 deletion, with the remaining changes isolated to the helper and focused tests.

Risk

Low. The existing run-codex-exec interface is unchanged. Explicit user output files keep their current behavior. The only behavioral change is that the action supplies and later removes its own temporary output path when the caller left output-file empty.

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@sylvesterkaczmarek

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

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.

Failed Codex runs can leave temporary output directories behind

1 participant