Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions src/Capacitor.Cli.Core/Acp/AcpSessionModelList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,57 @@ static IReadOnlyList<AvailableModelDto> FromModelsObject(JsonElement result) {
}
}

/// <summary>
/// The vendor's CURRENT/default model marker from a <c>session/new</c> result — <c>models.
/// currentModelId</c> (precedence, same reason as <see cref="Extract"/>) or the <c>configOptions</c>
/// <c>model</c> entry's <c>currentValue</c> — or null when neither shape publishes one. Read only
/// as a default-launch fallback (nothing was requested), never to claim a requested model was
/// applied. Total: a shape it cannot read contributes nothing.
/// </summary>
public static string? ExtractCurrentModel(JsonElement sessionNewResult) {
if (sessionNewResult.ValueKind != JsonValueKind.Object)
return null;

return CurrentFromModelsObject(sessionNewResult) ?? CurrentFromConfigOptions(sessionNewResult);
}

static string? CurrentFromModelsObject(JsonElement result) {
if (!result.TryGetProperty("models", out var models))
return null;

try {
var id = JsonSerializer
.Deserialize(models.GetRawText(), CapacitorJsonContext.Default.SessionModelsInfo)
?.CurrentModelId;
return string.IsNullOrWhiteSpace(id) ? null : id;
} catch (JsonException) {
return null;
}
}

static string? CurrentFromConfigOptions(JsonElement result) {
if (!result.TryGetProperty("configOptions", out var options) ||
options.ValueKind != JsonValueKind.Array)
return null;

foreach (var element in options.EnumerateArray()) {
SessionConfigOptionDto? option;
try {
option = JsonSerializer.Deserialize(
element.GetRawText(), CapacitorJsonContext.Default.SessionConfigOptionDto);
} catch (JsonException) {
continue;
}

if (!string.Equals(option?.Id, ModelConfigId, StringComparison.Ordinal))
continue;

return string.IsNullOrWhiteSpace(option!.CurrentValue) ? null : option.CurrentValue;
}

return null;
}

