Skip to content

Commit 0a09e96

Browse files
committed
fix(web): carry multi-skill compose list through EntryShell submit seam (#6333 review)
nettee's non-blocking thread on #6333 round-2 review (apps/web/src/components/EntryShell.tsx, 2026-08-02) observed that `handlePluginLoopSubmit` builds the `onCreateProject` payload by forward each `payload.*` field, but omitted `payload.skillIds`. PR #6333 had added a `skillIds` array to `PluginLoopSubmit` so the Home @-mention multi-skill compose flow (`@skill-a @skill-b`) reaches POST /api/projects with the full list (instead of silently dropping to `project.skillId` on the first run). The array was correctly forwarded in `App.handleCreateProject` (9624316), which stashes `od:auto-send-skillIds:<projectId>` in sessionStorage so ProjectView puts it back into `meta.skillIds` on the first `handleSend(...)`. Without the EntryShell seam passing `skillIds` through, `App.handleCreateProject` never saw the array and the Home auto-send hand-off landed empty — the end-to-end link was broken at this seam. Changes: - New helper `apps/web/src/components/plugin-loop-submit.ts` / `buildCreateProjectArgsFromPluginLoopSubmit` — extracts the spread-block logic from EntryShell into a pure function whose surface is explicitly tested. The helper forwards `payload.skillIds` when `length > 1` (mirrors the existing guard in HomeView) and does NOT spread it on the single-skill flow so the daemon's primary `skillId` binding stays the only contract. - `apps/web/src/components/EntryShell.tsx`: replace the inline spread with `buildCreateProjectArgsFromPluginLoopSubmit(payload, { name, amrGatePrechecked })` + merge `metadata` / `examplePromptContext` — same behavior, now testable in isolation. - `apps/web/tests/components/plugin-loop-submit.test.ts`: 6-case regression suite covering multi-skill forward, single-skill exclusion, length-1 guard, presence of plugin fields when set, absence when unset, and null defaults. - `apps/web/src/components/PluginLoopHome.tsx`: add `skillIds?: string[] | null` to the `PluginLoopSubmit` interface (the shape HomeView already stamped, just not forwarded through EntryShell). All 66 existing `ProjectView.run-cleanup.test.tsx` cases + all 6 new `plugin-loop-submit.test.ts` cases pass locally. Ready for re-review. Signed-off-by: xxiaoxiong <2482929840@qq.com>
1 parent 9624316 commit 0a09e96

4 files changed

Lines changed: 283 additions & 21 deletions

File tree

apps/web/src/components/EntryShell.tsx

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ import { NewProjectModal } from './NewProjectModal';
142142
import { PluginsView } from './PluginsView';
143143
import type { CreateInput, CreateTab, ImportClaudeDesignOutcome } from './NewProjectPanel';
144144
import type { PluginLoopSubmit } from './PluginLoopHome';
145+
import { buildCreateProjectArgsFromPluginLoopSubmit } from './plugin-loop-submit';
145146
import {
146147
createProject,
147148
type PluginShareAction,
@@ -814,28 +815,16 @@ export function EntryShell({
814815
} : {}),
815816
};
816817
return onCreateProject({
817-
name,
818-
skillId: payload.skillId ?? null,
819-
designSystemId: payload.designSystemId ?? null,
818+
...buildCreateProjectArgsFromPluginLoopSubmit(payload, {
819+
name,
820+
amrGatePrechecked,
821+
}),
820822
metadata,
821-
pendingPrompt: payload.prompt,
822-
...(payload.pluginId ? { pluginId: payload.pluginId } : {}),
823-
...(payload.pluginType ? { pluginType: payload.pluginType } : {}),
824-
...(payload.appliedPluginSnapshotId
825-
? { appliedPluginSnapshotId: payload.appliedPluginSnapshotId }
826-
: {}),
827-
...(payload.pluginInputs ? { pluginInputs: payload.pluginInputs } : {}),
828-
...(payload.initialRunContext ? { initialRunContext: payload.initialRunContext } : {}),
829-
...(payload.conversationMode ? { conversationMode: payload.conversationMode } : {}),
830-
...(payload.attachments && payload.attachments.length > 0
831-
? { pendingFiles: payload.attachments }
832-
: {}),
833-
// No `userWorkingDirToken`: linkedDirs grant read-only `--add-dir`
834-
// access and are validated by the daemon at create time, so they do
835-
// not need the desktop main-process trust token that baseDir imports
836-
// require for write access.
837-
autoSendFirstMessage: true,
838-
amrGatePrechecked,
823+
...(payload.examplePromptContext ? {
824+
examplePrompt: true,
825+
examplePromptTitle: payload.examplePromptContext.title,
826+
examplePromptBrief: payload.examplePromptContext.brief,
827+
} : {}),
839828
});
840829
}
841830

