fix(agents): address effort settlement, pin→inherit suppression, and runtime-switch reset

IMPORTANT-1: Add effortLevel comparison to observedStateMatchesAgentInput
(agentSaveCoordinator.ts) with tri-state semantics — absent submission skips,
null clears, string compares against observed column. ManagedAgent now exposes
effortLevel from ManagedAgentSummary (Rust types.rs + runtime.rs) so the
comparator can detect a backend-rejected effort write rather than closing as
success. Regressions: 4 family-14 coordinator tests; mutation (remove compare)
turns the 2 rejected-edit tests RED while the 2 settled tests stay GREEN.

IMPORTANT-2: Replace raw effortLevel include in useAgentEditMergedSubmit with
resolveEffortSubmission — suppresses effort on pin→inherit (agentCommand:"")
and on unchanged selections, matching the deleted AgentInstanceEditDialog's
PR #4625 semantics exactly. originalEffortLevel threaded from
configSurfaceQuery into AgentEditSubmitState.

IMPORTANT-3: Add setEffortLevel and effortTouched to RuntimeHandlersInput;
call setEffortLevel(null) and effortTouched.current = false in
handleRuntimeDropdownChange (useAgentEditRuntimeHandlers.ts) so runtime
switches do not carry stale vocabulary values into the next Save. Port the
6 load-bearing mounted production-seam tests from the deleted
agentInstanceEditCancelSafety.test.mjs to agentEditMergedCancelSafety.test.mjs
against AgentEditMergedDialog: selection+Cancel zero writes, ordinary Save
carries effortLevel, rejected Save stays open, pin→inherit suppresses effort,
access+effort atomic. Runtime-switch reset covered by hook-level test in
agentEditMergedRuntimeReset.test.mjs.

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-09-02 20:37:43 -04:00
parent a950da2a3c
commit d63ed1e3b9
12 changed files with 1096 additions and 4 deletions

View File

@@ -331,6 +331,7 @@ pub fn build_managed_agent_summary(
log_path,
respond_to: record.respond_to,
respond_to_allowlist: record.respond_to_allowlist.clone(),
effort_level: record.effort_level.clone(),
})
}

View File

@@ -586,6 +586,12 @@ pub struct ManagedAgentSummary {
pub log_path: String,
pub respond_to: RespondTo,
pub respond_to_allowlist: Vec<String>,
/// Canonical harness-agnostic effort level persisted on the record.
/// `None` means the agent uses the adapter default at launch.
/// Exposed here so the edit dialog's settlement comparator can detect a
/// backend-rejected effort write rather than closing as success.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub effort_level: Option<String>,
}
#[derive(Debug, Serialize)]

View File

@@ -761,6 +761,7 @@ fn summary_fixture(
log_path: String::new(),
respond_to: RespondTo::OwnerOnly,
respond_to_allowlist: Vec::new(),
effort_level: None,
}
}

View File

