Align the Claude Code plugin with GPT-5.6 runtime and prompting - #638
Align the Claude Code plugin with GPT-5.6 runtime and prompting#638eureka-pd wants to merge 6 commits into
Conversation
- port GPT-5.6 Sol/Terra/Luna model-catalog support from upstream PR openai#471 - support max/ultra efforts and model/effort selection in review flows - refresh stale shared brokers safely across plugin and Codex CLI upgrades - replace active GPT-5.4-specific prompt shaping with version-neutral codex-prompting guidance - keep gpt-5-4-prompting as a deprecated compatibility alias - update fork installation docs, changelog, and regression coverage Based-on: openai#471
![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: 866fdf8a09
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return null; | ||
| } | ||
| } | ||
| teardownExistingBroker(cwd, existing, options.killProcess ?? terminateProcessTree); |
There was a problem hiding this comment.
When a broker crashes and leaves broker.json behind, its endpoint becomes unreachable but its PID can later be reused; this unconditionally passes that stale PID to terminateProcessTree. On Windows this kills the reused process tree, and on Unix it can terminate an unrelated process group when the reused PID is a group leader. Verify that the process still belongs to this broker, or avoid killing by PID when the endpoint cannot confirm broker identity.
Useful? React with 👍 / 👎.
| if (Number.isFinite(ownerPid) && isProcessAlive(ownerPid)) { | ||
| return false; |
There was a problem hiding this comment.
If a CLI process dies while holding this lock and the OS later assigns its PID to an unrelated long-lived process, isProcessAlive remains true and this branch ignores staleMs indefinitely. Every subsequent broker startup or reuse attempt then waits five seconds and fails until that unrelated process exits, effectively disabling the shared runtime for the workspace. Include process-start identity in the lock or allow sufficiently old locks to be recovered safely.
Useful? React with 👍 / 👎.
Adds Phase 1 Claude-native Multi-Codex orchestration with read-only DAG execution, bounded independent App Server workers, durable lifecycle state, cancellation, and orchestration-aware command surfaces. Verified on Ubuntu, macOS, and Windows; Ubuntu passed the full suite three consecutive times. Phase 1 contains no writer worktrees, snapshot refs, integration branches, automatic commits, or danger-full-access orchestration.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3444f60f4a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const activePackageIds = this.pool.getSnapshot().active.map((entry) => entry.packageId); | ||
| for (const packageId of activePackageIds) { | ||
| await this.pool.cancel(packageId, { graceMs: options.graceMs }); |
There was a problem hiding this comment.
When two orchestrations are running concurrently in the same workspace, this snapshot contains workers from both runs, so cancelling either orchestration interrupts every active package in the shared pool. The unrelated orchestration then retries or fails packages even though the user never cancelled it; filter active workers by orchestration ID before calling pool.cancel.
Useful? React with 👍 / 👎.
| exitPromise | ||
| }; | ||
| child.once("exit", () => resolveExit()); | ||
| this.active.set(packageSpec.id, record); |
There was a problem hiding this comment.
Concurrent orchestrations commonly reuse package IDs such as review or explore, but this workspace-wide map is keyed only by packageSpec.id. Starting the second package overwrites the first record, and either package's cleanup can delete the other's record, making snapshots and cancellation target the wrong process or report no active process at all. Use an orchestration-qualified key throughout active, cancel, and cleanup.
Useful? React with 👍 / 👎.
| const ROOT = path.resolve(fileURLToPath(new URL("..", import.meta.url))); | ||
| function run(script, args) { const result = spawnSync(process.execPath, [script, ...args], { cwd: process.cwd(), env: process.env, encoding: "utf8" }); process.stdout.write(result.stdout ?? ""); process.stderr.write(result.stderr ?? ""); process.exitCode = result.status ?? 1; } | ||
| function isOrchestrationRef(reference) { if (!reference) return false; try { resolveOrchestrationReference(resolveWorkspaceRoot(process.cwd()), reference); return true; } catch { return false; } } | ||
| const [command, ...args] = process.argv.slice(2); const reference = args.find((arg) => !arg.startsWith("--")) ?? ""; |
There was a problem hiding this comment.
The command definitions invoke this dispatcher with "$ARGUMENTS", so a lifecycle command containing more than one token arrives here as one string. For example, /codex:status orch-123 --wait produces the candidate reference "orch-123 --wait", fails orchestration resolution, and is routed to the legacy job command instead. Normalize the single raw argument with the same argument splitter used by codex-companion.mjs before detecting the reference.
Useful? React with 👍 / 👎.
| const orchestrationCli = path.join(ROOT, "orchestration", "cli.mjs"); const companion = path.join(ROOT, "codex-companion.mjs"); | ||
| if (command === "status" && !reference) { | ||
| run(companion, ["status", ...args]); | ||
| if (listOrchestrations(resolveWorkspaceRoot(process.cwd())).length) run(orchestrationCli, ["status", ...args]); |
There was a problem hiding this comment.
When any orchestration exists, a reference-free documented command such as /codex:status --wait, --all, or --timeout-ms ... forwards those companion-only flags to the orchestration CLI as well. That CLI treats an unknown flag such as --wait as a positional reference and exits with No orchestration or package found for "--wait", turning an otherwise valid status request into an error. Pass only orchestration-supported options to the appended status call.
Useful? React with 👍 / 👎.
| const config = loadOrchestrationConfig(workspaceRoot); | ||
| const pool = new WorkerPool({ workspaceRoot, size: config.workers.workspacePoolSize, globalTopLevelLimit: config.workers.globalTopLevelLimit, globalActiveCodexLimit: config.workers.globalActiveCodexLimit, onEvent: () => {} }); | ||
| const controller = new OrchestrationController({ workspaceRoot, config, pool, controllerIdentity: identity }); |
There was a problem hiding this comment.
The validated workers.idleTtlMinutes setting is never passed to the pool or used by the controller server, and there is no idle shutdown timer elsewhere in the orchestration runtime. Consequently every workspace that starts an orchestration leaves a detached controller process running indefinitely, including when the user explicitly configures an idle TTL of zero. Wire the setting into controller/pool lifecycle and exit only after the configured idle interval with no active work.
Useful? React with 👍 / 👎.
Add the Phase 2 implementation plan and normative self-review amendments for clean-tree writer orchestration.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efc95af4f1
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const code = await new Promise((resolve, reject) => { | ||
| child.once("error", reject); | ||
| child.once("exit", resolve); |
There was a problem hiding this comment.
If a package worker exits while onStarted is awaiting its state-file mutation—for example, because Codex startup fails immediately—the listener registered earlier at line 105 observes the exit, but this newly registered listener never receives the already-emitted event. execute then waits forever, never releases its worker slot or global lease, and the timeout callback cannot resolve that promise. Capture the exit code/error in the pre-attached promise and await that promise here.
Useful? React with 👍 / 👎.
| const child = spawn(process.execPath, [fileURLToPath(script), requestFile], { | ||
| cwd: this.workspaceRoot, | ||
| env: process.env, | ||
| stdio: ["pipe", "pipe", "pipe"], | ||
| windowsHide: true |
There was a problem hiding this comment.
When a soft interrupt cannot stop a hung worker, both cancellation and package timeout call terminateProcessTree(child.pid). On Unix that helper first signals process group -pid and returns without a direct-PID fallback on ESRCH, but this non-detached child inherits the controller's process group rather than creating one whose ID equals its PID. Consequently the hard-stop path leaves the worker alive and execute remains blocked; launch it detached/in its own group or make termination fall back to the child PID.
Useful? React with 👍 / 👎.
| fs.rmSync(runtimeFile, { force: true }); | ||
| if (existing?.endpoint?.startsWith("unix:")) { | ||
| fs.rmSync(parseControllerEndpoint(existing.endpoint).path, { force: true }); |
There was a problem hiding this comment.
If the status probe times out while the recorded controller PID is still alive, this removes its Unix socket and starts another controller without terminating the old one. The old controller and its package workers can continue mutating orchestration state but are no longer reachable, while recovery skips their nonterminal state because the previous PID is alive; this can leave results racing or an orchestration permanently reported as running. Preserve the endpoint and fail, or terminate and verify the old controller before replacing it.
Useful? React with 👍 / 👎.
Implement Phase 2 clean-tree writer orchestration with isolated package worktrees, controller-owned package commits, deterministic integration, risk-triggered review, full verification, and guarded fast-forward application.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d722fc966
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| }); | ||
|
|
||
| await client.close().catch(() => {}); |
There was a problem hiding this comment.
When runAppServerTurnWithClient rejects before this line—for example, when turn/start rejects an invalid model—the close call is skipped. Because the directly spawned codex app-server still owns live pipes, fail() merely setting process.exitCode does not let the package worker exit, so the controller remains blocked until the 15–60 minute hard timeout instead of recording or retrying the failure. Close the client in a finally block.
Useful? React with 👍 / 👎.
| if (/Subagent .* completed|Native child completed/i.test(message)) { | ||
| nativeChildren = Math.max(0, nativeChildren - 1); |
There was a problem hiding this comment.
When a Root starts a native child, the collaboration tool invocation can complete as soon as the child is launched while the child's turn continues running; decrementing here therefore reports zero active children prematurely. Starting multiple children sequentially can consequently bypass both nativeSubagents.maxChildren and the plugin-wide active-Codex cap, since the worker sends this counter to the global lease registry and later returns its locally derived peak instead of result.nativeChildPeak. Track turn/started and turn/completed for child thread IDs rather than tool-call messages.
Useful? React with 👍 / 👎.
1 participant
Summary
This PR aligns the Claude Code Codex plugin with the current GPT-5.6
Sol/Terra/Luna runtime and prompting model.
It:
maxandultrareasoning efforts where the selected model advertises themminimaleffort from the plugin-facing runtime and command surfacesparkalias togpt-5.6-lunagpt-5-4-promptingskill with version-neutralcodex-promptingguidance--modeland--effortconsistently for review and adversarial-review flowsdanger-full-accesssandboxContext
The runtime/model-catalog and broker-lifecycle portions of this change are
based on and extend #471 by @alexandrereyes.
That PR already addresses the underlying GPT-5.6 runtime and stale-broker
issues. This PR carries that work forward while also updating the
Claude-side model policy and prompting layer for the current GPT-5.6
generation.
In particular, it addresses the stale GPT-5.4 prompting path reported in
#485 and removes the remaining generation-specific assumptions from the
active rescue context.
The plugin now treats:
gpt-5.6-solas the highest-capability tiergpt-5.6-terraas the balanced tiergpt-5.6-lunaas the efficient/high-volume tierReasoning effort remains a separate inference-budget dimension and is
validated against the current Codex model catalog rather than a
generation-specific model matrix.
Model and effort handling
The companion exposes the following reasoning-effort values:
none,low,medium,high,xhigh,max,ultraminimalhas been removed from the plugin surface.Known OpenAI model/effort combinations are validated using
model/listfrom the app server. Older Codex versions that do not expose the model
catalog retain the existing compatibility fallback, and custom providers
or unknown future model names are not blocked by a plugin model allowlist.
The
sparkconvenience alias now resolves to:gpt-5.6-lunainstead of the previous GPT-5.3 Spark model.
Prompting
codex-rescueno longer loads a GPT-5.4-specific prompting skill.The new
codex-promptingskill is generation-neutral and uses a lean,outcome-first task contract:
The old
gpt-5-4-promptingskill and references are removed so they cannotbe injected into new rescue contexts.
Broker/runtime behavior
The shared app-server broker now records enough runtime identity to detect
plugin or Codex CLI upgrades.
Stale brokers are recycled when safe, while brokers with active work are
preserved so that:
Verification
Validated on Node.js 22 with the current Codex CLI:
npm cinpm run check-versionnpm test— 122/122 passingnpm run buildgit diff --checkmax/ultraspark→gpt-5.6-lunaminimalRelated
If maintainers prefer to land #471 independently, I am happy to split the
prompting/model-policy changes into a smaller follow-up PR on top of it.