Skip to content

Resolve the running model for Codex and ACP default launches - #910

Merged
realtonyyoung merged 4 commits into
mainfrom
claude-tyoung/daemon-model-resolution
Sep 11, 2026
Merged

realtonyyoung merged 4 commits into
mainfrom
claude-tyoung/daemon-model-resolution

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

Closes #899 — AI-2717

What & why

The rail's model chip (PR #904) only showed for Pi, because Pi was the only vendor whose local status carried a model on a default launch. This resolves it for the rest, best-effort:

  • Codex now writes the resolved model to the local AgentInstance.Model (it was reported to the server only), so a local Codex row shows the chip.
  • ACP vendors (cursor/copilot/gemini/kiro/opencode) report the handshake's current model when nothing was requested — the same current-model marker the session/new result already carries.

Claude PTY default launches (no model requested, no handshake to read) stay blank — the accepted best-effort gap.

Where to look

The ACP fallback fires only when nothing was requested, so a requested-but-unmatched model still reports null — the signal EmitModelFallbackNote and registration depend on. AgentInstance.Model became settable (shadowing the positional param) and is written only through SetResolvedModel, which pulses the local status frame the way SetResolvedTitle does.

Verification

dotnet build Capacitor.slnx → 0 warnings; CLI and daemon dotnet publish -c Release → no IL2026/IL3050. Tests: AcpSessionModelListTests 25/25 (new: current-model extraction, both shapes + precedence + null), AcpHostedAgentRuntimeModelSelectionTests 8/8 (new: default launch exposes the handshake current model, no set_* sent), AgentStatusSnapshotTests 19/19 (new: SetResolvedModel updates the snapshot), AgentOrchestratorVendorTests 61/61 (Codex model now also on the local frame).

Codex now updates the local AgentInstance.Model when it resolves (was server-only), so a local Codex row shows its model chip. ACP runtimes report the handshake's current model when nothing was requested — only then, so a requested-but-unmatched model still reports null. Claude PTY default launches remain the known best-effort gap.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Resolve models for Codex and default ACP launches

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Expose Codex’s resolved model through local agent status updates.
• Derive default ACP models from vendor handshake metadata.
• Preserve null resolution when an explicitly requested model cannot be matched.
Diagram

graph TD
  ACP["ACP Handshake"] -->|current model| EXTRACT["Model Extractor"] --> RUNTIME["ACP Runtime"] --> ORCH["Agent Orchestrator"] --> STATUS["Local Status"] --> RAIL["Desktop Rail"]
  CODEX["Codex Runtime"] -->|resolved model| ORCH
Loading
High-Level Assessment

The approach is appropriate: it reuses existing ACP handshake metadata and the established resolved-title mutation-and-pulse pattern. Dynamically deriving models during snapshot serialization or introducing a new runtime event abstraction would add coupling without improving correctness, while the explicit no-request guard preserves unmatched-model signaling.

Files changed (7) +155 / -0

Bug fix (3) +78 / -0
AcpSessionModelList.csExtract current models from ACP session metadata +51/-0

Extract current models from ACP session metadata

• Adds total, best-effort extraction of the vendor’s current model from either 'models.currentModelId' or the model 'configOptions.currentValue'. The standardized 'models' shape takes precedence, and malformed or blank values yield null.

src/Capacitor.Cli.Core/Acp/AcpSessionModelList.cs

AcpHostedAgentRuntime.csResolve default ACP launches from handshake metadata +7/-0

Resolve default ACP launches from handshake metadata

• Uses the handshake’s current-model marker when no model was requested and model selection produced no result. Explicitly requested but unmatched models remain unresolved so fallback notes and registration behavior are preserved.

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs

AgentOrchestrator.csPublish post-launch model resolution locally +20/-0

Publish post-launch model resolution locally

• Makes 'AgentInstance.Model' mutable through a dedicated 'SetResolvedModel' path that pulses local status. Codex model reports now update the local agent instance before reporting the resolution to the server.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

Tests (4) +77 / -0
AcpSessionModelListTests.csCover ACP current-model extraction +36/-0

Cover ACP current-model extraction

