fix(desktop): Phase 1 round-7 — CI e2e failures: mock bridge persistence, phantom commandUpdate, llmProviderFieldVisible, model label, combobox, harness save routing

Six deterministic CI failure families closed:

Family 1 (Save-still-open: edit-agent:156/190, agent-access-warning:155):
  Root cause: unlinked agents (personaId=null) generated a phantom agentCommand
  write every save because the second clause of commandUpdate was checking
  agentCommandOverride==null && pinned.length>0 — true for every unlinked agent
  with a default runtime. The submitted agentCommand never matched the mock's
  stale observe value → settlement always failed → dialog stayed open.
  Fix: guard the second clause with inst.personaId!=null so unlinked agents
  never generate a harness write on an untouched save.

  Root cause also: mock handleUpdateManagedAgent did not persist provider,
  agentCommand, harnessOverride, agentArgs, acpCommand, or parallelism.
  Any submit that included provider (test 190: Anthropic + custom model)
  would fail settlement because observed.provider stayed null.
  Fix: extend handleUpdateManagedAgent and RawManagedAgent in e2eBridge.ts
  to handle all fields that observedStateMatchesAgentInput checks, including
  agent_command_override to support harnessOverride settlement.

Family 2 (model label source=global, edit-agent:318):
  resolveInheritedModelLabel only handled source=="build"; source=="global"
  fell through to the generic "Default (model)" label instead of calling
  getDefaultLlmModelLabel which renders "Use agent defaults (model)".
  Fix: add source==="global" branch.

Family 3 (test 11 provider picker visible: global-agent-config-screenshots:899):
  llmProviderFieldVisible used prospectiveRuntimeId (instance harness pin,
  derived from inst.agentCommand="goose") in D+I context. The D-section
  provider picker is driven by the definition runtime, not the instance pin.
  A runtime-less definition with a goose-pinned instance was incorrectly
  exposing the provider picker.
  Fix: use definitionRuntimeId in D+I context; prospectiveRuntimeId only in
  pure I-only context.

Family 4 (model combobox not found: agent-provider-dropdowns:147):
  Auto-seed effect set definitionRuntimeId but not selectedRuntimeId in D-only
  context. useAgentEditRuntimeState derives selectedRuntime from selectedRuntimeId
  which drove model discovery — a stale "custom" value left discovery unable to
  resolve a runtime and the model list stayed empty.
  Fix: auto-seed also calls setSelectedRuntimeId in D-only context (!inst).
  Also added pickModelComboboxOption helper to edit-agent.spec.ts (uses
  getByRole("button") not menuitemradio for PersonaModelCombobox items).

Family 5 (Databricks text leak: agent-provider-dropdowns:213):
  Test used globalAgentConfig.model="global-databricks-model" which produced
  a "Use agent defaults (global-databricks-model)" label containing "Databricks".
  Fix: change test fixture to "global-openai-model".

Family 6 (saved harness not selected in definition edit: inline-custom-harness:122):
  selectSavedHarness (usePendingHarnessSelection) called handleRuntimeDropdownChange
  in all contexts, but in D-only context the harness dropdown is driven by
  definitionRuntimeId via handleDefinitionRuntimeChange. The I-pin handler
  set selectedRuntimeId, which is hidden in D-only context.
  Fix: route to handleDefinitionRuntimeChange when showDef && !showInst.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
Duncan
2026-08-08 21:03:18 -04:00
parent 2103bb7413
commit 0e4cb69a7a
6 changed files with 84 additions and 5 deletions

View File

