Repository navigation
[AI-68] Hosted Codex agents on macOS and Linux (CLI/daemon) - #71
Merged
Merged
Conversation
Captures the strategy-pattern split of AgentOrchestrator into per-vendor IHostedAgentLauncher impls, the Tomlyn-based ~/.codex/config.toml pre-trust writer, LocalPermissionBridge per-vendor URL routing, and the CLI codex-hook PermissionRequest bridge bounce. Includes the confirmed Codex pre-trust spike findings. Out-of-scope follow-ups tracked in AI-632 (PR review) and AI-633 (sandbox/approval selection). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Six follow-ups from review on /Users/alexey/.../design.md (file:line refs): - Add §3.5 covering the kapacitor-server wire-contract changes (DaemonCommands, CapacitorHub, LaunchAgentDialog, AgentRunStarted read-model) so AI-68 is scoped as a single cross-repo issue with a paired-PR rollout ordering. - §4.3 CodexLauncher.Prepare gains a fail-fast hook preflight that throws CodexHooksNotInstalledException when ~/.codex/hooks.json doesn't reference kapacitor codex-hook for SessionStart / Stop / PermissionRequest — keeps hosted Codex from silently running without session linking, watcher, or permission round-trip. - §4.4 adds AgentInstance.Vendor and routes the failed-launch cleanup path (orchestrator line 361) through launcher.Cleanup instead of inlined Claude-specific calls. - §5.2 switches CodexPaths.Home from init-once to a computed property so HOME overrides take effect mid-test, and creates the parent .codex directory before atomic write so first-time Codex users get pre-trusted. - §7.3 Codex CLI bridge bounce becomes fail-closed (deny + nonzero exit on any HTTP failure), with loopback validation extracted from PermissionRequestCommand.cs:70 as a shared DaemonBridgeUrl helper. - §8 test list updated for the new fail-closed semantics, the directory creation case, the hook preflight, and the failed-after-prepare cleanup regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five follow-ups + the open question from the second review pass: - §7.0 (new) explicitly updates PermissionRequestCommand.cs:63 to post to /claude/permission-request when the bridge URL split lands. Adds the hosted-Claude regression test under §8.3 so the URL migration cannot break Claude silently. - §3.5 server rollout: RequestPermission gains an OPTIONAL fifth `vendor` parameter (defaulted to "claude") instead of a required one. SignalR's positional-arg dispatch lets old daemons (four-arg call sites) keep working against the new server. Server-first ordering is now safe. LaunchAgentCommand.Vendor on the server side also defaults to "claude" for old-daemon JSON-deserialisation compatibility. - §4.3 hook preflight accepts either user-scope (~/.codex/hooks.json) OR project-scope (worktree/.codex/hooks.json after overlay) installs. Overlay runs BEFORE preflight so project hooks copied from the source repo satisfy the check. Splits hooks across the two scopes are also accepted in aggregate. - §4.3 + §10 move EntryReferencesKapacitorCodexHook + CodexHookEvents + hooks.json parsing helpers to new src/Kapacitor.Core/CodexHooksParser.cs so the daemon can use them (Kapacitor.Daemon only references Core, not the CLI project). - §8.8 AOT publish covers BOTH src/kapacitor/kapacitor.csproj AND src/Kapacitor.Daemon/Kapacitor.Daemon.csproj. Daemon is where Tomlyn lands so its publish has to be the one that's grepped for IL3050/IL2026. - Open question resolved: §3.5 + §10.2 pull permission-request UI vendor awareness into AI-68 scope. PendingPermission carries Vendor; PermissionRequestCard hides Always Allow + Update Input for Codex. Server tests added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four follow-ups from third review:
- §3.5 + §6.2 + §10.2 switch from "optional fifth parameter on
RequestPermission" to "new RequestPermissionV2 hub method with the same
four args plus vendor". This repo's existing SignalR regression notes
(SendTranscriptBatchWireTests.cs:15, RemoteAgentService.cs:142) document
that HubConnection.InvokeAsync dispatches by exact arity and does NOT
backfill optional values. The named-method approach is the only safe
pattern. Old daemons keep calling four-arg RequestPermission; new
daemons invoke V2. Wire-level tests added.
- §3.5 expands permission-UI vendor awareness beyond PermissionRequestCard
to cover AgentChatView.razor:62 ("Yes, always" inline option) and
ElicitationOption.cs:13 (Claude-only "always" variant). Vendor threads
through InterruptIssued, PendingPermission, SignalR PermissionRequested
push, AgentChatContext, and AgentChatView. Server-side tests added for
each surface.
- §6.2 updates the daemon's ServerConnection.RequestPermissionAsync
signature to take string vendor and to invoke "RequestPermissionV2".
New LocalPermissionBridgeTests cover that POST /{token}/codex/...
reaches the server with vendor = "codex" and the claude path with
vendor = "claude".
- §5.4 reconciles the "best-effort vs fail-fast" semantics that previously
contradicted §4.3. Scopes the swallow-and-continue policy to filesystem
/ TOML errors only; CodexHooksNotInstalledException stays fail-fast and
produces LaunchFailed. Documents the wrapper shape with explicit catch
ordering so future fail-fast preconditions follow the same pattern.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four follow-ups from fourth review: - PermissionRequested SignalR broadcast hits the same arity trap as RequestPermission. Spec switches to a dual-broadcast: keep the existing five-arg PermissionRequested unchanged for old clients, add a sibling six-arg PermissionRequestedV2 that includes vendor. RemoteAgentService subscribes to both. New wire-regression tests cover both shapes firing on the same trigger. - InterruptIssued comes from the external Kurrent.Agent.Schema package and is not editable here. Vendor is carried on Capacitor-owned ClaudeCodePermissionExtension instead (ClaudeCodeExtensions.cs:107). §3.5 + §10.2 updated to reflect the extension as the carrier. - IHostedAgentLauncher.Prepare XML doc rewritten to acknowledge both failure modes — filesystem/parse errors swallowed inside the launcher, typed preflight exceptions allowed to propagate to LaunchFailed — pointing to §5.4's wrapper shape for the orchestrator-side catch ordering. - Permission-elicitation UI surfaces corrected to match the actual code shape: PendingPermission lives in ElicitationOption.cs (add Vendor field), AgentChatView's "Yes, always" is hardcoded markup (Razor-render conditional), AgentChatContext exposes scalar fields (add PermissionVendor, not a nested CurrentPermission object). Dropped the misnamed ElicitationOption_factory test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts the V2 hub method, dual SignalR broadcast, and nullable-default workarounds added in prior amendments. Staging is the only environment and the deployment is single-controlled — server, daemon, and clients upgrade together as part of this Linear issue. The compatibility layers were carrying rollout-window complexity for a window that does not exist in practice. Changes: - §3.1 LaunchAgentCommand.Vendor is a required positional parameter (no C# default). Required positionals moved before optional ones to satisfy C# language rules. - §3.5 RequestPermission bumped to five args (adds vendor); no V2 alias. - §3.5 PermissionRequested broadcast bumped to six args (adds vendor); no V2 alias, no dual-broadcast. - §3.5 DaemonCommands.LaunchAgentCommand.Vendor required (no "claude" default). Old-daemon JSON deserialisation compat language removed. - §6.2 daemon-side ServerConnection.RequestPermissionAsync invokes plain RequestPermission (not RequestPermissionV2). - §10.2 server file list and tests pruned: dropped Old_daemon_simulator, PermissionRequested_old_event_still_fires, Server_dual_broadcasts, LaunchAgentCommand_deserialises_with_default_vendor. Added LaunchAgentCommand_round_trips_required_vendor_field and LaunchAgentCommand_rejects_unknown_vendor_at_controller_boundary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… repo) 21-task TDD plan covering the §10.1 surface — Tomlyn dep + AOT gate, CodexHooksParser move to Core, CodexPaths computed properties, DaemonBridgeUrl loopback helper, CodexHooksNotInstalledException, CodexConfigWriter, LaunchAgentCommand/AgentRunStarted Vendor fields, ServerConnection vendor plumbing, Claude CLI hook URL migration, IHostedAgentLauncher abstraction, ClaudeLauncher extraction, AgentInstance.Vendor, orchestrator vendor routing, DaemonConfig.CodexPath, CodexLauncher (overlay + preflight + pre-trust + BuildArgs), LocalPermissionBridge per-vendor routing, Codex CLI hook bridge bounce (fail-closed), DI registration, README updates, final AOT gate. Cross-repo §10.2 server-side work is explicitly scoped out — paired PR plan lives in the kapacitor-server worktree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…prior float ranges
Move EntryReferencesKapacitorCodexHook and CodexHookEvents from PluginCommand into a new CodexHooksParser in Kapacitor.Core so Kapacitor.Daemon can reference the predicate without taking a dependency on the CLI project. Update all call sites (PluginCommand, StatusCommand) to delegate to CodexHooksParser. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nfig.toml Uses Tomlyn 2.4.0 TomlTable model with AOT-safe GetBuiltInTypeInfo<TomlTable> via a thin TomlSerializerContext subclass — avoids RequiresDynamicCode/ RequiresUnreferencedCode on the generic Deserialize<T> overload. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add required string Vendor positional to LaunchAgentCommand (between AttachmentIds and Kind to satisfy C# required-before-optional rule). Daemon validates at top of HandleLaunchAgent; rejects anything other than "claude" or "codex" via LaunchFailedAsync. Wire-format test added to confirm round-trip serialization. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…trator) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Launcher HandleLaunchAgent now looks up the vendor-specific launcher from a constructor-injected dictionary, invokes Prepare/BuildArgs/Cleanup, and surfaces CodexHooksNotInstalledException as an actionable LaunchFailed. CleanupAgentAsync and the failed-launch catch block dispatch teardown through the same dictionary, with a NoopPtyProcess stand-in so the launcher can run without a live PTY. A few ServerConnection methods become virtual so tests can override them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…se shapes
Switch from flat /{token}/permission-request to /{token}/{vendor}/permission-request.
Claude responses carry the full shape (applyPermissions, updatedInput); Codex responses
emit behavior only. Vendor is forwarded to RequestPermissionAsync replacing the "claude"
stub. Legacy URL now returns 404. FakeServerConnection captures vendor for assertion.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l-closed) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Review Summary by QodoAdd vendor-specific hosted agent launchers for Codex on macOS/Linux with daemon permission bridge
WalkthroughsDescription• Adds vendor-specific hosted agent launcher strategy pattern (IHostedAgentLauncher) enabling
Codex CLI support alongside Claude on macOS and Linux
• Implements CodexLauncher with sandbox (--sandbox workspace-write), approval
(--ask-for-approval on-request), and hooks preflight validation
• Implements ClaudeLauncher extracted from AgentOrchestrator preserving existing Claude behavior
with overlay, MCP config, and pre-trust logic
• Adds Codex pre-trust mechanism via CodexConfigWriter writing TOML entries to
~/.codex/config.toml (AOT-safe via Tomlyn source-gen)
• Splits LocalPermissionBridge URLs by vendor (/{token}/{vendor}/permission-request) with
vendor-shaped responses; Codex hook bridges through daemon with fail-closed semantics
• Refactors CodexHookCommand.HandlePermissionRequest to validate loopback URLs and POST to daemon
bridge when KAPACITOR_DAEMON_URL is set
• Adds Vendor field to LaunchAgentCommand and AgentRunStarted wire format (required, no
default)
• Moves CodexHooksParser to Kapacitor.Core for shared CLI/daemon access; exports hook event
constants and validation predicates
• Converts CodexPaths to computed properties enabling test HOME environment variable scoping
• Centralizes package version management via Directory.Packages.props across all projects
• Comprehensive test coverage for vendor routing, launcher dispatch, permission bridge routing,
hooks preflight, TOML pre-trust, loopback validation, and wire format serialization
• Updates documentation with hosted Codex agent setup, daemon config settings, and
permission-request hook daemon-bridge behavior
Diagramflowchart LR
Dashboard["Dashboard<br/>vendor selector"]
LaunchCmd["LaunchAgentCommand<br/>+ Vendor field"]
Orchestrator["AgentOrchestrator<br/>dispatch by vendor"]
ClaudeLauncher["ClaudeLauncher<br/>overlay + MCP + pre-trust"]
CodexLauncher["CodexLauncher<br/>sandbox + approval + hooks"]
PermBridge["LocalPermissionBridge<br/>/{token}/{vendor}/permission-request"]
CodexHook["CodexHookCommand<br/>daemon-bridge mode"]
CodexConfig["CodexConfigWriter<br/>TOML pre-trust"]
Dashboard -->|"Vendor"| LaunchCmd
LaunchCmd -->|"Vendor"| Orchestrator
Orchestrator -->|"claude"| ClaudeLauncher
Orchestrator -->|"codex"| CodexLauncher
ClaudeLauncher -->|"overlay"| PermBridge
CodexLauncher -->|"hooks preflight"| CodexConfig
CodexLauncher -->|"permission-request"| CodexHook
CodexHook -->|"POST"| PermBridge
File Changes1. src/Kapacitor.Daemon/Services/AgentOrchestrator.cs
|
Code Review by Qodo
1. allow = [] JsonArray expression
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+133
to
+135
| if (permissions["allow"] is not JsonArray allow) { | ||
| allow = []; | ||
| permissions["allow"] = allow; |
There was a problem hiding this comment.
1. allow = [] jsonarray expression 📘 Rule violation ☼ Reliability
ClaudeLauncher.MergeToolPermissions creates a JsonArray using the collection-expression syntax [], which can introduce dynamic-code requirements and break NativeAOT/trimming scenarios. This violates the requirement to use AOT-safe JsonArray construction patterns.
Agent Prompt
## Issue description
`JsonArray` is created via collection-expression syntax (`[]`) in `ClaudeLauncher.MergeToolPermissions`, which is disallowed for AOT compatibility.
## Issue Context
Compliance requires avoiding `JsonArray` collection expressions (e.g. `[]`, `[item1, item2]`) and using constructors like `new JsonArray()` / `new JsonArray(item1, item2)` instead.
## Fix Focus Areas
- src/Kapacitor.Daemon/Services/ClaudeLauncher.cs[133-135]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…ecisions aren't killed at 30s Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add claude_path and codex_path as settable daemon profile fields so users can override the binary paths via `kapacitor config set daemon.claude_path` / `daemon.codex_path` or via KAPACITOR_CLAUDE_PATH / KAPACITOR_CODEX_PATH env vars. Previously DaemonConfig carried these fields but nothing populated them from the profile — the daemon always resolved "claude" / "codex" via PATH. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…I-635/AI-636 codex hooks)
Conflicts resolved:
- src/Kapacitor.Core/CodexPaths.cs — kept our computed-property form
from Task 3 (so tests can scope HOME); added main's new UserSkillsDir
in the same computed form.
- src/Kapacitor.Core/Resources/help-codex-hook.txt — kept the daemon-
bridge section from AI-68 and reconciled the stub-branch description
with AI-636's record-and-yield behaviour (no more allow-stub fallback).
- src/Kapacitor.Daemon/DaemonRunner.cs — dropped our redundant
KAPACITOR_DAEMON_NAME env handling (now subsumed by main's shared
DaemonNameResolver.Resolve from AI-630). Dropped our OS-username
fallback for the same reason. Kept our profile-based ClaudePath /
CodexPath population AND our KAPACITOR_CLAUDE_PATH /
KAPACITOR_CODEX_PATH env overrides. Kept main's DaemonNameResolver
call with its arg-validation try/catch.
- src/kapacitor/Commands/CodexHookCommand.cs — kept our dispatcher
(HandlePermissionRequestStub vs HandlePermissionRequestViaBridge) and
rewrote the stub branch to match AI-636: bounded 2s POST to
/hooks/permission-record, emit "{}" so Codex's in-CLI approval prompt
asks the user. Updated the class-level remarks comment to describe
both branches.
- src/kapacitor/Commands/PluginCommand.cs — dropped our local
CodexHookEvents and EntryReferencesKapacitorCodexHook (now in
CodexHooksParser per Task 2). Kept main's new CodexSkillNames,
InstallCodexSkills, RemoveCodexSkills, CopyDirectory from AI-629
(skills install). Kept our PermissionRequestTimeout = 86400 from the
P1 review fix.
- test/kapacitor.Tests.Unit/CodexHookCommandTests.cs — collapsed our
PermissionRequestGroup and main's ConsoleSerialGroup into one shared
ConsoleSerialGroup. Updated every NotInParallel(PermissionRequestGroup)
attribute to the new name. Took main's record-and-yield assertions for
the no-daemon-URL case (was: assert allow-stub, now: assert no
decision) AND kept our env-var isolation pattern. Dropped my old
PermissionRequest_without_daemon_url_still_uses_legacy_stub test —
AI-636's PermissionRequest_records_event_and_yields_decision_to_codex
now covers the stub branch with the correct new contract.
Verified: 719 unit + 9 integration tests green, AOT clean on both
kapacitor.csproj and Kapacitor.Daemon.csproj.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eSystemOverlay against symlinks Qodo review findings #2 (empty HOME breaks paths) and #3 (overlay follows symlinks): - PathHelpers.HomeDirectory now treats HOME as missing when it's empty, whitespace, or a non-rooted relative path — falling back to Environment.SpecialFolder.UserProfile in all three cases. Previously HOME="" or HOME="foo" would silently produce a relative HomeDirectory, causing daemon/CLI config reads/writes to land in CWD instead of the user profile. - FileSystemOverlay.OverlayDirectory passes EnumerationOptions { AttributesToSkip = FileAttributes.ReparsePoint } to both GetFiles and GetDirectories. Symlinks (files and dirs) and other reparse points are now skipped during overlay, preventing symlink-loop recursion and accidental copy of out-of-repo content into the worktree. Both ClaudeLauncher and CodexLauncher use this helper to merge .claude/.codex from the source repo into the spawned worktree. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fix in-suite Console.SetOut race Both tests use Console.SetOut to capture stdout. Stop_maps_to_session_end_codex_route takes ~3m20s (real WatcherManager path through InlineDrainAsync + WhatsDoneGenerator spawn against an absent transcript), holding Console.Out captured for the full duration. With the suite at 727 tests, the long capture window plus TUnit's parallel scheduling drops captured output for the other ConsoleSerialGroup test intermittently — both pass in isolation, fail together in suite. Tightening from NotInParallel(ConsoleSerialGroup) (group-scoped) to plain NotInParallel (globally sequential) for these two tests guarantees they don't overlap with any other test, removing the race surface. Other ConsoleSerialGroup tests in the file (the new PermissionRequest_with_daemon_url_* set from AI-68) keep their group-scoped attribute — they're fast and serializing only against each other is sufficient.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
vendor: codexsupport to the daemon's hosted-agent path. The dashboard can now launch a Codex CLI under PTY supervision with the same overlay / pre-trust / permission-bridge guarantees Claude already has on macOS and Linux.AgentOrchestratorto dispatch through anIHostedAgentLauncherstrategy (ClaudeLauncher+CodexLauncher). Claude behavior is preserved; Codex is new.LocalPermissionBridgeURLs by vendor (/{token}/{vendor}/permission-request) with vendor-shaped responses; Codex hook bounces fail-closed through the daemon.Scope: CLI / daemon repo only. Paired server-repo PR in
kapacitor-server(new requiredVendorfield onDaemonCommands.LaunchAgentCommand, 5-argRequestPermission, 6-argPermissionRequestedbroadcast,ClaudeCodePermissionExtension.Vendor, dashboard vendor selector, vendor-aware permission UI) ships together — staging is single-controlled and the components upgrade in lockstep. No back-compat shims.Design + plan:
docs/superpowers/specs/2026-05-14-ai-68-codex-hosted-agents-design.mdanddocs/superpowers/plans/2026-05-14-ai-68-codex-hosted-agents.md.Follow-ups: AI-632 (PR review for Codex), AI-633 (sandbox/approval selector in launch dialog), AI-72 (Windows hosted Codex).
Test plan
Key changes
Wire format
Strategy abstraction
Codex pre-trust
Permission bridge
CLI hooks
Config / DI
Docs
Tests