mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): stop the create-agent provider config probe from erasing keystrokes
The WhereToRunSection probe effect depended on the whole run draft, so every keystroke in a provider config field re-fired the effect, re-probed the provider binary, and — because each probe result is a fresh object — kept re-probing in a loop for as long as the dialog sat on a provider. Each resolution then reset providerConfig to schema defaults, erasing whatever the user had typed. Fields without a schema default (the Kubernetes 'Kubeconfig context') read as completely dead. Fix: - Probe once per provider selection, keyed on the provider's stable binary path — not the draft, not the provider object (a providers-query refresh must not reprobe an unchanged selection). - Resolve with latest-state semantics via useEffectEvent + a new applyProbeResult helper that merges schema defaults BENEATH the current providerConfig, so a probe landing after the user typed can never clobber in-flight input. Tests: - unit: applyProbeResult merge semantics (defaults under typed values, user-cleared fields stay cleared, schema-less results). - e2e (new where-to-run-config.spec.ts, red-first verified against the unfixed component): typing sticks + exactly one probe per selection, probe-gated form render with a slow probe, provider->local->provider reset. Mock bridge gains backendProviders / backendProviderProbeResult / backendProviderProbeDelayMs seams. Reported by Tyler in buzz-remote-agents (channel 29414326, thread db76677a): could not type into the Kubeconfig context field. Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
This commit is contained in:
parent
318fbf896e
commit
8eb7680795
@@ -132,6 +132,7 @@ export default defineConfig({
|
||||
"**/harness-management.spec.ts",
|
||||
"**/harness-catalog-screenshots.spec.ts",
|
||||
"**/inline-custom-harness.spec.ts",
|
||||
"**/where-to-run-config.spec.ts",
|
||||
"**/huddle-transcription.spec.ts",
|
||||
],
|
||||
use: {
|
||||
|
||||
@@ -5,7 +5,11 @@ import { useBackendProvidersQuery } from "@/features/agents/hooks";
|
||||
import { probeBackendProvider } from "@/shared/api/tauri";
|
||||
|
||||
import { ProviderConfigFields } from "./ProviderConfigFields";
|
||||
import { emptyWhereToRunDraft, type WhereToRunDraft } from "./whereToRunIntent";
|
||||
import {
|
||||
applyProbeResult,
|
||||
emptyWhereToRunDraft,
|
||||
type WhereToRunDraft,
|
||||
} from "./whereToRunIntent";
|
||||
|
||||
/** Optional remote-backend selector. Buzz shared compute is an LLM provider, not a run destination. */
|
||||
export function WhereToRunSection({
|
||||
@@ -26,32 +30,37 @@ export function WhereToRunSection({
|
||||
[backendProviders, draft.runOn],
|
||||
);
|
||||
|
||||
// Latest-state seam for probe resolution: an Effect Event always sees the
|
||||
// draft as it is *now*. Without this, the probe promise closes over the
|
||||
// draft from probe start, and anything typed while the probe was in flight
|
||||
// gets thrown away when it resolves (a second, subtler Typewriter Eraser).
|
||||
const applyProbe = React.useEffectEvent(
|
||||
(result: Awaited<ReturnType<typeof probeBackendProvider>>) => {
|
||||
onDraftChange(applyProbeResult(draft, result));
|
||||
},
|
||||
);
|
||||
|
||||
// Probe once per provider *selection*, keyed on the provider's stable
|
||||
// path — never on the draft. Depending on the draft made every keystroke
|
||||
// refire the probe, and each resolution reset providerConfig to schema
|
||||
// defaults, which erased what the user was typing (the Typewriter Eraser)
|
||||
// and spawned the provider binary in a loop for as long as the dialog was
|
||||
// open. Keying on the path (not the provider object) also keeps a
|
||||
// providers-query refresh from reprobing an unchanged selection.
|
||||
const selectedBinaryPath = isProviderMode
|
||||
? (selectedBackendProvider?.binaryPath ?? null)
|
||||
: null;
|
||||
React.useEffect(() => {
|
||||
if (!isProviderMode || !selectedBackendProvider) {
|
||||
if (!selectedBinaryPath) {
|
||||
setProbeError(null);
|
||||
return;
|
||||
}
|
||||
let cancelled = false;
|
||||
setProbeError(null);
|
||||
void probeBackendProvider(selectedBackendProvider.binaryPath)
|
||||
void probeBackendProvider(selectedBinaryPath)
|
||||
.then((result) => {
|
||||
if (cancelled) return;
|
||||
const defaults: Record<string, string> = {};
|
||||
const properties =
|
||||
(result.config_schema as Record<string, unknown> | undefined)
|
||||
?.properties ?? {};
|
||||
for (const [key, property] of Object.entries(properties) as [
|
||||
string,
|
||||
Record<string, unknown>,
|
||||
][]) {
|
||||
if (property.default != null)
|
||||
defaults[key] = String(property.default);
|
||||
}
|
||||
onDraftChange({
|
||||
...draft,
|
||||
probedProvider: result,
|
||||
providerConfig: defaults,
|
||||
});
|
||||
applyProbe(result);
|
||||
})
|
||||
.catch((error: unknown) => {
|
||||
if (!cancelled) {
|
||||
@@ -61,7 +70,7 @@ export function WhereToRunSection({
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [draft, isProviderMode, onDraftChange, selectedBackendProvider]);
|
||||
}, [selectedBinaryPath]);
|
||||
|
||||
if (backendProviders.length === 0) return null;
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import {
|
||||
applyProbeResult,
|
||||
canSubmitWhereToRun,
|
||||
emptyWhereToRunDraft,
|
||||
providerConfigComplete,
|
||||
@@ -59,3 +60,72 @@ test("provider draft resolves with coerced config values", () => {
|
||||
config: { region: "us", size: 3 },
|
||||
});
|
||||
});
|
||||
|
||||
// ── applyProbeResult: probe resolution must merge, not overwrite ─────────────
|
||||
//
|
||||
// Pins the seam that fixed the "Typewriter Eraser" (agent-create dialog's
|
||||
// provider config fields losing keystrokes): a probe resolution prefills
|
||||
// schema defaults *beneath* the user's in-flight config, never over it. The
|
||||
// effect in WhereToRunSection keys probing on the provider's binary path, so
|
||||
// the only probe writes that reach providerConfig are the ones pinned here.
|
||||
|
||||
const probeWithDefaults = {
|
||||
ok: true,
|
||||
config_schema: {
|
||||
properties: {
|
||||
context: { type: "string", title: "Kubeconfig context" },
|
||||
namespace: { type: "string", default: "buzz-agents-x1y2z3" },
|
||||
inactivity_seconds: { type: "number", default: 1800 },
|
||||
},
|
||||
required: ["namespace"],
|
||||
},
|
||||
};
|
||||
|
||||
const unprobedDraft = {
|
||||
...emptyWhereToRunDraft,
|
||||
runOn: "kubernetes",
|
||||
};
|
||||
|
||||
test("probe resolution prefills schema defaults on a fresh draft", () => {
|
||||
const next = applyProbeResult(unprobedDraft, probeWithDefaults);
|
||||
assert.equal(next.probedProvider, probeWithDefaults);
|
||||
assert.deepEqual(next.providerConfig, {
|
||||
namespace: "buzz-agents-x1y2z3",
|
||||
inactivity_seconds: "1800",
|
||||
});
|
||||
});
|
||||
|
||||
test("probe resolution keeps user-typed values over schema defaults", () => {
|
||||
const typed = {
|
||||
...unprobedDraft,
|
||||
providerConfig: { context: "prod-us-west", namespace: "my-ns" },
|
||||
};
|
||||
const next = applyProbeResult(typed, probeWithDefaults);
|
||||
assert.deepEqual(next.providerConfig, {
|
||||
context: "prod-us-west",
|
||||
namespace: "my-ns",
|
||||
inactivity_seconds: "1800",
|
||||
});
|
||||
});
|
||||
|
||||
test("probe resolution keeps a user-cleared field cleared", () => {
|
||||
// "" is a deliberate user state — coerceConfigValues drops empty numerics
|
||||
// and required-gating treats "" as incomplete; the probe must not undo it.
|
||||
const cleared = { ...unprobedDraft, providerConfig: { namespace: "" } };
|
||||
const next = applyProbeResult(cleared, probeWithDefaults);
|
||||
assert.equal(next.providerConfig.namespace, "");
|
||||
});
|
||||
|
||||
test("a schema-less probe result records the probe without touching config", () => {
|
||||
const typed = { ...unprobedDraft, providerConfig: { context: "abc" } };
|
||||
const next = applyProbeResult(typed, { ok: true });
|
||||
assert.deepEqual(next.providerConfig, { context: "abc" });
|
||||
assert.deepEqual(next.probedProvider, { ok: true });
|
||||
});
|
||||
|
||||
test("probe resolution preserves unrelated draft fields", () => {
|
||||
assert.equal(
|
||||
applyProbeResult(unprobedDraft, probeWithDefaults).runOn,
|
||||
"kubernetes",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -15,6 +15,35 @@ export const emptyWhereToRunDraft: WhereToRunDraft = {
|
||||
probedProvider: null,
|
||||
};
|
||||
|
||||
/**
|
||||
* Fold a completed probe into the draft the user has *now* — not the draft
|
||||
* that existed when the probe started. Schema defaults prefill only the keys
|
||||
* the user has not touched: anything already in `providerConfig` (typed while
|
||||
* the probe was in flight) wins over the default. Overwriting instead of
|
||||
* merging is the "Typewriter Eraser" bug — every probe resolution silently
|
||||
* erased in-flight keystrokes.
|
||||
*/
|
||||
export function applyProbeResult(
|
||||
current: WhereToRunDraft,
|
||||
result: BackendProviderProbeResult,
|
||||
): WhereToRunDraft {
|
||||
const defaults: Record<string, string> = {};
|
||||
const properties =
|
||||
(result.config_schema as Record<string, unknown> | undefined)?.properties ??
|
||||
{};
|
||||
for (const [key, property] of Object.entries(properties) as [
|
||||
string,
|
||||
Record<string, unknown>,
|
||||
][]) {
|
||||
if (property.default != null) defaults[key] = String(property.default);
|
||||
}
|
||||
return {
|
||||
...current,
|
||||
probedProvider: result,
|
||||
providerConfig: { ...defaults, ...current.providerConfig },
|
||||
};
|
||||
}
|
||||
|
||||
export function providerConfigComplete(draft: WhereToRunDraft): boolean {
|
||||
if (draft.runOn === "local") return true;
|
||||
if (!draft.probedProvider) return false;
|
||||
|
||||
@@ -507,6 +507,11 @@ type E2eConfig = {
|
||||
* returning a catalog.
|
||||
*/
|
||||
discoverAgentModelsError?: string;
|
||||
// Backend provider mocks for the create-agent "Run on" section. See
|
||||
// tests/helpers/bridge.ts:MockBridgeOptions for semantics.
|
||||
backendProviders?: Array<{ id: string; binaryPath: string }>;
|
||||
backendProviderProbeResult?: Record<string, unknown>;
|
||||
backendProviderProbeDelayMs?: number;
|
||||
};
|
||||
relayHttpUrl?: string;
|
||||
relayWsUrl?: string;
|
||||
@@ -11064,9 +11069,22 @@ export function maybeInstallE2eTauriMocks() {
|
||||
activeConfig,
|
||||
);
|
||||
case "discover_backend_providers":
|
||||
return [];
|
||||
case "probe_backend_provider":
|
||||
return { ok: false, error: "mock: no providers available" };
|
||||
return activeConfig?.mock?.backendProviders ?? [];
|
||||
case "probe_backend_provider": {
|
||||
const probeDelayMs =
|
||||
activeConfig?.mock?.backendProviderProbeDelayMs ?? 0;
|
||||
if (probeDelayMs > 0) {
|
||||
await new Promise((resolve) =>
|
||||
window.setTimeout(resolve, probeDelayMs),
|
||||
);
|
||||
}
|
||||
return (
|
||||
activeConfig?.mock?.backendProviderProbeResult ?? {
|
||||
ok: false,
|
||||
error: "mock: no providers available",
|
||||
}
|
||||
);
|
||||
}
|
||||
case "discover_managed_agent_prereqs":
|
||||
return handleDiscoverManagedAgentPrereqs(
|
||||
payload as Parameters<typeof handleDiscoverManagedAgentPrereqs>[0],
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
/**
|
||||
* E2E spec for the create-agent "Run on" provider config fields.
|
||||
*
|
||||
* Pins the fix for the "Typewriter Eraser": WhereToRunSection's probe effect
|
||||
* used to depend on the whole draft, so every keystroke re-probed the
|
||||
* provider and every probe resolution reset providerConfig to schema
|
||||
* defaults — typing into a defaultless field (the k8s "Kubeconfig context")
|
||||
* looked completely dead, and the provider binary respawned in a loop.
|
||||
*
|
||||
* Covers:
|
||||
* - typing into a defaultless provider field sticks, and the provider is
|
||||
* probed exactly once for the selection (not once per keystroke)
|
||||
* - the config form is gated on probe resolution (no half-rendered form),
|
||||
* and defaults prefill exactly once when a slow probe lands
|
||||
* - switching provider → local → provider re-probes and resets cleanly
|
||||
*
|
||||
* The stale-closure merge on probe resolution (defaults beneath in-flight
|
||||
* typing) is unreachable through this UI because the fields render only
|
||||
* after the probe resolves; it is pinned at the unit level in
|
||||
* whereToRunIntent.test.mjs (applyProbeResult).
|
||||
*/
|
||||
import { expect, test } from "@playwright/test";
|
||||
|
||||
import { installMockBridge } from "../helpers/bridge";
|
||||
|
||||
type Page = import("@playwright/test").Page;
|
||||
|
||||
const PROVIDER = {
|
||||
id: "kubernetes",
|
||||
binaryPath: "/mock/buzz-backend-kubernetes",
|
||||
};
|
||||
|
||||
const PROBE_RESULT = {
|
||||
ok: true,
|
||||
name: "kubernetes",
|
||||
version: "0.0.0-mock",
|
||||
config_schema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
context: {
|
||||
type: "string",
|
||||
title: "Kubeconfig context",
|
||||
description: "Context from your kubeconfig.",
|
||||
},
|
||||
namespace: {
|
||||
type: "string",
|
||||
title: "Namespace",
|
||||
default: "buzz-agents-mock01",
|
||||
},
|
||||
},
|
||||
required: ["namespace"],
|
||||
},
|
||||
};
|
||||
|
||||
async function probeInvocations(page: Page): Promise<number> {
|
||||
return page.evaluate(
|
||||
() =>
|
||||
(
|
||||
window as Window & { __BUZZ_E2E_COMMANDS__?: string[] }
|
||||
).__BUZZ_E2E_COMMANDS__?.filter(
|
||||
(command) => command === "probe_backend_provider",
|
||||
).length ?? 0,
|
||||
);
|
||||
}
|
||||
|
||||
/** Open the create-agent dialog and select the mocked provider in "Run on". */
|
||||
async function openCreateDialogOnProvider(page: Page) {
|
||||
await page.goto("/", { waitUntil: "domcontentloaded" });
|
||||
await page.getByTestId("open-agents-view").click();
|
||||
await page.getByTestId("new-agent-card").click();
|
||||
await page.getByRole("menuitem", { name: "Create agent" }).click();
|
||||
const dialog = page.getByTestId("persona-dialog");
|
||||
await expect(dialog).toBeVisible({ timeout: 10_000 });
|
||||
await dialog.locator("#agent-run-on").selectOption(PROVIDER.id);
|
||||
return dialog;
|
||||
}
|
||||
|
||||
test("typing into a defaultless provider field sticks and probes only once", async ({
|
||||
page,
|
||||
}) => {
|
||||
await installMockBridge(page, {
|
||||
backendProviders: [PROVIDER],
|
||||
backendProviderProbeResult: PROBE_RESULT,
|
||||
});
|
||||
const dialog = await openCreateDialogOnProvider(page);
|
||||
|
||||
const contextField = dialog.locator("#provider-cfg-context");
|
||||
await expect(contextField).toBeVisible({ timeout: 10_000 });
|
||||
// Defaults prefilled from the schema; context has none.
|
||||
await expect(dialog.locator("#provider-cfg-namespace")).toHaveValue(
|
||||
"buzz-agents-mock01",
|
||||
);
|
||||
await expect(contextField).toHaveValue("");
|
||||
|
||||
await contextField.pressSequentially("prod-us-west", { delay: 20 });
|
||||
await expect(contextField).toHaveValue("prod-us-west");
|
||||
|
||||
// One selection, one probe — keystrokes must not refire it.
|
||||
expect(await probeInvocations(page)).toBe(1);
|
||||
});
|
||||
|
||||
test("config fields render only after a slow probe resolves, with defaults", async ({
|
||||
page,
|
||||
}) => {
|
||||
// The fields are gated on the probe result (draft.probedProvider), which is
|
||||
// what makes mid-flight typing unreachable through the UI — the stale-probe
|
||||
// merge seam (applyProbeResult) is pinned at the unit level instead. This
|
||||
// spec holds the gate: no half-rendered form before the probe lands, and
|
||||
// defaults appear exactly once when it does.
|
||||
await installMockBridge(page, {
|
||||
backendProviders: [PROVIDER],
|
||||
backendProviderProbeResult: PROBE_RESULT,
|
||||
backendProviderProbeDelayMs: 1_000,
|
||||
});
|
||||
const dialog = await openCreateDialogOnProvider(page);
|
||||
|
||||
// Pre-resolution: the security warning is up, the form is not.
|
||||
await expect(dialog.getByText("will receive your agent")).toBeVisible();
|
||||
await expect(dialog.locator("#provider-cfg-context")).toHaveCount(0);
|
||||
|
||||
// Post-resolution: fields render with schema defaults prefilled.
|
||||
await expect(dialog.locator("#provider-cfg-context")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(dialog.locator("#provider-cfg-namespace")).toHaveValue(
|
||||
"buzz-agents-mock01",
|
||||
);
|
||||
expect(await probeInvocations(page)).toBe(1);
|
||||
});
|
||||
|
||||
test("provider → local → provider re-probes and resets the config", async ({
|
||||
page,
|
||||
}) => {
|
||||
await installMockBridge(page, {
|
||||
backendProviders: [PROVIDER],
|
||||
backendProviderProbeResult: PROBE_RESULT,
|
||||
});
|
||||
const dialog = await openCreateDialogOnProvider(page);
|
||||
|
||||
const contextField = dialog.locator("#provider-cfg-context");
|
||||
await expect(contextField).toBeVisible({ timeout: 10_000 });
|
||||
await contextField.fill("stale-value");
|
||||
|
||||
await dialog.locator("#agent-run-on").selectOption("local");
|
||||
await expect(contextField).toHaveCount(0);
|
||||
|
||||
await dialog.locator("#agent-run-on").selectOption(PROVIDER.id);
|
||||
await expect(dialog.locator("#provider-cfg-context")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
// Fresh selection = fresh draft: the stale value must not leak back.
|
||||
await expect(dialog.locator("#provider-cfg-context")).toHaveValue("");
|
||||
expect(await probeInvocations(page)).toBe(2);
|
||||
});
|
||||
@@ -524,6 +524,23 @@ type MockBridgeOptions = {
|
||||
* returning a catalog. Exercises the discovery-failure UI path.
|
||||
*/
|
||||
discoverAgentModelsError?: string;
|
||||
/**
|
||||
* Providers returned by `discover_backend_providers`. Defaults to `[]`
|
||||
* (the "Run on" section stays hidden). Setting this renders the remote
|
||||
* backend selector in the create-agent dialog.
|
||||
*/
|
||||
backendProviders?: Array<{ id: string; binaryPath: string }>;
|
||||
/**
|
||||
* Result returned by `probe_backend_provider`. Defaults to
|
||||
* `{ ok: false, error: "mock: no providers available" }`.
|
||||
*/
|
||||
backendProviderProbeResult?: Record<string, unknown>;
|
||||
/**
|
||||
* Delay (ms) applied to `probe_backend_provider` so a spec can type into
|
||||
* provider config fields while the probe is still in flight (pins the
|
||||
* latest-state merge on probe resolution).
|
||||
*/
|
||||
backendProviderProbeDelayMs?: number;
|
||||
};
|
||||
|
||||
type BridgeOptions = {
|
||||
|
||||
Reference in New Issue
Block a user