feat(harness): Initial Codex Support - #2645
Conversation
|
Demo with Codex: output.mp4 |
2025b21 to
c449664
Compare
c449664 to
dda4e71
Compare
dda4e71 to
cdaa3f7
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds initial Codex native harness support end-to-end (compiler → controller status → actor runtime + image), and introduces shared runtime utilities so Claude and Codex can reuse atomic state + process-group handling.
Changes:
- Introduces Codex harness runtime (adapter/driver/config/cmd), Docker image, and E2E coverage.
- Refactors controller compilation output to include non-blocking compilation warnings, and publishes them in AgentTemplate status + CRDs.
- Adds shared runtime utilities (bounded output, atomic private files, process-group signaling) and migrates Claude to the new continuation store + utils.
Reviewed changes
Copilot reviewed 66 out of 68 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| helm/kagent-crds/templates/kagent.dev_agenttemplates.yaml | Adds warnings field to Helm-rendered AgentTemplate CRD schema |
| go/harness/runtime/utils/utils.go | Adds shared OS helpers (bounded buffer, private dir/file, process-group wrappers) |
| go/harness/runtime/utils/utils_test.go | Tests for bounded buffer and atomic private file replacement |
| go/harness/runtime/continuation/store.go | Adds shared continuation store (persist one runtime continuation ID per actor) |
| go/harness/runtime/continuation/store_test.go | Tests continuation store persistence, formatting, and validation |
| go/harness/runtime/a2a/executor.go | Logs runner failures via A2A logger; adds/updates doc comments |
| go/harness/codex/testdata/app-server-tools.jsonl | Adds Codex app-server protocol fixtures (tool events) |
| go/harness/codex/testdata/app-server-success.jsonl | Adds Codex app-server protocol fixtures (success path) |
| go/harness/codex/testdata/app-server-failure.jsonl | Adds Codex app-server protocol fixtures (failure/interrupted) |
| go/harness/codex/internal/driver/rpc.go | Implements Codex JSON-RPC newline framing client |
| go/harness/codex/internal/driver/rpc_test.go | Tests RPC client rejection/acceptance behavior |
| go/harness/codex/internal/driver/process.go | Implements Codex app-server process lifecycle + cancellation + event consumption |
| go/harness/codex/internal/driver/process_test.go | Tests pinned protocol requests, resume semantics, and workspace config rejection |
| go/harness/codex/internal/driver/events.go | Translates Codex notifications into runtime-neutral events + tool lifecycle |
| go/harness/codex/internal/driver/events_test.go | Tests translation of pinned Codex notifications into runtime events |
| go/harness/codex/internal/adapter/adapter.go | Materializes CODEX_HOME, TOML config, agents/skills, env wiring; returns ProcessDriver |
| go/harness/codex/internal/adapter/adapter_test.go | Tests adapter materialization, permissions, symlink rejection, and pinned CLI acceptance |
| go/harness/codex/config/config.go | Defines Codex runtime config contract + validation + defaults |
| go/harness/codex/config/config_test.go | Tests config round-trip and unsafe configuration rejection |
| go/harness/codex/cmd/main.go | Adds Codex actor entrypoint wiring adapter + continuation store + A2A app |
| go/harness/codex/README.md | Documents Codex harness structure, scope, and usage |
| go/harness/codex/Dockerfile | Adds Codex harness image build (bundles pinned codex-cli) |
| go/harness/claude/internal/session/store.go | Removes Claude-specific session store (replaced by shared continuation store) |
| go/harness/claude/internal/session/store_test.go | Removes tests for deleted Claude session store |
| go/harness/claude/internal/driver/process_group_unix.go | Refactors process-group helpers (intended move to shared utils) |
| go/harness/claude/internal/driver/process_group_windows.go | Refactors process-group helpers (intended move to shared utils) |
| go/harness/claude/internal/driver/process_group_unix_test.go | Removes unix-only descendant termination test |
| go/harness/claude/internal/driver/process.go | Uses shared utils for process groups + bounded stderr; adds doc comments |
| go/harness/claude/internal/driver/parser.go | Adds exported doc comment for ParseJSONL |
| go/harness/claude/internal/adapter/adapter.go | Uses shared utils for private dirs/files; uses config env var constants |
| go/harness/claude/internal/adapter/adapter_test.go | Updates env var constant reference |
| go/harness/claude/config/config.go | Adds reserved env var constants + OwnsEnvironment helper |
| go/harness/claude/config/config_test.go | Adds tests for OwnsEnvironment |
| go/harness/claude/cmd/main.go | Switches to shared continuation store; fixes validate ctx parent; adds UUID validator |
| go/harness/claude/cmd/main_test.go | Adds validateSessionID unit test |
| go/harness/claude/README.md | Adds code-structure section; minor formatting cleanup |
| go/harness/README.md | New shared harness README explaining layout and execution flow |
| go/core/v2/translator/revision.go | Introduces Compilation wrapper to carry warnings outside Revision |
| go/core/v2/translator/revision_test.go | Updates digest test to use Compilation warnings (excluded from digest) |
| go/core/v2/translator/compiler.go | Updates compiler interface to return *Compilation |
| go/core/v2/translator/compiler_test.go | Updates tests for new Compilation return type |
| go/core/v2/translator/kagent/compiler.go | Updates Kagent harness compiler to return *Compilation |
| go/core/v2/translator/codex/compiler.go | Adds Codex harness compiler (providers, MCP, agents, provenance, env resolution) |
| go/core/v2/translator/codex/mcp.go | Adds Codex MCP compilation logic (constraints + header env generation) |
| go/core/v2/translator/codex/compiler_test.go | Adds Codex compiler unit tests (providers, MCP, agents, determinism, leakage checks) |
| go/core/v2/translator/claude/compiler.go | Returns *Compilation; uses OwnsEnvironment + warning plumbing |
| go/core/v2/translator/claude/mcp.go | Converts some previously-fatal MCP incompatibilities into warnings |
| go/core/v2/translator/claude/compiler_test.go | Updates env var constant usage; adds tests for MCP warnings + protocol rejection |
| go/core/v2/controller/reconciler.go | Wires Codex compiler; carries warnings into reconciliation state |
| go/core/v2/controller/status.go | Publishes compilation warnings into AgentTemplateHarnessStatus (defensive copy) |
| go/core/v2/controller/status_test.go | Tests warnings are published and not aliased |
| go/core/v2/controller/collections_test.go | Adds Codex reconciliation compilation test |
| go/core/test/e2e/manifests/lifecycle.yaml.tmpl | Adds Codex harness manifest for e2e lifecycle |
| go/core/test/e2e/mocks/invoke_codex_agent.json | Adds mockllm fixture for Codex agent interaction |
| go/core/test/e2e/mocks/invoke_codex_builtin_tools.json | Adds mockllm fixture for Codex built-in tool events |
| go/core/test/e2e/mocks/invoke_codex_resources.json | Adds mockllm fixture for Codex MCP resource flow |
| go/core/test/e2e/codex_interaction_test.go | Adds Codex E2E tests (streaming, persistence/resume, checkpoint/fork, tools, MCP) |
| go/api/v1alpha3/agenttemplate_types.go | Adds Warnings []string to AgentTemplateHarnessStatus type |
| go/api/config/crd/bases/kagent.dev_agenttemplates.yaml | Adds warnings field to generated CRD schema |
| go/go.mod | Bumps mockllm; adds direct go-toml dependency |
| go/go.sum | Updates sums for mockllm bump |
| go/README.md | Documents new top-level harness/ tree and build targets |
| go/.gitignore | Ignores generated Codex schema bundle directory |
| Makefile | Adds codex-harness image build/push targets; includes in build-all/build/versions |
| .github/workflows/ci.yaml | Builds codex-harness in CI and wires digest into E2E env |
| .github/workflows/tag.yaml | Adds codex-harness to tag workflow matrix |
| .github/workflows/image-scan.yaml | Adds codex-harness to image scan matrix |
Files not reviewed (1)
- go/api/v1alpha3/zz_generated.deepcopy.go: Generated file
Suppressed comments (5)
go/harness/codex/internal/driver/rpc.go:1
- Response ID matching is too strict: JSON-RPC allows
idto be a string or number, andjson.RawMessagewill include quotes for string IDs. If the server ever returns\"1\"instead of1, this will incorrectly fail. Consider decodingmessage.IDinto either anintorstring(or normalizing by unmarshalling intoanyand comparing) rather than comparing raw bytes as strings.
go/harness/codex/config/config.go:1 - This converts
[]byte→string→Reader, which makes an extra allocation/copy proportional to the config size. Using a byte-backed reader (e.g.,bytes.NewReader(data)) avoids the copy while preserving the same decoding behavior.
go/harness/codex/internal/adapter/adapter.go:1 - The
inlineoption on TOML tags is typically intended for embedded structs, not map fields. Keeping it here risks subtle encode/decode differences across TOML library versions. Prefer plain tags (toml:\"http_headers,omitempty\"andtoml:\"env_http_headers,omitempty\") unless the Codex CLI strictly requires an inlined shape.
go/harness/codex/config/config.go:1 - Unlike
anthropicBaseURLHostname(which also rejects query strings), this URL validator permits?query=.... If these URLs are used as base URLs for upstream requests, allowing queries can be surprising and potentially hazardous. Consider also rejectingparsed.RawQuery != \"\"(and possibly normalizing/validating paths) for parity and safer configuration.
go/harness/runtime/utils/utils.go:1 - If the intent is crash-safe durability (not just atomic replacement), it's typical to
fsyncthe parent directory afteros.Renameso the rename itself is persisted across power loss/kernel crashes. Consider adding a best-effort directory sync (with platform-appropriate handling) to strengthen the 'atomically replaces' guarantee.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
2453790 to
dafb621
Compare
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Runs Codex on top of Substrate. Reuses most plumbings from Claude harness, plus some refactoring, and address some follow-up comments on #2602. Feature coverage for Codex is identical to Claude Code right now, and is mostly in parity with Kagent's own ADK harness. The harness driver uses Codex App Server (JSON-RPC over stdio).
What Changed
AgentTemplateharness status; Claude now reports ignored MCP TLS, timeout, and terminateOnClose settings there instead of rejecting the template.Validation