mirror of
https://github.com/block/buzz.git
synced 2026-10-29 21:41:33 +00:00
fix(desktop): keep Relay Mesh runtime switch on the shown layer and name the observed agent
handleProviderDropdownChange forced buzz-agent through the instance handler unconditionally, so a linked edit pinned the clicked instance to buzz-agent while the definition kept its old runtime. Route through the definition handler when a definition is shown and consult that layer's runtime id. The full-success toast preferred latestAgent?.name, which only advances on a non-throwing updateManagedAgent, so a rename that committed but whose command threw after commit reported the stale pre-save name. Prefer the observed name from the final refetch. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -1202,3 +1202,56 @@ test("test_drift_guard_inert_when_no_expected_updatedAt_supplied", async () => {
|
||||
assert.equal(result, true, "null expected updatedAt skips the guard");
|
||||
assert.equal(opts._calls.updatePersona, 1);
|
||||
});
|
||||
|
||||
// ── Test family 9: success toast names the observed (persisted) agent (P2) ───
|
||||
//
|
||||
// `latestAgent` only advances on a NON-throwing updateManagedAgent. A rename
|
||||
// that commits to disk but whose command throws afterward leaves `latestAgent`
|
||||
// at the pre-save instance, so the success toast must read the name from the
|
||||
// observed refetch — not from `latestAgent` — or a committed Alice→Bob rename
|
||||
// falsely reports "Alice saved."
|
||||
|
||||
test("test_success_toast_uses_observed_name_on_thrown_but_persisted_rename", async () => {
|
||||
const cap = captureToasts();
|
||||
try {
|
||||
const opts = makeOpts({
|
||||
ctx: {
|
||||
kind: "instance-only",
|
||||
instance: makeInstance({ name: "Alice" }),
|
||||
},
|
||||
agentInput: makeAgentInput({ name: "Bob" }),
|
||||
// The rename commits to disk, but the command throws after commit, so
|
||||
// `latestAgent` is never reassigned and stays at the pre-save "Alice".
|
||||
updateManagedAgent: async () => {
|
||||
throw new Error("summary build failed after commit");
|
||||
},
|
||||
// The final refetch observes the persisted rename.
|
||||
refetchStores: async () => ({
|
||||
persona: null,
|
||||
agent: makeInstance({ name: "Bob" }),
|
||||
}),
|
||||
});
|
||||
|
||||
const result = await runAgentSaveCoordinator(opts);
|
||||
|
||||
assert.equal(
|
||||
result,
|
||||
true,
|
||||
"a thrown-but-persisted write is a full success (observed state matches)",
|
||||
);
|
||||
const successes = cap.captured.filter((c) => c.kind === "success");
|
||||
assert.equal(successes.length, 1, "exactly one success toast");
|
||||
assert.match(
|
||||
successes[0].message,
|
||||
/^Bob saved\./,
|
||||
"toast must name the observed (persisted) rename, not the stale pre-save name",
|
||||
);
|
||||
assert.doesNotMatch(
|
||||
successes[0].message,
|
||||
/Alice/,
|
||||
"toast must not report the stale pre-save name",
|
||||
);
|
||||
} finally {
|
||||
cap.restore();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -359,8 +359,14 @@ export async function runAgentSaveCoordinator(
|
||||
|
||||
if (!observedRemainder) {
|
||||
// Full success — every submitted write is reflected in observed state.
|
||||
// Prefer the OBSERVED name from the final refetch over `latestAgent`.
|
||||
// `latestAgent` only advances on a non-throwing `updateManagedAgent`; a
|
||||
// rename that persisted but whose command threw after commit leaves it at
|
||||
// the pre-save `inst`, so a committed Alice→Bob would report "Alice saved."
|
||||
// The observed refetch reflects what is actually on disk (the stopped-state
|
||||
// path below already prefers `observedAgent` for the same reason).
|
||||
const agentName =
|
||||
latestAgent?.name ?? observedAgent?.name ?? def?.displayName ?? "Agent";
|
||||
observedAgent?.name ?? latestAgent?.name ?? def?.displayName ?? "Agent";
|
||||
|
||||
if (profileSyncError) {
|
||||
showAgentProfileSyncWarning(agentName, profileSyncError);
|
||||
|
||||
@@ -0,0 +1,179 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { after, before, test } from "node:test";
|
||||
|
||||
import { JSDOM } from "jsdom";
|
||||
|
||||
// useAgentEditRuntimeHandlers is a React hook (it calls
|
||||
// usePendingHarnessSelection internally), so it must be mounted to exercise
|
||||
// the real handler wiring. JSDOM provides the DOM globals renderHook needs.
|
||||
const dom = new JSDOM("<!doctype html><html><body></body></html>", {
|
||||
url: "http://localhost",
|
||||
});
|
||||
|
||||
before(() => {
|
||||
Object.assign(globalThis, {
|
||||
document: dom.window.document,
|
||||
HTMLElement: dom.window.HTMLElement,
|
||||
IS_REACT_ACT_ENVIRONMENT: true,
|
||||
window: dom.window,
|
||||
localStorage: dom.window.localStorage,
|
||||
});
|
||||
});
|
||||
|
||||
after(() => dom.window.close());
|
||||
|
||||
let hookModule;
|
||||
before(async () => {
|
||||
hookModule = await import("./useAgentEditRuntimeHandlers.ts");
|
||||
});
|
||||
|
||||
// Build a RuntimeHandlersInput whose every setter records its calls, so a test
|
||||
// can assert which layer (definition vs instance) a handler wrote to.
|
||||
function makeInput(overrides = {}) {
|
||||
const calls = {
|
||||
setDefinitionRuntimeId: [],
|
||||
setSelectedRuntimeId: [],
|
||||
setInheritHarness: [],
|
||||
setAgentCommand: [],
|
||||
setAgentArgs: [],
|
||||
setDProvider: [],
|
||||
setDModel: [],
|
||||
setIProvider: [],
|
||||
setIModel: [],
|
||||
runtimeTouchedWrites: 0,
|
||||
};
|
||||
const noop = () => {};
|
||||
const runtimeTouched = {
|
||||
get current() {
|
||||
return this._v ?? false;
|
||||
},
|
||||
set current(v) {
|
||||
this._v = v;
|
||||
calls.runtimeTouchedWrites++;
|
||||
},
|
||||
};
|
||||
const input = {
|
||||
showDef: true,
|
||||
showInst: true,
|
||||
dProvider: "openai",
|
||||
dModel: "gpt-4o",
|
||||
dIsCustomProviderEditing: false,
|
||||
dIsCustomModelEditing: false,
|
||||
setDProvider: (v) => calls.setDProvider.push(v),
|
||||
setDModel: (v) => calls.setDModel.push(v),
|
||||
setDIsCustomProviderEditing: noop,
|
||||
setDIsCustomModelEditing: noop,
|
||||
envVars: {},
|
||||
setEnvVars: noop,
|
||||
definitionRuntimeId: "goose",
|
||||
setDefinitionRuntimeId: (v) => calls.setDefinitionRuntimeId.push(v),
|
||||
iProvider: "openai",
|
||||
iModel: "gpt-4o",
|
||||
iIsCustomProviderEditing: false,
|
||||
iIsCustomModelEditing: false,
|
||||
setIProvider: (v) => calls.setIProvider.push(v),
|
||||
setIModel: (v) => calls.setIModel.push(v),
|
||||
setIIsCustomProviderEditing: noop,
|
||||
setIIsCustomModelEditing: noop,
|
||||
instanceEnvVars: {},
|
||||
setInstanceEnvVars: noop,
|
||||
selectedRuntimeId: "goose",
|
||||
setSelectedRuntimeId: (v) => calls.setSelectedRuntimeId.push(v),
|
||||
setInheritHarness: (v) => calls.setInheritHarness.push(v),
|
||||
setAgentCommand: (v) => calls.setAgentCommand.push(v),
|
||||
setAgentArgs: (v) => calls.setAgentArgs.push(v),
|
||||
runtimeTouched,
|
||||
setIsAddHarnessOpen: noop,
|
||||
runtimes: [{ id: "buzz-agent", command: "", defaultArgs: [] }],
|
||||
selectedRuntime: { id: "goose", command: "", defaultArgs: [] },
|
||||
open: true,
|
||||
...overrides,
|
||||
};
|
||||
return { input, calls };
|
||||
}
|
||||
|
||||
async function renderHandlers(input) {
|
||||
const { renderHook } = await import("@testing-library/react");
|
||||
return renderHook(() => hookModule.useAgentEditRuntimeHandlers(input));
|
||||
}
|
||||
|
||||
// ── P1: Relay Mesh selection must switch the runtime on the SHOWN layer ───────
|
||||
//
|
||||
// Carl's finding: `handleProviderDropdownChange` forced buzz-agent through the
|
||||
// INSTANCE handler unconditionally. In a linked edit (showDef + showInst) that
|
||||
// pinned the clicked instance to buzz-agent while the definition kept its old
|
||||
// runtime — a D/I ownership-boundary violation. Selecting Relay Mesh while a
|
||||
// definition is shown must change the DEFINITION runtime and leave the instance
|
||||
// pin untouched. Reverting the fix (routing back through the instance handler,
|
||||
// or consulting the instance runtime id) makes both assertions below fail.
|
||||
|
||||
test("test_relay_mesh_in_linked_edit_switches_definition_runtime_not_instance", async () => {
|
||||
const { act } = await import("@testing-library/react");
|
||||
const { input, calls } = makeInput();
|
||||
const { result } = await renderHandlers(input);
|
||||
|
||||
act(() => result.current.handleProviderDropdownChange("relay-mesh"));
|
||||
|
||||
assert.deepEqual(
|
||||
calls.setDefinitionRuntimeId,
|
||||
["buzz-agent"],
|
||||
"definition runtime must switch to buzz-agent when Relay Mesh is chosen with the definition shown",
|
||||
);
|
||||
assert.deepEqual(
|
||||
calls.setSelectedRuntimeId,
|
||||
[],
|
||||
"the instance runtime pin must NOT be touched in a linked edit",
|
||||
);
|
||||
assert.equal(
|
||||
calls.runtimeTouchedWrites,
|
||||
0,
|
||||
"the instance runtimeTouched ref must NOT be set in a linked edit",
|
||||
);
|
||||
assert.deepEqual(
|
||||
calls.setInheritHarness,
|
||||
[],
|
||||
"instance harness inheritance must NOT be cleared in a linked edit",
|
||||
);
|
||||
// The definition provider/model still land on the relay-mesh selection.
|
||||
assert.equal(
|
||||
calls.setDProvider.at(-1),
|
||||
"relay-mesh",
|
||||
"definition provider must become relay-mesh",
|
||||
);
|
||||
assert.equal(
|
||||
calls.setDModel.at(-1),
|
||||
"auto",
|
||||
"definition model must become auto for relay-mesh",
|
||||
);
|
||||
});
|
||||
|
||||
test("test_relay_mesh_in_instance_only_edit_switches_instance_runtime", async () => {
|
||||
const { act } = await import("@testing-library/react");
|
||||
// Instance-only context: no definition shown, so the instance handler is the
|
||||
// correct layer and buzz-agent must be pinned on the instance.
|
||||
const { input, calls } = makeInput({
|
||||
showDef: false,
|
||||
showInst: true,
|
||||
selectedRuntimeId: "goose",
|
||||
definitionRuntimeId: "goose",
|
||||
});
|
||||
const { result } = await renderHandlers(input);
|
||||
|
||||
act(() => result.current.handleProviderDropdownChange("relay-mesh"));
|
||||
|
||||
assert.deepEqual(
|
||||
calls.setSelectedRuntimeId,
|
||||
["buzz-agent"],
|
||||
"instance runtime must switch to buzz-agent in an instance-only edit",
|
||||
);
|
||||
assert.deepEqual(
|
||||
calls.setDefinitionRuntimeId,
|
||||
[],
|
||||
"definition runtime must NOT be touched in an instance-only edit",
|
||||
);
|
||||
assert.equal(
|
||||
calls.setIProvider.at(-1),
|
||||
"relay-mesh",
|
||||
"instance provider must become relay-mesh",
|
||||
);
|
||||
});
|
||||
@@ -209,8 +209,19 @@ export function useAgentEditRuntimeHandlers(input: RuntimeHandlersInput) {
|
||||
function handleProviderDropdownChange(nextValue: string) {
|
||||
const nextProvider =
|
||||
nextValue === AUTO_PROVIDER_DROPDOWN_VALUE ? "" : nextValue;
|
||||
if (nextProvider === "relay-mesh" && selectedRuntimeId !== "buzz-agent") {
|
||||
handleRuntimeDropdownChange("buzz-agent");
|
||||
// Relay Mesh is only offered by the buzz-agent runtime, so selecting it
|
||||
// forces that runtime. Route through the same layer the provider dropdown
|
||||
// writes: in a definition-showing context switch the DEFINITION runtime
|
||||
// (handleDefinitionRuntimeChange), never the instance handler — otherwise a
|
||||
// linked edit silently pins the clicked instance to buzz-agent while the
|
||||
// definition keeps its old runtime. Consult that same layer's runtime id.
|
||||
const currentRuntimeId = showDef ? definitionRuntimeId : selectedRuntimeId;
|
||||
if (nextProvider === "relay-mesh" && currentRuntimeId !== "buzz-agent") {
|
||||
if (showDef) {
|
||||
handleDefinitionRuntimeChange("buzz-agent");
|
||||
} else {
|
||||
handleRuntimeDropdownChange("buzz-agent");
|
||||
}
|
||||
}
|
||||
const sel = showDef ? dSelection : iSelection;
|
||||
const applyFn = showDef ? applyDSelection : applyISelection;
|
||||
|
||||
Reference in New Issue
Block a user