Codex CLI installation fixed

[andrey-helldar]

Summary

• Resolve Codex SDK and CLI versions dynamically during Docker builds.
• Default to the npm latest tag while supporting exact versions and semver ranges.
• Prevent version drift between the SDK, CLI, and runtime executable.

Changes

• Added the CODEX_VERSION Docker build argument and Compose configuration.
• Installed @openai/codex-sdk and its matching CLI in an isolated Docker stage.
• Removed the duplicate global Codex CLI installation.
• Configured CODEX_CLI_PATH and PATH to use the SDK-provided CLI.
• Changed the runtime SDK dependency constraint from ^0.118.0 to *.
• Updated lock files and generated App Server protocol artifacts to the 0.145.0 baseline.
• Regenerated App Server protocol types during Docker builds.
• Updated the Codex adapter for the 0.145.0 protocol changes.
• Added regression coverage for Docker version resolution and CLI selection.
• Updated environment examples and provider/setup documentation.

Formatting, linting, tests, runtime coverage, protocol validation, TypeScript builds, and docker compose build pass. The full ai:validate command remains blocked by the existing ai:perf readiness timeout despite Vite reporting that it is ready.

Before

After

AI Model

Which AI model was used to generate or assist with this code?

• GPT (5.6 Sol Max)
• Claude Fable (latest)
• Claude Opus (latest)
• Claude Sonnet (latest)
• Other:
• No AI used

Test Plan

• Tests pass (npm test)
• Lint passes (npm run lint)
• Documentation updated (if applicable)
• Manually verified the change works as expected

[lee-to]

Code review

This PR fixes a real SDK/CLI drift problem and the handwritten change is much smaller than GitHub's raw numbers suggest. The raw diff is 505 files / 43,502 additions / 14,253 deletions, but 487 files and 57,502 changed lines are generated Codex App Server protocol artifacts. Another 2 files are lockfiles. The reviewable handwritten portion is 16 files / 171 changed lines: Docker wiring, protocol compatibility updates, tests, and docs.

Must fix

1. Pin the reviewed Codex version by default — .docker/Dockerfile:25-26, packages/runtime/package.json:27

   CODEX_VERSION=latest plus "@openai/codex-sdk": "*" makes production images non-reproducible. A later no-cache rebuild can silently install a newer SDK/CLI and regenerate a different protocol at Docker build time (.docker/Dockerfile:58) than the committed and reviewed 0.145.0 artifacts. That bypasses the lockfile, this PR's compatibility changes, and its tests; a future protocol change can therefore break or alter a production image without any repository change.

   Keep the selector override, but default it to the reviewed baseline: pin the package dependency and Docker/Compose fallback to 0.145.0. Operators can still explicitly set CODEX_VERSION=latest or a semver range when they intentionally accept that rollout. If automatic tracking of latest is a hard requirement, it needs an off-by-default rollout path and CI that builds/tests the dynamically resolved version before it can become the production default.

Should fix

1. Add CODEX_VERSION to the canonical configuration reference — docs/configuration.md:18

   The variable is documented in .env.example, Getting Started, README, and providers docs, but it is missing from the repository's canonical environment-variable table. Document its type, safe default, build-time scope, supported selector forms, and cache behavior there as well.

Context gates

• Architecture: pass — package boundaries are unchanged.
• Rules: pass — no DB, migration, UI, or structured-error violations found.
• Roadmap: warn — this runtime/Docker change is not linked to a roadmap milestone or issue.
• CHECKLIST compliance (touched package: runtime): pass — Codex-specific protocol compatibility is covered; other-adapter parity is not applicable; runtime and protocol tests are present.
• Docs sync: warn — provider/Docker docs are updated, but docs/configuration.md is missing the new build variable.
• PR size: warn — raw size is enormous, but after excluding 487 generated files and 2 lockfiles the handwritten diff is 16 files / 171 lines and remains one cohesive concern. A separate generated-artifacts commit would make future reviews easier.
• Risk gating: error — the moving dependency/runtime selector defaults on instead of using the reviewed pinned baseline.
• CI: build, lint, MCP, and tests are green. The current npm audit failure is not introduced by this PR: auditing the base and head lockfiles now yields the same 7 production findings (including the same fast-uri high advisory), so I am treating it as a newly surfaced base/advisory-state issue rather than a PR regression.