@@ -322,6 +322,14 @@ export function AgentEditMergedDialog({
const defaultRuntime = getDefaultPersonaRuntime(runtimes);
if (!defaultRuntime) return;
setDefinitionRuntimeId(defaultRuntime.id);
// In definition-only context, selectedRuntimeId is the same conceptual field
// as definitionRuntimeId (there is no separate I-harness pin). Sync it so
// that model discovery runs: useAgentEditRuntimeState derives `selectedRuntime`
// from `selectedRuntimeId`, and a stale "custom" value leaves discovery unable
// to resolve a runtime → model list stays empty.
if (!inst) {
setSelectedRuntimeId(defaultRuntime.id);
}
}, [open, runtimes.length, showDef]);
// Re-derive runtime id when catalog loads
@@ -545,7 +553,12 @@ export function AgentEditMergedDialog({
const selectSavedHarness = usePendingHarnessSelection(
runtimes,
handleRuntimeDropdownChange,
// In definition-only context the harness dropdown drives `definitionRuntimeId`
// (via handleDefinitionRuntimeChange), not the I-harness pin. Route to the
// correct handler so the saved harness appears in the D-section dropdown.
showDef && !showInst
? handleDefinitionRuntimeChange
: handleRuntimeDropdownChange,
open,
);

View File

@@ -181,9 +181,18 @@ export function useAgentEditMergedSubmit(
commandUpdate = inst.agentCommandOverride != null ? "" : undefined;
} else {
const pinned = (effectiveRuntime?.command ?? s.agentCommand).trim();
// Only write a commandUpdate when the command actually changed OR
// when a linked agent (personaId != null) is creating a new pin that
// matches the current inherited command (no override yet). Unlinked
// agents have no inherit/pin distinction — their agentCommandOverride
// is always null, so the second clause must not trigger for them or
// every unmodified save generates a phantom harness write that
// `observedStateMatchesAgentInput` then fails to settle.
if (
pinned !== inst.agentCommand ||
(inst.agentCommandOverride == null && pinned.length > 0)
(inst.personaId != null &&
inst.agentCommandOverride == null &&
pinned.length > 0)
) {
commandUpdate = pinned;
}

View File

@@ -31,6 +31,7 @@ import {
sortPersonaRuntimes,
type PersonaDropdownOption,
getProviderApiKeyEnvVar,
getDefaultLlmModelLabel,
} from "./agentConfigOptions";
import {
MODEL_DISCOVERY_LOADING_VALUE,
@@ -221,7 +222,16 @@ export function useAgentEditRuntimeState({
]);
const llmProviderFieldVisible = runtimeSupportsLlmProviderSelection(
showInst ? prospectiveRuntimeId : selectedRuntimeId,
// In linked context (showDef && showInst), the D-section provider/model
// picker is driven by the definition's runtime (definitionRuntimeId), not by
// the instance's harness pin (prospectiveRuntimeId). Using prospectiveRuntimeId
// here would make the D-section picker visible whenever the instance's
// agentCommand matches a provider-supporting runtime, even if the definition
// has no runtime configured — a phantom provider field exposure.
// Instance-only context (showInst && !showDef) correctly uses prospectiveRuntimeId
// because the instance's harness is the only runtime in scope.
// D-only context falls through to definitionRuntimeId via selectedRuntimeId (they equal).
showInst && !showDef ? prospectiveRuntimeId : definitionRuntimeId,
);
const prospectiveRuntime = runtimes.find(
(r) => r.id === prospectiveRuntimeId,
@@ -460,5 +470,6 @@ function resolveInheritedModelLabel(
const model = inherited.value;
if (!model) return "Default model";
if (inherited.source === "build") return getBakedModelInheritLabel(model);
if (inherited.source === "global") return getDefaultLlmModelLabel(model);
return `Default (${model})`;
}

View File

@@ -857,6 +857,8 @@ type RawManagedAgent = {
| { type: "local" }
| { type: "provider"; id: string; config: Record<string, unknown> };
backend_agent_id: string | null;
/** Non-null when the instance has an explicit harness pin (user chose a specific command). */
agent_command_override?: string | null;
respond_to: "owner-only" | "allowlist" | "anyone";
respond_to_allowlist: string[];
};
@@ -1687,6 +1689,7 @@ function cloneManagedAgent(agent: MockManagedAgent): RawManagedAgent {
auto_restart_on_config_change: agent.auto_restart_on_config_change ?? true,
backend: agent.backend ?? { type: "local" as const },
backend_agent_id: agent.backend_agent_id ?? null,
agent_command_override: agent.agent_command_override ?? null,
respond_to: agent.respond_to ?? "owner-only",
respond_to_allowlist: agent.respond_to_allowlist
? [...agent.respond_to_allowlist]
@@ -8670,10 +8673,16 @@ async function handleUpdateManagedAgent(args: {
pubkey: string;
name?: string;
model?: string | null;
provider?: string | null;
systemPrompt?: string | null;
envVars?: Record<string, string>;
respondTo?: "owner-only" | "allowlist" | "anyone";
respondToAllowlist?: string[];
agentCommand?: string;
harnessOverride?: boolean;
agentArgs?: string[];
acpCommand?: string;
parallelism?: number;
};
}): Promise<{ agent: RawManagedAgent; profile_sync_error: string | null }> {
const agent = getMockManagedAgent(args.input.pubkey);
@@ -8683,6 +8692,9 @@ async function handleUpdateManagedAgent(args: {
if (args.input.model !== undefined) {
agent.model = args.input.model;
}
if (args.input.provider !== undefined) {
agent.provider = args.input.provider;
}
if (args.input.systemPrompt !== undefined) {
agent.system_prompt = args.input.systemPrompt;
}
@@ -8695,6 +8707,25 @@ async function handleUpdateManagedAgent(args: {
if (args.input.respondToAllowlist !== undefined) {
agent.respond_to_allowlist = args.input.respondToAllowlist;
}
if (args.input.agentCommand !== undefined) {
agent.agent_command = args.input.agentCommand;
// Mirror Rust backend: non-empty command + harnessOverride=true → set override;
// empty string (inherit sentinel) → clear override.
if (args.input.agentCommand.length === 0) {
agent.agent_command_override = null;
} else if (args.input.harnessOverride) {
agent.agent_command_override = args.input.agentCommand;
}
}
if (args.input.agentArgs !== undefined) {
agent.agent_args = [...args.input.agentArgs];
}
if (args.input.acpCommand !== undefined) {
agent.acp_command = args.input.acpCommand;
}
if (args.input.parallelism !== undefined) {
agent.parallelism = args.input.parallelism;
}
agent.updated_at = new Date().toISOString();
return { agent: cloneManagedAgent(agent), profile_sync_error: null };
}

View File

@@ -216,7 +216,7 @@ test.describe("agent provider dropdown screenshots", () => {
await installMockBridge(page, {
globalAgentConfig: {
provider: "databricks_v2",
model: "global-databricks-model",
model: "global-openai-model",
env_vars: {},
},
personas: [

View File

@@ -78,6 +78,21 @@ async function pickDropdownOption(
await page.getByRole("menuitemradio", { name: optionName }).click();
}
/**
* Pick an option from a PersonaModelCombobox (popover-based combobox with
* <button> items, not a PersonaDropdownField with menuitemradio items).
*/
async function pickModelComboboxOption(
page: import("@playwright/test").Page,
triggerId: string,
optionName: string | RegExp,
) {
await page.locator(`#${triggerId}`).click();
// Items in PersonaModelCombobox are plain <button> elements inside the popover.
// Query at page level because the popover is portaled outside the dialog.
await page.getByRole("button", { name: optionName }).click();
}
test.describe("agent definition dialog", () => {
test("owner-only-access build shows disabled agent access with an explanation", async ({
page,
@@ -205,7 +220,7 @@ test.describe("edit agent dialog", () => {
// Pick a provider so model discovery has a scope, then set a custom model.
await pickDropdownOption(page, "edit-agent-llm-provider", "Anthropic");
await pickDropdownOption(page, "edit-agent-model", "Custom model...");
await pickModelComboboxOption(page, "edit-agent-model", "Custom model...");
await page.locator("#edit-agent-custom-model").fill("claude-opus-4-5");
// Anthropic requires a credential before save unlocks.
await page.getByLabel("Anthropic API Key").fill("sk-test-edit-agent-e2e");