Repository navigation
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Codex stop-time review gate execution path so the stop-gate “one-shot” review runs as an ephemeral, untracked, read-only task, preventing orphaned persisted threads and job-catalog churn while keeping the hook’s stdout contract intact.
Changes:
- Teach
executeTaskRunto honorrequest.persistThread(defaulting totrue) and avoid generating a persistent thread name when persistence is disabled. - Route stop-gate marker prompts through a foreground, untracked one-shot path in
handleTaskthat forcespersistThread: falseand omitswrite(read-only). - Update the stop-hook end-to-end test to assert the thread is ephemeral and that no “Codex Stop Gate Review” job appears in the catalog/status output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/runtime.test.mjs | Adds assertions that stop-gate runs produce an ephemeral thread and do not create catalog jobs/status entries. |
| plugins/codex/scripts/codex-companion.mjs | Implements persistThread support in task execution and adds an early stop-gate one-shot execution path that bypasses tracked jobs and forces read-only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5406e18692
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| resumeLast | ||
| }); | ||
|
|
||
| if (!resumeLast && String(prompt ?? "").includes(STOP_REVIEW_TASK_MARKER)) { |
There was a problem hiding this comment.
Any normal /codex:task prompt or --prompt-file that contains this marker sentence now takes the stop-gate path before --background or --write are honored. For example, asking Codex to edit or discuss plugins/codex/prompts/stop-review-gate.md with --write would be run as an untracked ephemeral read-only one-shot instead of a write-capable task, so the requested edits cannot be made and the job cannot be resumed or inspected. Consider keying this branch off an internal flag/env from stop-review-gate-hook.mjs rather than user-controlled prompt text alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 4acaf32. Routing no longer reads the prompt: stop-review-gate-hook.mjs now passes an explicit --stop-gate flag, and handleTask keys the ephemeral/untracked/read-only branch off that flag instead of the marker sentence. So a normal /codex:task (or --prompt-file) whose text happens to contain the marker — e.g. editing prompts/stop-review-gate.md with --write — now runs as a normal write-capable, tracked, resumable task. Added a regression test asserting exactly that. Thanks for the catch!
5406e18 to
4acaf32
Compare
|
Bumping this — it's been running in production on our side for ~4 weeks (patched into the local 1.0.4 plugin copy) and behaving exactly as intended: the stop-gate review runs as an ephemeral, untracked, read-only one-shot, so it no longer persists a rollout to This supersedes #384: routing is now on an explicit The only reason we carry it as a local patch is that a plugin auto-update would orphan it, so merging upstream is what makes it durable. Happy to rebase or adjust if anything's blocking review. Thanks! |
|
This issue is even more problematic when using the ChatGPT/Codex desktop app which makes the sidebar nearly unusable. Please fix. |
…ne-shot
When the stop-time review gate is enabled, the Stop hook runs
`codex-companion.mjs task` every turn. That path forced
`persistThread: true` (persisting the thread as an on-disk rollout) and
registered a tracked "Codex Stop Gate Review" job in the companion
catalog (state.json + jobs/<id>.{json,log}).
But the stop-gate review is a one-shot: the hook consumes its result
inline (parsing ALLOW:/BLOCK: from stdout) and never resumes or inspects
it via /codex:status, /codex:result, or --resume-last. So every turn
orphaned a Codex rollout on disk and churned the 50-job catalog cap with
stop-gate noise, pushing out real tasks and reviews.
stop-review-gate-hook.mjs now marks its invocation with an explicit
`--stop-gate` flag, and handleTask routes on that flag (not
user-controlled prompt text) through an ephemeral, untracked, read-only
path: the check runs before the --background branch (so a backgrounded
run can't create a tracked job), passes persistThread: false (ephemeral
thread, no rollout, no thread name), and omits write so the review can
never select a writable sandbox. executeTaskRun now honors
request.persistThread (default true). The stdout contract is unchanged;
normal /codex:task runs are unaffected, even when their prompt text
happens to contain the stop-gate marker sentence.
4acaf32 to
9eb2306
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9eb2306812
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const { options, positionals } = parseCommandInput(argv, { | ||
| valueOptions: ["model", "effort", "cwd", "prompt-file"], | ||
| booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], | ||
| booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background", "stop-gate"], |
There was a problem hiding this comment.
When a task is supplied as the normal single quoted prompt argument and its text mentions --stop-gate, normalizeArgv re-tokenizes that argument with splitRawArgumentString, and this new boolean option is therefore set from prompt text. For example, task "Explain the --stop-gate path" strips the token and takes the ephemeral, untracked branch; a raw one-argument request containing both --write and this text also silently loses write access. The fresh evidence beyond the earlier marker-sentence report is the single-argument re-tokenization at codex-companion.mjs:130-137, so use a hook-only signal (or otherwise separate internal controls from prompt tokens).
Useful? React with 👍 / 👎.
| prompt, | ||
| resumeLast: false, | ||
| jobId: null, | ||
| persistThread: false, |
There was a problem hiding this comment.
When the review gate is enabled, persistThread: false plus the direct untracked path means the stop-gate run has neither a persisted rollout nor a job/session ID exposed by /codex:status or /codex:result. README.md:290 still promises that every stop-gate run can be resumed with codex resume using an ID from those commands, so users following the documented handoff flow cannot do so; update that guidance with this behavior change.
Useful? React with 👍 / 👎.
Pass the hook's internal stop-gate mode through its child environment instead of argv. Literal --stop-gate text remains part of a normal tracked prompt, including a single raw argument carrying --write, while the hook keeps its ephemeral read-only thread and leaves no job record. Clarify that delegated tasks can be resumed but ephemeral stop-gate checks cannot be resumed or retrieved through status/result. Add raw-argument regressions and strengthen the hook's sandbox/catalog assertions.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a639b86252
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }); | ||
|
|
||
| // The hook signals out of band so prompt text cannot select this untracked path. | ||
| if (process.env.CODEX_COMPANION_STOP_GATE === "1") { |
There was a problem hiding this comment.
When a stop-gate run is the first Codex command for a workspace, this marker remains in process.env while the lazy detached broker and app server are started; lib/app-server.mjs:190-193 forwards that environment, so commands spawned by the review (and later by the shared runtime) can inherit CODEX_COMPANION_STOP_GATE=1. This breaks any such command that invokes the companion—for example, this repository's test fixture copies process.env, causing its normal task tests to take this untracked/read-only branch and fail because no job state is created. Capture and remove the marker (or pass a sanitized environment) before starting the Codex runtime.
Useful? React with 👍 / 👎.
Problem
When the stop-time review gate is enabled, the
Stophook runscodex-companion.mjs taskon every turn. Thetaskpath always setpersistThread: true(persisting the thread as an on-disk rollout) and registered a tracked job in the companion catalog (state.json+jobs/<id>.{json,log}).But the stop-gate review is a one-shot: the hook consumes its result inline (parsing
ALLOW:/BLOCK:from stdout) and never resumes or inspects it via/codex:status,/codex:result, or--resume-last. So every turn leaves behind:On a busy machine this accumulates into hundreds of orphaned rollouts and a catalog dominated by stop-gate noise.
Fix
Route the stop-gate one-shot (already recognized via
STOP_REVIEW_TASK_MARKER) through an ephemeral, untracked, read-only path inhandleTask:executeTaskRunnow honorsrequest.persistThread(defaulttrue), so the stop-gate passespersistThread: false→ the thread startsephemeral: true(no rollout) with no persistent thread name.runForegroundCommand/runTrackedJob, so no job record or log file is written. The same JSON payload is still printed to stdout, so the hook contract (and the gate's ALLOW/BLOCK behavior) is unchanged.--backgroundbranch, so a backgrounded marker prompt can't create a tracked job either.write, so the review can never select aworkspace-writesandbox regardless of CLI flags — it is always read-only.Normal
/codex:taskruns are unaffected (persistThreaddefaults totrue).Tests
Updated the stop-hook end-to-end test to assert the new contract: the review still runs and blocks on findings, the review thread is
ephemeral, and no "Codex Stop Gate Review" job is added to the catalog.