static IReadOnlyList<AvailableModelDto> FromConfigOptions(JsonElement result) {
if (!result.TryGetProperty("configOptions", out var options) ||
options.ValueKind != JsonValueKind.Array)
Expand Down
10 changes: 9 additions & 1 deletion src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1071,9 +1071,17 @@ public async Task StartAsync(
ct)
.ConfigureAwait(false);

// Default launch (nothing requested): no selection ran, so report the vendor's CURRENT model
// from the handshake — the same "show the running model" the desktop rail gets from Pi. Only
// when nothing was requested, so a requested-but-unmatched model still reports null (the
// signal EmitModelFallbackNote and registration rely on).
if (_resolvedModel is null && string.IsNullOrWhiteSpace(requestedModel))
_resolvedModel = AcpSessionModelList.ExtractCurrentModel(sessionNewResult);
Comment thread
qodo-code-review[bot] marked this conversation as resolved.

// Handshake is now fully complete (initialize + session/new + best-effort model selection) —
// one consolidated Info log carrying the negotiated protocol version, loadSession, and the
// resolved model (null if none was requested/matched).
// resolved model (the applied selection, the handshake's current model for a no-request
// launch, or null when a requested model did not match or no current marker was published).
LogHandshakeOk(_agentId, _negotiatedProtocolVersion, _negotiatedCapabilities.LoadSession, _resolvedModel);

// A dropped model is only knowable after session/new publishes the vendor's list, so nothing
Expand Down
49 changes: 38 additions & 11 deletions src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ public string? Title {
/// <see cref="AgentOrchestrator.SetResolvedTitle"/> so the pulse cannot be forgotten.</summary>
public string? ResolvedTitle { get; set; }

/// <summary>Settable override of the positional <c>Model</c> above, for runtimes that only learn
/// the running model after the handshake (Codex resolves it from its config). Written only
/// through <see cref="AgentOrchestrator.SetResolvedModel"/> so the status pulse cannot be
/// forgotten — the same discipline as <see cref="ResolvedTitle"/>. Initialized from the
/// positional parameter so every existing construction site still sets it.</summary>
public string? Model { get; set; } = Model;

/// First non-blank line of the launch prompt, trimmed, capped at 80 chars total (ellipsis when
/// cut, never splitting a surrogate pair) — the status payload is re-sent on every revision,
/// so the full prompt never rides it.
Expand Down Expand Up @@ -905,6 +912,15 @@ internal void SetResolvedTitle(AgentInstance agent, string title) {
_statusNotifier.Pulse();
}

/// The running model, learned after launch (Codex resolves it from its config). Mutate first,
/// pulse second — same ordering as SetResolvedTitle — so the local status frame re-pushes and
/// the desktop rail's model chip fills in.
internal void SetResolvedModel(AgentInstance agent, string model) {
if (agent.Model == model) return;
agent.Model = model;
_statusNotifier.Pulse();
}

/// The attribution ladder: the payload's agent id (raw, then canonical GUID), the resolved
/// vendor session id, the worktree path — each rung only on exactly one live match. Live is
/// "present in _agents"; teardown withdraws whatever was attributed during that window.
Expand Down Expand Up @@ -2388,16 +2404,16 @@ await _server.LaunchFailedAsync(agentId,
LogBridgeDefeatingPosture(agentId, applied.Sandbox, applied.Approval);
}

// An ACP runtime confirms model application during its StartAsync handshake:
// Transcript.ResolvedModel is the id actually applied, or null when the request did not
// take (no availableModels match / the agent rejected the option — the vendor's default
// runs in every null case). Register the CONFIRMED value, never the request: agent.Model
// An ACP runtime confirms its running model during the StartAsync handshake:
// Transcript.ResolvedModel is the applied selection, or — when no model was requested —
// the handshake's current model. It is null when a REQUESTED model did not take (no
// availableModels match / the agent rejected it) OR when a no-request launch published no
// current marker; the vendor default runs with no known id in both. Register that
// CONFIRMED value (null included) rather than a requested-but-unconfirmed one: agent.Model
// feeds AgentRegisteredAsync (live model chip + hosted_agent_started analytics),
// AgentRunStarted (agent_runs), every reconnect re-registration, and the local
// supervision status payload (SnapshotAgentsForStatus). Same requested-vs-running rule
// as ModelSelectionLaunchPolicy, applied per-request instead of per-capability. PTY
// runtimes have no confirmation seam (Transcript is null) and keep reporting
// effectiveModel.
// AgentRunStarted (agent_runs), every reconnect re-registration, and the local supervision
// status payload (SnapshotAgentsForStatus). Only PTY runtimes (Transcript is null) fall
// back to effectiveModel.
var registeredModel = start.Transcript is { } confirmed ? confirmed.ResolvedModel : effectiveModel;

var agent = new AgentInstance(agentId, prompt, registeredModel, effort, repoPath, cmd.Vendor, runtime, worktree, cts) {
Expand Down Expand Up @@ -2536,7 +2552,9 @@ await _server.LaunchFailedAsync(agentId,
// Legacy path (name/arity/behavior UNCHANGED): the dispatched `model` may be the "default"
// no-override sentinel, in which case Codex resolves the model from ~/.codex/config.toml.
// Codex-only — Claude/other agents never call the ReportAgentResolvedModel hub.
ReportResolvedModel(agentId, cmd.Vendor, model);
// updateLocal only on the PTY path: an app-server Codex (Transcript present) already
// registered its confirmed handshake model, which must not be overwritten locally.
ReportResolvedModel(agentId, cmd.Vendor, model, updateLocal: start.Transcript is null);
}

// Phase B2-b (sequenced-settlement design §4.2.2): the launch executed — the agent is registered.
Expand Down Expand Up @@ -2641,7 +2659,7 @@ async Task DiscardFailedLaunchJournalAsync(TranscriptJournal? journal, string ag
/// <c>~/.codex/config.toml</c> — we resolve the same value here. Never throws: a resolve/report
/// failure must not break launch.
/// </summary>
void ReportResolvedModel(string agentId, string vendor, string model) {
void ReportResolvedModel(string agentId, string vendor, string model, bool updateLocal) {
try {
var isDefault = string.IsNullOrEmpty(model) || string.Equals(model, "default", StringComparison.OrdinalIgnoreCase);

Expand All @@ -2651,6 +2669,15 @@ void ReportResolvedModel(string agentId, string vendor, string model) {

if (string.IsNullOrEmpty(resolved)) return;

// Surface it on the LOCAL status frame only for a runtime with no authoritative handshake
// model — the PTY path (updateLocal). An app-server Codex already registered the confirmed
// model at launch, so overwriting it with a config-derived value would show the wrong one;
// and the unresolved "default" sentinel is never a real model to display.
if (updateLocal
&& !string.Equals(resolved, "default", StringComparison.OrdinalIgnoreCase)
&& _agents.TryGetValue(agentId, out var agent))
SetResolvedModel(agent, resolved);

_ = _server.ReportAgentResolvedModelAsync(agentId, resolved);
} catch (Exception ex) {
LogReportResolvedModelFailed(ex, agentId);
Expand Down
17 changes: 9 additions & 8 deletions src/Capacitor.Cli.Daemon/Services/IAcpTranscriptSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ internal interface IAcpTranscriptSource {
string Cwd { get; }

/// <summary>
/// The model id actually resolved AND applied by model selection (i.e. the vendor's
/// model-selection RPC — <c>session/set_config_option</c> or <c>session/set_model</c>,
/// whichever the descriptor's selector sends — was sent and answered without error) — or
/// <see langword="null"/> when no model was requested, resolution found no match in
/// <c>session/new</c>'s <c>availableModels</c>, or the agent rejected the option. In every "null"
/// case the vendor's own default model applies (Cursor's for a Cursor session, Kiro's for a
/// Kiro session, …), which is why this is nullable rather than falling back to the
/// requested-but-unconfirmed id.
/// The confirmed running model of the ACP session: either the id resolved AND applied by model
/// selection (the vendor's model-selection RPC — <c>session/set_config_option</c> or
/// <c>session/set_model</c> — was sent and answered without error), OR, when NO model was
/// requested, the current model the <c>session/new</c> handshake reported. It is
/// <see langword="null"/> in two cases: a REQUESTED model that did not take (no match in
/// <c>availableModels</c> / the agent rejected the option), and a no-request launch whose
/// <c>session/new</c> published no current-model marker. In both the vendor's own default runs and
/// its specific id is unknown — so a non-null value is always a model the session is actually
/// running, never a requested-but-unconfirmed or guessed one.
/// </summary>
string? ResolvedModel { get; }

Expand Down
36 changes: 36 additions & 0 deletions test/Capacitor.Cli.Core.Tests.Unit/Acp/AcpSessionModelListTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -186,4 +186,40 @@ public async Task ResolvesAgainstTheOpenCodeList(string requested, string? expec

await Assert.That(AcpModelResolver.Resolve(requested, models)).IsEqualTo(expected);
}

/// The current/default model marker — read only as a default-launch fallback so the desktop
/// rail shows the running model even when nothing was requested. Both shapes carry it.
[Test]
public async Task ExtractCurrentModel_reads_the_models_object_current_id() {
await Assert.That(AcpSessionModelList.ExtractCurrentModel(Result(ModelsResult)))
.IsEqualTo("claude-sonnet-4-5[thinking=true]");
}

[Test]
public async Task ExtractCurrentModel_reads_the_configOptions_current_value() {
await Assert.That(AcpSessionModelList.ExtractCurrentModel(Result(OpenCodeResult)))
.IsEqualTo("opencode/big-pickle");
}

/// Same precedence as Extract: the standardized `models` shape wins over a `configOptions` mirror.
[Test]
public async Task ExtractCurrentModel_prefers_the_models_object_over_configOptions() {
var both = Result("""
{ "sessionId": "s",
"models": { "currentModelId": "from-models" },
"configOptions": [ { "id": "model", "currentValue": "from-config",
"options": [ { "value": "from-config", "name": "c" } ] } ] }
""");

await Assert.That(AcpSessionModelList.ExtractCurrentModel(both)).IsEqualTo("from-models");
}

[Test]
[Arguments("""{ "sessionId": "s" }""")]
[Arguments("""{ "sessionId": "s", "models": { "availableModels": [] } }""")]
[Arguments("""{ "sessionId": "s", "configOptions": [ { "id": "mode", "currentValue": "build" } ] }""")]
[Arguments("""[ "not", "an", "object" ]""")]
public async Task ExtractCurrentModel_yields_null_when_no_current_marker(string json) {
await Assert.That(AcpSessionModelList.ExtractCurrentModel(Result(json))).IsNull();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,25 @@ await h.Runtime.StartAsync(
await Assert.That(h.Runtime.ResolvedModel).IsEqualTo("claude-haiku-4.5");
}

/// A default launch (no requested model) applies no selection, but the desktop rail still needs
/// the running model — so the runtime reports the handshake's current model as ResolvedModel,
/// the same "show the running model" Pi has. No set_* is sent.
[Test]
public async Task StartAsync_DefaultLaunch_ExposesTheHandshakeCurrentModelAsResolved() {
await using var h = new Harness();
h.Fake.SetSessionNewResult(FakeAcpAgent.BuildSessionNewResult(
FakeAcpAgent.FixedSessionId, currentModelId: "composer-2.5[fast=true]", TeamAvailableModels));
h.StartFakeAgentLoop();

await h.Runtime.StartAsync(
"/abs/worktree", "do the thing", h.Cts.Token, requestedModel: null
).WaitAsync(HangGuard);

await Assert.That(h.Fake.ReceivedCalls.Any(c => c.Method == "session/set_config_option")).IsFalse();
await Assert.That(h.Fake.ReceivedCalls.Any(c => c.Method == "session/set_model")).IsFalse();
await Assert.That(h.Runtime.ResolvedModel).IsEqualTo("composer-2.5[fast=true]");
}

[Test]
public async Task StartAsync_ExactModelId_SendsSetConfigOptionWithThatExactId_BeforeThePrompt() {
await using var h = new Harness();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1664,6 +1664,9 @@ public async Task NewDaemon_oldServer_legacy_codex_launch_uses_the_unchanged_Rep
await Assert.That(server.ReportAgentResolvedModelCalls).Contains(("agent-legacy", "gpt-5-codex"));
await Assert.That(server.ExplicitReviewerModelReports).IsEmpty();

// ...and the model reaches the LOCAL status frame too, so a local Codex row shows the chip.
await Assert.That(orch.SnapshotAgentsForStatus().Single(a => a.Id == "agent-legacy").Model).IsEqualTo("gpt-5-codex");

await orch.HandleStopAgentForTest("agent-legacy");

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,25 @@ public async Task Snapshot_normalizes_blank_model_to_null_and_passes_real_model_
}
}

/// <summary>A model learned after launch (Codex resolves it post-handshake from its config) is
/// written through <c>SetResolvedModel</c>, and the next snapshot re-reads it — so a local Codex
/// row's model chip fills in without a re-registration.</summary>
[Test]
public async Task SetResolvedModel_updates_the_model_the_snapshot_reports() {
var fixture = Build();
var orch = fixture.Orchestrator;
try {
var agent = orch.SeedAgentForTest("codex-default", model: null);
await Assert.That(orch.SnapshotAgentsForStatus().Single(a => a.Id == "codex-default").Model).IsNull();

orch.SetResolvedModel(agent, "gpt-5-codex");

await Assert.That(orch.SnapshotAgentsForStatus().Single(a => a.Id == "codex-default").Model).IsEqualTo("gpt-5-codex");
} finally {
await fixture.CleanupAsync();
}
}

/// <summary>
/// Pins <see cref="AgentOrchestrator.SnapshotAgentsForStatus"/>'s stamping of
/// <c>HasTerminal</c> from the agent's own runtime — a PTY runtime (<c>SeedAgentForTest</c>'s
Expand Down