mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(agents): explain why Create/Save is disabled and fix Codex/Claude gate (#2050)
Signed-off-by: Atish Patel <atish@squareup.com> Co-authored-by: WorkerBeeGPT <c5c455215c2506cb8ba776518cec804af62d3a0526e32d496a22072e395042b9@buzz> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
WorkerBeeGPT
Claude Opus 4.8
parent
79598bdb3f
commit
87dc4dccba
@@ -31,6 +31,7 @@ import {
|
||||
emptyPersonaBehaviorDraft,
|
||||
personaBehaviorDraftValid,
|
||||
} from "./personaBehaviorDraft";
|
||||
import { personaSubmitBlock } from "./personaSubmitBlock";
|
||||
import {
|
||||
AUTO_MODEL_DROPDOWN_VALUE,
|
||||
AUTO_PROVIDER_DROPDOWN_VALUE,
|
||||
@@ -415,22 +416,31 @@ export function AgentDefinitionDialog({
|
||||
secretEnvVar: topLevelSecretEnvVar,
|
||||
value: apiKeyValue,
|
||||
} = apiKeyFieldState;
|
||||
// Provider required-ness is a static property of the runtime — it does not
|
||||
// change based on whether the field is currently filled. Using the dynamic
|
||||
// missingNormalizedFields check would flip the asterisk off once a value is
|
||||
// selected, which is incoherent (required means required, not "required until
|
||||
// satisfied"). runtimeSupportsLlmProviderSelection is the authoritative gate.
|
||||
// Provider required-ness is a static property of the field's visibility — it
|
||||
// does not change based on whether the field is currently filled. Using the
|
||||
// dynamic missingNormalizedFields check would flip the asterisk off once a
|
||||
// value is selected, which is incoherent (required means required, not
|
||||
// "required until satisfied"). runtimeCanChooseLlmProvider is the authoritative
|
||||
// gate: it tracks exactly when the provider picker is shown (Buzz Agent/Goose,
|
||||
// plus runtime-less legacy/builtin definitions), so the required marker never
|
||||
// drifts from whether Save actually needs a provider.
|
||||
const providerIsRequired =
|
||||
aiConfigurationMode === "custom" &&
|
||||
runtimeSupportsLlmProviderSelection(runtime);
|
||||
aiConfigurationMode === "custom" && runtimeCanChooseLlmProvider;
|
||||
const modelFieldVisible =
|
||||
runtime.trim().length > 0 || blankRuntimeModelProviderEditable;
|
||||
// Customize pins a complete provider/model pair. Shared compute's concrete
|
||||
// automatic-routing value is the only valid non-model-id choice.
|
||||
const isExplicitModelRequired = aiConfigurationMode === "custom";
|
||||
// Gate the provider requirement on the field's actual visibility, not the raw
|
||||
// runtime capability. Codex/Claude hide the provider picker (they drive their
|
||||
// own provider), so Customize must not require a provider there. But a
|
||||
// runtime-less legacy/builtin definition still exposes the picker via
|
||||
// blankRuntimeModelProviderEditable, so it must keep requiring a provider —
|
||||
// otherwise Save could persist `provider: undefined` despite the visible field.
|
||||
const customAiPairSatisfied = agentAiConfigurationModeSatisfied(
|
||||
aiConfigurationMode,
|
||||
{ provider, model },
|
||||
runtimeCanChooseLlmProvider,
|
||||
);
|
||||
const isCreateMode = Boolean(initialValues && !("id" in initialValues));
|
||||
const selectedRuntimeIsAvailable =
|
||||
@@ -452,6 +462,29 @@ export function AgentDefinitionDialog({
|
||||
customAiPairSatisfied &&
|
||||
!isAvatarUploadPending;
|
||||
|
||||
// Derive the single, deterministic reason the action is disabled from the
|
||||
// same gate outputs that feed canSubmit — no policy is recomputed here.
|
||||
// Precedence mirrors canSubmit's term order, so the reason is `null` exactly
|
||||
// when the form can be submitted (transient Saving/Uploading states aside).
|
||||
const submitBlockReason = personaSubmitBlock({
|
||||
isPending,
|
||||
isAvatarUploadPending,
|
||||
displayNameEmpty: displayName.trim().length === 0,
|
||||
isCreateMode,
|
||||
runtimeChosen: runtime.trim().length > 0,
|
||||
runtimeAvailable: selectedRuntimeIsAvailable,
|
||||
createBackendBlocked: createSubmitBlocked,
|
||||
allowlistEmpty: !personaBehaviorDraftValid(behaviorDraft),
|
||||
aiConfigurationMode,
|
||||
localModeSatisfied,
|
||||
localModeMissingFields: localModeGate.missingNormalizedFields,
|
||||
localModeMissingEnvKeys: localModeGate.missingEnvKeys,
|
||||
customAiPairSatisfied,
|
||||
runtimeNeedsProviderSelection: runtimeCanChooseLlmProvider,
|
||||
customProviderEmpty: provider.trim().length === 0,
|
||||
customModelEmpty: model.trim().length === 0,
|
||||
});
|
||||
|
||||
// Merge global env as the base layer so credential keys satisfied via global
|
||||
// config are available to model discovery — same rationale as in AgentInstanceEditDialog.
|
||||
const envVarsForDiscovery = React.useMemo(
|
||||
@@ -700,7 +733,16 @@ export function AgentDefinitionDialog({
|
||||
title={title}
|
||||
footer={
|
||||
<div className="flex w-full items-center justify-between gap-3">
|
||||
<div className="flex min-h-9 items-center" />
|
||||
<div className="flex min-h-9 items-center">
|
||||
{submitBlockReason ? (
|
||||
<p
|
||||
className="text-2xs text-muted-foreground"
|
||||
data-testid="persona-dialog-submit-reason"
|
||||
>
|
||||
{submitBlockReason}
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
<div className="flex items-center gap-2">
|
||||
<Button
|
||||
|
||||
@@ -50,6 +50,71 @@ test("Customize requires a complete explicit pair", () => {
|
||||
);
|
||||
});
|
||||
|
||||
test("Codex/Claude Customize needs only a model, not the hidden provider", () => {
|
||||
// needsProviderSelection=false → the intentionally hidden provider must not
|
||||
// gate Save (the create/edit "Save stays disabled" regression).
|
||||
assert.equal(
|
||||
agentAiConfigurationModeSatisfied(
|
||||
"custom",
|
||||
{ provider: "", model: "gpt-5-codex" },
|
||||
false,
|
||||
),
|
||||
true,
|
||||
);
|
||||
// Still needs a model even when the provider is hidden.
|
||||
assert.equal(
|
||||
agentAiConfigurationModeSatisfied(
|
||||
"custom",
|
||||
{ provider: "", model: "" },
|
||||
false,
|
||||
),
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test("Buzz Agent/Goose Customize still requires both provider and model", () => {
|
||||
assert.equal(
|
||||
agentAiConfigurationModeSatisfied(
|
||||
"custom",
|
||||
{ provider: "", model: "llama" },
|
||||
true,
|
||||
),
|
||||
false,
|
||||
);
|
||||
assert.equal(
|
||||
agentAiConfigurationModeSatisfied(
|
||||
"custom",
|
||||
{ provider: "databricks_v2", model: "llama" },
|
||||
true,
|
||||
),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("runtime-less editable definition still requires the visible provider", () => {
|
||||
// A legacy/builtin definition with no runtime but a saved model exposes the
|
||||
// provider picker (runtimeCanChooseLlmProvider === true), so the dialog passes
|
||||
// needsProviderSelection=true here. An empty provider must NOT satisfy the
|
||||
// pair — otherwise Save persists `provider: undefined` despite the visible
|
||||
// picker (wesbillman's blocking review point).
|
||||
assert.equal(
|
||||
agentAiConfigurationModeSatisfied(
|
||||
"custom",
|
||||
{ provider: "", model: "claude-opus-4-5" },
|
||||
true,
|
||||
),
|
||||
false,
|
||||
);
|
||||
assert.equal(
|
||||
agentAiConfigurationModeSatisfied(
|
||||
"custom",
|
||||
{ provider: "anthropic", model: "claude-opus-4-5" },
|
||||
true,
|
||||
),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("Defaults clears provider and model together", () => {
|
||||
assert.deepEqual(
|
||||
agentAiConfigurationPairForMode({
|
||||
|
||||
@@ -30,12 +30,27 @@ export function agentAiConfigurationPairForMode({
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a Customize (explicit) AI pair is complete enough to submit.
|
||||
*
|
||||
* `needsProviderSelection` reflects whether the provider picker is actually
|
||||
* shown to the user: Buzz Agent / Goose expose it (and runtime-less legacy /
|
||||
* builtin definitions do too), so both provider and model are required, while
|
||||
* Codex / Claude drive their own provider and hide the field, so requiring a
|
||||
* provider there would gate Save on a value the user can never set (the
|
||||
* create/edit "Save stays disabled" regression). Callers should pass the
|
||||
* field-visibility capability (`runtimeCanChooseLlmProvider`), not the raw
|
||||
* runtime capability, so the gate never diverges from the visible picker. It
|
||||
* defaults to `true` so existing callers keep the provider+model requirement.
|
||||
*/
|
||||
export function agentAiConfigurationModeSatisfied(
|
||||
mode: AgentAiConfigurationMode,
|
||||
pair: AgentAiConfigurationPair,
|
||||
needsProviderSelection = true,
|
||||
) {
|
||||
return (
|
||||
mode === "defaults" ||
|
||||
(pair.provider.trim().length > 0 && pair.model.trim().length > 0)
|
||||
);
|
||||
if (mode === "defaults") {
|
||||
return true;
|
||||
}
|
||||
const providerOk = !needsProviderSelection || pair.provider.trim().length > 0;
|
||||
return providerOk && pair.model.trim().length > 0;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,158 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { personaSubmitBlock } from "./personaSubmitBlock.ts";
|
||||
|
||||
/** A fully valid, submittable form: personaSubmitBlock returns null. */
|
||||
function submittable(overrides = {}) {
|
||||
return {
|
||||
isPending: false,
|
||||
isAvatarUploadPending: false,
|
||||
displayNameEmpty: false,
|
||||
isCreateMode: true,
|
||||
runtimeChosen: true,
|
||||
runtimeAvailable: true,
|
||||
createBackendBlocked: false,
|
||||
allowlistEmpty: false,
|
||||
aiConfigurationMode: "defaults",
|
||||
localModeSatisfied: true,
|
||||
localModeMissingFields: [],
|
||||
localModeMissingEnvKeys: [],
|
||||
customAiPairSatisfied: true,
|
||||
runtimeNeedsProviderSelection: true,
|
||||
customProviderEmpty: false,
|
||||
customModelEmpty: false,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
test("a valid form has no disabled reason", () => {
|
||||
assert.equal(personaSubmitBlock(submittable()), null);
|
||||
});
|
||||
|
||||
test("missing name is reported first", () => {
|
||||
assert.equal(
|
||||
personaSubmitBlock(submittable({ displayNameEmpty: true })),
|
||||
"Enter a name for this agent.",
|
||||
);
|
||||
});
|
||||
|
||||
test("Buzz Agent + Use AI defaults with no global provider/model names the fix", () => {
|
||||
const reason = personaSubmitBlock(
|
||||
submittable({
|
||||
aiConfigurationMode: "defaults",
|
||||
localModeSatisfied: false,
|
||||
localModeMissingFields: ["provider", "model"],
|
||||
}),
|
||||
);
|
||||
assert.match(reason, /global AI defaults are incomplete/);
|
||||
assert.match(reason, /a provider and a model/);
|
||||
assert.match(reason, /Settings → AI defaults/);
|
||||
});
|
||||
|
||||
test("incomplete defaults also names missing credential keys", () => {
|
||||
const reason = personaSubmitBlock(
|
||||
submittable({
|
||||
localModeSatisfied: false,
|
||||
localModeMissingFields: [],
|
||||
localModeMissingEnvKeys: ["ANTHROPIC_API_KEY"],
|
||||
}),
|
||||
);
|
||||
assert.match(reason, /a value for ANTHROPIC_API_KEY/);
|
||||
});
|
||||
|
||||
test("the reason disappears once the blocking input is corrected", () => {
|
||||
const blocked = submittable({
|
||||
localModeSatisfied: false,
|
||||
localModeMissingFields: ["provider", "model"],
|
||||
});
|
||||
assert.notEqual(personaSubmitBlock(blocked), null);
|
||||
// Correct the blocking input: defaults now resolve.
|
||||
const corrected = {
|
||||
...blocked,
|
||||
localModeSatisfied: true,
|
||||
localModeMissingFields: [],
|
||||
};
|
||||
assert.equal(personaSubmitBlock(corrected), null);
|
||||
});
|
||||
|
||||
test("create mode requires a chosen, available runtime", () => {
|
||||
assert.equal(
|
||||
personaSubmitBlock(submittable({ runtimeChosen: false })),
|
||||
"Choose where this agent runs.",
|
||||
);
|
||||
assert.equal(
|
||||
personaSubmitBlock(submittable({ runtimeAvailable: false })),
|
||||
"The selected runtime isn't available on this machine.",
|
||||
);
|
||||
});
|
||||
|
||||
test("runtime gates do not apply in edit mode", () => {
|
||||
assert.equal(
|
||||
personaSubmitBlock(
|
||||
submittable({ isCreateMode: false, runtimeChosen: false }),
|
||||
),
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
test("empty allowlist is reported (create and edit)", () => {
|
||||
const reason = personaSubmitBlock(
|
||||
submittable({ isCreateMode: false, allowlistEmpty: true }),
|
||||
);
|
||||
assert.match(reason, /allowed sender/);
|
||||
});
|
||||
|
||||
test("Customize with an empty pair but satisfied global fallback points at the pair", () => {
|
||||
const reason = personaSubmitBlock(
|
||||
submittable({
|
||||
aiConfigurationMode: "custom",
|
||||
localModeSatisfied: true,
|
||||
customAiPairSatisfied: false,
|
||||
customProviderEmpty: true,
|
||||
customModelEmpty: true,
|
||||
}),
|
||||
);
|
||||
assert.match(reason, /Select a provider and a model/);
|
||||
assert.match(reason, /Use AI defaults/);
|
||||
});
|
||||
|
||||
test("Customize on Codex/Claude asks only for a model, never a provider", () => {
|
||||
const reason = personaSubmitBlock(
|
||||
submittable({
|
||||
aiConfigurationMode: "custom",
|
||||
customAiPairSatisfied: false,
|
||||
runtimeNeedsProviderSelection: false,
|
||||
customProviderEmpty: true,
|
||||
customModelEmpty: true,
|
||||
}),
|
||||
);
|
||||
assert.match(reason, /Select a model/);
|
||||
assert.doesNotMatch(reason, /provider/);
|
||||
});
|
||||
|
||||
test("precedence: a missing name outranks incomplete AI defaults", () => {
|
||||
assert.equal(
|
||||
personaSubmitBlock(
|
||||
submittable({
|
||||
displayNameEmpty: true,
|
||||
localModeSatisfied: false,
|
||||
localModeMissingFields: ["provider", "model"],
|
||||
}),
|
||||
),
|
||||
"Enter a name for this agent.",
|
||||
);
|
||||
});
|
||||
|
||||
test("in-flight save/upload shows no reason (the button label communicates it)", () => {
|
||||
assert.equal(
|
||||
personaSubmitBlock(
|
||||
submittable({ isPending: true, displayNameEmpty: true }),
|
||||
),
|
||||
null,
|
||||
);
|
||||
assert.equal(
|
||||
personaSubmitBlock(submittable({ isAvatarUploadPending: true })),
|
||||
null,
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,135 @@
|
||||
import type { AgentAiConfigurationMode } from "./agentAiConfigurationPolicy";
|
||||
|
||||
/**
|
||||
* Inputs for {@link personaSubmitBlock}. Every field is an OUTPUT of a gate the
|
||||
* dialog already computes for `canSubmit` — this module maps those outputs to a
|
||||
* single human-readable reason. It must not recompute policy: the derivation
|
||||
* stays a pure function of the gate results so the message can never disagree
|
||||
* with whether the button is actually disabled.
|
||||
*/
|
||||
export type PersonaSubmitBlockInput = {
|
||||
/** A save/create request is in flight (button shows "Saving..."). */
|
||||
isPending: boolean;
|
||||
/** The avatar upload is in flight (button shows "Uploading..."). */
|
||||
isAvatarUploadPending: boolean;
|
||||
/** Trimmed display name is empty. */
|
||||
displayNameEmpty: boolean;
|
||||
/** Create (new definition) vs edit (existing). Some gates are create-only. */
|
||||
isCreateMode: boolean;
|
||||
/** A runtime has been chosen (create-only gate). */
|
||||
runtimeChosen: boolean;
|
||||
/** The chosen runtime is available on this machine (create-only gate). */
|
||||
runtimeAvailable: boolean;
|
||||
/** The remote / where-to-run backend selection is incomplete (create-only). */
|
||||
createBackendBlocked: boolean;
|
||||
/** Respond-to allowlist mode is selected but the allowlist is empty. */
|
||||
allowlistEmpty: boolean;
|
||||
/** Selected AI configuration mode: inherit global defaults vs customize. */
|
||||
aiConfigurationMode: AgentAiConfigurationMode;
|
||||
/** `computeLocalModeGate(...).satisfied` — resolved AI config is complete. */
|
||||
localModeSatisfied: boolean;
|
||||
/** `computeLocalModeGate(...).missingNormalizedFields`, e.g. ["provider"]. */
|
||||
localModeMissingFields: readonly string[];
|
||||
/** `computeLocalModeGate(...).missingEnvKeys` — required credentials unset. */
|
||||
localModeMissingEnvKeys: readonly string[];
|
||||
/** `agentAiConfigurationModeSatisfied(...)` for the Customize pair. */
|
||||
customAiPairSatisfied: boolean;
|
||||
/** Runtime exposes a provider picker (Buzz Agent / Goose), not Codex/Claude. */
|
||||
runtimeNeedsProviderSelection: boolean;
|
||||
/** Customize provider field is empty. */
|
||||
customProviderEmpty: boolean;
|
||||
/** Customize model field is empty. */
|
||||
customModelEmpty: boolean;
|
||||
};
|
||||
|
||||
function joinWithAnd(parts: readonly string[]): string {
|
||||
if (parts.length <= 1) return parts[0] ?? "";
|
||||
if (parts.length === 2) return `${parts[0]} and ${parts[1]}`;
|
||||
return `${parts.slice(0, -1).join(", ")}, and ${parts[parts.length - 1]}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Describe the concrete missing pieces behind an unsatisfied AI-config gate,
|
||||
* naming the actual fix rather than a generic "configuration incomplete".
|
||||
*/
|
||||
function describeMissingAiPieces(
|
||||
fields: readonly string[],
|
||||
envKeys: readonly string[],
|
||||
): string {
|
||||
const parts: string[] = [];
|
||||
if (fields.includes("provider")) parts.push("a provider");
|
||||
if (fields.includes("model")) parts.push("a model");
|
||||
for (const key of envKeys) parts.push(`a value for ${key}`);
|
||||
return joinWithAnd(parts);
|
||||
}
|
||||
|
||||
/**
|
||||
* Human-readable reason the Create/Save button is disabled, or `null` when the
|
||||
* form can be submitted. Precedence mirrors the `canSubmit` term order in
|
||||
* AgentDefinitionDialog so the surfaced reason is deterministic and always the
|
||||
* first blocking input — correcting it makes the reason advance or disappear.
|
||||
*
|
||||
* While a request or avatar upload is in flight the button communicates the
|
||||
* progress itself ("Saving..." / "Uploading..."), so no reason is returned.
|
||||
*/
|
||||
export function personaSubmitBlock(
|
||||
input: PersonaSubmitBlockInput,
|
||||
): string | null {
|
||||
if (input.isPending || input.isAvatarUploadPending) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// 1. Required definition fields.
|
||||
if (input.displayNameEmpty) {
|
||||
return "Enter a name for this agent.";
|
||||
}
|
||||
|
||||
// 2–4. Create-only runtime / backend gates.
|
||||
if (input.isCreateMode) {
|
||||
if (!input.runtimeChosen) {
|
||||
return "Choose where this agent runs.";
|
||||
}
|
||||
if (!input.runtimeAvailable) {
|
||||
return "The selected runtime isn't available on this machine.";
|
||||
}
|
||||
if (input.createBackendBlocked) {
|
||||
return "Finish configuring the remote backend before creating this agent.";
|
||||
}
|
||||
}
|
||||
|
||||
// 5. Access / allowlist crash-loop guard (create and edit).
|
||||
if (input.allowlistEmpty) {
|
||||
return "Add at least one allowed sender, or change who this agent responds to.";
|
||||
}
|
||||
|
||||
// 6. Resolved AI configuration (provider/model/credentials) incomplete.
|
||||
if (!input.localModeSatisfied) {
|
||||
const missing = describeMissingAiPieces(
|
||||
input.localModeMissingFields,
|
||||
input.localModeMissingEnvKeys,
|
||||
);
|
||||
if (input.aiConfigurationMode === "defaults") {
|
||||
const detail = missing ? ` — missing ${missing}` : "";
|
||||
return `Your global AI defaults are incomplete${detail}. Set them in Settings → AI defaults, or choose Customize to configure this agent directly.`;
|
||||
}
|
||||
return missing
|
||||
? `This agent's AI configuration is missing ${missing}.`
|
||||
: "Complete this agent's AI configuration.";
|
||||
}
|
||||
|
||||
// 7. Customize pair incomplete (form provider/model empty while a global
|
||||
// fallback keeps localMode satisfied). Provider only counts where the runtime
|
||||
// exposes a picker — Codex/Claude drive their own provider.
|
||||
if (!input.customAiPairSatisfied) {
|
||||
const needProvider =
|
||||
input.runtimeNeedsProviderSelection && input.customProviderEmpty;
|
||||
const pieces: string[] = [];
|
||||
if (needProvider) pieces.push("a provider");
|
||||
if (input.customModelEmpty) pieces.push("a model");
|
||||
const what =
|
||||
pieces.length > 0 ? joinWithAnd(pieces) : "the AI configuration";
|
||||
return `Select ${what} for this agent, or switch to Use AI defaults.`;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
@@ -99,6 +99,10 @@ type MockPersonaSeed = {
|
||||
isActive?: boolean;
|
||||
sourceTeam?: string | null;
|
||||
envVars?: Record<string, string>;
|
||||
runtime?: string | null;
|
||||
model?: string | null;
|
||||
provider?: string | null;
|
||||
namePool?: string[];
|
||||
};
|
||||
|
||||
type MockTeamSeed = {
|
||||
@@ -1962,6 +1966,10 @@ function resetMockPersonas(config?: E2eConfig) {
|
||||
display_name: persona.displayName,
|
||||
avatar_url: persona.avatarUrl ?? null,
|
||||
system_prompt: persona.systemPrompt,
|
||||
runtime: persona.runtime ?? null,
|
||||
model: persona.model ?? null,
|
||||
provider: persona.provider ?? null,
|
||||
name_pool: persona.namePool ?? [],
|
||||
is_builtin: false,
|
||||
is_active: persona.isActive ?? true,
|
||||
source_team: persona.sourceTeam ?? null,
|
||||
|
||||
@@ -1,16 +1,10 @@
|
||||
import { expect, test } from "@playwright/test";
|
||||
|
||||
import { installMockBridge } from "../helpers/bridge";
|
||||
import { waitForAnimations } from "../helpers/animations";
|
||||
import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge";
|
||||
|
||||
const SHOTS = "test-results/global-agent-config";
|
||||
|
||||
// Settle any in-flight CSS / Web Animations before capture.
|
||||
async function settleAnimations(page: import("@playwright/test").Page) {
|
||||
await page.evaluate(() =>
|
||||
Promise.all(document.getAnimations().map((a) => a.finished)),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Open Settings → Agents through the app UI and wait for the defaults card to
|
||||
* load. CI serves the built SPA with a static file server, so navigating to
|
||||
@@ -46,6 +40,128 @@ async function customizeAgentAi(page: import("@playwright/test").Page) {
|
||||
await page.getByRole("tab", { name: "Customize for this agent" }).click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Pick an option from a PersonaDropdownField (menu-based, not a native
|
||||
* <select>): focus the trigger, open it, then click the matching
|
||||
* menuitemradio. Mirrors the helper in agent-readiness-screenshots.spec.ts.
|
||||
*/
|
||||
async function selectDropdownOption(
|
||||
page: import("@playwright/test").Page,
|
||||
trigger: import("@playwright/test").Locator,
|
||||
optionName: string | RegExp,
|
||||
) {
|
||||
await expect(trigger).toBeVisible({ timeout: 10_000 });
|
||||
await trigger.press("Enter");
|
||||
await page
|
||||
.getByRole("menuitemradio", { name: optionName })
|
||||
.click({ timeout: 5_000 });
|
||||
}
|
||||
|
||||
// A runtime catalog with both a provider-selection runtime (buzz-agent) and a
|
||||
// CLI-login runtime (Claude Code) marked available, so Claude Code appears and
|
||||
// is selectable in the harness dropdown. Same shape the readiness spec uses.
|
||||
const CATALOG_WITH_CLAUDE = [
|
||||
{
|
||||
id: "buzz-agent",
|
||||
label: "Buzz Agent",
|
||||
avatar_url: "",
|
||||
availability: "available",
|
||||
command: "buzz-agent",
|
||||
binary_path: "/usr/local/bin/buzz-agent",
|
||||
default_args: [],
|
||||
mcp_command: "buzz-dev-mcp",
|
||||
install_hint: "Ships with the Buzz desktop app.",
|
||||
install_instructions_url: "https://github.com/block/buzz",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: null,
|
||||
},
|
||||
{
|
||||
id: "claude",
|
||||
label: "Claude Code",
|
||||
avatar_url: "",
|
||||
availability: "available",
|
||||
command: "/usr/local/bin/claude-agent",
|
||||
binary_path: "/usr/local/bin/claude-agent",
|
||||
default_args: ["acp"],
|
||||
mcp_command: null,
|
||||
install_hint: "Install the Claude Code ACP adapter via npm.",
|
||||
install_instructions_url:
|
||||
"https://www.npmjs.com/package/@anthropic-ai/claude-agent-acp",
|
||||
can_auto_install: true,
|
||||
underlying_cli_path: "/usr/local/bin/claude",
|
||||
},
|
||||
];
|
||||
|
||||
// A runtime catalog with Codex marked available (the default catalog ships it
|
||||
// as `not_installed`). Codex is a CLI-login runtime — it drives its own
|
||||
// provider, so the definition dialog hides the provider picker for it. Used by
|
||||
// the Edit/Save-mode test to seed an editable Codex agent.
|
||||
const CATALOG_WITH_CODEX = [
|
||||
{
|
||||
id: "buzz-agent",
|
||||
label: "Buzz Agent",
|
||||
avatar_url: "",
|
||||
availability: "available",
|
||||
command: "buzz-agent",
|
||||
binary_path: "/usr/local/bin/buzz-agent",
|
||||
default_args: [],
|
||||
mcp_command: "buzz-dev-mcp",
|
||||
install_hint: "Ships with the Buzz desktop app.",
|
||||
install_instructions_url: "https://github.com/block/buzz",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: null,
|
||||
},
|
||||
{
|
||||
id: "codex",
|
||||
label: "Codex",
|
||||
avatar_url: "",
|
||||
availability: "available",
|
||||
command: "/usr/local/bin/codex-agent",
|
||||
binary_path: "/usr/local/bin/codex-agent",
|
||||
default_args: ["acp"],
|
||||
mcp_command: null,
|
||||
install_hint: "The codex-acp adapter must be built from source.",
|
||||
install_instructions_url: "https://github.com/openai/codex",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: "/usr/local/bin/codex",
|
||||
},
|
||||
];
|
||||
|
||||
// A catalog where every runtime is unavailable (not installed). With nothing
|
||||
// available, getDefaultPersonaRuntime returns null, so the definition dialog's
|
||||
// runtime auto-seed effect is a no-op and a runtime-less definition keeps its
|
||||
// empty runtime — the precondition for blankRuntimeModelProviderEditable.
|
||||
const CATALOG_NONE_AVAILABLE = [
|
||||
{
|
||||
id: "buzz-agent",
|
||||
label: "Buzz Agent",
|
||||
avatar_url: "",
|
||||
availability: "not_installed",
|
||||
command: "buzz-agent",
|
||||
binary_path: null,
|
||||
default_args: [],
|
||||
mcp_command: "buzz-dev-mcp",
|
||||
install_hint: "Ships with the Buzz desktop app.",
|
||||
install_instructions_url: "https://github.com/block/buzz",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: null,
|
||||
},
|
||||
{
|
||||
id: "goose",
|
||||
label: "Goose",
|
||||
avatar_url: "",
|
||||
availability: "not_installed",
|
||||
command: "goose",
|
||||
binary_path: null,
|
||||
default_args: [],
|
||||
mcp_command: null,
|
||||
install_hint: "Install Goose to use this runtime.",
|
||||
install_instructions_url: "https://github.com/block/goose",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: null,
|
||||
},
|
||||
];
|
||||
|
||||
test.describe("global agent config screenshots", () => {
|
||||
test.use({ viewport: { width: 1280, height: 900 } });
|
||||
|
||||
@@ -75,7 +191,7 @@ test.describe("global agent config screenshots", () => {
|
||||
|
||||
const card = page.getByTestId("settings-global-agent-config");
|
||||
await card.scrollIntoViewIfNeeded();
|
||||
await settleAnimations(page);
|
||||
await waitForAnimations(page);
|
||||
|
||||
await card.screenshot({
|
||||
path: `${SHOTS}/01-global-agent-config-card-populated.png`,
|
||||
@@ -218,7 +334,16 @@ test.describe("global agent config screenshots", () => {
|
||||
await expect(page.getByTestId("persona-dialog-submit")).toBeDisabled({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await settleAnimations(page);
|
||||
|
||||
// The footer must explain WHY it is disabled (regression guard for the
|
||||
// submitBlockReason wiring, not just the boolean gate): defaults mode with
|
||||
// no resolvable provider names the missing piece and points to Settings.
|
||||
const reason = page.getByTestId("persona-dialog-submit-reason");
|
||||
await expect(reason).toBeVisible({ timeout: 10_000 });
|
||||
await expect(reason).toContainText("provider");
|
||||
await expect(reason).toContainText("Settings → AI defaults");
|
||||
|
||||
await waitForAnimations(page);
|
||||
|
||||
const dialog = page.getByRole("dialog");
|
||||
await dialog.screenshot({
|
||||
@@ -243,11 +368,231 @@ test.describe("global agent config screenshots", () => {
|
||||
await expect(page.getByTestId("persona-dialog-submit")).toBeEnabled({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await settleAnimations(page);
|
||||
// The reason is null exactly when the form can submit — no footer reason.
|
||||
await expect(page.getByTestId("persona-dialog-submit-reason")).toHaveCount(
|
||||
0,
|
||||
);
|
||||
|
||||
await waitForAnimations(page);
|
||||
|
||||
const dialog = page.getByRole("dialog");
|
||||
await dialog.screenshot({
|
||||
path: `${SHOTS}/05-create-enabled-with-global-provider.png`,
|
||||
});
|
||||
});
|
||||
|
||||
// Shot 09: CLI-login runtime (Claude Code / Codex) drives its own provider,
|
||||
// so the provider picker is intentionally hidden. This is Ian's regression:
|
||||
// before the provider-aware gate, the hidden provider left the button
|
||||
// permanently disabled with no explanation. Now the provider is not required,
|
||||
// the button is enabled, and — critically — no spurious provider reason is
|
||||
// shown in the footer. Create and Save share this rendering path.
|
||||
test("09-cli-login-runtime-enabled-no-reason", async ({ page }) => {
|
||||
await installMockBridge(page, {
|
||||
acpRuntimesCatalog: CATALOG_WITH_CLAUDE,
|
||||
});
|
||||
|
||||
await openCreateDialog(page);
|
||||
|
||||
// Switch the auto-selected buzz-agent runtime to the CLI-login runtime.
|
||||
await selectDropdownOption(
|
||||
page,
|
||||
page.locator("#persona-runtime"),
|
||||
"Claude Code",
|
||||
);
|
||||
|
||||
// Provider picker hidden — the runtime drives its own provider.
|
||||
await expect(page.locator("#persona-llm-provider")).not.toBeVisible();
|
||||
// The hidden provider must not block submit, and must not surface a reason.
|
||||
await expect(page.getByTestId("persona-dialog-submit")).toBeEnabled({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(page.getByTestId("persona-dialog-submit-reason")).toHaveCount(
|
||||
0,
|
||||
);
|
||||
|
||||
await waitForAnimations(page);
|
||||
|
||||
const dialog = page.getByRole("dialog");
|
||||
await dialog.screenshot({
|
||||
path: `${SHOTS}/09-cli-login-runtime-enabled-no-reason.png`,
|
||||
});
|
||||
});
|
||||
|
||||
// Shot 10: the ORIGINAL defect — Ian's "Save button stays disabled after
|
||||
// editing an agent." This drives the real EDIT/Save path (not create): a
|
||||
// persona-linked Codex agent with an explicit custom model and no provider is
|
||||
// opened via the Agents view → profile → Edit affordance, which mounts
|
||||
// AgentDefinitionDialog in edit mode (id present in initialValues, "Save
|
||||
// changes" label). Before the provider-aware gate, the hidden Codex provider
|
||||
// left Save permanently disabled on a value the user could never set. Now:
|
||||
// provider picker hidden, Save enabled, and no submit-block reason. Create
|
||||
// and Save share this rendering path, but the defect was Save-specific, so
|
||||
// this exercises Save directly.
|
||||
test("10-edit-codex-custom-model-save-enabled-no-reason", async ({
|
||||
page,
|
||||
}) => {
|
||||
const PERSONA_ID = "persona-codex-edit-e2e";
|
||||
await installMockBridge(page, {
|
||||
acpRuntimesCatalog: CATALOG_WITH_CODEX,
|
||||
managedAgents: [
|
||||
{
|
||||
pubkey: TEST_IDENTITIES.tyler.pubkey,
|
||||
name: "Codex Editor",
|
||||
personaId: PERSONA_ID,
|
||||
status: "stopped",
|
||||
channelNames: ["agents"],
|
||||
},
|
||||
],
|
||||
personas: [
|
||||
{
|
||||
id: PERSONA_ID,
|
||||
displayName: "Codex Editor",
|
||||
systemPrompt: "You are the Codex edit-mode e2e persona.",
|
||||
// CLI-login runtime with an explicit custom model and NO provider —
|
||||
// the exact shape that used to pin Save disabled.
|
||||
runtime: "codex",
|
||||
model: "gpt-5-codex",
|
||||
provider: null,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
// Agents view → persona-grouped agent card → Edit quick action.
|
||||
await page.goto("/");
|
||||
await page.getByTestId("open-agents-view").click();
|
||||
const agentButton = page.getByRole("button", {
|
||||
name: "Codex Editor agent profile",
|
||||
});
|
||||
await expect(agentButton).toBeVisible({ timeout: 10_000 });
|
||||
await agentButton.click();
|
||||
await expect(page.getByTestId("user-profile-panel")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await page.getByTestId("user-profile-edit-agent").click();
|
||||
|
||||
// The definition dialog opens in EDIT mode ("Save changes"), seeded from
|
||||
// the persona — confirm it's the edit path, not create.
|
||||
await expect(page.getByTestId("persona-dialog")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(page.locator("#persona-display-name")).toHaveValue(
|
||||
"Codex Editor",
|
||||
);
|
||||
await expect(page.getByTestId("persona-dialog-submit")).toHaveText(
|
||||
/Save changes/,
|
||||
);
|
||||
|
||||
// The core assertions: Codex hides the provider picker, so the hidden
|
||||
// provider must NOT block Save and must NOT surface a reason.
|
||||
await expect(page.locator("#persona-llm-provider")).not.toBeVisible();
|
||||
await expect(page.getByTestId("persona-dialog-submit")).toBeEnabled({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(page.getByTestId("persona-dialog-submit-reason")).toHaveCount(
|
||||
0,
|
||||
);
|
||||
|
||||
await waitForAnimations(page);
|
||||
|
||||
const dialog = page.getByRole("dialog");
|
||||
await dialog.screenshot({
|
||||
path: `${SHOTS}/10-edit-codex-custom-model-save-enabled-no-reason.png`,
|
||||
});
|
||||
});
|
||||
|
||||
// Shot 11: the inverse of Ian's fix, and wesbillman's blocking review point.
|
||||
// A runtime-LESS legacy/builtin definition (no runtime, but a saved model)
|
||||
// still EXPOSES the provider picker via blankRuntimeModelProviderEditable, so
|
||||
// an empty provider must keep Save DISABLED. The gate must key off the field's
|
||||
// visibility (runtimeCanChooseLlmProvider), not the raw runtime capability —
|
||||
// otherwise Save persists `provider: undefined` despite the visible picker.
|
||||
// A global provider/model default keeps localMode satisfied, so the ONLY thing
|
||||
// that can block Save here is the Customize-pair provider gate (step 7), which
|
||||
// is exactly what this regression pins.
|
||||
test("11-edit-runtime-less-provider-required-save-blocked", async ({
|
||||
page,
|
||||
}) => {
|
||||
const PERSONA_ID = "persona-runtime-less-edit-e2e";
|
||||
await installMockBridge(page, {
|
||||
// No runtime is available, so getDefaultPersonaRuntime returns null and
|
||||
// the dialog does NOT auto-seed a runtime on open — the runtime-less
|
||||
// definition stays runtime-less, which is the only state where
|
||||
// blankRuntimeModelProviderEditable exposes the provider picker.
|
||||
acpRuntimesCatalog: CATALOG_NONE_AVAILABLE,
|
||||
// Global defaults satisfy localMode, so any block is the pair gate alone.
|
||||
globalAgentConfig: {
|
||||
provider: "anthropic",
|
||||
model: "claude-opus-4-5",
|
||||
env_vars: { ANTHROPIC_API_KEY: "sk-ant-global-value" },
|
||||
},
|
||||
managedAgents: [
|
||||
{
|
||||
pubkey: TEST_IDENTITIES.tyler.pubkey,
|
||||
name: "Legacy Editor",
|
||||
personaId: PERSONA_ID,
|
||||
status: "stopped",
|
||||
channelNames: ["agents"],
|
||||
},
|
||||
],
|
||||
personas: [
|
||||
{
|
||||
id: PERSONA_ID,
|
||||
displayName: "Legacy Editor",
|
||||
systemPrompt: "You are the runtime-less edit-mode e2e persona.",
|
||||
// Runtime-less definition with a saved model and NO provider — the
|
||||
// picker is editable-without-runtime, so the provider stays required.
|
||||
runtime: null,
|
||||
model: "claude-opus-4-5",
|
||||
provider: null,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
// Agents view → persona-grouped agent card → Edit quick action.
|
||||
await page.goto("/");
|
||||
await page.getByTestId("open-agents-view").click();
|
||||
const agentButton = page.getByRole("button", {
|
||||
name: "Legacy Editor agent profile",
|
||||
});
|
||||
await expect(agentButton).toBeVisible({ timeout: 10_000 });
|
||||
await agentButton.click();
|
||||
await expect(page.getByTestId("user-profile-panel")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await page.getByTestId("user-profile-edit-agent").click();
|
||||
|
||||
// Confirm the real EDIT dialog, seeded from the persona.
|
||||
await expect(page.getByTestId("persona-dialog")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(page.locator("#persona-display-name")).toHaveValue(
|
||||
"Legacy Editor",
|
||||
);
|
||||
await expect(page.getByTestId("persona-dialog-submit")).toHaveText(
|
||||
/Save changes/,
|
||||
);
|
||||
|
||||
// The provider picker IS visible (runtime-less editable definition) …
|
||||
await expect(page.locator("#persona-llm-provider")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
// … so the empty provider must block Save …
|
||||
await expect(page.getByTestId("persona-dialog-submit")).toBeDisabled({
|
||||
timeout: 10_000,
|
||||
});
|
||||
// … and the reason must be the Customize-pair provider gate, not the
|
||||
// global-defaults gate (which would say "Settings → AI defaults").
|
||||
const reason = page.getByTestId("persona-dialog-submit-reason");
|
||||
await expect(reason).toBeVisible({ timeout: 10_000 });
|
||||
await expect(reason).toContainText("Select a provider");
|
||||
await expect(reason).not.toContainText("Settings → AI defaults");
|
||||
|
||||
await waitForAnimations(page);
|
||||
|
||||
const dialog = page.getByRole("dialog");
|
||||
await dialog.screenshot({
|
||||
path: `${SHOTS}/11-edit-runtime-less-provider-required-save-blocked.png`,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -90,6 +90,18 @@ type MockPersonaSeed = {
|
||||
isActive?: boolean;
|
||||
sourceTeam?: string | null;
|
||||
envVars?: Record<string, string>;
|
||||
/**
|
||||
* Runtime the persona is pinned to (e.g. "goose", "codex", "claude"). Lets a
|
||||
* spec seed a CLI-login runtime whose provider picker is hidden, so the Edit
|
||||
* dialog's provider-aware submit gate can be driven end-to-end. Omitted →
|
||||
* null (definition inherits the app default at open).
|
||||
*/
|
||||
runtime?: string | null;
|
||||
/** Model pinned on the persona (a custom model id for Customize mode). */
|
||||
model?: string | null;
|
||||
/** Provider pinned on the persona. Leave empty for Codex/Claude runtimes. */
|
||||
provider?: string | null;
|
||||
namePool?: string[];
|
||||
};
|
||||
|
||||
type MockTeamSeed = {
|
||||
|
||||
Reference in New Issue
Block a user