Positive notes

• SDK and CLI now come from one package graph, removing the previous global-CLI drift.
• All initialize call sites were updated for the new required requestAttestation capability.
• The removed on-failure App Server policy is normalized explicitly and covered by a regression test.

Verdict: REQUEST_CHANGES — pin the default version/rollout, then this becomes a focused and reviewable protocol upgrade despite the generated diff.

[lee-to]

Code review — follow-up

Previous review

Response pass

• Addressed: ✔ Pinned the reviewed Codex baseline — .docker/Dockerfile:25, packages/runtime/package.json:27, and both Compose files now default to 0.145.0; the regression test covers the manifests, locks, Docker, Compose, and docs.
• Addressed: ✔ Documented CODEX_VERSION canonically — docs/configuration.md:29 now records the type, safe default, build-time scope, selectors, and cache behavior.
• New: ⚠ Commits 55f675c and 9ca2e4b add model-effort discovery and a Hono/WebSocket security migration after the prior review.
• Still unresolved: ✖ None of the findings from the previous review; the blockers below are new.

The requested fixes are complete and all current checks are green. However, the follow-up delta now contains two substantial, unrelated behavior changes, and both add rollout/correctness risk that was not present in the reviewed Codex version fix.

Must fix

1. Decompose the newly expanded PR — follow-up delta / commits 55f675c..6a156c3

   The delta since the prior review is 1,902 changed lines across 41 unique files. Even after conservatively excluding lockfiles and the three plan/spec documents, it is still 850 lines across 36 files, above both hard limits (>800 lines and >30 files), and it mixes three independently reviewable concerns.

   Split it into actionable boundaries:

   • Codex SDK/CLI/protocol upgrade: keep 5c113bc plus the pin/docs correction in 6a156c3.
   • Model-specific effort discovery: move 55f675c (packages/runtime/src/modelEffort.ts, the four adapter directories, RuntimeProfileForm*, runtime tests, and the provider-doc section) to its own PR.
   • Dependency/security and WebSocket migration: move 9ca2e4b (package*.json, lockfiles, packages/api/**, packages/agent/package.json, and its plan) to a separate PR.

2. Do not treat every non-empty profile value as provider-advertised effort — packages/runtime/src/modelEffort.ts:1-7, packages/runtime/src/adapters/codex/appServer/run.ts:738-740, packages/runtime/src/adapters/openrouter/api.ts:172-176

   normalizeModelEffort() only trims and lowercases. The execution call sites consume persisted/API-supplied profile options directly; they never verify that the value came from the selected model's discovered metadata. The App Server path then casts that arbitrary string to the generated ReasoningEffort union, while the other transports forward it to SDK/CLI/network requests. An API-created or legacy profile containing bogus/a stale level now reaches the provider instead of being ignored, so task execution can fail even though docs/providers.md:83 says these are provider-advertised values.

   Validate the configured value against the selected model's cached discovery metadata before execution. When metadata is unavailable, retain the runtime-specific fallback allowlist. Reject or ignore invalid values through a structured runtime error/warning; do not use a type assertion to claim an arbitrary string is ReasoningEffort.

3. Add off-by-default rollout flags for both risky behavior changes — packages/runtime/src/modelEffort.ts:1, packages/api/src/ws.ts:21-54

   The effort change replaces stable validation/selection behavior across every adapter and changes provider request payloads. The Hono v2 migration replaces the WebSocket request hot path. Neither is guarded.

   Add AIF_RUNTIME_MODEL_EFFORT_DISCOVERY_ENABLED=false and AIF_API_NODE_SERVER_V2_WEBSOCKET_ENABLED=false in packages/shared/src/env.ts, document both in .env.example and docs/configuration.md, read each once outside hot loops, and keep the current behavior fully reachable when the flag is off. The model-effort off path must preserve the existing allowlists; the WebSocket off path must preserve the existing bridge. Enable each independently only after its focused PR is validated.

Should fix

1. Exercise the real WebSocket handshake — packages/api/src/__tests__/serverBootstrap.test.ts

   The updated test mocks createAdaptorServer, so it proves option wiring but not that the new upgradeWebSocket path yields the ws.raw object used by the client maps. Add an integration test on an ephemeral port using the real adapter and ws: connect to /ws, assert the ws:connected payload, verify sendToClient/broadcast, close, and verify cleanup.

2. Sync the adapter template with the new model-metadata convention — packages/runtime/src/adapters/TEMPLATE.ts

   docs/providers.md now declares model-specific effort discovery as a cross-adapter convention, but the runtime checklist requires TEMPLATE.ts to reflect changed adapter conventions. Add a listModels() metadata example for supportsEffort, supportedEffortLevels, and the explicit unsupported case.

Nits

• None.

Context gates

• Architecture: pass — package dependency boundaries and the @aif/data DB boundary remain intact.
• Rules: warn — no DB, migration, UI/Pencil, expensive-CSS, nullable-cast, or string-based error-classification violation found. CI is green, but the PR body reports that full npm run ai:validate still ends at the existing ai:perf readiness timeout.
• Roadmap: warn — the model-effort feature and WebSocket/security migration are not linked to a roadmap milestone or issue.
• CHECKLIST compliance (touched packages: runtime, web, api, agent): warn — all four runtime adapters were enumerated and tested; web changes reuse the existing form and add no CSS/component/API surface; API changes do not alter the WS event contract; agent is manifest-only. The remaining gaps are the runtime template sync and real WebSocket integration coverage above.
• Docs sync: warn — provider/configuration/Docker docs are synchronized, but TEMPLATE.ts does not yet teach the new model-metadata convention.
• PR size (850 non-lock/non-plan lines / 36 files / 3 concerns; raw follow-up: 1,902 / 41): error — hard limit exceeded; use the split above.
• Risk gating (feature flag): error — missing AIF_RUNTIME_MODEL_EFFORT_DISCOVERY_ENABLED=false and AIF_API_NODE_SERVER_V2_WEBSOCKET_ENABLED=false.
• CI: pass — Build, ESLint, MCP Unit, npm audit, Tests, and mcpunit are green at 6a156c3.

Positive notes

• The prior reproducibility and configuration-doc findings were addressed thoroughly and now have regression coverage.
• The effort-discovery implementation covers every adapter directory (claude, codex, opencode, openrouter) rather than leaving a parity gap.
• The Hono v2 server wiring matches the upstream createAdaptorServer({ websocket: { server } }) contract; the requested project-level integration test would close the remaining confidence gap.

Verdict: REQUEST_CHANGES — the prior findings are fixed, but the newly added concerns need decomposition, runtime validation, and off-by-default rollout guards before this PR is safe to merge.

[lee-to]

Code review — follow-up

Previous review

Response pass

• Addressed: ✔ Runtime effort validation — 35a8bae adds selected-model validation in the registry, preserves stable fallback allowlists, strips unsupported values before adapter execution, and emits structured warning fields instead of branching on message text.
• Addressed: ✔ Off-by-default rollout flags — both flags are boolean, default to false, are documented in .env.example and docs/configuration.md, are included in turbo.json, and are read during registry/API initialization rather than inside hot loops.
• Addressed: ✔ WebSocket rollback path and integration coverage — both the legacy injected bridge and Node Server v2 path complete a real handshake, direct send, broadcast, close, and cleanup test.
• Addressed: ✔ Adapter parity and template documentation — all four adapter directories have enforcement coverage, and TEMPLATE.ts now demonstrates effort metadata.
• Superseded: ✔ Decomposition — maintainers explicitly accepted keeping the PR together; I am treating the size as an accepted review-risk exception, not a blocker.
• New: ⚠ One OpenRouter supported_efforts: null edge case remains, but the affected behavior is dark while AIF_RUNTIME_MODEL_EFFORT_DISCOVERY_ENABLED=false.
• Still unresolved: ✖ No merge-blocking findings from the previous review.

The new commit closes the previous blockers and current CI is green. The default-off paths retain the stable effort allowlists and legacy WebSocket strategy, so the two follow-ups below do not block merging; the model-effort flag should remain disabled until the OpenRouter case is corrected.

Must fix

• None.

Should fix

1. Preserve OpenRouter's supported_efforts: null semantics before enabling rollout — packages/runtime/src/adapters/openrouter/api.ts:724-733, packages/runtime/src/modelEffort.ts:13-19, packages/runtime/src/modelEffort.ts:194-206

   OpenRouter documents supported_efforts: null as accepting every gateway effort value. The adapter currently records only supportsEffort: true; validation then sees no discovered list and falls back to minimal|low|medium|high|xhigh, silently dropping valid gateway values such as max and none. This also leaves those values absent from the profile control.

   Keep the stable off-path list unchanged, but when discovery returns null, emit an explicit discovered gateway list (max, xhigh, high, medium, low, minimal, none) or model the “all gateway values” state separately. Add coverage for null, max, and none in the OpenRouter adapter, central validation, and form behavior. See OpenRouter reasoning discovery semantics.

2. Make the adapter template's capability match its implementation — packages/runtime/src/adapters/TEMPLATE.ts:265-269, packages/runtime/src/adapters/TEMPLATE.ts:308

   The template now implements listModels(), but supportsModelDiscovery: true remains commented out. A copied adapter therefore exposes an unreachable method because capability checks reject discovery. Either enable the capability in the example or move listModels() back into the optional commented block so the skeleton is internally consistent.

Nits

• None.

Context gates

• Architecture: pass — registry policy remains inside @aif/runtime; API and agent only pass validated env configuration, and DB boundaries are unchanged.
• Rules: warn — no DB, migration, UI/Pencil, expensive-CSS, nullable-cast, or string-based error-classification violation found. CI is green; the PR body still records the existing full ai:validate performance-readiness timeout.
• Roadmap: warn — the effort-discovery and WebSocket/security work is not linked to a roadmap milestone or issue.
• CHECKLIST compliance (touched packages: shared, runtime, api, agent): warn — env/schema/type/state-machine checks are either covered or not applicable; all adapters and production registry entry points are covered; API has real tests for both WS strategies; agent changes only plumb the registry flag. The remaining template capability mismatch is noted above.
• Docs sync: pass — env examples, canonical configuration, provider behavior, Turbo env propagation, and adapter metadata conventions are synchronized.
• PR size (1,689-line latest fix commit; overall PR remains above the normal hard limit): warn, accepted — maintainers explicitly chose not to decompose.
• Risk gating (feature flag): pass — AIF_RUNTIME_MODEL_EFFORT_DISCOVERY_ENABLED=false and AIF_API_NODE_SERVER_V2_WEBSOCKET_ENABLED=false satisfy declaration, documentation, single-read, naming, and off-path requirements.
• CI: pass — Build, ESLint, MCP Unit, npm audit, Tests, and mcpunit are green at 35a8bae.

Positive notes

• The validation marker prevents a dynamically discovered value from bypassing adapter-local fallback checks without registry approval.
• Discovery is cached and strips the effort option from the discovery profile, avoiding per-value cache fragmentation.
• The WebSocket integration test exercises both rollout branches using the real adapter instead of only asserting mocked wiring.

Verdict: APPROVE — the previous blockers are resolved. Keep model-effort discovery disabled until the OpenRouter null semantics follow-up lands.