mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-05 20:38:41 +00:00
feat(frontend): reopen the skill list after a skill is selected (#4639)
* feat(frontend): reopen the skill list after a skill is selected Selecting a skill closed the composer's skill list for good: `/` no longer reopened it, so a skill could not be looked up or swapped without deleting the chip first. The list now reopens from the editable text beside the chip, and picking an entry swaps the chip rather than stacking a second activation, since the wire format carries exactly one leading /skill. Builtin commands are withheld in that state because they own the whole composer line, and Enter navigates the list before submitting except while an IME is composing. The trigger is unchanged: a slash still opens the list only at the start of the input. * fix(frontend): keep builtin names reserved in the reopened skill list Withholding the builtin list from getMatchingSkillSuggestions in chip mode also disabled the reserved-name filter it drives, so a custom skill named after a builtin command became selectable there. Nothing rejects such a name at install time, and submitting the resulting chip runs the command instead of the skill. Pass the builtin list as before and drop the builtin entries from the result instead. The new regression covers both sides of the reservation, and the reopen test now waits for the list before pressing Enter. * fix(frontend): hide skills the slash parsers refuse from the picker The composer picker reserved only the two builtin command names, while both slash parsers refuse the seven names in the shared contract. A skill named bootstrap, help, memory, models, new or status was therefore offered, could be selected into a chip, and submitted — and then activated nothing, because parse_slash_skill_reference drops the name on the way in. The turn reached the model as literal text with no skill loaded and no error anywhere. Reserve the contract names alongside the builtin ones, so the picker cannot offer what the parsers will not honour.
This commit is contained in:
parent
78f7a5f7c0
commit
da282811b8
@ -97,6 +97,8 @@ Auth UI note: the login page's "keep me signed in" option submits only `remember
|
||||
|
||||
`/goal` and `/compact` are built-in composer commands, not skill activations. `src/components/workspace/input-box.tsx` intercepts `/goal`, `/goal clear`, and `/goal <condition>` before normal chat submission, calling Gateway `GET/PUT/DELETE /api/threads/{thread_id}/goal`. Setting `/goal <condition>` also submits the condition text as the next user task so the agent starts running immediately; status and clear do not start a run. Goal and compact requests are tied to the current `threadId` with an `AbortController`, so switching threads or unmounting the composer aborts in-flight requests and stale responses cannot update the new thread's composer state. The chat pages render `GoalStatus` above the composer from `AgentThreadState.goal`, with local optimistic state until the next stream `values` update arrives. `/compact` calls `POST /api/threads/{thread_id}/compact` to summarize older active context while leaving the full visible chat history intact; it is skipped on new/empty threads and blocked server-side while a run is in flight. Thread rename uses the same serialized state-write route; the rename dialog stays open and surfaces the server error when an active run returns 409.
|
||||
|
||||
The `/` skill list stays reachable after a skill is selected: typing `/` in the editable text beside the chip reopens it, and picking an entry swaps the chip rather than adding a second one, because the wire format carries exactly one leading `/skill`. That list offers skills only while a chip is selected — a builtin command owns the whole composer line, so `/goal` behind a selected skill would submit as chat text instead of running the command. The trigger itself is unchanged: a slash only opens the list at the start of the input (`getLeadingSlashSkillQuery`), pinned by `tests/e2e/chat.spec.ts`.
|
||||
|
||||
Human input requests are a structured message protocol layered on normal chat history. The backend writes request payloads to `ToolMessage.artifact.human_input`, `src/core/messages/human-input.ts` owns the runtime validators/types, and `src/components/workspace/messages/human-input-card.tsx` renders the reusable card. The protocol is versioned on the request side only: v1 covers `free_text` / `choice_with_other`, and v2 adds `form` (typed fields — text/textarea/number/select/multi_select/checkbox/date — with required-field validation in the card). Replies deliberately stay on the v1 response protocol: the form card submits a `response_kind: "text"` reply whose value is the human-readable summary plus one JSON block keyed by stable field names (`buildHumanInputFormSubmissionValue` — the readable part alone is ambiguous because labels/values may contain the separators), so the model can reconstruct the submitted mapping without a structured response kind. The validators reject unknown versions/modes (and field names colliding with JS `Object.prototype` members) so future protocol bumps degrade to the plain-text ToolMessage fallback rather than rendering a broken card. Form values are read through own-property access only (`readHumanInputFormValue`); select fields stay controlled from their empty-string placeholder state through selection; checkbox fields are native `<input type="checkbox">` controls seeded to an explicit `false` (`buildInitialHumanInputFormValues`) so an untouched checkbox submits as "no" while a `required` checkbox keeps must-agree semantics (no HTML `required` attribute — native constraint validation would intercept the custom submit path), and form controls carry label/`htmlFor`, `aria-required` plus a visually-hidden localized "required" marker, and `aria-invalid`/error associations whose error node stays mounted while any field is still invalid. Composer-bypass closure: `deriveHumanInputThreadState` treats a visible plain human message as answering the latest unanswered request opened before it (only the latest — nothing guarantees a single outstanding request across runs, and closing all would silently swallow older decisions; an older request left open simply becomes the active card again). This lets current users bypass a structured form through the normal composer and preserves compatibility with old v1-only frontends that degrade a v2 request to plain text. `MessageList` owns answered/latest/pending state for visible cards, but derives answered responses from raw `thread.messages` because replies are hidden; pending cards clear when the hidden reply appears, when dispatch is dropped, or when a new `thread.error` reports an async stream failure. Page-level card submit callbacks must send a normal human message and put `hide_from_ui: true` plus the response payload in the fourth `sendMessage(..., options)` argument as `options.additionalKwargs`; the third argument remains run context such as `{ agent_name }`. Composer entry points remain enabled while a human-input request is open; a normal visible message intentionally bypasses the card and starts the next run without structured response metadata.
|
||||
|
||||
Tool-calling AI messages can contain user-visible text as well as `tool_calls`. `core/messages/utils.ts` keeps these turns in an `assistant:processing` group, and `components/workspace/messages/message-group.tsx` must render the visible text as a processing step instead of treating the message as only tool metadata. This preserves provider text such as error explanations or "trying another approach" notes during tool-heavy runs.
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
import type { Skill } from "@/core/skills";
|
||||
import { RESERVED_SLASH_SKILL_NAMES, type Skill } from "@/core/skills";
|
||||
export {
|
||||
SUGGESTION_TEMPLATE_PLACEHOLDER_PATTERN,
|
||||
findSuggestionTemplatePlaceholder,
|
||||
@ -167,9 +167,15 @@ export function getMatchingSkillSuggestions(
|
||||
builtinCommands: SlashSuggestion[],
|
||||
): SlashSuggestion[] {
|
||||
const normalizedQuery = query.toLowerCase();
|
||||
const builtinCommandNames = new Set(
|
||||
builtinCommands.map(({ name }) => name.toLowerCase()),
|
||||
);
|
||||
// A name the slash parsers refuse must not be offered here either. Both
|
||||
// parsers drop `RESERVED_SLASH_SKILL_NAMES` (the shared contract), and the
|
||||
// builtin commands own their own names in the composer, so a skill carrying
|
||||
// either one is unreachable: submitting it either runs the command or
|
||||
// reaches the model as literal text with nothing activated.
|
||||
const reservedNames = new Set([
|
||||
...RESERVED_SLASH_SKILL_NAMES,
|
||||
...builtinCommands.map(({ name }) => name.toLowerCase()),
|
||||
]);
|
||||
|
||||
const builtinMatches = builtinCommands.filter(({ name, description }) => {
|
||||
if (!normalizedQuery) {
|
||||
@ -191,7 +197,7 @@ export function getMatchingSkillSuggestions(
|
||||
if (!skill.enabled) {
|
||||
return false;
|
||||
}
|
||||
if (builtinCommandNames.has(name)) {
|
||||
if (reservedNames.has(name)) {
|
||||
return false;
|
||||
}
|
||||
return !normalizedQuery || name.includes(normalizedQuery);
|
||||
|
||||
@ -1296,21 +1296,30 @@ export function InputBox({
|
||||
() => getGoalObjectiveCounter(textInput.value ?? ""),
|
||||
[textInput.value],
|
||||
);
|
||||
const skillSuggestions = useMemo(
|
||||
() =>
|
||||
slashSkillQuery === null
|
||||
? []
|
||||
: getMatchingSkillSuggestions(
|
||||
skills,
|
||||
slashSkillQuery,
|
||||
builtinSlashCommands,
|
||||
),
|
||||
[builtinSlashCommands, skills, slashSkillQuery],
|
||||
);
|
||||
const skillSuggestions = useMemo(() => {
|
||||
if (slashSkillQuery === null) {
|
||||
return [];
|
||||
}
|
||||
const matches = getMatchingSkillSuggestions(
|
||||
skills,
|
||||
slashSkillQuery,
|
||||
builtinSlashCommands,
|
||||
);
|
||||
// Builtin commands own the whole composer line, so they cannot be combined
|
||||
// with a skill activation: `/goal` behind a selected skill would submit as
|
||||
// chat text instead of running the command. Drop them from the result
|
||||
// rather than withholding them from the helper, which needs the list to
|
||||
// reserve their names — a skill named after a builtin is unusable for the
|
||||
// mirrored reason, its submitted text runs the command, not the skill.
|
||||
return selectedSlashSkill
|
||||
? matches.filter(({ kind }) => kind === "skill")
|
||||
: matches;
|
||||
}, [builtinSlashCommands, selectedSlashSkill, skills, slashSkillQuery]);
|
||||
// A selected skill does not close the catalog: `/` reopens it so a skill can
|
||||
// be found by browsing and swapped without first clearing the chip.
|
||||
const showSkillSuggestions =
|
||||
!disabled &&
|
||||
textareaFocused &&
|
||||
!selectedSlashSkill &&
|
||||
slashSkillQuery !== null &&
|
||||
skillSuggestions.length > 0 &&
|
||||
dismissedSkillSuggestionValue !== textInput.value;
|
||||
@ -1896,6 +1905,16 @@ export function InputBox({
|
||||
|
||||
const handleInlineSkillKeyDown = useCallback(
|
||||
(event: KeyboardEvent<HTMLSpanElement>) => {
|
||||
// The catalog can be reopened from here, so its navigation keys must win
|
||||
// over Enter-to-submit. Skip it mid-composition, where Enter belongs to
|
||||
// the IME candidate rather than the list.
|
||||
if (!isIMEComposing(event, inlineSkillComposingRef.current)) {
|
||||
handleSkillSuggestionKeyDown(event);
|
||||
if (event.defaultPrevented) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
handleSelectedSlashSkillKeyDown(event);
|
||||
if (event.defaultPrevented) {
|
||||
return;
|
||||
@ -1920,7 +1939,11 @@ export function InputBox({
|
||||
|
||||
event.currentTarget.closest("form")?.requestSubmit();
|
||||
},
|
||||
[handleSelectedSlashSkillKeyDown, updateInlineSkillTextInput],
|
||||
[
|
||||
handleSelectedSlashSkillKeyDown,
|
||||
handleSkillSuggestionKeyDown,
|
||||
updateInlineSkillTextInput,
|
||||
],
|
||||
);
|
||||
|
||||
const clearSelectedSlashSkill = useCallback(() => {
|
||||
|
||||
@ -460,6 +460,136 @@ test.describe("Chat workspace", () => {
|
||||
.toBe("/data-analysis summarize this dataset");
|
||||
});
|
||||
|
||||
test("reopens the skill list with a slash after a skill is selected", async ({
|
||||
page,
|
||||
}) => {
|
||||
let submittedText: string | undefined;
|
||||
await page.route("**/runs/stream", (route) => {
|
||||
const body = route.request().postDataJSON() as {
|
||||
input?: { messages?: Array<{ content?: unknown }> };
|
||||
};
|
||||
submittedText = textFromMessageContent(
|
||||
body.input?.messages?.at(-1)?.content,
|
||||
);
|
||||
return handleRunStream(route);
|
||||
});
|
||||
|
||||
await page.goto("/workspace/chats/new");
|
||||
|
||||
const textarea = page.getByPlaceholder(/how can i assist you/i);
|
||||
await expect(textarea).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
await textarea.fill("/dat");
|
||||
await expect(
|
||||
page.getByRole("option", { name: /data-analysis/i }),
|
||||
).toBeVisible();
|
||||
await textarea.press("Enter");
|
||||
await expect(page.getByText("/data-analysis")).toBeVisible();
|
||||
|
||||
const skillInput = page.getByRole("textbox", {
|
||||
name: /how can i assist you/i,
|
||||
});
|
||||
await expect(skillInput).toBeVisible();
|
||||
|
||||
await skillInput.pressSequentially("/");
|
||||
|
||||
const dataAnalysis = page.getByRole("option", { name: /data-analysis/i });
|
||||
const frontendDesign = page.getByRole("option", {
|
||||
name: /frontend-design/i,
|
||||
});
|
||||
await expect(dataAnalysis).toBeVisible();
|
||||
await expect(frontendDesign).toBeVisible();
|
||||
// Builtin commands own the whole composer line, so they stay out of the
|
||||
// list while a skill is selected even though an empty query matches them.
|
||||
await expect(page.getByRole("option", { name: /goal/i })).toBeHidden();
|
||||
|
||||
await skillInput.pressSequentially("fro");
|
||||
await expect(frontendDesign).toHaveAttribute("aria-selected", "true");
|
||||
|
||||
await skillInput.press("Enter");
|
||||
|
||||
await expect(page.getByText("/frontend-design")).toBeVisible();
|
||||
await expect(page.getByText("/data-analysis")).toBeHidden();
|
||||
|
||||
await skillInput.pressSequentially("polish the composer");
|
||||
await skillInput.press("Enter");
|
||||
|
||||
await expect
|
||||
.poll(() => submittedText)
|
||||
.toBe("/frontend-design polish the composer");
|
||||
});
|
||||
|
||||
test("does not offer a skill whose name a slash command owns", async ({
|
||||
page,
|
||||
}) => {
|
||||
// Registered after the shared mock, so it wins: nothing rejects these
|
||||
// names when the skill is created.
|
||||
await page.route("**/api/skills", (route) =>
|
||||
route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({
|
||||
skills: [
|
||||
{
|
||||
name: "data-analysis",
|
||||
description: "Analyze structured data and produce charts.",
|
||||
category: "public",
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
name: "compact",
|
||||
description: "A custom skill named after a builtin command.",
|
||||
category: "custom",
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
name: "status",
|
||||
description: "A custom skill named after a reserved command.",
|
||||
category: "custom",
|
||||
enabled: true,
|
||||
},
|
||||
],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
await page.goto("/workspace/chats/new");
|
||||
|
||||
const textarea = page.getByPlaceholder(/how can i assist you/i);
|
||||
await expect(textarea).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
await textarea.fill("/comp");
|
||||
// Reserved outside chip mode: the builtin is offered, the skill is not.
|
||||
await expect(
|
||||
page.getByRole("option", { name: /compact/i }),
|
||||
).toHaveAccessibleName(/Compact earlier context/i);
|
||||
|
||||
// A contract-reserved name has no builtin standing in for it, so the list
|
||||
// is empty rather than showing a skill both slash parsers would refuse.
|
||||
await textarea.fill("/stat");
|
||||
await expect(page.getByRole("option", { name: /status/i })).toBeHidden();
|
||||
|
||||
await textarea.fill("/dat");
|
||||
await expect(
|
||||
page.getByRole("option", { name: /data-analysis/i }),
|
||||
).toBeVisible();
|
||||
await textarea.press("Enter");
|
||||
await expect(page.getByText("/data-analysis")).toBeVisible();
|
||||
|
||||
const skillInput = page.getByRole("textbox", {
|
||||
name: /how can i assist you/i,
|
||||
});
|
||||
await skillInput.pressSequentially("/comp");
|
||||
|
||||
// Reserved in chip mode too. Selecting it would set a `/compact` chip that
|
||||
// `parseCompactCommand` intercepts on submit, so context compaction would
|
||||
// run instead of the skill.
|
||||
await expect(page.getByRole("option", { name: /compact/i })).toBeHidden();
|
||||
|
||||
await skillInput.fill("/stat");
|
||||
await expect(page.getByRole("option", { name: /status/i })).toBeHidden();
|
||||
});
|
||||
|
||||
test("goal command sets a goal and starts an agent run", async ({ page }) => {
|
||||
let streamCalls = 0;
|
||||
await page.goto("/workspace/chats/new");
|
||||
|
||||
@ -21,7 +21,7 @@ import {
|
||||
readGoalResponseError,
|
||||
type SlashSuggestion,
|
||||
} from "@/components/workspace/input-box-helpers";
|
||||
import type { Skill } from "@/core/skills";
|
||||
import { RESERVED_SLASH_SKILL_NAMES, type Skill } from "@/core/skills";
|
||||
|
||||
function makeSkill(name: string, enabled = true): Skill {
|
||||
return {
|
||||
@ -333,6 +333,29 @@ describe("getMatchingSkillSuggestions", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("excludes skills that collide with a reserved slash name", () => {
|
||||
// The picker must not offer what the slash parsers refuse. Both sides drop
|
||||
// these names, so such a skill can never activate — picking it would send
|
||||
// literal text to the model with nothing loaded.
|
||||
for (const reserved of RESERVED_SLASH_SKILL_NAMES) {
|
||||
const result = getMatchingSkillSuggestions(
|
||||
[makeSkill(reserved), makeSkill(`${reserved}-helper`)],
|
||||
reserved,
|
||||
builtins,
|
||||
);
|
||||
|
||||
expect(
|
||||
result.filter((s) => s.kind === "skill").map((s) => s.name),
|
||||
).toEqual([`${reserved}-helper`]);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps reserved names out even when no builtin commands are passed", () => {
|
||||
const result = getMatchingSkillSuggestions([makeSkill("status")], "", []);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it("caps the number of suggestions", () => {
|
||||
const skills = Array.from({ length: 10 }, (_, i) =>
|
||||
makeSkill(`skill-${i}`),
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user