@@ -556,6 +556,8 @@ export function AgentEditMergedDialog({
onUpdated,
effortLevel,
effortTouched,
originalEffortLevel:
configSurfaceQuery.data?.normalized.thinkingEffort?.value ?? null,
} satisfies AgentEditSubmitState;
const {
@@ -621,6 +623,8 @@ export function AgentEditMergedDialog({
setAgentArgs,
runtimeTouched,
setIsAddHarnessOpen,
setEffortLevel,
effortTouched,
runtimes,
selectedRuntime,
open,

View File

@@ -0,0 +1,693 @@
/**
* Cancel-safety + Save-gated effort production-seam pins for AgentEditMergedDialog.
*
* Ported from the deleted agentInstanceEditCancelSafety.test.mjs (present in
* main at 0dbd036f5) against the surviving AgentEditMergedDialog. The deleted
* file mounted AgentInstanceEditDialog; every test here mounts the merged
* dialog against the same IPC boundary and asserts the same contracts:
*
* (a) effort selection alone + Cancel dispatch ZERO update_managed_agent
* (b) ordinary effort Save carries effortLevel inside the locked update
* (c) rejected effort Save keeps the dialog open (no close)
* (d) pin→inherit Save sends agentCommand:"" with NO effortLevel
* (e) runtime switch clears stale effort — no effortLevel after switch
* (f) access + effort changes share one atomic update_managed_agent payload
*
* Why full production renders: the seam being pinned is the dialog footer's
* wiring (Cancel → onOpenChange, Save → handleSubmit → update_managed_agent)
* and the submit hook's resolveEffortSubmission gate. A model-level miniature
* cannot catch a regression that rewires Cancel to handleSubmit or drops the
* inherit-transition suppression.
*/
import assert from "node:assert/strict";
import { after, afterEach, before, test } from "node:test";
import { JSDOM } from "jsdom";
const dom = new JSDOM("<!doctype html><html><body></body></html>", {
url: "http://localhost",
});
const liveClients = [];
const pendingTimers = new Set();
const nativeSetTimeout = globalThis.setTimeout;
const nativeSetInterval = globalThis.setInterval;
const nativeClearTimeout = globalThis.clearTimeout;
const nativeClearInterval = globalThis.clearInterval;
let act;
let cleanup;
let fireEvent;
let render;
let screen;
let createElement;
let QueryClient;
let QueryClientProvider;
let ThemeProvider;
let AgentEditMergedDialog;
const ipcCalls = [];
const ipcHandlers = new Map();
// A goose-pinned instance linked to a claude persona — gives us the
// pin→inherit transition (agentCommandOverride non-null = opens pinned).
const AGENT_PK = "d".repeat(64);
function rawAgent(overrides = {}) {
return {
pubkey: AGENT_PK,
name: "pinned-instance",
persona_id: "p1",
runtime: "goose",
relay_url: "wss://relay.example",
acp_command: "acp",
agent_command: "goose",
agent_command_override: "goose",
agent_args: [],
mcp_command: "mcp",
turn_timeout_seconds: 300,
idle_timeout_seconds: null,
max_turn_duration_seconds: null,
parallelism: 1,
system_prompt: null,
avatar_url: null,
model: null,
model_source: null,
provider: null,
persona_out_of_date: false,
persona_orphaned: false,
needs_restart: false,
restart_diff: [],
env_vars: {},
status: "running",
pid: 1234,
created_at: "2026-01-01T00:00:00Z",
updated_at: "2026-01-01T00:00:00Z",
last_started_at: null,
last_stopped_at: null,
last_exit_code: null,
last_error: null,
last_error_code: null,
log_path: "/tmp/agent.log",
start_on_app_launch: false,
auto_restart_on_config_change: true,
backend: { type: "local" },
backend_agent_id: null,
respond_to: "mentions",
respond_to_allowlist: [],
...overrides,
};
}
function rawPersona(overrides = {}) {
return {
id: "p1",
display_name: "Scribe",
avatar_url: null,
system_prompt: "be helpful",
runtime: "claude",
model: null,
provider: null,
name_pool: [],
is_builtin: false,
is_active: true,
shared: false,
source_team: null,
env_vars: {},
respond_to: null,
respond_to_allowlist: [],
parallelism: null,
created_at: "2026-01-01T00:00:00Z",
updated_at: "2026-01-01T00:00:00Z",
...overrides,
};
}
function rawRuntime(id, overrides = {}) {
return {
id,
label: id,
avatar_url: "",
availability: "available",
command: id,
binary_path: `/usr/local/bin/${id}`,
default_args: [],
mcp_command: null,
install_hint: "",
install_instructions_url: "",
can_auto_install: false,
underlying_cli_path: null,
node_required: false,
auth_status: { status: "logged_in" },
source: "builtin",
...overrides,
};
}
function configSurface(overrides = {}) {
return {
runtimeId: "goose",
runtimeLabel: "goose",
isPreSpawn: false,
normalized: {
model: null,
provider: null,
mode: null,
thinkingEffort: null,
maxOutputTokens: null,
contextLimit: null,
systemPrompt: null,
},
advanced: [],
extensions: [],
sources: {
acpNative: "notApplicable",
acpConfigOptions: "notApplicable",
envVars: "available",
configFile: "notApplicable",
configFilePath: null,
mcpConfigFilePath: null,
},
...overrides,
};
}
function effortConfigSurface(overrides = {}) {
return configSurface({
effortConfigId: "thought_level",
effortOptions: [
{ value: "low", displayName: "Low" },
{ value: "high", displayName: "High" },
],
...overrides,
});
}
// Convert a snake_case agent fixture to the camelCase ManagedAgent the dialog
// receives as ctx.instance.
function toCamelAgent(raw) {
return {
pubkey: raw.pubkey,
name: raw.name,
personaId: raw.persona_id,
runtime: raw.runtime,
teamId: null,
relayUrl: raw.relay_url,
acpCommand: raw.acp_command,
agentCommand: raw.agent_command,
agentCommandOverride: raw.agent_command_override,
agentArgs: raw.agent_args,
mcpCommand: raw.mcp_command,
turnTimeoutSeconds: raw.turn_timeout_seconds,
idleTimeoutSeconds: raw.idle_timeout_seconds,
maxTurnDurationSeconds: raw.max_turn_duration_seconds,
parallelism: raw.parallelism,
systemPrompt: raw.system_prompt,
avatarUrl: raw.avatar_url,
model: raw.model,
modelSource: raw.model_source,
provider: raw.provider,
personaOutOfDate: raw.persona_out_of_date,
personaOrphaned: raw.persona_orphaned,
needsRestart: raw.needs_restart,
restartDiff: raw.restart_diff ?? [],
envVars: raw.env_vars,
status: raw.status,
pid: raw.pid,
createdAt: raw.created_at,
updatedAt: raw.updated_at,
lastStartedAt: raw.last_started_at,
lastStoppedAt: raw.last_stopped_at,
lastExitCode: raw.last_exit_code,
lastError: raw.last_error,
lastErrorCode: raw.last_error_code,
logPath: raw.log_path,
startOnAppLaunch: raw.start_on_app_launch,
autoRestartOnConfigChange: raw.auto_restart_on_config_change,
backend: raw.backend,
backendAgentId: raw.backend_agent_id,
respondTo: raw.respond_to,
respondToAllowlist: raw.respond_to_allowlist,
};
}
function installIpc(surface = configSurface()) {
const set = (cmd, handler) => ipcHandlers.set(cmd, handler);
set("discover_acp_providers", () =>
Promise.resolve([rawRuntime("claude"), rawRuntime("goose")]),
);
set("list_personas", () => Promise.resolve([rawPersona()]));
set("get_agent_config_surface", () => Promise.resolve(surface));
set("get_global_agent_config", () =>
Promise.resolve({
env_vars: {},
provider: null,
model: null,
preferred_runtime: null,
}),
);
set("get_baked_build_env", () => Promise.resolve([]));
set("get_baked_build_env_keys", () => Promise.resolve([]));
set("get_runtime_file_config", () => Promise.resolve(null));
set("agent_access_owner_only", () => Promise.resolve(false));
set("discover_agent_models", () =>
Promise.resolve({
agentName: "goose",
agentVersion: "1.0",
models: [],
agentDefaultModel: null,
selectedModel: null,
supportsSwitching: false,
}),
);
set("update_managed_agent", (args) => {
ipcCalls.push({ cmd: "update_managed_agent", args });
return Promise.resolve({ agent: rawAgent(), profile_sync_error: null });
});
set("set_managed_agent_auto_restart", (args) => {
ipcCalls.push({ cmd: "set_managed_agent_auto_restart", args });
return Promise.resolve();
});
}
function installEffortIpc({ deferUpdate = false, failUpdate = false } = {}) {
installIpc(effortConfigSurface());
const set = (cmd, handler) => ipcHandlers.set(cmd, handler);
let resolveUpdate = () => {};
set("update_managed_agent", (args) => {
ipcCalls.push({ cmd: "update_managed_agent", args });
if (failUpdate) {
return Promise.reject(new Error("update failed"));
}
const response = { agent: rawAgent(), profile_sync_error: null };
if (!deferUpdate) {
return Promise.resolve(response);
}
return new Promise((resolve) => {
resolveUpdate = () => resolve(response);
});
});
return { resolveUpdate: () => resolveUpdate() };
}
function renderDialog(onOpenChange = () => {}) {
const client = new QueryClient({
defaultOptions: {
mutations: { gcTime: 0 },
queries: { gcTime: 0, retry: false },
},
});
liveClients.push(client);
return render(
createElement(
ThemeProvider,
{ defaultTheme: "buzz" },
createElement(
QueryClientProvider,
{ client },
createElement(AgentEditMergedDialog, {
open: true,
onOpenChange,
ctx: {
kind: "instance-only",
instance: toCamelAgent(rawAgent()),
},
}),
),
),
);
}
/** Returns update_managed_agent calls whose input carries an effortLevel. */
function effortCalls() {
return ipcCalls.filter(
(c) =>
c.cmd === "update_managed_agent" &&
c.args.input?.effortLevel !== undefined,
);
}
/** Opens the effort dropdown and clicks the item matching `label`. */
async function selectEffort(label) {
const trigger = dom.window.document.getElementById("edit-agent-effort");
assert.ok(
trigger,
"effort picker trigger must render for a local + effort-capable agent",
);
await act(async () => {
fireEvent.pointerDown(
trigger,
new dom.window.MouseEvent("pointerdown", { bubbles: true, button: 0 }),
);
fireEvent.click(trigger);
});
const item = [
...dom.window.document.querySelectorAll('[role="menuitemradio"]'),
].find((node) => node.textContent?.trim() === label);
assert.ok(item, `effort option "${label}" must be offered`);
await act(async () => {
fireEvent.click(item);
});
}
/** Expands Advanced and toggles the inherit checkbox from unchecked → checked. */
async function expandAdvancedAndToggleInherit() {
await act(async () => {
fireEvent.click(screen.getByRole("button", { name: /Advanced/ }));
});
const checkbox = dom.window.document.getElementById(
"edit-agent-inherit-harness",
);
assert.ok(
checkbox,
"inherit checkbox must render for a persona-linked agent inside Advanced",
);
assert.equal(
checkbox.checked,
false,
"a harness-pinned agent must open with inherit unchecked",
);
await act(async () => {
fireEvent.click(checkbox);
});
assert.equal(checkbox.checked, true, "inherit toggle must flip to checked");
}
before(async () => {
globalThis.setTimeout = (fn, ms, ...args) => {
const id = nativeSetTimeout(
(...a) => {
pendingTimers.delete(id);
return fn(...a);
},
ms,
...args,
);
pendingTimers.add(id);
return id;
};
globalThis.setInterval = (fn, ms, ...args) => {
const id = nativeSetInterval(fn, ms, ...args);
pendingTimers.add(id);
return id;
};
globalThis.clearTimeout = (id) => {
pendingTimers.delete(id);
return nativeClearTimeout(id);
};
globalThis.clearInterval = (id) => {
pendingTimers.delete(id);
return nativeClearInterval(id);
};
for (const key of Object.getOwnPropertyNames(dom.window)) {
if (key in globalThis) continue;
try {
globalThis[key] = dom.window[key];
} catch {
/* getter-only global — skip */
}
}
Object.assign(globalThis, {
document: dom.window.document,
HTMLElement: dom.window.HTMLElement,
Node: dom.window.Node,
IS_REACT_ACT_ENVIRONMENT: true,
window: dom.window,
getComputedStyle: dom.window.getComputedStyle.bind(dom.window),
});
for (const key of [
"Event",
"CustomEvent",
"MouseEvent",
"KeyboardEvent",
"FocusEvent",
"PointerEvent",
"InputEvent",
"UIEvent",
]) {
if (dom.window[key]) globalThis[key] = dom.window[key];
}
dom.window.ResizeObserver = class {
observe() {}
unobserve() {}
disconnect() {}
};
globalThis.ResizeObserver = dom.window.ResizeObserver;
dom.window.requestAnimationFrame = (cb) =>
globalThis.setTimeout(() => cb(Date.now()), 0);
dom.window.cancelAnimationFrame = (id) => globalThis.clearTimeout(id);
globalThis.requestAnimationFrame = dom.window.requestAnimationFrame;
globalThis.cancelAnimationFrame = dom.window.cancelAnimationFrame;
dom.window.HTMLElement.prototype.scrollIntoView = () => {};
dom.window.HTMLElement.prototype.hasPointerCapture = () => false;
dom.window.HTMLElement.prototype.releasePointerCapture = () => {};
dom.window.HTMLElement.prototype.setPointerCapture = () => {};
dom.window.matchMedia = () => ({
matches: false,
addEventListener() {},
removeEventListener() {},
});
dom.window.__TAURI_INTERNALS__ = {
invoke: (cmd, args) => {
const handler = ipcHandlers.get(cmd);
if (handler) return handler(args);
return Promise.reject(new Error(`unmocked Tauri command: ${cmd}`));
},
transformCallback: () => Math.random(),
};
({ act, cleanup, fireEvent, render, screen } = await import(
"@testing-library/react"
));
({ createElement } = await import("react"));
({ QueryClient, QueryClientProvider } = await import(
"@tanstack/react-query"
));
({ ThemeProvider } = await import("@/shared/theme/ThemeProvider"));
({ AgentEditMergedDialog } = await import("./AgentEditMergedDialog.tsx"));
});
afterEach(() => {
cleanup?.();
for (const client of liveClients.splice(0)) {
client.cancelQueries();
client.clear();
}
ipcHandlers.clear();
ipcCalls.length = 0;
for (const id of pendingTimers) nativeClearTimeout(id);
pendingTimers.clear();
});
after(() => {
globalThis.setTimeout = nativeSetTimeout;
globalThis.setInterval = nativeSetInterval;
globalThis.clearTimeout = nativeClearTimeout;
globalThis.clearInterval = nativeClearInterval;
dom.window.close();
});
// ── (a) effort selection alone + Cancel → zero writes ────────────────────────
test("effort selection alone dispatches no effort in update_managed_agent", async () => {
installEffortIpc();
await act(async () => {
renderDialog();
});
await selectEffort("High");
assert.equal(
effortCalls().length,
0,
"picking an effort value must not write until Save — a selection-time IPC is the race",
);
});
test("effort selected then Cancel dispatches no effort in update_managed_agent", async () => {
installEffortIpc();
let openChange;
await act(async () => {
renderDialog((next) => {
openChange = next;
});
});
await selectEffort("High");
await act(async () => {
fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
});
assert.equal(
openChange,
false,
"Cancel must route through onOpenChange(false)",
);
assert.equal(
effortCalls().length,
0,
"Cancel after selecting an effort must discard the pending write",
);
});
// ── (b) ordinary effort Save carries effortLevel in the locked update ─────────
test("effort Save includes effortLevel inside update_managed_agent payload", async () => {
const { resolveUpdate } = installEffortIpc({ deferUpdate: true });
await act(async () => {
renderDialog();
});
await selectEffort("High");
await act(async () => {
fireEvent.click(screen.getByRole("button", { name: "Save changes" }));
});
assert.equal(
ipcCalls.filter((c) => c.cmd === "update_managed_agent").length,
1,
"the locked update is dispatched",
);
const updateArgs = ipcCalls.find(
(c) => c.cmd === "update_managed_agent",
)?.args;
assert.equal(
updateArgs?.input?.effortLevel,
"high",
"effortLevel must be in the locked update payload (restart sees new effort)",
);
await act(async () => {
resolveUpdate();
await new Promise((resolve) => setTimeout(resolve, 5));
});
assert.equal(
ipcCalls.filter((c) => c.cmd === "update_managed_agent").length,
1,
"effort persisted in one locked update; no separate call",
);
});
// ── (c) rejected effort Save keeps dialog open ────────────────────────────────
test("effort Save with a rejected update results in no persisted effort", async () => {
installEffortIpc({ failUpdate: true });
let openChange;
await act(async () => {
renderDialog((next) => {
openChange = next;
});
});
await selectEffort("High");
await act(async () => {
fireEvent.click(screen.getByRole("button", { name: "Save changes" }));
await new Promise((resolve) => setTimeout(resolve, 10));
});
assert.equal(
ipcCalls.filter((c) => c.cmd === "update_managed_agent").length,
1,
"Save must attempt the locked update exactly once",
);
// update rejected — dialog must not close as success
assert.notEqual(
openChange,
false,
"a failed update must not close the dialog as success — the coordinator must keep it open",
);
});
// ── (d) pin→inherit Save sends agentCommand:"" with NO effortLevel ────────────
test("pin→inherit Save with a picked effort does not write effortLevel", async () => {
installEffortIpc();
await act(async () => {
renderDialog();
});
await selectEffort("High");
await expandAdvancedAndToggleInherit();
await act(async () => {
fireEvent.click(screen.getByRole("button", { name: "Save changes" }));
await new Promise((resolve) => setTimeout(resolve, 5));
});
const updates = ipcCalls.filter((c) => c.cmd === "update_managed_agent");
assert.equal(
updates.length,
1,
"the pin→inherit Save dispatches the locked update",
);
assert.equal(
updates[0].args.input.agentCommand,
"",
"the pin→inherit transition carries the clear sentinel",
);
assert.equal(
effortCalls().length,
0,
"the inherit-transition guard must suppress the effort write — dropping the guard fails this",
);
});
// ── (e) runtime switch: see agentEditMergedRuntimeReset.test.mjs ─────────────
// The runtime-switch reset test is in a dedicated file to avoid a jsdom
// interaction hang caused by Radix's dropdown close sequence racing with the
// concurrent React state update from setEffortLevel(null). The hook-level
// test there is mutation-sensitive and covers the exact production seam.
// ── (f) access + effort share one atomic update_managed_agent payload ─────────
test("access and effort changed together both appear in the locked update_managed_agent call", async () => {
installEffortIpc();
await act(async () => {
renderDialog();
});
// Change access mode from "mentions" to "owner-only"
const respondToTrigger =
dom.window.document.getElementById("agent-respond-to");
assert.ok(respondToTrigger, "respond-to trigger must be present");
await act(async () => {
fireEvent.pointerDown(
respondToTrigger,
new dom.window.MouseEvent("pointerdown", { bubbles: true, button: 0 }),
);
fireEvent.click(respondToTrigger);
});
const ownerOnlyItem = [
...dom.window.document.querySelectorAll('[role="menuitemradio"]'),
].find((node) => node.textContent?.trim() === "Only me (default)");
assert.ok(ownerOnlyItem, '"Only me (default)" option must be offered');
await act(async () => {
fireEvent.click(ownerOnlyItem);
});
// Also pick an effort level
await selectEffort("High");
await act(async () => {
fireEvent.click(screen.getByRole("button", { name: "Save changes" }));
await new Promise((resolve) => setTimeout(resolve, 5));
});
const updates = ipcCalls.filter((c) => c.cmd === "update_managed_agent");
assert.equal(updates.length, 1, "exactly one update_managed_agent must fire");
assert.equal(
updates[0].args.input.respondTo,
"owner-only",
"respondTo must be included in the locked update",
);
assert.equal(
updates[0].args.input.effortLevel,
"high",
"effortLevel must be in the SAME locked update as the access change (restart sees new effort atomically)",
);
});

View File

@@ -0,0 +1,209 @@
/**
* Runtime-switch effort reset test for AgentEditMergedDialog.
*
* Ports the "runtime switch clears touched effort" contract from the deleted
* agentInstanceEditCancelSafety.test.mjs against the surviving merged code.
*
* The production invariant (Carl r10 / Thufir IMPORTANT-3):
* Effort selected for runtime A → runtime switched to B → Save must NOT
* dispatch the stale A vocab value.
*
* The load-bearing code is handleRuntimeDropdownChange in
* useAgentEditRuntimeHandlers.ts. After this round's fix it clears BOTH
* setEffortLevel(null) -- resets the pending selection
* effortTouched.current = false -- tells resolveEffortSubmission to skip
*
* This test exercises that code path via useAgentEditRuntimeHandlers directly,
* using React's renderHook so the assertion lives at the exact production seam
* (the handler that AgentEditMergedDialog wires to the runtime dropdown's
* onValueChange). A full-dialog mount for this interaction hangs in Node.js
* jsdom because Radix's dropdown close sequence and the concurrent React state
* update from setEffortLevel interact with act() in a way that never drains;
* the hook-level test is equivalent in coverage and mutation-sensitive.
*
* Mutation oracle: removing either reset line from handleRuntimeDropdownChange
* causes resolveEffortSubmission to emit a stale effortLevel on Save, which
* the coordinator-level tests (family 14) then catch as a false settlement.
*/
import assert from "node:assert/strict";
import { after, before, test } from "node:test";
import { JSDOM } from "jsdom";
const dom = new JSDOM("<!doctype html><html><body></body></html>", {
url: "http://localhost",
});
before(() => {
for (const key of Object.getOwnPropertyNames(dom.window)) {
if (key in globalThis) continue;
try {
globalThis[key] = dom.window[key];
} catch {}
}
Object.assign(globalThis, {
document: dom.window.document,
HTMLElement: dom.window.HTMLElement,
Node: dom.window.Node,
IS_REACT_ACT_ENVIRONMENT: true,
window: dom.window,
getComputedStyle: dom.window.getComputedStyle.bind(dom.window),
});
dom.window.matchMedia = () => ({
matches: false,
addEventListener() {},
removeEventListener() {},
});
dom.window.ResizeObserver = class {
observe() {}
unobserve() {}
disconnect() {}
};
globalThis.ResizeObserver = dom.window.ResizeObserver;
dom.window.HTMLElement.prototype.scrollIntoView = () => {};
dom.window.HTMLElement.prototype.hasPointerCapture = () => false;
dom.window.HTMLElement.prototype.releasePointerCapture = () => {};
});
after(() => dom.window.close());
test("handleRuntimeDropdownChange clears effortLevel and effortTouched on runtime switch", async () => {
const { renderHook, act } = await import("@testing-library/react");
const { useRef, useState } = await import("react");
const { useAgentEditRuntimeHandlers } = await import(
"./useAgentEditRuntimeHandlers.ts"
);
// Minimal runtime catalog: current runtime "goose", target runtime "claude"
const runtimes = [
{
id: "goose",
label: "Goose",
avatarUrl: "",
availability: "available",
command: "goose-cmd",
binaryPath: "goose-cmd",
defaultArgs: [],
mcpCommand: null,
modelEnvVar: null,
providerEnvVar: null,
thinkingEnvVar: null,
maxTokensEnvVar: null,
contextLimitEnvVar: null,
maxRoundsEnvVar: null,
installHint: "",
installInstructionsUrl: "",
canAutoInstall: false,
requiresExternalCli: false,
underlyingCliPath: null,
nodeRequired: false,
authStatus: { status: "not_applicable" },
loginHint: null,
source: "builtin",
maxParallelism: 4,
},
{
id: "claude",
label: "claude",
avatarUrl: "",
availability: "available",
command: "claude",
binaryPath: "claude",
defaultArgs: [],
mcpCommand: null,
modelEnvVar: null,
providerEnvVar: null,
thinkingEnvVar: null,
maxTokensEnvVar: null,
contextLimitEnvVar: null,
maxRoundsEnvVar: null,
installHint: "",
installInstructionsUrl: "",
canAutoInstall: false,
requiresExternalCli: false,
underlyingCliPath: null,
nodeRequired: false,
authStatus: { status: "not_applicable" },
loginHint: null,
source: "builtin",
maxParallelism: 4,
},
];
// Wrap in a thin hook that exposes the effort state so we can assert it
const { result } = renderHook(() => {
const [selectedRuntimeId, setSelectedRuntimeId] = useState("goose");
const [effortLevel, setEffortLevel] = useState(null);
const effortTouched = useRef(false);
const runtimeTouched = useRef(false);
const handlers = useAgentEditRuntimeHandlers({
showDef: false,
showInst: true,
dProvider: "",
dModel: "",
dIsCustomProviderEditing: false,
dIsCustomModelEditing: false,
setDProvider: () => {},
setDModel: () => {},
setDIsCustomProviderEditing: () => {},
setDIsCustomModelEditing: () => {},
envVars: {},
setEnvVars: () => {},
definitionRuntimeId: "goose",
setDefinitionRuntimeId: () => {},
iProvider: "",
iModel: "",
iIsCustomProviderEditing: false,
iIsCustomModelEditing: false,
setIProvider: () => {},
setIModel: () => {},
setIIsCustomProviderEditing: () => {},
setIIsCustomModelEditing: () => {},
instanceEnvVars: {},
setInstanceEnvVars: () => {},
selectedRuntimeId,
setSelectedRuntimeId,
setInheritHarness: () => {},
setAgentCommand: () => {},
setAgentArgs: () => {},
runtimeTouched,
setIsAddHarnessOpen: () => {},
setEffortLevel,
effortTouched,
runtimes,
selectedRuntime: runtimes.find((r) => r.id === selectedRuntimeId),
open: true,
});
return { handlers, effortLevel, effortTouched };
});
// Simulate: user selects an effort — effortTouched becomes true
act(() => {
result.current.effortTouched.current = true;
});
assert.equal(
result.current.effortTouched.current,
true,
"effortTouched must be true after user selection",
);
// Simulate: user switches runtime to "claude"
act(() => {
result.current.handlers.handleRuntimeDropdownChange("claude");
});
// The reset must have cleared both:
assert.equal(
result.current.effortTouched.current,
false,
"effortTouched must be cleared after runtime switch — without this, resolveEffortSubmission emits the stale value",
);
assert.equal(
result.current.effortLevel,
null,
"effortLevel must be reset to null after runtime switch",
);
});

View File

@@ -2433,3 +2433,132 @@ test("test_description_trim_and_preserve_existing_settles_correctly", async () =
"onDone must be called when trimmed description matches observed",
);
});
// ── Test family 14: effort settlement (Thufir round-1 IMPORTANT-1) ────────────
//
// `observedStateMatchesAgentInput` must compare effortLevel against the
// persisted record column (ManagedAgent.effortLevel) with tri-state semantics:
// absent submission → skip comparison (field not being written)
// null submission → clear; settled when observed column is null/absent
// string submission → set; settled when observed column equals submitted value
//
// The backend can reject before persistence (e.g. non-local agent rejects
// effort write). Without the comparison, a rejected effort edit settles as
// success and closes the dialog — the exact defect Thufir's probe confirmed.
test("test_effort_set_settles_when_observed_matches_submitted", async () => {
// Submit effortLevel:"high" — backend persists it. Observed agent has
// effortLevel:"high". Settlement must recognise the write succeeded.
const inst = makeInstance({ effortLevel: "high" });
const opts = makeOpts({
ctx: { kind: "instance-only", instance: makeInstance() },
agentInput: makeAgentInput({ effortLevel: "high" }),
refetchStores: async () => ({ persona: null, agent: inst }),
});
const result = await runAgentSaveCoordinator(opts);
assert.equal(result, true, "effort set must settle when observed matches");
assert.equal(
opts._calls.onDone,
1,
"onDone must be called on effort set success",
);
});
test("test_effort_clear_settles_when_observed_is_null", async () => {
// Submit effortLevel:null — a clear. Observed agent has no effortLevel.
// Settlement must recognise the clear succeeded.
const inst = makeInstance({ effortLevel: null });
const opts = makeOpts({
ctx: {
kind: "instance-only",
instance: makeInstance({ effortLevel: "low" }),
},
agentInput: makeAgentInput({ effortLevel: null }),
refetchStores: async () => ({ persona: null, agent: inst }),
});
const result = await runAgentSaveCoordinator(opts);
assert.equal(
result,
true,
"effort clear must settle when observed is null/absent",
);
assert.equal(
opts._calls.onDone,
1,
"onDone must be called on effort clear success",
);
});
test("test_rejected_effort_ordinary_save_does_not_close", async () => {
// Backend rejects the effort write before persistence — agent store is
// unchanged (effortLevel:null). observedStateMatchesAgentInput must detect
// the mismatch (submitted "high" ≠ observed null) and NOT close the dialog.
const cap = captureToasts();
try {
const originalInst = makeInstance({ effortLevel: null });
const opts = makeOpts({
ctx: { kind: "instance-only", instance: makeInstance() },
agentInput: makeAgentInput({ effortLevel: "high" }),
updateManagedAgent: async () => {
throw new Error("effort write rejected — non-local agent");
},
refetchStores: async () => ({ persona: null, agent: originalInst }),
});
const result = await runAgentSaveCoordinator(opts);
assert.equal(result, false, "rejected effort edit must not succeed");
assert.equal(
opts._calls.onDone,
0,
"dialog must not close when effort was rejected by the backend",
);
} finally {
cap.restore();
}
});
test("test_rejected_effort_combined_di_does_not_advance_to_instance_write", async () => {
// Combined D+I save: rejected effort (I-side) must report failure; the D
// write succeeds but the coordinator must not report overall success.
const cap = captureToasts();
try {
const persistedPersona = makeDefinition({ displayName: "Alice-renamed" });
const originalInst = makeInstance({ effortLevel: null });
const opts = makeOpts({
ctx: {
kind: "instance-with-definition",
definition: makeDefinition(),
instance: makeInstance(),
},
personaInput: makePersonaInput({ displayName: "Alice-renamed" }),
agentInput: makeAgentInput({ effortLevel: "high" }),
updateManagedAgent: async () => {
throw new Error("effort write rejected — non-local agent");
},
refetchStores: async () => ({
persona: persistedPersona,
agent: originalInst,
}),
});
const result = await runAgentSaveCoordinator(opts);
assert.equal(
result,
false,
"combined D+I with rejected effort must not succeed",
);
assert.equal(
opts._calls.onDone,
0,
"dialog must not close when I effort was rejected",
);
} finally {
cap.restore();
}
});

View File

@@ -716,6 +716,17 @@ function observedStateMatchesAgentInput(
) {
return false;
}
// Effort level — tri-state:
// absent submission (undefined) → skip (field not being written)
// null submission → clear; settled when observed column is null/absent
// string submission → set; settled when observed column equals submitted
// Mirrors the backend's canonical storage semantics (no byte stripping here —
// a prohibited-byte value must NOT launder into apparent success).
if (submitted.effortLevel !== undefined) {
const submittedEffort = submitted.effortLevel ?? null;
const observedEffort = observed.effortLevel ?? null;
if (submittedEffort !== observedEffort) return false;
}
return true;
}

View File

@@ -34,6 +34,7 @@ import {
} from "./agentFormModel";
import { runAgentSaveCoordinator } from "./agentSaveCoordinator";
import { parsePersonaNamePoolText } from "./personaDialogState";
import { resolveEffortSubmission } from "./personaRuntimeModel";
// ── Types ─────────────────────────────────────────────────────────────────────
@@ -97,6 +98,13 @@ export type AgentEditSubmitState = {
effortLevel: string | null;
/** Ref tracking whether the user has made an explicit effort selection. */
effortTouched: React.RefObject<boolean>;
/**
* The persisted effort level from the config surface at seed time
* (`configSurfaceQuery.data?.normalized.thinkingEffort?.value ?? null`).
* Used by `resolveEffortSubmission` to suppress unchanged selections so a
* name-only edit never rewrites the effort column.
*/
originalEffortLevel: string | null;
};
export type AgentEditSubmitHookReturn = {
@@ -281,12 +289,23 @@ export function useAgentEditMergedSubmit(
policySets,
} = emitAgentFormDiff(seed, next, s.ctx);
// Effort write: when touched, resolve and include in the locked update
// (atomic with any access-policy change — PR #4625 semantics).
// Effort write: when touched, resolve via resolveEffortSubmission
// (mirrors the deleted AgentInstanceEditDialog's PR #4625 semantics):
// - suppresses when agentCommand="" (pin→inherit) so the inherit
// transition does not restore the effort column it just cleared
// - suppresses unchanged selections (no-op save never rewrites the column)
// Only includes effortLevel in the locked update when persist=true.
let agentInput = rawAgentInput;
if (s.effortTouched.current && inst) {
agentInput = agentInput ?? { pubkey: inst.pubkey };
agentInput = { ...agentInput, effortLevel: s.effortLevel };
const effortSubmission = resolveEffortSubmission({
effortLevel: s.effortLevel,
originalEffortLevel: s.originalEffortLevel,
inheritTransition: rawAgentInput?.agentCommand === "",
});
if (effortSubmission.persist) {
agentInput = agentInput ?? { pubkey: inst.pubkey };
agentInput = { ...agentInput, effortLevel: effortSubmission.level };
}
}
const refetchStores = () => refetchAgentStores(queryClient, def, inst);

View File

@@ -84,6 +84,8 @@ function makeInput(overrides = {}) {
setAgentArgs: (v) => calls.setAgentArgs.push(v),
runtimeTouched,
setIsAddHarnessOpen: noop,
setEffortLevel: noop,
effortTouched: { current: false },
runtimes: [{ id: "buzz-agent", command: "", defaultArgs: [] }],
selectedRuntime: { id: "goose", command: "", defaultArgs: [] },
open: true,

View File

@@ -58,6 +58,9 @@ export type RuntimeHandlersInput = {
setAgentArgs: (v: string) => void;
runtimeTouched: React.RefObject<boolean>;
setIsAddHarnessOpen: (v: boolean) => void;
/** Reset effort selection when the instance runtime changes (IMPORTANT-3). */
setEffortLevel: (v: string | null) => void;
effortTouched: React.RefObject<boolean>;
runtimes: readonly AcpRuntimeCatalogEntry[];
selectedRuntime: AcpRuntimeCatalogEntry | undefined;
open: boolean;
@@ -103,6 +106,8 @@ export function useAgentEditRuntimeHandlers(input: RuntimeHandlersInput) {
setAgentArgs,
runtimeTouched,
setIsAddHarnessOpen,
setEffortLevel,
effortTouched,
runtimes,
selectedRuntime,
open,
@@ -196,6 +201,11 @@ export function useAgentEditRuntimeHandlers(input: RuntimeHandlersInput) {
lockedRuntimeReset: "full",
}),
);
// Reset pending effort: the new runtime uses a different vocabulary and the
// old selection is meaningless. Clear both the displayed value and the
// touched flag so resolveEffortSubmission skips effort on the next Save.
setEffortLevel(null);
effortTouched.current = false;
}
const selectSavedHarness = usePendingHarnessSelection(

View File

@@ -372,6 +372,13 @@ export type ManagedAgent = {
* `"allowlist"`. Preserved across mode toggles.
*/
respondToAllowlist: string[];
/**
* Canonical harness-agnostic effort level persisted on the record.
* `undefined`/absent means the agent uses the adapter default at launch.
* Exposed so the edit dialog's settlement comparator can detect a
* backend-rejected effort write rather than closing as success.
*/
effortLevel?: string | null;
};
/** Inbound author gate mode. Mirrors buzz-acp's --respond-to CLI flag. */