From bacc9b1767752dff8079921d53e0707befbd925a Mon Sep 17 00:00:00 2001 From: realtonyyoung <6655045+realtonyyoung@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:06:30 -0400 Subject: [PATCH 1/8] feat: CodexLaunchPosture wire block + CodexPosturePolicy validation/resolution --- src/Capacitor.Cli.Core/Models.cs | 32 ++++- .../Services/CodexPosturePolicy.cs | 74 ++++++++++ .../Codex/CodexPosturePolicyTests.cs | 136 ++++++++++++++++++ 3 files changed, 239 insertions(+), 3 deletions(-) create mode 100644 src/Capacitor.Cli.Daemon/Services/CodexPosturePolicy.cs create mode 100644 test/Capacitor.Cli.Tests.Unit/Codex/CodexPosturePolicyTests.cs diff --git a/src/Capacitor.Cli.Core/Models.cs b/src/Capacitor.Cli.Core/Models.cs index f45f5bbc6..4da570ddc 100644 --- a/src/Capacitor.Cli.Core/Models.cs +++ b/src/Capacitor.Cli.Core/Models.cs @@ -1298,9 +1298,22 @@ public readonly record struct LaunchAgentCommand( // wire-compatible with older daemons (ignore it) and older servers (never set it). The daemon // launches with the exact LaunchModel VERBATIM and, post-launch, reports the concrete resolved // model back keyed on LaunchAttemptId (see ExplicitReviewerModelResolvedV1). - ExplicitReviewerModelLaunch? ExplicitReviewerModel = null + ExplicitReviewerModelLaunch? ExplicitReviewerModel = null, + // Caller-selected Codex sandbox/approval posture for an INTERACTIVE daemon-owned-worktree + // launch; null preserves the derived behaviour byte-for-byte. Supplied for any other launch + // shape (review-flow, PR review, borrowed cwd, non-codex vendor) the daemon fails closed + // with a coded LaunchFailed — never silently ignored, never honoured. Appended last as an + // optional field so the SignalR positional/name binding stays wire-compatible with older + // daemons (ignore it) and older servers (never set it). + CodexLaunchPosture? CodexPosture = null ); +/// Caller-selected Codex launch posture. Valid ONLY for interactive, daemon-owned-worktree +/// launches ( and not borrowed); the daemon fails closed on any other +/// launch shape. Both fields are required — a partial block is malformed. Values are the Codex CLI's +/// own lowercase tokens, compared ordinally (see the daemon's CodexPosturePolicy). +public sealed record CodexLaunchPosture(string Sandbox, string Approval); + public sealed record ReviewerCertificationRequirement( string Vendor, string AllowedCliRanges, @@ -1747,7 +1760,12 @@ public sealed record UnattendedVendorCapability( bool SupportsReviewerModelResolution = false, // This vendor's reviewer-model policy version (distinct from LauncherPolicyVersion); the server // echoes it to detect a mid-flight policy change. Null when no resolver is advertised. - string? ReviewerModelPolicyVersion = null + string? ReviewerModelPolicyVersion = null, + // Whether this daemon accepts a caller-selected launch posture for this vendor + // (CodexLaunchPosture on LaunchAgentCommand). Defaults false — a legacy or mid-rollout daemon is + // never widened to "supported" by any fallback, so the server refuses posture selection rather + // than sending a block that would be silently ignored. + bool SupportsLaunchPosture = false ); public readonly record struct AgentRegistered( @@ -1755,7 +1773,15 @@ public readonly record struct AgentRegistered( string? Prompt, string? Model, string? Effort, - string? RepoPath + string? RepoPath, + // Applied Codex posture echo: the pair actually passed to --sandbox / --ask-for-approval, + // non-null ONLY for a hosted interactive Codex launch (Kind == Default, owned worktree), + // whether that pair was caller-selected or derived. Review-flow / PR-review / local-attach + // agents and non-codex vendors always report null, so a consumer can render it without any + // launch-kind discriminator. Trailing name-bound fields on a single JSON DTO — an older + // server ignores them, an older daemon never sets them. + string? SandboxPolicy = null, + string? ApprovalPolicy = null ); public readonly record struct AgentStatusChanged( diff --git a/src/Capacitor.Cli.Daemon/Services/CodexPosturePolicy.cs b/src/Capacitor.Cli.Daemon/Services/CodexPosturePolicy.cs new file mode 100644 index 000000000..22740f75a --- /dev/null +++ b/src/Capacitor.Cli.Daemon/Services/CodexPosturePolicy.cs @@ -0,0 +1,74 @@ +using Capacitor.Cli.Core; +using Capacitor.Cli.Core.LocalIpc; + +namespace Capacitor.Cli.Daemon.Services; + +/// +/// Single source of truth for the caller-selected Codex launch posture: eligibility + token +/// validation (, consumed by the orchestrator's pre-flight guard) and +/// effective resolution (, consumed by +/// AND by the launch path that stamps the applied pair onto the AgentInstance, so the registration +/// echo can never diverge from the argv). +/// +/// Selection applies ONLY to interactive daemon-owned-worktree launches. A borrowed cwd is the +/// user's real checkout and is always read-only; a review-flow reviewer is unattended and always +/// runs with approvals off. Those two values are containment guarantees, not preferences, so a +/// posture supplied for either fails the launch rather than being silently dropped or honoured. +/// +internal static class CodexPosturePolicy { + static readonly HashSet Sandboxes = new(StringComparer.Ordinal) { + "read-only", "workspace-write", "danger-full-access" + }; + + static readonly HashSet Approvals = new(StringComparer.Ordinal) { + "untrusted", "on-request", "never" + }; + + /// Coded rejection for an ineligible or malformed posture block; null when the + /// launch may continue — which includes every posture-less launch, whatever its shape. + public static string? RejectionReason(LaunchAgentCommand cmd) { + if (cmd.CodexPosture is not { } posture) return null; + + if (!string.Equals(cmd.Vendor, "codex", StringComparison.OrdinalIgnoreCase)) + return $"codex_posture_wrong_vendor: a Codex launch posture was supplied for vendor '{cmd.Vendor}' — " + + "the posture block applies only to Codex launches."; + + if (cmd.Kind == LaunchKind.ReviewFlow) + return "codex_posture_not_overridable: review-flow reviewers run unattended, so their approval " + + "policy is daemon-derived and cannot be overridden."; + + if (cmd.Kind == LaunchKind.Review) + return "codex_posture_not_overridable: PR-review launches use a fixed posture; selection applies " + + "only to interactive launches."; + + if (cmd.Borrowed) + return "codex_posture_not_overridable: a borrowed launch runs in the requester's real checkout and " + + "is always read-only, so its posture is daemon-derived and cannot be overridden."; + + if (string.IsNullOrWhiteSpace(posture.Sandbox) || string.IsNullOrWhiteSpace(posture.Approval)) + return "codex_posture_invalid: a posture block must carry both a sandbox and an approval policy."; + + if (!Sandboxes.Contains(posture.Sandbox)) + return $"codex_posture_invalid: unknown sandbox '{posture.Sandbox}'. " + + "Valid values: read-only, workspace-write, danger-full-access."; + + if (string.Equals(posture.Approval, "on-failure", StringComparison.Ordinal)) + return "codex_posture_invalid: approval policy 'on-failure' is deprecated upstream and not supported."; + + if (!Approvals.Contains(posture.Approval)) + return $"codex_posture_invalid: unknown approval policy '{posture.Approval}'. " + + "Valid values: untrusted, on-request, never."; + + return null; + } + + /// The effective sandbox/approval pair: the selected posture when one is present, else + /// the derived containment defaults. Callers establish eligibility via + /// before passing a non-null posture. + public static (string Sandbox, string Approval) Resolve( + WorkLocation work, bool isReviewFlow, CodexLaunchPosture? posture) => + posture is not null + ? (posture.Sandbox, posture.Approval) + : (work == WorkLocation.BorrowedCwd ? "read-only" : "workspace-write", + isReviewFlow ? "never" : "on-request"); +} diff --git a/test/Capacitor.Cli.Tests.Unit/Codex/CodexPosturePolicyTests.cs b/test/Capacitor.Cli.Tests.Unit/Codex/CodexPosturePolicyTests.cs new file mode 100644 index 000000000..722dd88f7 --- /dev/null +++ b/test/Capacitor.Cli.Tests.Unit/Codex/CodexPosturePolicyTests.cs @@ -0,0 +1,136 @@ +using Capacitor.Cli.Core; +using Capacitor.Cli.Core.LocalIpc; +using Capacitor.Cli.Daemon.Services; + +namespace Capacitor.Cli.Tests.Unit.Codex; + +public class CodexPosturePolicyTests { + static LaunchAgentCommand Cmd( + string vendor = "codex", + LaunchKind kind = LaunchKind.Default, + bool borrowed = false, + CodexLaunchPosture? posture = null + ) => new( + "agent1", "prompt", "default", null, "/repo", null, null, vendor, + Kind: kind, Borrowed: borrowed, CodexPosture: posture + ); + + // === Absent block: every launch shape stays acceptable, unchanged === + + [Test] + public async Task Absent_block_is_accepted_for_every_launch_shape() { + await Assert.That(CodexPosturePolicy.RejectionReason(Cmd())).IsNull(); + await Assert.That(CodexPosturePolicy.RejectionReason(Cmd(kind: LaunchKind.ReviewFlow))).IsNull(); + await Assert.That(CodexPosturePolicy.RejectionReason(Cmd(kind: LaunchKind.Review))).IsNull(); + await Assert.That(CodexPosturePolicy.RejectionReason(Cmd(borrowed: true))).IsNull(); + await Assert.That(CodexPosturePolicy.RejectionReason(Cmd(vendor: "claude"))).IsNull(); + } + + // === The selectable case: every supported sandbox x approval combination === + + [Test] + [Arguments("read-only", "untrusted")] + [Arguments("read-only", "on-request")] + [Arguments("read-only", "never")] + [Arguments("workspace-write", "untrusted")] + [Arguments("workspace-write", "on-request")] + [Arguments("workspace-write", "never")] + [Arguments("danger-full-access", "untrusted")] + [Arguments("danger-full-access", "on-request")] + [Arguments("danger-full-access", "never")] + public async Task All_nine_combinations_are_accepted_on_interactive_owned_launches(string sandbox, string approval) { + await Assert.That(CodexPosturePolicy.RejectionReason(Cmd(posture: new(sandbox, approval)))).IsNull(); + } + + // === Fail-closed eligibility: the containment invariants === + + [Test] + public async Task Posture_on_review_flow_fails_closed_with_coded_reason() { + var reason = CodexPosturePolicy.RejectionReason( + Cmd(kind: LaunchKind.ReviewFlow, posture: new("workspace-write", "on-request"))); + + await Assert.That(reason).StartsWith("codex_posture_not_overridable:"); + await Assert.That(reason).Contains("review-flow"); + } + + [Test] + public async Task Posture_on_pr_review_fails_closed_with_coded_reason() { + var reason = CodexPosturePolicy.RejectionReason( + Cmd(kind: LaunchKind.Review, posture: new("read-only", "on-request"))); + + await Assert.That(reason).StartsWith("codex_posture_not_overridable:"); + await Assert.That(reason).Contains("PR-review"); + } + + [Test] + public async Task Posture_on_borrowed_launch_fails_closed_with_coded_reason() { + var reason = CodexPosturePolicy.RejectionReason( + Cmd(borrowed: true, posture: new("read-only", "never"))); + + await Assert.That(reason).StartsWith("codex_posture_not_overridable:"); + await Assert.That(reason).Contains("borrowed"); + } + + [Test] + public async Task Posture_on_non_codex_vendor_fails_closed_with_coded_reason() { + var reason = CodexPosturePolicy.RejectionReason( + Cmd(vendor: "claude", posture: new("read-only", "never"))); + + await Assert.That(reason).StartsWith("codex_posture_wrong_vendor:"); + } + + /// A borrowed review-flow launch violates BOTH invariants; the reason names one of them + /// rather than reporting nothing. + [Test] + public async Task Posture_on_borrowed_review_flow_still_fails_closed() { + var reason = CodexPosturePolicy.RejectionReason( + Cmd(kind: LaunchKind.ReviewFlow, borrowed: true, posture: new("workspace-write", "never"))); + + await Assert.That(reason).StartsWith("codex_posture_not_overridable:"); + } + + // === Token validation === + + [Test] + [Arguments("full-access", "on-request")] // unknown sandbox + [Arguments("Read-Only", "on-request")] // mixed case: rejected, never normalized + [Arguments("read-only", "Never")] // mixed case on the approval axis + [Arguments("workspace-write", "on-failure")] // deprecated upstream + [Arguments("workspace-write", "always")] // unknown approval + [Arguments("", "on-request")] // partial: empty sandbox + [Arguments("read-only", "")] // partial: empty approval + [Arguments(" ", "on-request")] // partial: whitespace sandbox + public async Task Invalid_tokens_and_partial_blocks_are_rejected(string sandbox, string approval) { + var reason = CodexPosturePolicy.RejectionReason(Cmd(posture: new(sandbox, approval))); + + await Assert.That(reason).StartsWith("codex_posture_invalid:"); + } + + [Test] + public async Task On_failure_rejection_names_the_upstream_deprecation() { + var reason = CodexPosturePolicy.RejectionReason(Cmd(posture: new("workspace-write", "on-failure"))); + + await Assert.That(reason).Contains("deprecated"); + } + + // === Effective resolution === + + [Test] + public async Task Resolve_returns_selected_pair_when_posture_present() { + var pair = CodexPosturePolicy.Resolve(WorkLocation.OwnedWorktree, false, new("read-only", "never")); + + await Assert.That(pair).IsEqualTo(("read-only", "never")); + } + + [Test] + public async Task Resolve_returns_derived_pairs_when_posture_absent() { + await Assert.That(CodexPosturePolicy.Resolve(WorkLocation.OwnedWorktree, false, null)) + .IsEqualTo(("workspace-write", "on-request")); + await Assert.That(CodexPosturePolicy.Resolve(WorkLocation.OwnedWorktree, true, null)) + .IsEqualTo(("workspace-write", "never")); + await Assert.That(CodexPosturePolicy.Resolve(WorkLocation.BorrowedCwd, true, null)) + .IsEqualTo(("read-only", "never")); + await Assert.That(CodexPosturePolicy.Resolve(WorkLocation.BorrowedCwd, false, null)) + .IsEqualTo(("read-only", "on-request")); + } +} From 33503f2d20e089cfcd7d2888cfe8699589319a0f Mon Sep 17 00:00:00 2001 From: realtonyyoung <6655045+realtonyyoung@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:11:06 -0400 Subject: [PATCH 2/8] feat: fail-closed pre-flight guard for a caller-selected Codex posture --- .../Services/AgentOrchestrator.cs | 11 ++ .../AgentOrchestratorVendorTests.cs | 110 ++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs index b11526c5b..6d078b614 100644 --- a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs +++ b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs @@ -916,6 +916,17 @@ await _server.LaunchFailedAsync(cmd.AgentId, return new CommandOutcome(CommandOutcomeKind.LaunchRejected, agentId, RejectReason: CommandRejectedReason.Semantic); } + // A caller-selected Codex posture is validated here — before the repo checks and the worktree + // creation below, so a rejection leaves nothing to clean up. Selection is only ever valid for + // an interactive daemon-owned-worktree launch: a posture on a borrowed, review-flow or + // PR-review launch would defeat a containment invariant, so it fails the launch outright + // rather than being silently dropped or silently honoured. + if (CodexPosturePolicy.RejectionReason(cmd) is { } postureRejection) { + await _server.LaunchFailedAsync(cmd.AgentId, postureRejection); + + return new CommandOutcome(CommandOutcomeKind.LaunchRejected, agentId, RejectReason: CommandRejectedReason.Semantic); + } + if (isReviewFlow && cmd.ReviewerCertification is { } certification) { var version = string.Equals(cmd.Vendor, "claude", StringComparison.Ordinal) ? DaemonRunner.ProbeCliVersionForLaunch(_config.ClaudePath) diff --git a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs index ce32a7753..e2b71e4a7 100644 --- a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs @@ -169,6 +169,116 @@ public async Task Launch_with_unknown_vendor_emits_launch_failed_and_does_not_sp await Assert.That(ptyFactory.SpawnCalls).IsEqualTo(0); } + // ── Caller-selected Codex posture: fail-closed pre-flight guard ────────────────────────── + // Every case below uses a repo path that is NOT allowed and does NOT exist, so the assertions + // are position-sensitive: the posture guard must run BEFORE the repo-path checks (and therefore + // before any worktree work). Move the guard later and the reason becomes "Repo path …" and these + // tests fail — which is exactly the regression they exist to catch. + + static LaunchAgentCommand PostureCmd( + string agentId, + CodexLaunchPosture? posture, + string vendor = "codex", + LaunchKind kind = LaunchKind.Default, + bool borrowed = false + ) => new( + AgentId: agentId, + Prompt: "hi", + Model: "default", + Effort: null, + RepoPath: "/tmp/kcap-posture-guard-nonexistent", + Tools: null, + AttachmentIds: null, + Vendor: vendor, + Kind: kind, + Borrowed: borrowed, + CodexPosture: posture + ); + + static AgentOrchestrator BuildPostureOrchestrator(CaptureServerConnection server, SpyPtyProcessFactory ptyFactory) => + BuildOrchestrator( + server, ptyFactory, new Dictionary(), + extraRuntimeFactories: [ + new SpyHostedAgentRuntimeFactory("codex") { SupportsUnattended = true, SupportsBorrowedReviewFlow = true }, + new SpyHostedAgentRuntimeFactory("claude") { SupportsUnattended = true } + ]); + + [Test] + public async Task Posture_on_review_flow_launch_is_rejected_before_any_worktree_work() { + var server = new CaptureServerConnection(); + var ptyFactory = new SpyPtyProcessFactory(); + + await using var orch = BuildPostureOrchestrator(server, ptyFactory); + + await orch.HandleLaunchAgentForTest( + PostureCmd("agent-posture-flow", new("workspace-write", "on-request"), kind: LaunchKind.ReviewFlow)); + + await Assert.That(server.LaunchFailedCalls.Count).IsEqualTo(1); + await Assert.That(server.LaunchFailedCalls[0].AgentId).IsEqualTo("agent-posture-flow"); + await Assert.That(server.LaunchFailedCalls[0].Reason).StartsWith("codex_posture_not_overridable:"); + await Assert.That(ptyFactory.SpawnCalls).IsEqualTo(0); + } + + [Test] + public async Task Posture_on_borrowed_launch_is_rejected_before_any_worktree_work() { + var server = new CaptureServerConnection(); + var ptyFactory = new SpyPtyProcessFactory(); + + await using var orch = BuildPostureOrchestrator(server, ptyFactory); + + await orch.HandleLaunchAgentForTest( + PostureCmd("agent-posture-borrowed", new("read-only", "never"), borrowed: true)); + + await Assert.That(server.LaunchFailedCalls.Count).IsEqualTo(1); + await Assert.That(server.LaunchFailedCalls[0].Reason).StartsWith("codex_posture_not_overridable:"); + await Assert.That(ptyFactory.SpawnCalls).IsEqualTo(0); + } + + [Test] + public async Task Posture_on_a_non_codex_launch_is_rejected() { + var server = new CaptureServerConnection(); + var ptyFactory = new SpyPtyProcessFactory(); + + await using var orch = BuildPostureOrchestrator(server, ptyFactory); + + await orch.HandleLaunchAgentForTest( + PostureCmd("agent-posture-claude", new("read-only", "never"), vendor: "claude")); + + await Assert.That(server.LaunchFailedCalls.Count).IsEqualTo(1); + await Assert.That(server.LaunchFailedCalls[0].Reason).StartsWith("codex_posture_wrong_vendor:"); + await Assert.That(ptyFactory.SpawnCalls).IsEqualTo(0); + } + + [Test] + public async Task Posture_with_an_invalid_token_is_rejected() { + var server = new CaptureServerConnection(); + var ptyFactory = new SpyPtyProcessFactory(); + + await using var orch = BuildPostureOrchestrator(server, ptyFactory); + + await orch.HandleLaunchAgentForTest( + PostureCmd("agent-posture-bad-token", new("workspace-write", "on-failure"))); + + await Assert.That(server.LaunchFailedCalls.Count).IsEqualTo(1); + await Assert.That(server.LaunchFailedCalls[0].Reason).StartsWith("codex_posture_invalid:"); + await Assert.That(ptyFactory.SpawnCalls).IsEqualTo(0); + } + + [Test] + public async Task A_valid_interactive_posture_passes_the_guard() { + // This launch still fails downstream (the repo path is deliberately bogus), but it must get + // PAST the posture guard — proving the guard rejects only ineligible/malformed blocks. + var server = new CaptureServerConnection(); + var ptyFactory = new SpyPtyProcessFactory(); + + await using var orch = BuildPostureOrchestrator(server, ptyFactory); + + await orch.HandleLaunchAgentForTest( + PostureCmd("agent-posture-ok", new("read-only", "never"))); + + await Assert.That(server.LaunchFailedCalls.Any(c => c.Reason.StartsWith("codex_posture_"))).IsFalse(); + } + // Qodo review on #234: a null Vendor (SignalR boundary — non-null annotation not enforced) must // emit LaunchFailed, NOT throw ArgumentNullException from the dictionary lookup (which SafeInvoke // would swallow, dropping the launch silently). From 4be23633faee0a3b7a8bf890fea5c6a0df97ecf4 Mon Sep 17 00:00:00 2001 From: realtonyyoung <6655045+realtonyyoung@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:22:22 -0400 Subject: [PATCH 3/8] feat: applied Codex posture echo on AgentRegistered (AgentInstance-owned, reconnect-safe) + bridge-defeating warning --- .../Services/AgentOrchestrator.cs | 44 +++- .../Services/CodexLauncher.cs | 38 ++-- .../Services/IHostedAgentLauncher.cs | 6 + .../Services/IHostedAgentRuntimeFactory.cs | 6 +- .../Services/PtyHostedAgentRuntimeFactory.cs | 3 +- .../Services/ServerConnection.cs | 9 +- .../AgentOrchestratorLocalAttachTests.cs | 2 +- .../AgentOrchestratorVendorTests.cs | 211 +++++++++++++++++- .../Codex/CodexLauncherTests.cs | 74 ++++++ 9 files changed, 371 insertions(+), 22 deletions(-) diff --git a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs index 6d078b614..8766f8711 100644 --- a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs +++ b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs @@ -41,6 +41,14 @@ CancellationTokenSource ReadCts public string? FlowRunId { get; init; } public string? FlowRole { get; init; } + /// The applied Codex sandbox/approval pair — the values actually passed to the vendor + /// CLI, whether caller-selected or derived. Set only for an interactive Codex launch on a + /// daemon-owned worktree; null everywhere else. Stored HERE (not recomputed at each send) so the + /// initial registration and every reconnect re-registration report the same pair, which is what + /// lets the value survive a server restart. + public string? SandboxPolicy { get; init; } + public string? ApprovalPolicy { get; init; } + /// Phase B (D1): single-flight teardown latch — a plain field (not a property) so /// can gate it. Exactly /// one teardown runs even if the launch-catch and the read-loop's finally race. @@ -1148,7 +1156,8 @@ await _server.LaunchFailedAsync(agentId, Work: work, DaemonId: _daemonId, // Phase B (D4 §6.4(3)): child env markers for the OrphanReaper scan DaemonEpoch: _daemonEpoch, - IsBorrowedSnapshot: snapshotBorrow + IsBorrowedSnapshot: snapshotBorrow, + CodexPosture: cmd.CodexPosture ); HostedRuntimeStart start; @@ -1191,11 +1200,34 @@ await _server.LaunchFailedAsync(agentId, var cts = new CancellationTokenSource(); + // Applied-posture echo, stamped only for an interactive Codex launch on a daemon-owned + // worktree. A review-flow or PR-review launch reports nothing (its posture is the + // containment invariant, not a choice), and so does a borrowed launch — `work` is + // resolved from cmd.Borrowed independently of Kind, so the owned-worktree arm is what + // keeps a borrowed Default command from reporting a posture it never selected. + (string Sandbox, string Approval)? appliedPosture = + string.Equals(cmd.Vendor, "codex", StringComparison.OrdinalIgnoreCase) + && cmd.Kind == LaunchKind.Default + && work == WorkLocation.OwnedWorktree + ? CodexPosturePolicy.Resolve(work, isReviewFlow, cmd.CodexPosture) + : null; + + // The permission bridge can never prompt under `never`, and danger-full-access reaches + // outside the worktree entirely. The dashboard warns the user before launch; this is the + // operator-side record of what actually ran. + if (appliedPosture is { } applied + && (string.Equals(applied.Approval, "never", StringComparison.Ordinal) + || string.Equals(applied.Sandbox, "danger-full-access", StringComparison.Ordinal))) { + LogBridgeDefeatingPosture(agentId, applied.Sandbox, applied.Approval); + } + var agent = new AgentInstance(agentId, prompt, effectiveModel, effort, repoPath, cmd.Vendor, runtime, worktree, cts) { McpConfigPath = mcpConfigPath, CurrentCols = HostedPtyCols, CurrentRows = HostedPtyRows, Work = work, + SandboxPolicy = appliedPosture?.Sandbox, + ApprovalPolicy = appliedPosture?.Approval, ReviewerBridgeToken = reviewerToken, BorrowedSnapshotSource = borrowedSnapshotSource, Kind = cmd.Kind, // Phase B (D2): flow identity + kind for LiveAgents/status report @@ -2194,7 +2226,7 @@ ReviewerModelResolveResponseV1 HandleResolveReviewerModel(ReviewerModelResolveRe async Task RegisterAgentAsync(AgentInstance agent) { if (agent.IsPrivate) return; - await _server.AgentRegisteredAsync(agent.Id, agent.Prompt, agent.Model, agent.Effort, agent.RepoPath); + await _server.AgentRegisteredAsync(agent.Id, agent.Prompt, agent.Model, agent.Effort, agent.RepoPath, agent.SandboxPolicy, agent.ApprovalPolicy); // Report the PTY size so read-only viewers lock their xterm to it. Best-effort. try { @@ -2432,7 +2464,7 @@ async Task ReRegisterAgentsAsync() { foreach (var agent in _agents.Values.Where(a => (a.Status is "Starting" or "Running") && !a.IsPrivate)) { for (var attempt = 1; ; attempt++) { try { - await _server.AgentRegisteredAsync(agent.Id, agent.Prompt, agent.Model, agent.Effort, agent.RepoPath); + await _server.AgentRegisteredAsync(agent.Id, agent.Prompt, agent.Model, agent.Effort, agent.RepoPath, agent.SandboxPolicy, agent.ApprovalPolicy); await _server.AgentStatusChangedAsync(agent.Id, agent.Status, agent.SessionId); // Re-send the fixed PTY dims. The server stores them in memory, so a @@ -2817,6 +2849,12 @@ static string ExtractTerminalText(TerminalOutputBuffer buffer) { [LoggerMessage(Level = LogLevel.Information, Message = "Agent {AgentId} spawned (PID={Pid}, worktree={Worktree}, vendor={Vendor})")] partial void LogAgentSpawned(string agentId, int pid, string worktree, string vendor); + [LoggerMessage( + Level = LogLevel.Warning, + Message = "Interactive Codex agent {AgentId} launched with sandbox={Sandbox} approval={Approval}: " + + "kcap approval prompts will never appear for this agent and/or it can reach outside its worktree")] + partial void LogBridgeDefeatingPosture(string agentId, string sandbox, string approval); + [LoggerMessage(Level = LogLevel.Information, Message = "Agent {AgentId} exited with code {ExitCode}")] partial void LogAgentExited(string agentId, int? exitCode); diff --git a/src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs b/src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs index 0cad80d71..4911cbafc 100644 --- a/src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs +++ b/src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs @@ -16,10 +16,11 @@ ILogger logger public bool SupportsUnattended => true; public bool SupportsBorrowedReviewFlow => true; - // Review-flow launches pass --ask-for-approval never (see BuildArgs); every other launch keeps - // on-request. So approval prompts are off exactly for review-flow launches (any worktree, since - // codex uses `never` regardless of owned/borrowed). - public bool DisablesApprovalPrompts(LauncherContext ctx) => ctx.IsReviewFlow; + // Approval prompts are off for review-flow launches (always `never`, any worktree) and for an + // interactive launch whose caller-selected posture chose `never`. In both cases no dialog an + // Enter could accept can appear, which is what gates the PTY submit strategy. + public bool DisablesApprovalPrompts(LauncherContext ctx) => + ctx.IsReviewFlow || string.Equals(ctx.CodexPosture?.Approval, "never", StringComparison.Ordinal); public string BorrowedReviewContainment => "native-tool-clamp"; /// Codex owns its own reviewer-model policy (known OpenAI/Codex slug families → @@ -107,20 +108,27 @@ public LaunchArgs BuildArgs(LauncherContext ctx) { return BuildReviewArgs(ctx, launch); } + // Re-assert the orchestrator's pre-flight guard. A posture here means that guard was + // bypassed, and honouring it would break a containment invariant — a borrowed reviewer runs + // in the user's REAL checkout, and a review-flow reviewer has no human to answer a prompt. + if (ctx.CodexPosture is not null && (ctx.Work == WorkLocation.BorrowedCwd || ctx.IsReviewFlow)) { + throw new InvalidOperationException( + "codex_posture_not_overridable: a launch posture reached a borrowed or review-flow launch"); + } + + // Selected pair for an interactive owned-worktree launch; otherwise the derived containment + // values — borrowed cwd is read-only (read-only is proven in the headless runner, including + // with MCP injection, so the flow-result tool still works), and a review-flow reviewer never + // pauses for approval while an interactive agent keeps the user in the loop. + var (sandbox, approval) = CodexPosturePolicy.Resolve(ctx.Work, ctx.IsReviewFlow, ctx.CodexPosture); + var args = new List { "--cd", ctx.Worktree.Path, "--sandbox", - // A borrowed reviewer runs in the user's REAL checkout (not a daemon-owned, - // throwaway worktree) — workspace-write would let it mutate that real repo. - // read-only is already proven in the headless runner (CodexCliRunner), including - // with MCP injection, so the flow-result MCP tool still works. - ctx.Work == WorkLocation.BorrowedCwd ? "read-only" : "workspace-write", - // Review-flow reviewers (LaunchKind.ReviewFlow) run unattended → never pause for - // approval (writes stay confined by the workspace-write sandbox). Interactive rendered - // agents keep the default on-request approval so the user stays in the loop. + sandbox, "--ask-for-approval", - ctx.IsReviewFlow ? "never" : "on-request" + approval }; // Review-flow reviewers get exactly ONE MCP server: kcap-flow-result (+ any @@ -322,6 +330,10 @@ static void AddModelArg(List args, string? model) { /// ephemeral `-c` overrides (no ~/.codex/config.toml mutation, nothing to clean /// up), and pass the rendered review prompt as Codex's initial prompt (Codex has /// no --system-prompt equivalent). + /// + /// Sandbox/approval stay FIXED here by design: the PR-review path sits outside the + /// caller-posture seam, and a posture supplied with this launch kind is rejected upstream by + /// CodexPosturePolicy rather than reaching this method. static LaunchArgs BuildReviewArgs(LauncherContext ctx, ReviewLaunchBuilder.ReviewLaunch launch) { const string serverName = "kcap-review"; var mcp = launch.Mcp; diff --git a/src/Capacitor.Cli.Daemon/Services/IHostedAgentLauncher.cs b/src/Capacitor.Cli.Daemon/Services/IHostedAgentLauncher.cs index ff148d29e..9886d6eff 100644 --- a/src/Capacitor.Cli.Daemon/Services/IHostedAgentLauncher.cs +++ b/src/Capacitor.Cli.Daemon/Services/IHostedAgentLauncher.cs @@ -112,6 +112,12 @@ internal sealed record LauncherContext( /// MCP config, stripping any flow-starting server regardless of listing. Null/local-spawn /// launches (e.g. kcap agent start) never set this. public string[]? McpAllowlist { get; init; } + + /// Caller-selected Codex sandbox/approval posture. Non-null only for an interactive + /// daemon-owned-worktree launch whose block already passed the orchestrator's + /// CodexPosturePolicy guard; null for every other launch, which keeps the derived + /// containment values. + public CodexLaunchPosture? CodexPosture { get; init; } } internal readonly record struct LaunchArgs(string[] Args, string? McpConfigPath); diff --git a/src/Capacitor.Cli.Daemon/Services/IHostedAgentRuntimeFactory.cs b/src/Capacitor.Cli.Daemon/Services/IHostedAgentRuntimeFactory.cs index a049782cb..8388931ec 100644 --- a/src/Capacitor.Cli.Daemon/Services/IHostedAgentRuntimeFactory.cs +++ b/src/Capacitor.Cli.Daemon/Services/IHostedAgentRuntimeFactory.cs @@ -129,5 +129,9 @@ internal sealed record RuntimeStartContext( IReadOnlyList? McpServers = null, // True only when a borrowed request has been materialized into a fully independent, // daemon-owned repository snapshot. Factories use this to revalidate exact artifacts. - bool IsBorrowedSnapshot = false + bool IsBorrowedSnapshot = false, + // Caller-selected Codex sandbox/approval posture, carried verbatim from + // LaunchAgentCommand.CodexPosture through to LauncherContext.CodexPosture. Non-null only for + // an interactive daemon-owned-worktree Codex launch that passed the orchestrator's guard. + CodexLaunchPosture? CodexPosture = null ); diff --git a/src/Capacitor.Cli.Daemon/Services/PtyHostedAgentRuntimeFactory.cs b/src/Capacitor.Cli.Daemon/Services/PtyHostedAgentRuntimeFactory.cs index f1b371a9e..aa1b80c09 100644 --- a/src/Capacitor.Cli.Daemon/Services/PtyHostedAgentRuntimeFactory.cs +++ b/src/Capacitor.Cli.Daemon/Services/PtyHostedAgentRuntimeFactory.cs @@ -64,7 +64,8 @@ public async Task StartAsync(RuntimeStartContext ctx, Cancel : null ) { McpAllowlist = ctx.McpAllowlist, - Work = ctx.Work + Work = ctx.Work, + CodexPosture = ctx.CodexPosture }; try { diff --git a/src/Capacitor.Cli.Daemon/Services/ServerConnection.cs b/src/Capacitor.Cli.Daemon/Services/ServerConnection.cs index 754e8198e..d67f36e85 100644 --- a/src/Capacitor.Cli.Daemon/Services/ServerConnection.cs +++ b/src/Capacitor.Cli.Daemon/Services/ServerConnection.cs @@ -732,8 +732,13 @@ public virtual async Task UpdateRepoPathsAsync() { } // Outgoing messages to server - public virtual Task AgentRegisteredAsync(string agentId, string? prompt, string? model, string? effort, string? repoPath) - => _hub.InvokeAsync("AgentRegistered", new AgentRegistered(agentId, prompt, model, effort, repoPath), cancellationToken: _ct); + public virtual Task AgentRegisteredAsync( + string agentId, string? prompt, string? model, string? effort, string? repoPath, + string? sandboxPolicy = null, string? approvalPolicy = null) + => _hub.InvokeAsync( + "AgentRegistered", + new AgentRegistered(agentId, prompt, model, effort, repoPath, sandboxPolicy, approvalPolicy), + cancellationToken: _ct); /// /// Reports the hosted agent's fixed PTY dimensions to the server, which stores diff --git a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorLocalAttachTests.cs b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorLocalAttachTests.cs index 2d1002b2c..d67462cfa 100644 --- a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorLocalAttachTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorLocalAttachTests.cs @@ -711,7 +711,7 @@ sealed class TripwireServerConnection() : ServerConnection( public override Task SendTerminalDimensionsAsync(string agentId, int cols, int rows) { LastDims = (cols, rows); Calls.Add(nameof(SendTerminalDimensionsAsync)); return Task.CompletedTask; } public override Task LaunchFailedAsync(string agentId, string reason) { Calls.Add(nameof(LaunchFailedAsync)); return Task.CompletedTask; } - public override Task AgentRegisteredAsync(string agentId, string? prompt, string? model, string? effort, string? repoPath) { Calls.Add(nameof(AgentRegisteredAsync)); return Task.CompletedTask; } + public override Task AgentRegisteredAsync(string agentId, string? prompt, string? model, string? effort, string? repoPath, string? sandboxPolicy = null, string? approvalPolicy = null) { Calls.Add(nameof(AgentRegisteredAsync)); return Task.CompletedTask; } public override Task AgentStatusChangedAsync(string agentId, string status, string? sessionId) { Calls.Add(nameof(AgentStatusChangedAsync)); return Task.CompletedTask; } public override Task AgentUnregisteredAsync(string agentId) { Calls.Add(nameof(AgentUnregisteredAsync)); return Task.CompletedTask; } public override Task UpdateRepoPathsAsync() { Calls.Add(nameof(UpdateRepoPathsAsync)); return Task.CompletedTask; } diff --git a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs index e2b71e4a7..a4ded5f91 100644 --- a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs @@ -279,6 +279,208 @@ await orch.HandleLaunchAgentForTest( await Assert.That(server.LaunchFailedCalls.Any(c => c.Reason.StartsWith("codex_posture_"))).IsFalse(); } + // ── Applied-posture echo on registration ──────────────────────────────────────────────── + // The echo is stamped on the AgentInstance so the initial registration AND every reconnect + // re-registration report the same pair. It exists only for an interactive Codex launch on a + // daemon-owned worktree; every other launch shape reports nulls, which is what lets a consumer + // render it without any launch-kind discriminator. + + async Task<(CaptureServerConnection Server, SpyHostedAgentRuntimeFactory Codex)> LaunchForEchoAsync( + string repoPath, + string agentId, + CodexLaunchPosture? posture, + LaunchKind kind = LaunchKind.Default, + bool borrowed = false, + string vendor = "codex", + ILogger? logger = null) { + var server = new CaptureServerConnection(); + var ptyFactory = new SpyPtyProcessFactory(); + var codexSpy = new SpyHostedAgentRuntimeFactory("codex") { + EmitsTerminalOutput = false, SupportsUnattended = true, SupportsBorrowedReviewFlow = true + }; + var claudeSpy = new SpyHostedAgentRuntimeFactory("claude") { + EmitsTerminalOutput = false, SupportsUnattended = true + }; + + // No explicit allowlist: an empty AllowedRepoPaths allows every local git repo, which is what + // the borrow tests rely on too — a textual allowlist entry would not match the canonicalized + // (symlink-resolved) cwd the borrow authorizer compares against. + await using var orch = BuildOrchestrator( + server, ptyFactory, new Dictionary(), + extraRuntimeFactories: [codexSpy, claudeSpy], logger: logger); + + await orch.HandleLaunchAgentForTest(new LaunchAgentCommand( + AgentId: agentId, + Prompt: "do a thing", + Model: "default", + Effort: null, + RepoPath: repoPath, + Tools: null, + AttachmentIds: null, + Vendor: vendor, + Kind: kind, + Borrowed: borrowed, + BorrowCwd: borrowed ? repoPath : null, + CodexPosture: posture)); + + return (server, codexSpy); + } + + [Test] + public async Task Interactive_codex_launch_echoes_the_selected_posture_on_registration() { + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var (server, _) = await LaunchForEchoAsync(repoPath, "agent-echo-selected", new("read-only", "never")); + + await Assert.That(server.AgentRegisteredPostures).Contains(("agent-echo-selected", "read-only", "never")); + } finally { + cleanup(); + } + } + + [Test] + public async Task Interactive_codex_launch_without_a_posture_echoes_the_derived_pair() { + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var (server, _) = await LaunchForEchoAsync(repoPath, "agent-echo-derived", posture: null); + + await Assert.That(server.AgentRegisteredPostures) + .Contains(("agent-echo-derived", "workspace-write", "on-request")); + } finally { + cleanup(); + } + } + + [Test] + public async Task Review_flow_launch_echoes_no_posture() { + // A reviewer's `never` is the containment invariant, not a selection — reporting it would + // make every reviewer look like a user-chosen bridge-defeating launch. + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var (server, _) = await LaunchForEchoAsync( + repoPath, "agent-echo-flow", posture: null, kind: LaunchKind.ReviewFlow); + + await Assert.That(server.AgentRegisteredPostures).Contains(("agent-echo-flow", null, null)); + } finally { + cleanup(); + } + } + + [Test] + public async Task Borrowed_default_launch_echoes_no_posture() { + // `work` is resolved from cmd.Borrowed independently of Kind, so a posture-LESS borrowed + // Default command is accepted — and must still echo nothing rather than a derived pair. + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var (server, _) = await LaunchForEchoAsync( + repoPath, "agent-echo-borrowed", posture: null, borrowed: true); + + await Assert.That(server.AgentRegisteredPostures).Contains(("agent-echo-borrowed", null, null)); + } finally { + cleanup(); + } + } + + [Test] + public async Task Non_codex_launch_echoes_no_posture() { + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var (server, _) = await LaunchForEchoAsync( + repoPath, "agent-echo-claude", posture: null, vendor: "claude"); + + await Assert.That(server.AgentRegisteredPostures).Contains(("agent-echo-claude", null, null)); + } finally { + cleanup(); + } + } + + [Test] + public async Task Reregistration_resends_the_same_applied_posture() { + // A server restart wipes the in-memory echo; the reconnect path rebuilds it from the + // AgentInstance, so the pair must survive rather than silently becoming null. + var server = new CaptureServerConnection(); + var ptyFactory = new SpyPtyProcessFactory(); + + await using var orch = BuildOrchestrator(server, ptyFactory, new Dictionary()); + + orch.RegisterAgentForTest(new AgentInstance( + "agent-rereg-posture", null, "", null, "/tmp", "codex", + new PtyHostedAgentRuntime("codex", new StubPtyProcess()), + new WorktreeInfo("/tmp", "", "/tmp", IsStandalone: true), new CancellationTokenSource() + ) { + SandboxPolicy = "danger-full-access", ApprovalPolicy = "never" + }); + + await server.ReRegisterAgentsHook!(); + + await Assert.That(server.AgentRegisteredPostures) + .Contains(("agent-rereg-posture", "danger-full-access", "never")); + } + + [Test] + [Arguments("workspace-write", "never")] + [Arguments("danger-full-access", "on-request")] + public async Task Bridge_defeating_posture_logs_a_warning(string sandbox, string approval) { + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var logger = new CapturingLogger(); + + await LaunchForEchoAsync(repoPath, "agent-warn", new(sandbox, approval), logger: logger); + + // Matched on the posture warning's own wording — an unrelated warning that merely names + // the agent (e.g. the terminal-dimensions send failure this harness always provokes) + // must not be able to satisfy this. + var postureWarnings = logger.Entries + .Where(e => e.Level == LogLevel.Warning + && e.Message.Contains("agent-warn") + && e.Message.Contains($"sandbox={sandbox}") + && e.Message.Contains($"approval={approval}")) + .ToList(); + await Assert.That(postureWarnings).IsNotEmpty(); + } finally { + cleanup(); + } + } + + [Test] + public async Task A_prompting_posture_logs_no_bridge_warning() { + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var logger = new CapturingLogger(); + + await LaunchForEchoAsync(repoPath, "agent-no-warn", new("read-only", "untrusted"), logger: logger); + + // Scoped to the posture warning's wording: this harness emits unrelated warnings (no real + // server to accept terminal dimensions), so a bare "any warning" assertion would be false. + var postureWarnings = logger.Entries + .Where(e => e.Level == LogLevel.Warning && e.Message.Contains("sandbox=read-only")) + .ToList(); + await Assert.That(postureWarnings).IsEmpty(); + } finally { + cleanup(); + } + } + + sealed class CapturingLogger : ILogger { + public List<(LogLevel Level, string Message)> Entries { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel level, EventId eventId, TState state, Exception? ex, + Func formatter) { + lock (Entries) Entries.Add((level, formatter(state, ex))); + } + } + // Qodo review on #234: a null Vendor (SignalR boundary — non-null annotation not enforced) must // emit LaunchFailed, NOT throw ArgumentNullException from the dictionary lookup (which SafeInvoke // would swallow, dropping the launch silently). @@ -1502,10 +1704,17 @@ public override Task LaunchFailedAsync(string agentId, string reason) { /// LaunchModel, not the dispatched cmd.Model). public List<(string AgentId, string? Model)> AgentRegisteredCalls { get; } = []; - public override Task AgentRegisteredAsync(string agentId, string? prompt, string? model, string? effort, string? repoPath) { + /// The applied Codex posture echoed on each registration, in call order — proves the + /// initial registration and every reconnect re-registration report the same pair. + public List<(string AgentId, string? Sandbox, string? Approval)> AgentRegisteredPostures { get; } = []; + + public override Task AgentRegisteredAsync( + string agentId, string? prompt, string? model, string? effort, string? repoPath, + string? sandboxPolicy = null, string? approvalPolicy = null) { AgentRegisteredCallCount++; lock (AcpCallOrder) AcpCallOrder.Add($"register:{agentId}"); lock (AgentRegisteredCalls) AgentRegisteredCalls.Add((agentId, model)); + lock (AgentRegisteredPostures) AgentRegisteredPostures.Add((agentId, sandboxPolicy, approvalPolicy)); return AgentRegisteredCallCount <= AgentRegisteredFailTimes ? Task.FromException(new InvalidOperationException("transient re-register failure")) diff --git a/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs b/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs index afdeea8a4..16017fb56 100644 --- a/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs @@ -779,4 +779,78 @@ public async Task BuildArgs_owned_worktree_keeps_workspace_write_sandbox() { await Assert.That(args[sIdx + 1]).IsEqualTo("workspace-write"); await Assert.That(args).DoesNotContain("read-only"); } + + // === Caller-selected posture (interactive, daemon-owned worktree) === + + static LauncherContext InteractiveCtx(CodexLaunchPosture? posture) => + NewCtx() with { Work = WorkLocation.OwnedWorktree, CodexPosture = posture }; + + [Test] + [Arguments("read-only", "untrusted")] + [Arguments("read-only", "on-request")] + [Arguments("read-only", "never")] + [Arguments("workspace-write", "untrusted")] + [Arguments("workspace-write", "on-request")] + [Arguments("workspace-write", "never")] + [Arguments("danger-full-access", "untrusted")] + [Arguments("danger-full-access", "on-request")] + [Arguments("danger-full-access", "never")] + public async Task BuildArgs_selected_posture_lands_verbatim_in_argv(string sandbox, string approval) { + var args = NewLauncher().BuildArgs(InteractiveCtx(new(sandbox, approval))).Args; + + var sIdx = Array.IndexOf(args, "--sandbox"); + var aIdx = Array.IndexOf(args, "--ask-for-approval"); + + await Assert.That(args[sIdx + 1]).IsEqualTo(sandbox); + await Assert.That(args[aIdx + 1]).IsEqualTo(approval); + } + + [Test] + public async Task BuildArgs_without_a_posture_is_byte_identical_to_the_derived_argv() { + // The regression that matters most: an absent block must not perturb a single argument. + // NewCtx() never sets CodexPosture, so this compares "today's shape" against an explicit null. + var derived = NewLauncher().BuildArgs(NewCtx() with { Work = WorkLocation.OwnedWorktree }).Args; + var explicitNull = NewLauncher().BuildArgs(InteractiveCtx(null)).Args; + + await Assert.That(explicitNull).IsEquivalentTo(derived, CollectionOrdering.Matching); + + var sIdx = Array.IndexOf(derived, "--sandbox"); + var aIdx = Array.IndexOf(derived, "--ask-for-approval"); + await Assert.That(derived[sIdx + 1]).IsEqualTo("workspace-write"); + await Assert.That(derived[aIdx + 1]).IsEqualTo("on-request"); + } + + [Test] + public async Task BuildArgs_throws_when_a_posture_reaches_a_review_flow_context() { + // Belt-and-braces: the orchestrator guard rejects this upstream, so reaching the launcher + // means the guard was bypassed — fail loudly rather than silently honouring the override. + var ctx = NewCtx(isReviewFlow: true) with { + Work = WorkLocation.OwnedWorktree, CodexPosture = new("workspace-write", "on-request") + }; + + await Assert.That(() => NewLauncher().BuildArgs(ctx)).Throws(); + } + + [Test] + public async Task BuildArgs_throws_when_a_posture_reaches_a_borrowed_context() { + var ctx = NewCtx() with { + Work = WorkLocation.BorrowedCwd, CodexPosture = new("workspace-write", "never") + }; + + await Assert.That(() => NewLauncher().BuildArgs(ctx)).Throws(); + } + + [Test] + public async Task DisablesApprovalPrompts_tracks_the_effective_approval_policy() { + var launcher = NewLauncher(); + + // Selected `never` turns prompts off for an interactive agent... + await Assert.That(launcher.DisablesApprovalPrompts(InteractiveCtx(new("workspace-write", "never")))).IsTrue(); + // ...while any prompting policy leaves the submit strategy alone. + await Assert.That(launcher.DisablesApprovalPrompts(InteractiveCtx(new("workspace-write", "on-request")))).IsFalse(); + await Assert.That(launcher.DisablesApprovalPrompts(InteractiveCtx(new("read-only", "untrusted")))).IsFalse(); + // Unchanged for every posture-less launch. + await Assert.That(launcher.DisablesApprovalPrompts(InteractiveCtx(null))).IsFalse(); + await Assert.That(launcher.DisablesApprovalPrompts(NewCtx(isReviewFlow: true))).IsTrue(); + } } From 21044059f7e36694fbd1fd1294dd9578c42ae534 Mon Sep 17 00:00:00 2001 From: realtonyyoung <6655045+realtonyyoung@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:23:25 -0400 Subject: [PATCH 4/8] feat: advertise SupportsLaunchPosture for codex --- src/Capacitor.Cli.Daemon/DaemonRunner.cs | 5 ++++- .../DaemonRunnerCursorAvailabilityTests.cs | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Capacitor.Cli.Daemon/DaemonRunner.cs b/src/Capacitor.Cli.Daemon/DaemonRunner.cs index 91afa9c97..23c9ae65e 100644 --- a/src/Capacitor.Cli.Daemon/DaemonRunner.cs +++ b/src/Capacitor.Cli.Daemon/DaemonRunner.cs @@ -628,7 +628,10 @@ internal static IReadOnlyList ComputeUnattendedVendo borrowedSupported, borrowedSupported ? factory.BorrowedReviewContainment : null, SupportsReviewerModelResolution: modelResolver is not null, - ReviewerModelPolicyVersion: modelResolver?.PolicyVersion)); + ReviewerModelPolicyVersion: modelResolver?.PolicyVersion, + // Caller-selected launch posture, advertised per vendor rather than per platform — + // the seam is platform-neutral, and only the Codex launcher honours a posture block. + SupportsLaunchPosture: string.Equals(vendor, "codex", StringComparison.Ordinal))); } return capabilities; } diff --git a/test/Capacitor.Cli.Tests.Unit/Daemon/DaemonRunnerCursorAvailabilityTests.cs b/test/Capacitor.Cli.Tests.Unit/Daemon/DaemonRunnerCursorAvailabilityTests.cs index bfe846f0e..535dd2588 100644 --- a/test/Capacitor.Cli.Tests.Unit/Daemon/DaemonRunnerCursorAvailabilityTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Daemon/DaemonRunnerCursorAvailabilityTests.cs @@ -146,6 +146,23 @@ await Assert.That(claude.LauncherPolicyVersion) await Assert.That(claude.BorrowedReviewSupported).IsFalse(); } + /// Launch-posture support is advertised per vendor: Codex accepts a caller-selected + /// sandbox/approval block, every other vendor does not. The server refuses posture selection + /// unless this is explicitly true, so a wrong default here would either silently drop a + /// selection or offer one no launcher honours. + [Test] + public async Task ComputeUnattendedVendorCapabilities_AdvertisesLaunchPostureForCodexOnly() { + IHostedAgentRuntimeFactory[] factories = [ + new FakeRuntimeFactory("claude", isAvailable: true, supportsUnattended: true), + new FakeRuntimeFactory("codex", isAvailable: true, supportsUnattended: true), + ]; + + var capabilities = DaemonRunner.ComputeUnattendedVendorCapabilities(factories, new DaemonConfig()); + + await Assert.That(capabilities.Single(c => c.Vendor == "codex").SupportsLaunchPosture).IsTrue(); + await Assert.That(capabilities.Single(c => c.Vendor == "claude").SupportsLaunchPosture).IsFalse(); + } + // === Trust-by-default borrowed-review advertisement === // (docs/superpowers/specs/2026-07-27-ai1528-trust-by-default-borrowed-review-design.md) // From a2fcef5683c38845b5b75fa4d3c98e092a237e83 Mon Sep 17 00:00:00 2001 From: realtonyyoung <6655045+realtonyyoung@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:36:05 -0400 Subject: [PATCH 5/8] docs: README for hosted Codex launch-posture selection --- README.md | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 32a111488..496e0f6f4 100644 --- a/README.md +++ b/README.md @@ -715,7 +715,25 @@ The first five (`kcap-recap`, `kcap-errors`, `kcap-hide`, `kcap-disable`, `kcap- > > The `**.kcap.ai` wildcard covers every SaaS tenant — current and future — plus `auth.kcap.ai`, so switching profiles and adding tenants just work with no per-tenant edits. **Self-hosted** servers are added as exact-host entries, derived from every configured profile's `server_url` and refreshed on each `kcap setup`. Existing config is respected: if you already run a `network_proxy` policy, kcap's hosts are merged into your `domains` (yours preserved); if you've already opened the network (`network_access = true` with no proxy), nothing changes. Opt out with `--skip-codex-network-access` on either command (and the npm-postinstall `--if-installed` refresh never touches `config.toml`). A localhost dev server additionally needs `allow_local_binding = true`, which kcap does not set. Uninstall leaves these keys in place — they're your security posture, not kcap state. -The daemon starts Codex with `--sandbox workspace-write` and `--ask-for-approval on-request`. This lets Codex edit files in the agent's worktree but escalates sensitive operations (e.g. network calls, shell commands outside the worktree) through the daemon's permission bridge to the dashboard. +**Sandbox and approval posture.** By default the daemon starts Codex with `--sandbox workspace-write` and `--ask-for-approval on-request`. This lets Codex edit files in the agent's worktree but escalates sensitive operations (e.g. network calls, shell commands outside the worktree) through the daemon's permission bridge to the dashboard. + +An **interactive** hosted launch (one that runs in a daemon-created worktree) may select a different posture instead of those defaults: + +| Flag | Selectable values | +|---|---| +| `--sandbox` | `read-only`, `workspace-write` (default), `danger-full-access` | +| `--ask-for-approval` | `untrusted`, `on-request` (default), `never` | + +Two cases are **not** selectable, because their values are containment guarantees rather than preferences: + +- A **borrowed** launch runs in your own real checkout, so it is always `read-only` — anything else would let a hosted agent modify the repo you are working in. +- A **review-flow reviewer** runs unattended, so it is always `--ask-for-approval never` — any prompting mode would wedge the round waiting for an approval no human is there to give. + +PR-review launches also keep a fixed posture. A launch that supplies a posture for any of those three cases is **rejected** with a coded error (`codex_posture_not_overridable`) rather than having it silently ignored or silently applied; an unknown value, a half-specified pair, or the upstream-deprecated `on-failure` is rejected as `codex_posture_invalid`. + +Selecting `--ask-for-approval never` or `--sandbox danger-full-access` **disables the permission bridge for that agent** — it will never ask the dashboard before acting, and `danger-full-access` also reaches outside the worktree. The daemon logs a warning naming the agent whenever an interactive launch resolves to either. + +Posture selection is driven by the server (dashboard), and the daemon advertises support for it at connect. A server that offers the selector will refuse it against a daemon too old to advertise support, rather than sending a posture that would be quietly dropped — so update kcap on the daemon host if the dashboard says the selector is unavailable. This is separate from local `kcap agent start codex -- …`, where you pass Codex flags yourself and they are not restricted (see [Local agents](#local-agents-kcap-agent)). > **Upgrading from an earlier version of kcap?** Run `kcap update` (or `npm install -g @kurrent/kcap`) — the npm postinstall hook, and `kcap update` itself, refresh all user-scope kcap installations, so you always pick up the current CLI version's skills (`~/.agents/skills/kcap-*`), Codex hook commands (`~/.codex/hooks.json`), and Claude plugin registration (`~/.claude/settings.json`). Each refresh is gated on a marker file written by your previous setup — fresh systems that never opted in are left untouched. Project-scope installs (`--project`) are not auto-refreshed; re-run `kcap plugin install [--codex] --project` after upgrading if you want the latest config for a specific repo. > @@ -727,7 +745,7 @@ The daemon starts Codex with `--sandbox workspace-write` and `--ask-for-approval > > **Recovery:** upgrade kcap, then end the harness session and start a fresh one — or, in harnesses that expose it, reconnect the `kcap-flows` MCP server. To confirm the new schema is live, check that your client lists the server as connected (`claude mcp list`, `codex mcp get kcap-flows`, `gemini mcp list`, `cursor-agent mcp list`, `opencode mcp list`) and that `start_review_flow` now advertises `vendor`. -PR review is supported for hosted Codex agents as well as Claude — the same `kcap-review` MCP context is injected either way. The sandbox and approval-mode selectors in the launch dialog are planned as a follow-up (AI-633). +PR review is supported for hosted Codex agents as well as Claude — the same `kcap-review` MCP context is injected either way. #### Cursor IDE hooks From 3af6bd4519518037f4a9afc2f845d9e98c4fac6f Mon Sep 17 00:00:00 2001 From: realtonyyoung <6655045+realtonyyoung@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:47:17 -0400 Subject: [PATCH 6/8] fix: address codex code-review round 1 (positive kind eligibility, borrowed echo arm, handoff + argv pinning tests, README accuracy) --- README.md | 11 ++- .../Services/AgentOrchestrator.cs | 7 +- .../Services/CodexPosturePolicy.cs | 20 +++-- .../AgentOrchestratorVendorTests.cs | 80 +++++++++++++++++++ .../Codex/CodexLauncherTests.cs | 53 +++++++++--- .../Codex/CodexPosturePolicyTests.cs | 12 +++ 6 files changed, 159 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 496e0f6f4..4c8b09254 100644 --- a/README.md +++ b/README.md @@ -717,7 +717,7 @@ The first five (`kcap-recap`, `kcap-errors`, `kcap-hide`, `kcap-disable`, `kcap- **Sandbox and approval posture.** By default the daemon starts Codex with `--sandbox workspace-write` and `--ask-for-approval on-request`. This lets Codex edit files in the agent's worktree but escalates sensitive operations (e.g. network calls, shell commands outside the worktree) through the daemon's permission bridge to the dashboard. -An **interactive** hosted launch (one that runs in a daemon-created worktree) may select a different posture instead of those defaults: +The daemon also **accepts** a caller-selected posture for an **interactive** hosted launch (one that runs in a daemon-created worktree). Nothing selects one yet: the dashboard selector ships with a later Capacitor **server** release, and until then every launch uses the defaults above. The selectable values are: | Flag | Selectable values | |---|---| @@ -731,9 +731,14 @@ Two cases are **not** selectable, because their values are containment guarantee PR-review launches also keep a fixed posture. A launch that supplies a posture for any of those three cases is **rejected** with a coded error (`codex_posture_not_overridable`) rather than having it silently ignored or silently applied; an unknown value, a half-specified pair, or the upstream-deprecated `on-failure` is rejected as `codex_posture_invalid`. -Selecting `--ask-for-approval never` or `--sandbox danger-full-access` **disables the permission bridge for that agent** — it will never ask the dashboard before acting, and `danger-full-access` also reaches outside the worktree. The daemon logs a warning naming the agent whenever an interactive launch resolves to either. +Two selections weaken the protections you get by default, in different ways: -Posture selection is driven by the server (dashboard), and the daemon advertises support for it at connect. A server that offers the selector will refuse it against a daemon too old to advertise support, rather than sending a posture that would be quietly dropped — so update kcap on the daemon host if the dashboard says the selector is unavailable. This is separate from local `kcap agent start codex -- …`, where you pass Codex flags yourself and they are not restricted (see [Local agents](#local-agents-kcap-agent)). +- `--ask-for-approval never` **disables the permission bridge for that agent** — it never asks the dashboard before acting, so nothing pauses for your approval. +- `--sandbox danger-full-access` **removes the filesystem sandbox** — the agent can read and write outside its worktree. (Approvals are unaffected: with `on-request` it still escalates through the bridge.) + +The daemon logs a warning naming the agent whenever an interactive launch resolves to either. + +The daemon advertises posture support to the server at connect. A server that offers the selector refuses it against a daemon too old to advertise support, rather than sending a posture that would be quietly dropped — so update kcap on the daemon host if the dashboard says the selector is unavailable. This is all separate from local `kcap agent start codex -- …`, where you pass Codex flags yourself and they are not restricted (see [Local agents](#local-agents-kcap-agent)). > **Upgrading from an earlier version of kcap?** Run `kcap update` (or `npm install -g @kurrent/kcap`) — the npm postinstall hook, and `kcap update` itself, refresh all user-scope kcap installations, so you always pick up the current CLI version's skills (`~/.agents/skills/kcap-*`), Codex hook commands (`~/.codex/hooks.json`), and Claude plugin registration (`~/.claude/settings.json`). Each refresh is gated on a marker file written by your previous setup — fresh systems that never opted in are left untouched. Project-scope installs (`--project`) are not auto-refreshed; re-run `kcap plugin install [--codex] --project` after upgrading if you want the latest config for a specific repo. > diff --git a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs index 8766f8711..fb7ea5b00 100644 --- a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs +++ b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs @@ -1202,12 +1202,13 @@ await _server.LaunchFailedAsync(agentId, // Applied-posture echo, stamped only for an interactive Codex launch on a daemon-owned // worktree. A review-flow or PR-review launch reports nothing (its posture is the - // containment invariant, not a choice), and so does a borrowed launch — `work` is - // resolved from cmd.Borrowed independently of Kind, so the owned-worktree arm is what - // keeps a borrowed Default command from reporting a posture it never selected. + // containment invariant, not a choice), and neither does a borrowed one. Both borrow + // conditions are tested: `work` alone is not enough, because a snapshot-backed borrow + // maps to OwnedWorktree while still being a borrow the caller never chose a posture for. (string Sandbox, string Approval)? appliedPosture = string.Equals(cmd.Vendor, "codex", StringComparison.OrdinalIgnoreCase) && cmd.Kind == LaunchKind.Default + && !cmd.Borrowed && work == WorkLocation.OwnedWorktree ? CodexPosturePolicy.Resolve(work, isReviewFlow, cmd.CodexPosture) : null; diff --git a/src/Capacitor.Cli.Daemon/Services/CodexPosturePolicy.cs b/src/Capacitor.Cli.Daemon/Services/CodexPosturePolicy.cs index 22740f75a..e32e394c3 100644 --- a/src/Capacitor.Cli.Daemon/Services/CodexPosturePolicy.cs +++ b/src/Capacitor.Cli.Daemon/Services/CodexPosturePolicy.cs @@ -33,13 +33,19 @@ internal static class CodexPosturePolicy { return $"codex_posture_wrong_vendor: a Codex launch posture was supplied for vendor '{cmd.Vendor}' — " + "the posture block applies only to Codex launches."; - if (cmd.Kind == LaunchKind.ReviewFlow) - return "codex_posture_not_overridable: review-flow reviewers run unattended, so their approval " - + "policy is daemon-derived and cannot be overridden."; - - if (cmd.Kind == LaunchKind.Review) - return "codex_posture_not_overridable: PR-review launches use a fixed posture; selection applies " - + "only to interactive launches."; + // Positive eligibility: anything that is not an interactive launch is rejected, so a value + // outside the known enum (LaunchKind crosses the wire as a number) cannot deserialize into + // something that merely fails both `is ReviewFlow` and `is Review` and then gets treated as + // interactive. Known kinds keep their specific diagnostic. + if (cmd.Kind != LaunchKind.Default) + return "codex_posture_not_overridable: " + cmd.Kind switch { + LaunchKind.ReviewFlow => "review-flow reviewers run unattended, so their approval policy is " + + "daemon-derived and cannot be overridden.", + LaunchKind.Review => "PR-review launches use a fixed posture; selection applies only to " + + "interactive launches.", + _ => $"launch kind '{cmd.Kind}' is not an interactive launch; posture " + + "selection applies only to interactive launches." + }; if (cmd.Borrowed) return "codex_posture_not_overridable: a borrowed launch runs in the requester's real checkout and " diff --git a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs index a4ded5f91..434317501 100644 --- a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs @@ -339,6 +339,86 @@ public async Task Interactive_codex_launch_echoes_the_selected_posture_on_regist } } + /// The command→runtime handoff, which no other test covers: the echo is computed from + /// `cmd` and the launcher tests build a LauncherContext by hand, so dropping + /// `CodexPosture: cmd.CodexPosture` from the RuntimeStartContext construction would leave every + /// other posture test green while the agent silently launched on the old defaults — with + /// registration still advertising the selected pair. + [Test] + public async Task Interactive_codex_launch_threads_the_posture_into_the_runtime_start_context() { + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var (_, codexSpy) = await LaunchForEchoAsync(repoPath, "agent-thread", new("danger-full-access", "untrusted")); + + await Assert.That(codexSpy.LastContext).IsNotNull(); + await Assert.That(codexSpy.LastContext!.CodexPosture).IsNotNull(); + await Assert.That(codexSpy.LastContext!.CodexPosture!.Sandbox).IsEqualTo("danger-full-access"); + await Assert.That(codexSpy.LastContext!.CodexPosture!.Approval).IsEqualTo("untrusted"); + } finally { + cleanup(); + } + } + + [Test] + public async Task Interactive_codex_launch_without_a_posture_threads_null() { + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var (_, codexSpy) = await LaunchForEchoAsync(repoPath, "agent-thread-null", posture: null); + + await Assert.That(codexSpy.LastContext).IsNotNull(); + await Assert.That(codexSpy.LastContext!.CodexPosture).IsNull(); + } finally { + cleanup(); + } + } + + /// A snapshot-backed borrow maps to WorkLocation.OwnedWorktree, so `work` alone would + /// wrongly qualify it as interactive and echo a posture the caller never chose. Guards the + /// `!cmd.Borrowed` arm of the echo predicate. + [Test] + public async Task Snapshot_borrowed_launch_echoes_no_posture() { + var (repoPath, cleanup) = CreateGitRepo(); + + try { + var server = new CaptureServerConnection(); + var ptyFactory = new SpyPtyProcessFactory(); + var codexSpy = new SpyHostedAgentRuntimeFactory("codex") { + EmitsTerminalOutput = false, + SupportsUnattended = true, + SupportsBorrowedReviewFlow = true, + BorrowedReviewRequiresIndependentSnapshot = true + }; + + await using var orch = BuildOrchestrator( + server, ptyFactory, new Dictionary(), + extraRuntimeFactories: [codexSpy]); + + await orch.HandleLaunchAgentForTest(new LaunchAgentCommand( + AgentId: "agent-snapshot-borrow", + Prompt: "hi", + Model: "default", + Effort: null, + RepoPath: repoPath, + Tools: null, + AttachmentIds: null, + Vendor: "codex", + Kind: LaunchKind.Default, + Borrowed: true, + BorrowCwd: repoPath)); + + // Either the launch never registered (fine — nothing to echo) or it registered with nulls; + // what must never happen is a stamped pair on a borrowed command. + await Assert.That( + server.AgentRegisteredPostures.Any(p => p.AgentId == "agent-snapshot-borrow" + && (p.Sandbox is not null || p.Approval is not null))) + .IsFalse(); + } finally { + cleanup(); + } + } + [Test] public async Task Interactive_codex_launch_without_a_posture_echoes_the_derived_pair() { var (repoPath, cleanup) = CreateGitRepo(); diff --git a/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs b/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs index 16017fb56..7f1c82a20 100644 --- a/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs @@ -805,19 +805,50 @@ public async Task BuildArgs_selected_posture_lands_verbatim_in_argv(string sandb await Assert.That(args[aIdx + 1]).IsEqualTo(approval); } - [Test] - public async Task BuildArgs_without_a_posture_is_byte_identical_to_the_derived_argv() { - // The regression that matters most: an absent block must not perturb a single argument. - // NewCtx() never sets CodexPosture, so this compares "today's shape" against an explicit null. - var derived = NewLauncher().BuildArgs(NewCtx() with { Work = WorkLocation.OwnedWorktree }).Args; - var explicitNull = NewLauncher().BuildArgs(InteractiveCtx(null)).Args; + /// The regression that matters most: with no posture block the argv must be EXACTLY what + /// this launcher produced before posture selection existed. The expected sequences below are + /// literal transcriptions of the pre-change output, so an added, removed, renamed or REORDERED + /// argument fails here — comparing two runs of the current implementation would not, since a + /// change of that kind moves both sides identically. + [Test] + public async Task BuildArgs_without_a_posture_matches_the_pre_change_argv_exactly() { + string[] expectedInteractive = [ + "--cd", "/tmp/wt", + "--sandbox", "workspace-write", + "--ask-for-approval", "on-request", + "-m", "gpt-5.3-codex", + "--no-alt-screen" + ]; - await Assert.That(explicitNull).IsEquivalentTo(derived, CollectionOrdering.Matching); + // Both the shape that never mentions a posture and an explicit null must produce it. + await Assert.That(NewLauncher().BuildArgs(NewCtx() with { Work = WorkLocation.OwnedWorktree }).Args) + .IsEquivalentTo(expectedInteractive, CollectionOrdering.Matching); + await Assert.That(NewLauncher().BuildArgs(InteractiveCtx(null)).Args) + .IsEquivalentTo(expectedInteractive, CollectionOrdering.Matching); + + // Prompt + effort ride after the posture pair; pinned so their ordering can't drift either. + string[] expectedWithPromptAndEffort = [ + "--cd", "/tmp/wt", + "--sandbox", "workspace-write", + "--ask-for-approval", "on-request", + "-m", "gpt-5.3-codex", + "-c", "model_reasoning_effort=\"high\"", + "--no-alt-screen", + "--", "do the thing" + ]; - var sIdx = Array.IndexOf(derived, "--sandbox"); - var aIdx = Array.IndexOf(derived, "--ask-for-approval"); - await Assert.That(derived[sIdx + 1]).IsEqualTo("workspace-write"); - await Assert.That(derived[aIdx + 1]).IsEqualTo("on-request"); + await Assert.That( + NewLauncher().BuildArgs( + NewCtx(prompt: "do the thing", effort: "high") with { Work = WorkLocation.OwnedWorktree }).Args) + .IsEquivalentTo(expectedWithPromptAndEffort, CollectionOrdering.Matching); + + // Borrowed + review-flow derivations are pinned by their own tests above; this asserts the + // borrowed pair specifically still lands in the same positions with no block present. + var borrowed = NewLauncher().BuildArgs(NewCtx(isReviewFlow: true) with { Work = WorkLocation.BorrowedCwd }).Args; + await Assert.That(borrowed[2]).IsEqualTo("--sandbox"); + await Assert.That(borrowed[3]).IsEqualTo("read-only"); + await Assert.That(borrowed[4]).IsEqualTo("--ask-for-approval"); + await Assert.That(borrowed[5]).IsEqualTo("never"); } [Test] diff --git a/test/Capacitor.Cli.Tests.Unit/Codex/CodexPosturePolicyTests.cs b/test/Capacitor.Cli.Tests.Unit/Codex/CodexPosturePolicyTests.cs index 722dd88f7..d65f661ae 100644 --- a/test/Capacitor.Cli.Tests.Unit/Codex/CodexPosturePolicyTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Codex/CodexPosturePolicyTests.cs @@ -79,6 +79,18 @@ public async Task Posture_on_non_codex_vendor_fails_closed_with_coded_reason() { await Assert.That(reason).StartsWith("codex_posture_wrong_vendor:"); } + /// LaunchKind crosses the wire as a number, so a malformed or future value deserializes + /// as an unknown enum member. Eligibility is positive (only Default is interactive) precisely so + /// such a value cannot slip past "not ReviewFlow and not Review" and be honoured as interactive — + /// which would also skip the bridge-defeating warning, since that predicate requires Default. + [Test] + public async Task Posture_on_an_unknown_launch_kind_fails_closed() { + var reason = CodexPosturePolicy.RejectionReason( + Cmd(kind: (LaunchKind)99, posture: new("danger-full-access", "never"))); + + await Assert.That(reason).StartsWith("codex_posture_not_overridable:"); + } + /// A borrowed review-flow launch violates BOTH invariants; the reason names one of them /// rather than reporting nothing. [Test] From edddc8cd539ed5103939e31800e7b6c138331847 Mon Sep 17 00:00:00 2001 From: realtonyyoung <6655045+realtonyyoung@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:51:33 -0400 Subject: [PATCH 7/8] test: cover the factory half of the posture handoff; require registration in the snapshot-borrow echo test --- .../AgentOrchestratorVendorTests.cs | 18 ++++-- .../PtyHostedAgentRuntimeFactoryTests.cs | 62 +++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs index 434317501..d788e571a 100644 --- a/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs @@ -408,12 +408,18 @@ await orch.HandleLaunchAgentForTest(new LaunchAgentCommand( Borrowed: true, BorrowCwd: repoPath)); - // Either the launch never registered (fine — nothing to echo) or it registered with nulls; - // what must never happen is a stamped pair on a borrowed command. - await Assert.That( - server.AgentRegisteredPostures.Any(p => p.AgentId == "agent-snapshot-borrow" - && (p.Sandbox is not null || p.Approval is not null))) - .IsFalse(); + // The launch must actually REACH registration — otherwise an unrelated early failure + // would satisfy a "no non-null echo exists" assertion without ever exercising the + // predicate under test. Require exactly one registration, and require it to be null/null. + var registrations = server.AgentRegisteredPostures + .Where(p => p.AgentId == "agent-snapshot-borrow") + .ToList(); + + await Assert.That(registrations).Count().IsEqualTo(1); + await Assert.That(registrations[0].Sandbox).IsNull(); + await Assert.That(registrations[0].Approval).IsNull(); + // The runtime really started, so the snapshot-borrow path ran end to end. + await Assert.That(codexSpy.StartCalls).IsEqualTo(1); } finally { cleanup(); } diff --git a/test/Capacitor.Cli.Tests.Unit/PtyHostedAgentRuntimeFactoryTests.cs b/test/Capacitor.Cli.Tests.Unit/PtyHostedAgentRuntimeFactoryTests.cs index 2a7100b3a..dac1fb56f 100644 --- a/test/Capacitor.Cli.Tests.Unit/PtyHostedAgentRuntimeFactoryTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/PtyHostedAgentRuntimeFactoryTests.cs @@ -110,6 +110,68 @@ public async Task Review_launch_for_codex_builds_MCP_command_from_CapacitorPath_ } } + /// The factory half of the posture handoff. The orchestrator-level test proves + /// LaunchAgentCommand → RuntimeStartContext, but it substitutes a spy for this factory, so + /// dropping CodexPosture = ctx.CodexPosture here would leave that test green while the real + /// launcher silently received null — launching on the default posture even though registration + /// advertised the selected pair. is the same + /// LauncherContext instance BuildArgs consumes, so asserting on it pins what the launcher sees. + [Test] + public async Task Codex_launch_threads_the_posture_into_the_launcher_context() { + var launcher = new RecordingLauncher("codex", cliPath: "/opt/vendor/codex"); + var ptyFactory = new NullPtyProcessFactory(); + var factory = new PtyHostedAgentRuntimeFactory(launcher, ptyFactory, NullLogger.Instance); + + var ctx = BuildInteractiveContext("codex") with { CodexPosture = new("danger-full-access", "untrusted") }; + + var start = await factory.StartAsync(ctx, CancellationToken.None); + + try { + await Assert.That(launcher.LastPrepareCtx).IsNotNull(); + await Assert.That(launcher.LastPrepareCtx!.CodexPosture).IsNotNull(); + await Assert.That(launcher.LastPrepareCtx!.CodexPosture!.Sandbox).IsEqualTo("danger-full-access"); + await Assert.That(launcher.LastPrepareCtx!.CodexPosture!.Approval).IsEqualTo("untrusted"); + } finally { + await start.Runtime.DisposeAsync(); + } + } + + [Test] + public async Task Codex_launch_without_a_posture_threads_null_into_the_launcher_context() { + var launcher = new RecordingLauncher("codex", cliPath: "/opt/vendor/codex"); + var ptyFactory = new NullPtyProcessFactory(); + var factory = new PtyHostedAgentRuntimeFactory(launcher, ptyFactory, NullLogger.Instance); + + var start = await factory.StartAsync(BuildInteractiveContext("codex"), CancellationToken.None); + + try { + await Assert.That(launcher.LastPrepareCtx).IsNotNull(); + await Assert.That(launcher.LastPrepareCtx!.CodexPosture).IsNull(); + } finally { + await start.Runtime.DisposeAsync(); + } + } + + static RuntimeStartContext BuildInteractiveContext(string vendor) => + new( + AgentId: "agent-interactive-1", + Vendor: vendor, + SourceRepoPath: "/repo", + Worktree: new WorktreeInfo("/repo/.worktrees/agent-interactive-1", "wt-branch", "/repo"), + Prompt: null, + Model: "gpt-5.3-codex", + Effort: null, + Tools: null, + IsReview: false, + IsReviewFlow: false, + Review: null, + Cols: 120, + Rows: 40, + ServerUrl: "https://srv", + DaemonBridgeUrl: null, + CapacitorPath: "/opt/kcap/kcap" + ); + sealed class NullPtyProcessFactory : IPtyProcessFactory { public IPtyProcess Spawn( string command, From cc9186810d11fe59cd48148d0f5f50b873ac3468 Mon Sep 17 00:00:00 2001 From: realtonyyoung <6655045+realtonyyoung@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:55:18 -0400 Subject: [PATCH 8/8] fix: fail closed when a posture reaches a PR-review launch; trim wire comment Addresses both Qodo review-recommended findings on #417. --- src/Capacitor.Cli.Core/Models.cs | 8 ++------ .../Services/CodexLauncher.cs | 18 ++++++++++-------- .../Codex/CodexLauncherTests.cs | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/Capacitor.Cli.Core/Models.cs b/src/Capacitor.Cli.Core/Models.cs index 4da570ddc..845c37acd 100644 --- a/src/Capacitor.Cli.Core/Models.cs +++ b/src/Capacitor.Cli.Core/Models.cs @@ -1299,12 +1299,8 @@ public readonly record struct LaunchAgentCommand( // launches with the exact LaunchModel VERBATIM and, post-launch, reports the concrete resolved // model back keyed on LaunchAttemptId (see ExplicitReviewerModelResolvedV1). ExplicitReviewerModelLaunch? ExplicitReviewerModel = null, - // Caller-selected Codex sandbox/approval posture for an INTERACTIVE daemon-owned-worktree - // launch; null preserves the derived behaviour byte-for-byte. Supplied for any other launch - // shape (review-flow, PR review, borrowed cwd, non-codex vendor) the daemon fails closed - // with a coded LaunchFailed — never silently ignored, never honoured. Appended last as an - // optional field so the SignalR positional/name binding stays wire-compatible with older - // daemons (ignore it) and older servers (never set it). + // Interactive Codex launches only; any other launch shape is rejected by CodexPosturePolicy. + // Appended last so the wire stays compatible with older daemons and servers. CodexLaunchPosture? CodexPosture = null ); diff --git a/src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs b/src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs index 4911cbafc..2b4ae8aa9 100644 --- a/src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs +++ b/src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs @@ -104,16 +104,18 @@ public void Prepare(LauncherContext ctx) { } public LaunchArgs BuildArgs(LauncherContext ctx) { - if (ctx is { IsReview: true, ReviewLaunch: { } launch }) { - return BuildReviewArgs(ctx, launch); + // Re-assert the orchestrator's guard for every non-interactive shape, BEFORE the PR-review + // branch returns — a posture here means that guard was bypassed, and each of these launches + // owes its posture to a containment rule: a borrowed cwd is the user's real checkout, a + // review-flow reviewer has no human to answer a prompt, and PR review is fixed by contract. + if (ctx.CodexPosture is not null + && (ctx.IsReview || ctx.IsReviewFlow || ctx.Work == WorkLocation.BorrowedCwd)) { + throw new InvalidOperationException( + "codex_posture_not_overridable: a launch posture reached a borrowed, review-flow or PR-review launch"); } - // Re-assert the orchestrator's pre-flight guard. A posture here means that guard was - // bypassed, and honouring it would break a containment invariant — a borrowed reviewer runs - // in the user's REAL checkout, and a review-flow reviewer has no human to answer a prompt. - if (ctx.CodexPosture is not null && (ctx.Work == WorkLocation.BorrowedCwd || ctx.IsReviewFlow)) { - throw new InvalidOperationException( - "codex_posture_not_overridable: a launch posture reached a borrowed or review-flow launch"); + if (ctx is { IsReview: true, ReviewLaunch: { } launch }) { + return BuildReviewArgs(ctx, launch); } // Selected pair for an interactive owned-worktree launch; otherwise the derived containment diff --git a/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs b/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs index 7f1c82a20..dc7965af2 100644 --- a/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs @@ -871,6 +871,25 @@ public async Task BuildArgs_throws_when_a_posture_reaches_a_borrowed_context() { await Assert.That(() => NewLauncher().BuildArgs(ctx)).Throws(); } + /// The PR-review branch returns early, so the guard has to sit ABOVE it — otherwise a + /// posture reaching a PR-review launch is silently dropped instead of failing closed, which is + /// the one outcome the posture contract rules out for every non-interactive shape. + [Test] + public async Task BuildArgs_throws_when_a_posture_reaches_a_pr_review_context() { + var ctx = NewCtx() with { + IsReview = true, + Review = new ReviewLaunchInfo("acme", "widgets", 42), + ReviewLaunch = new ReviewLaunchBuilder.ReviewLaunch( + McpConfigPath: null, + SystemPrompt: "review it", + Mcp: new ReviewLaunchBuilder.ReviewMcpServer( + "/opt/kcap/kcap", ["mcp", "review"], new Dictionary())), + CodexPosture = new("workspace-write", "never") + }; + + await Assert.That(() => NewLauncher().BuildArgs(ctx)).Throws(); + } + [Test] public async Task DisablesApprovalPrompts_tracks_the_effective_approval_policy() { var launcher = NewLauncher();