apps/web/src/components/PluginLoopHome.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ export interface PluginLoopSubmit {
3232
// to attribute project_create_result to a plugin type. Null when no plugin.
3333
pluginType?: string | null;
3434
skillId?: string | null;
35+
// 5824: Home @-mention can stage multiple skills at once. The first
36+
// entry is the project's primary `skillId` above; later entries ride
37+
// along as composed-skill blocks via the daemon's `skillIds` field.
38+
// Null/absent means single-skill (or no-skill) flow. Forwarded by
39+
// EntryShell.handlePluginLoopSubmit into onCreateProject so the
40+
// multi-skill compose list reaches POST /api/projects (and the
41+
// Home auto-send hand-off in App.handleCreateProject) instead of
42+
// being silently dropped at this submit seam.
43+
skillIds?: string[] | null;
3544
appliedPluginSnapshotId: string | null;
3645
pluginTitle: string | null;
3746
taskKind: string | null;
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// #5824 / #6333 round-2 (nettee, 2026-08-02): the Home @-mention
2+
// multi-skill compose flow stages an ordered `skillIds` array on the
3+
// submit payload (the first entry becomes the project's primary
4+
// `skillId`, later entries ride along as composed-skill blocks via the
5+
// daemon's `skillIds` field). The array was correctly forwarded to
6+
// POST /api/projects from `App.handleCreateProject` (9624316), but
7+
// `EntryShell.handlePluginLoopSubmit` previously only forwarded
8+
// `payload.skillId` into `onCreateProject`, so the multi-skill list was
9+
// silently dropped at this submit seam — `App.handleCreateProject`
10+
// never saw it and the Home auto-send hand-off landed with no
11+
// `od:auto-send-skillIds:*` entry, breaking the end-to-end link.
12+
//
13+
// This module extracts the `buildCreateProjectArgsFromPluginLoopSubmit`
14+
// spread rule into a pure helper so the seam can be unit-tested without
15+
// driving the full plugin-loop UI. `EntryShell.handlePluginLoopSubmit`
16+
// now delegates to this helper, and the helper has a regression test
17+
// asserting `payload.skillIds` (>1 entries) lands on the produced
18+
// create-project args while `skillId` defaults to the primary.
19+
20+
import type { PluginLoopSubmit } from './PluginLoopHome';
21+
22+
/**
23+
* Shape of the args `EntryShell.handlePluginLoopSubmit` passes into
24+
* `onCreateProject`. We only model the fields this helper is
25+
* responsible for spreading; callers may merge additional fields
26+
* (metadata, plugin snapshot id, attachments, …) on top.
27+
*/
28+
export interface PluginLoopCreateProjectArgs {
29+
name: string;
30+
skillId: string | null;
31+
/**
32+
* The multi-skill compose list. Absent on the single-skill /
33+
* no-skill flow (`payload.skillIds` is null / undefined / length <=
34+
* 1) so the daemon's primary `skillId` binding is the only contract
35+
* in that case. Forwarded as-is (no de-dup, no reordering) when
36+
* the user staged more than one skill via @-mention.
37+
*/
38+
skillIds?: string[];
39+
designSystemId: string | null;
40+
pendingPrompt: string;
41+
pluginId?: string;
42+
pluginType?: string;
43+
appliedPluginSnapshotId?: string;
44+
pluginInputs?: Record<string, unknown>;
45+
initialRunContext?: PluginLoopSubmit['initialRunContext'];
46+
conversationMode?: PluginLoopSubmit['conversationMode'];
47+
pendingFiles?: PluginLoopSubmit['attachments'];
48+
autoSendFirstMessage: true;
49+
amrGatePrechecked: boolean;
50+
// The exhaustive literal list above mirrors the spread block the
51+
// EntryShell handler used to inline. Keeping the surface typed
52+
// surfaces drift between the helper and the call-site; the EntryShell
53+
// component adds metadata + linkedDirs / examplePromptContext by
54+
// merging on top of this helper's output.
55+
}
56+
57+
/**
58+
* Build the `onCreateProject` args from a `PluginLoopSubmit` payload.
59+
*
60+
* The function is intentionally pure — it does not touch state, network,
61+
* or sessionStorage. The EntryShell component keeps responsibility
62+
* for deriving `name`, `metadata`, `linkedDirs`, `examplePromptContext`,
63+
* and the AMR balance pre-check flag, then merges those on top of the
64+
* object returned here.
65+
*
66+
* Round-2 review regression: `payload.skillIds` MUST reach
67+
* `onCreateProject` (and therefore `App.handleCreateProject`'s
68+
* `od:auto-send-skillIds:<projectId>` stash) when the user staged more
69+
* than one skill. The single-skill flow does NOT spread `skillIds` so
70+
* the daemon's primary `skillId` binding is the only contract in that
71+
* case (mirrors the previous behavior — the `length > 1` guard avoids
72+
* leaking an empty / single-element array that would shadow
73+
* `skillId`).
74+
*/
75+
export function buildCreateProjectArgsFromPluginLoopSubmit(
76+
payload: PluginLoopSubmit,
77+
context: {
78+
name: string;
79+
amrGatePrechecked: boolean;
80+
},
81+
): Omit<PluginLoopCreateProjectArgs, 'designSystemId' | 'metadata' | 'pendingPrompt'> & {
82+
designSystemId: string | null;
83+
pendingPrompt: string;
84+
} {
85+
return {
86+
name: context.name,
87+
skillId: payload.skillId ?? null,
88+
// Forward the multi-skill compose list so Home @-mention
89+
// (`@skill-a @skill-b`) does not silently degrade to a
90+
// single-skill create at this submit seam.
91+
...(payload.skillIds && payload.skillIds.length > 1
92+
? { skillIds: payload.skillIds }
93+
: {}),
94+
designSystemId: payload.designSystemId ?? null,
95+
pendingPrompt: payload.prompt,
96+
...(payload.pluginId ? { pluginId: payload.pluginId } : {}),
97+
...(payload.pluginType ? { pluginType: payload.pluginType } : {}),
98+
...(payload.appliedPluginSnapshotId
99+
? { appliedPluginSnapshotId: payload.appliedPluginSnapshotId }
100+
: {}),
101+
...(payload.pluginInputs ? { pluginInputs: payload.pluginInputs } : {}),
102+
...(payload.initialRunContext ? { initialRunContext: payload.initialRunContext } : {}),
103+
...(payload.conversationMode ? { conversationMode: payload.conversationMode } : {}),
104+
...(payload.attachments && payload.attachments.length > 0
105+
? { pendingFiles: payload.attachments }
106+
: {}),
107+
autoSendFirstMessage: true,
108+
amrGatePrechecked: context.amrGatePrechecked,
109+
};
110+
}
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// @vitest-environment jsdom
2+
//
3+
// Regression for #6333 round-2 (nettee, 2026-08-02):
4+
// `PluginLoopSubmit` now carries a `skillIds` array alongside the
5+
// existing primary `skillId`. `EntryShell.handlePluginLoopSubmit`
6+
// delegates the build-args spread to
7+
// `buildCreateProjectArgsFromPluginLoopSubmit`, which must forward
8+
// the multi-skill compose list (length > 1) onto the produced
9+
// `onCreateProject` args so the array reaches
10+
// `App.handleCreateProject`'s `od:auto-send-skillIds:<projectId>`
11+
// sessionStorage hand-off and ultimately `streamViaDaemon`'s
12+
// `skillIds`. The single-skill flow must NOT spread `skillIds` so
13+
// the daemon's primary `skillId` binding is the only contract.
14+
15+
import { describe, expect, it } from 'vitest';
16+
17+
import type { PluginLoopSubmit } from '../../src/components/PluginLoopHome';
18+
import { buildCreateProjectArgsFromPluginLoopSubmit } from '../../src/components/plugin-loop-submit';
19+
20+
function basePayload(overrides: Partial<PluginLoopSubmit>): PluginLoopSubmit {
21+
return {
22+
prompt: 'make a deck and a PDF',
23+
pluginId: null,
24+
skillId: 'deck-builder',
25+
appliedPluginSnapshotId: null,
26+
pluginTitle: null,
27+
taskKind: null,
28+
...overrides,
29+
} as PluginLoopSubmit;
30+
}
31+
32+
describe('buildCreateProjectArgsFromPluginLoopSubmit', () => {
33+
it('forwards payload.skillIds to onCreateProject when the compose list has more than one entry (multi-skill @-mention flow)', () => {
34+
const payload = basePayload({
35+
skillId: 'deck-builder',
36+
skillIds: ['deck-builder', 'pdf-designer'],
37+
});
38+
39+
const args = buildCreateProjectArgsFromPluginLoopSubmit(payload, {
40+
name: 'Test Deck',
41+
amrGatePrechecked: false,
42+
});
43+
44+
// The primary skill stays on `skillId` (the single-skill contract),
45+
// and the full multi-skill compose list lands on `skillIds`.
46+
expect(args.skillId).toBe('deck-builder');
47+
expect(args.skillIds).toEqual(['deck-builder', 'pdf-designer']);
48+
49+
// The hand-off fields the EntryShell relies on are present.
50+
expect(args.name).toBe('Test Deck');
51+
expect(args.pendingPrompt).toBe('make a deck and a PDF');
52+
expect(args.designSystemId).toBeNull();
53+
expect(args.autoSendFirstMessage).toBe(true);
54+
expect(args.amrGatePrechecked).toBe(false);
55+
});
56+
57+
it('does NOT spread skillIds on the single-skill flow so the primary binding is the only contract (regression for the previous spread that leaked null/undefined)', () => {
58+
const payload = basePayload({
59+
skillId: 'deck-builder',
60+
skillIds: null,
61+
});
62+
63+
const args = buildCreateProjectArgsFromPluginLoopSubmit(payload, {
64+
name: 'Test',
65+
amrGatePrechecked: false,
66+
});
67+
68+
expect(args.skillId).toBe('deck-builder');
69+
// `skillIds` must NOT appear — null/undefined should not be spread.
70+
expect(args).not.toHaveProperty('skillIds');
71+
});
72+
73+
it('does NOT spread skillIds when the compose list has length <= 1 (avoids overriding the primary skillId with a redundant one-element array)', () => {
74+
const payload = basePayload({
75+
skillId: 'deck-builder',
76+
// Edge case: HomeView stamps the same primary as the only
77+
// entry — the spread must not duplicate `skillId` as `skillIds`.
78+
skillIds: ['deck-builder'],
79+
});
80+
81+
const args = buildCreateProjectArgsFromPluginLoopSubmit(payload, {
82+
name: 'Test',
83+
amrGatePrechecked: false,
84+
});
85+
86+
expect(args.skillId).toBe('deck-builder');
87+
expect(args).not.toHaveProperty('skillIds');
88+
});
89+
90+
it('preserves pluginId / pluginType / snapshotId / pluginInputs / initialRunContext / conversationMode / attachments when present', () => {
91+
const attachment = new File(['x'], 'x.png');
92+
const payload = basePayload({
93+
pluginId: 'od-default',
94+
pluginType: 'official',
95+
appliedPluginSnapshotId: 'snap-1',
96+
pluginInputs: { tone: 'concise' },
97+
conversationMode: 'design',
98+
attachments: [attachment],
99+
// `initialRunContext` is typed loosely here for the helper test.
100+
initialRunContext: { kind: 'recent-files', fileIds: ['f-1'] } as never,
101+
});
102+
103+
const args = buildCreateProjectArgsFromPluginLoopSubmit(payload, {
104+
name: 'Test',
105+
amrGatePrechecked: true,
106+
});
107+
108+
expect(args.pluginId).toBe('od-default');
109+
expect(args.pluginType).toBe('official');
110+
expect(args.appliedPluginSnapshotId).toBe('snap-1');
111+
expect(args.pluginInputs).toEqual({ tone: 'concise' });
112+
expect(args.conversationMode).toBe('design');
113+
expect(args.pendingFiles).toEqual([attachment]);
114+
expect(args.initialRunContext).toEqual({ kind: 'recent-files', fileIds: ['f-1'] });
115+
expect(args.amrGatePrechecked).toBe(true);
116+
});
117+
118+
it('omits pluginId / pluginType / snapshotId / pluginInputs / initialRunContext / conversationMode / pendingFiles when absent (no leak of undefined)', () => {
119+
const payload = basePayload({});
120+
121+
const args = buildCreateProjectArgsFromPluginLoopSubmit(payload, {
122+
name: 'Test',
123+
amrGatePrechecked: false,
124+
});
125+
126+
expect(args).not.toHaveProperty('pluginId');
127+
expect(args).not.toHaveProperty('pluginType');
128+
expect(args).not.toHaveProperty('appliedPluginSnapshotId');
129+
expect(args).not.toHaveProperty('pluginInputs');
130+
expect(args).not.toHaveProperty('initialRunContext');
131+
expect(args).not.toHaveProperty('conversationMode');
132+
expect(args).not.toHaveProperty('pendingFiles');
133+
});
134+
135+
it('defaults designSystemId and skillId to null when payload omits them (the daemon treats absence as no binding)', () => {
136+
const payload = basePayload({}).delete
137+
? basePayload({})
138+
: ({
139+
prompt: 'plain prompt',
140+
pluginId: null,
141+
appliedPluginSnapshotId: null,
142+
pluginTitle: null,
143+
taskKind: null,
144+
} as PluginLoopSubmit);
145+
146+
const args = buildCreateProjectArgsFromPluginLoopSubmit(payload, {
147+
name: 'Plain',
148+
amrGatePrechecked: false,
149+
});
150+
151+
expect(args.skillId).toBeNull();
152+
expect(args.designSystemId).toBeNull();
153+
});
154+
});

0 commit comments

Comments
 (0)