• Tests both supported response shapes, standardized-shape precedence, and null results for missing or invalid current-model markers.

test/Capacitor.Cli.Core.Tests.Unit/Acp/AcpSessionModelListTests.cs

AcpHostedAgentRuntimeModelSelectionTests.csVerify default ACP model reporting +19/-0

Verify default ACP model reporting

• Confirms a default launch exposes the handshake’s current model without sending either model-selection RPC.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AcpHostedAgentRuntimeModelSelectionTests.cs

AgentOrchestratorVendorTests.csVerify Codex model reaches local status +3/-0

Verify Codex model reaches local status

• Extends the legacy Codex launch test to assert that the resolved model appears in the orchestrator’s local status snapshot.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentOrchestratorVendorTests.cs

AgentStatusSnapshotTests.csCover post-launch status model updates +19/-0

Cover post-launch status model updates

• Verifies 'SetResolvedModel' changes the model emitted by subsequent local agent status snapshots without re-registration.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentStatusSnapshotTests.cs

@qodo-code-review

qodo-code-review Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Codex status can show the wrong model ✓ Resolved 🐞 Bug ≡ Correctness
Description
ReportResolvedModel overwrites the AgentInstance.Model initialized from the Codex app-server's
authoritative thread/start response with a value re-derived from the launch request and local
config. On a default launch whose handshake returns a concrete model while config.toml lacks a
top-level model, CodexResolvedModel returns the literal default sentinel, so the local status
pulse and later re-registration replace the confirmed model with that sentinel.
Code

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[2672]

+            if (_agents.TryGetValue(agentId, out var agent)) SetResolvedModel(agent, resolved);
Relevance

●●● Strong

PR #497 accepted separating confirmed runtime identity from requested-model fallbacks; this is
closely analogous.

PR-#497

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The production app-server factory returns the runtime as the transcript source, and that runtime
records the model returned by thread/start; the orchestrator initially treats this value as
confirmed. The added mutation subsequently replaces it with CodexResolvedModel, whose documented
fallback returns the original request when the config is unavailable, while both local status and
reconnect registration consume the mutable AgentInstance.Model.

src/Capacitor.Cli.Daemon/Harness/Codex/CodexHostedAgentRuntimeFactory.cs[134-156]
src/Capacitor.Cli.Daemon/Harness/Codex/CodexAppServerHostedAgentRuntime.cs[404-418]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[2407-2419]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[2660-2674]
src/Capacitor.Cli.Core/Harness/Codex/CodexConfigToml.cs[292-310]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.LocalIpc.cs[48-61]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[4678-4693]
PR-#497

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new local Codex model update can overwrite the authoritative model returned by the app-server handshake with a config-derived value or the literal `default` sentinel.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[2544-2555]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[2660-2674]
- src/Capacitor.Cli.Daemon/Harness/Codex/CodexHostedAgentRuntimeFactory.cs[134-156]
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentOrchestratorVendorTests.cs[1638-1672]

## Recommended Fix
Keep the legacy server reporting behavior, but perform the new local mutation only for Codex runtimes without an authoritative transcript source, such as the PTY path. Preserve `AgentInstance.Model` when it was initialized from `IAcpTranscriptSource.ResolvedModel`, and add coverage for a default app-server launch whose handshake returns a concrete model while the config has no top-level model.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Reconnected usage names a stale model ✓ Resolved 🐞 Bug ≡ Correctness
Description
ReapplyModelAsync preserves the startup _resolvedModel when _requestedModel is blank, even
though _lastLoadResult carries the same current-model shape that the new launch fallback reads. If
session/load reports a different current model after a child restart, every later usage envelope
is stamped with the old model and token usage is attributed to the wrong model.
Code

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[R1078-1079]

+        if (_resolvedModel is null && string.IsNullOrWhiteSpace(requestedModel))
+            _resolvedModel = AcpSessionModelList.ExtractCurrentModel(sessionNewResult);
Relevance

●● Moderate

The stale-model risk is plausible, but no close reconnect-specific precedent establishes team
acceptance.

