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 4 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 👍 / 👎.
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.