mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
test(desktop): fix harness-management e2e spec and wire it into CI
The C-10 harness-management spec was 5 passed / 5 failed and had never run in
CI — it was added without an entry in either playwright.config.ts project's
testMatch, so no gate covered it. Three test-side defects, no product defects.
1. Ambiguous "Save" selector. getByRole("button", {name: "Save"}) substring-
matches "Save defaults" from AgentDefaultsEditor, a sibling card in the
same Settings -> Agents panel. Scoped to the form, exact match.
2. The mock catalog ignored its own mutation store. handleDiscoverAcpRuntimes
returned a test-supplied acpRuntimesCatalog early, never merging
mockCustomHarnesses — so save/delete mutated a store the seeded specs
never read back, and add/edit/rename/delete all silently no-oped.
Fixed with one mergeMockCustomHarnesses used by both catalog paths. A
deleted *seeded* row has no store entry to remove, so removal needs a
tombstone set rather than just the map; the same mechanism covers the id
vacated by a rename.
3. The onboarding-nav test was unsatisfiable as written. It needs machine
onboarding to run (community must not vouch for the active identity) and
needsSetup to be false afterwards (or App.tsx renders WelcomeSetup instead
of the router). skipCommunitySeed leaves zero communities and fails the
second; the default seed stamps the active pubkey and fails the first.
Seeding a community under a foreign pubkey satisfies both.
Instrumenting this showed the navigation itself always worked — the hash
became #/settings?section=agents on click, with WelcomeSetup painted over
it because the router never mounted. The route intent is correct.
Adds unit coverage for the merge/tombstone logic; reverting the tombstone
filter fails exactly the delete and rename cases.
Verified at 91080e0a9: harness-management 10/10, unit suite 3560/3561 (the
one failure is the pre-existing styled-qr-code / missing qrcode package
fault, identical on a pristine tree), tsc clean, biome clean 1641 files,
check-file-sizes clean.
Co-authored-by: Dawn (sprout agent) <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
This commit is contained in:
@@ -120,6 +120,7 @@ export default defineConfig({
|
||||
"**/inbox-live-update.spec.ts",
|
||||
"**/mesh-compute.spec.ts",
|
||||
"**/observer-archive-policy.spec.ts",
|
||||
"**/harness-management.spec.ts",
|
||||
],
|
||||
use: {
|
||||
...devices["Desktop Chrome"],
|
||||
|
||||
@@ -4,7 +4,7 @@ import { decode } from "nostr-tools/nip19";
|
||||
import { finalizeEvent, getPublicKey } from "nostr-tools/pure";
|
||||
import { parse as yamlParse } from "yaml";
|
||||
import {
|
||||
mockCustomHarnesses,
|
||||
mergeMockCustomHarnesses,
|
||||
handleSaveCustomHarness,
|
||||
handleDeleteCustomHarness,
|
||||
} from "./e2eBridgeCustomHarnesses.ts";
|
||||
@@ -6964,7 +6964,9 @@ async function handleDiscoverAcpRuntimes(
|
||||
}
|
||||
const configured = config?.mock?.acpRuntimesCatalog;
|
||||
if (configured) {
|
||||
return configured.map(withMockRuntimeConfigMetadata);
|
||||
return mergeMockCustomHarnesses(
|
||||
configured.map(withMockRuntimeConfigMetadata),
|
||||
);
|
||||
}
|
||||
const defaultCatalog: RawAcpRuntimeCatalogEntry[] = [
|
||||
{
|
||||
@@ -7046,10 +7048,9 @@ async function handleDiscoverAcpRuntimes(
|
||||
login_hint: undefined,
|
||||
},
|
||||
];
|
||||
return [
|
||||
...defaultCatalog.map(withMockRuntimeConfigMetadata),
|
||||
...Array.from(mockCustomHarnesses.values()),
|
||||
];
|
||||
return mergeMockCustomHarnesses(
|
||||
defaultCatalog.map(withMockRuntimeConfigMetadata),
|
||||
);
|
||||
}
|
||||
|
||||
async function handleDiscoverAcpAuthMethods(
|
||||
|
||||
@@ -19,6 +19,7 @@ import { beforeEach, describe, it } from "node:test";
|
||||
|
||||
import {
|
||||
mockCustomHarnesses,
|
||||
mergeMockCustomHarnesses,
|
||||
resetMockCustomHarnesses,
|
||||
handleSaveCustomHarness,
|
||||
handleDeleteCustomHarness,
|
||||
@@ -149,11 +150,86 @@ describe("handleDeleteCustomHarness", () => {
|
||||
|
||||
// ── discover integration: store is shared by reference ───────────────────────
|
||||
|
||||
describe("mergeMockCustomHarnesses", () => {
|
||||
const seeded = (id, label = id) => ({ id, label, source: "custom" });
|
||||
|
||||
it("appends a newly saved harness that is not in the seeded catalog", () => {
|
||||
handleSaveCustomHarness(makeArgs({ id: "added", label: "Added" }));
|
||||
const merged = mergeMockCustomHarnesses([seeded("preset-a")]);
|
||||
assert.deepEqual(
|
||||
merged.map((e) => e.id),
|
||||
["preset-a", "added"],
|
||||
);
|
||||
});
|
||||
|
||||
it("replaces a seeded entry in place on same-id save — no duplicate row", () => {
|
||||
handleSaveCustomHarness(makeArgs({ id: "seeded-one", label: "V2" }));
|
||||
const merged = mergeMockCustomHarnesses([
|
||||
seeded("preset-a"),
|
||||
seeded("seeded-one", "V1"),
|
||||
]);
|
||||
assert.deepEqual(
|
||||
merged.map((e) => e.id),
|
||||
["preset-a", "seeded-one"],
|
||||
"must not duplicate the id",
|
||||
);
|
||||
assert.equal(
|
||||
merged.find((e) => e.id === "seeded-one").label,
|
||||
"V2",
|
||||
"saved entry must win over the seed",
|
||||
);
|
||||
});
|
||||
|
||||
it("drops a deleted seeded entry (tombstone, not just store removal)", () => {
|
||||
// The regression: a seeded row has no store entry to delete, so without a
|
||||
// tombstone the row survived the delete and the spec failed.
|
||||
handleDeleteCustomHarness({ id: "seeded-one" });
|
||||
const merged = mergeMockCustomHarnesses([
|
||||
seeded("preset-a"),
|
||||
seeded("seeded-one"),
|
||||
]);
|
||||
assert.deepEqual(
|
||||
merged.map((e) => e.id),
|
||||
["preset-a"],
|
||||
);
|
||||
});
|
||||
|
||||
it("drops the vacated old id after a rename and surfaces the new one", () => {
|
||||
handleSaveCustomHarness(
|
||||
makeArgs({ id: "new-id", label: "New", originalId: "old-id" }),
|
||||
);
|
||||
const merged = mergeMockCustomHarnesses([seeded("old-id", "Old")]);
|
||||
assert.deepEqual(
|
||||
merged.map((e) => e.id),
|
||||
["new-id"],
|
||||
);
|
||||
});
|
||||
|
||||
it("re-saving a deleted id resurrects it", () => {
|
||||
handleDeleteCustomHarness({ id: "seeded-one" });
|
||||
handleSaveCustomHarness(makeArgs({ id: "seeded-one", label: "Back" }));
|
||||
const merged = mergeMockCustomHarnesses([seeded("seeded-one", "Original")]);
|
||||
assert.deepEqual(
|
||||
merged.map((e) => e.id),
|
||||
["seeded-one"],
|
||||
);
|
||||
assert.equal(merged[0].label, "Back");
|
||||
});
|
||||
|
||||
it("leaves a seeded catalog untouched when nothing has been mutated", () => {
|
||||
const base = [seeded("preset-a"), seeded("preset-b")];
|
||||
assert.deepEqual(
|
||||
mergeMockCustomHarnesses(base).map((e) => e.id),
|
||||
["preset-a", "preset-b"],
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("mockCustomHarnesses Map reference", () => {
|
||||
it("handler writes are immediately visible to callers that read the exported Map", () => {
|
||||
// e2eBridge.ts reads `Array.from(mockCustomHarnesses.values())` in
|
||||
// handleDiscoverAcpRuntimes. The exported Map is the same object by reference,
|
||||
// so writes via the handler are visible to any reader of the Map.
|
||||
// handleDiscoverAcpRuntimes merges this store into the catalog it returns.
|
||||
// The exported Map is the same object by reference, so writes via the
|
||||
// handler are visible to any reader of the Map.
|
||||
assert.equal(
|
||||
mockCustomHarnesses.size,
|
||||
0,
|
||||
|
||||
@@ -10,9 +10,43 @@ import type { RawAcpRuntimeCatalogEntry } from "../shared/api/tauri.ts";
|
||||
/** In-memory store for custom harnesses saved via `save_custom_harness`. */
|
||||
export const mockCustomHarnesses = new Map<string, RawAcpRuntimeCatalogEntry>();
|
||||
|
||||
/**
|
||||
* Ids removed via `delete_custom_harness` (or vacated by a rename).
|
||||
*
|
||||
* Needed because a test's `acpRuntimesCatalog` seed is static config, not the
|
||||
* mutation store: deleting a seeded row leaves nothing to remove from
|
||||
* `mockCustomHarnesses`, so without a tombstone the row would survive the
|
||||
* delete and the mock would report success while the UI still shows it.
|
||||
*/
|
||||
export const mockDeletedCustomHarnesses = new Set<string>();
|
||||
|
||||
/** Reset the store between tests. */
|
||||
export function resetMockCustomHarnesses(): void {
|
||||
mockCustomHarnesses.clear();
|
||||
mockDeletedCustomHarnesses.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
* Overlay the mutation store onto a seeded catalog.
|
||||
*
|
||||
* Deleted ids drop out, saved ids replace their seeded entry in place (so a
|
||||
* same-id edit updates rather than duplicates), and newly added ids append.
|
||||
*/
|
||||
export function mergeMockCustomHarnesses(
|
||||
base: RawAcpRuntimeCatalogEntry[],
|
||||
): RawAcpRuntimeCatalogEntry[] {
|
||||
const merged = base.filter(
|
||||
(entry) => !mockDeletedCustomHarnesses.has(entry.id),
|
||||
);
|
||||
for (const entry of mockCustomHarnesses.values()) {
|
||||
const index = merged.findIndex((existing) => existing.id === entry.id);
|
||||
if (index === -1) {
|
||||
merged.push(entry);
|
||||
} else {
|
||||
merged[index] = entry;
|
||||
}
|
||||
}
|
||||
return merged;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -41,7 +75,10 @@ export function handleSaveCustomHarness(args: {
|
||||
// On rename: remove the old entry so the old id is no longer in the catalog.
|
||||
if (originalId && originalId !== id) {
|
||||
mockCustomHarnesses.delete(originalId);
|
||||
mockDeletedCustomHarnesses.add(originalId);
|
||||
}
|
||||
// A save resurrects an id that an earlier test step deleted.
|
||||
mockDeletedCustomHarnesses.delete(id);
|
||||
|
||||
const entry: RawAcpRuntimeCatalogEntry = {
|
||||
id,
|
||||
@@ -86,4 +123,5 @@ export function handleDeleteCustomHarness(
|
||||
}
|
||||
const id = args.id ?? "";
|
||||
mockCustomHarnesses.delete(id);
|
||||
mockDeletedCustomHarnesses.add(id);
|
||||
}
|
||||
|
||||
@@ -197,7 +197,10 @@ test.describe("add custom harness", () => {
|
||||
});
|
||||
|
||||
// Submit.
|
||||
await page.getByRole("button", { name: "Save" }).click();
|
||||
await page
|
||||
.getByTestId("custom-harness-form")
|
||||
.getByRole("button", { name: "Save", exact: true })
|
||||
.click();
|
||||
|
||||
// Row must appear in the list after save.
|
||||
await expect(
|
||||
@@ -247,7 +250,10 @@ test.describe("add custom harness", () => {
|
||||
await page.getByTestId("custom-harness-edit-my-custom-agent").click();
|
||||
await expect(page.getByTestId("custom-harness-form")).toBeVisible();
|
||||
await page.fill("#ch-label", "V2 Label");
|
||||
await page.getByRole("button", { name: "Save" }).click();
|
||||
await page
|
||||
.getByTestId("custom-harness-form")
|
||||
.getByRole("button", { name: "Save", exact: true })
|
||||
.click();
|
||||
|
||||
// Exactly one row with the same ID; label updated.
|
||||
const rows = page.locator(
|
||||
@@ -271,7 +277,10 @@ test.describe("add custom harness", () => {
|
||||
await expect(page.getByTestId("custom-harness-form")).toBeVisible();
|
||||
await page.fill("#ch-label", "New Harness");
|
||||
await page.fill("#ch-id", "new-harness");
|
||||
await page.getByRole("button", { name: "Save" }).click();
|
||||
await page
|
||||
.getByTestId("custom-harness-form")
|
||||
.getByRole("button", { name: "Save", exact: true })
|
||||
.click();
|
||||
|
||||
// Old row gone; new row present.
|
||||
await expect(
|
||||
@@ -381,6 +390,32 @@ test("onboarding setup More-harnesses click navigates to Settings → Agents", a
|
||||
skipCommunitySeed: true,
|
||||
skipOnboardingSeed: true,
|
||||
});
|
||||
// Seed a community stamped with a *foreign* pubkey. This is the only shape
|
||||
// that satisfies both preconditions of this test at once:
|
||||
// - machine onboarding must still run, so the community must NOT vouch for
|
||||
// the active identity (migrateMachineOnboardingCompletion only accepts a
|
||||
// community whose recorded pubkey matches — see machineOnboarding.ts:70).
|
||||
// - after onboarding completes, useCommunityInit must NOT report
|
||||
// needsSetup, or App.tsx:499 renders WelcomeSetup instead of the router
|
||||
// and the navigation lands on a screen that has no settings tree.
|
||||
// The default seed vouches (it uses the active pubkey) and skipping it
|
||||
// entirely leaves zero communities, so neither default gets there.
|
||||
await page.addInitScript(() => {
|
||||
const communityId = "e2e-default-community";
|
||||
window.localStorage.setItem(
|
||||
"buzz-communities",
|
||||
JSON.stringify([
|
||||
{
|
||||
id: communityId,
|
||||
name: "E2E Test",
|
||||
relayUrl: "ws://127.0.0.1:7777",
|
||||
pubkey: "f".repeat(64),
|
||||
addedAt: new Date().toISOString(),
|
||||
},
|
||||
]),
|
||||
);
|
||||
window.localStorage.setItem("buzz-active-community-id", communityId);
|
||||
});
|
||||
await page.goto("/");
|
||||
|
||||
// Reach the setup page: create a new identity key → skip backup step.
|
||||
|
||||
Reference in New Issue
Block a user