PR-#880

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new launch path stores the initial current marker in _resolvedModel, while reconnect stores a
fresh load response but calls a selector that necessarily returns null for a blank request and then
falls back to the old field. Repository fixtures confirm that session/load returns the same
models/configuration response shape, and the usage translator directly stamps _resolvedModel onto
each usage envelope.

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1074-1079]
src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[2455-2485]
src/Capacitor.Cli.Daemon/Acp/IAcpModelSelector.cs[73-89]
test/Capacitor.Cli.Daemon.Tests.Unit/Acp/FakeAcpAgent.cs[471-483]
src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1787-1799]
src/Capacitor.Cli.Daemon/Acp/AcpEventTranslator.cs[111-123]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Default ACP launches capture the current model from `session/new`, but reconnect processing retains that startup value rather than reading the current marker returned by `session/load`.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1074-1079]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[2455-2485]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1787-1799]
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/AcpHostedAgentRuntimeReconnectTests.cs[68-120]

## Recommended Fix
When `_requestedModel` is blank, replace `_resolvedModel` after each successful `session/load` with `AcpSessionModelList.ExtractCurrentModel(_lastLoadResult)` instead of preserving the startup value. Add a reconnect test where the candidate's load response advertises a different current model and assert that a subsequent usage envelope carries the refreshed model.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Two model checks bypass JSON helpers 📘 Rule violation ⚙ Maintainability
Description
ExtractCurrentModel and CurrentFromConfigOptions compare JsonElement.ValueKind directly for
object and array shapes instead of using IsObject and IsArray. Both checks run while extracting
the handshake model, so the new parsing path duplicates type-shape logic rather than using the
repository's shared extensions.
Code

src/Capacitor.Cli.Core/Acp/AcpSessionModelList.cs[R84-85]

+        if (sessionNewResult.ValueKind != JsonValueKind.Object)
+            return null;
Relevance

● Weak

Exact same-file helper-convention finding was rejected in PR #484.

PR-#484

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2270023 prohibits new direct JsonElement.ValueKind comparisons when equivalent
helpers exist. The changed file directly compares against Object and Array, while the shared
extensions define IsObject and IsArray for those checks.

Rule 2270023: Use JsonElementExtensions helpers instead of direct JsonValueKind comparisons
src/Capacitor.Cli.Core/Acp/AcpSessionModelList.cs[84-84]
src/Capacitor.Cli.Core/Acp/AcpSessionModelList.cs[104-107]
src/Capacitor.Models.Transcripts/JsonElementExtensions.cs[5-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new current-model extraction path directly compares `JsonElement.ValueKind` even though the repository provides shared `IsObject` and `IsArray` extensions.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Acp/AcpSessionModelList.cs[84-106]

## Recommended Fix
Replace `sessionNewResult.ValueKind != JsonValueKind.Object` with `!sessionNewResult.IsObject` and `options.ValueKind != JsonValueKind.Array` with `!options.IsArray`, preserving the existing control flow.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-server (sha: f582e72a)
Review mode: ⚖️ Balanced: This is a behavioral change spanning ACP handshake parsing, daemon model resolution, local status propagation, and Codex reporting, with several independent edge cases despite its modest size.

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs Outdated
Comment thread src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs
@linear-code

linear-code Bot commented Sep 11, 2026

Copy link
Copy Markdown

AI-2717

realtonyyoung and others added 3 commits September 11, 2026 15:33
The local Codex model update now runs only on the PTY path (no handshake model) and never writes the 'default' sentinel, so an app-server Codex keeps its confirmed model. On ACP reconnect, a default launch refreshes its model from session/load instead of keeping the stale startup value.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reconnect keeps its prior model behavior: surfacing a session/load model change would need the orchestrator to re-read the runtime and pulse status, which is out of scope here. ResolvedModel's contract doc now covers the no-request current-model case.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@realtonyyoung
realtonyyoung merged commit ad1aed6 into main Sep 11, 2026
8 checks passed
@realtonyyoung
realtonyyoung deleted the claude-tyoung/daemon-model-resolution branch September 11, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop: show the model in use for all hosted harnesses as chips in the agent list

1 participant