Skip to content

Commit d76fa06

Browse files
committed
fix(vscode): address PR review findings
A skill-catalog failure rejected out of the slash parser, whose caller awaits it outside any try block, so every message starting with "/" was dropped. It now degrades to the skill prefix check, matching how an unlisted skill is handled. Selecting a different provider, or leaving the form, kept the API key already typed, so a key entered for one provider could be saved under another's id.
1 parent 580a174 commit d76fa06

3 files changed

Lines changed: 38 additions & 3 deletions

File tree

apps/vscode/src/handlers/slash-command.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,14 @@ export async function parseHostSlashCommand(
5959
const args = match[2]?.trim() ?? "";
6060
if (HOST_COMMANDS.has(name)) return { name, args, raw };
6161

62-
if (listSkills === undefined) {
62+
const skills = listSkills === undefined ? undefined : await listSkills().catch(() => undefined);
63+
if (skills === undefined) {
64+
// The parser runs on every message that starts with "/", so a catalog
65+
// failure must degrade to the prefix check rather than reject and take the
66+
// whole send down with it.
6367
return name.startsWith("skill:") ? { name, args, raw, skillName: name.slice(6) } : undefined;
6468
}
65-
const { commandMap } = buildSkillSlashCommands(await listSkills());
69+
const { commandMap } = buildSkillSlashCommands(skills);
6670
const skillName = commandMap.get(name) ?? commandMap.get(match[1]!);
6771
if (skillName !== undefined) return { name, args, raw, skillName };
6872
// A skill the catalog no longer lists still reaches the engine, which reports

apps/vscode/test/pythinker-harness.integration.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,26 @@ describe("VS Code Pythinker harness integration (shares one in-process SDK home)
386386
await expect(parseHostSlashCommand([{ type: "text", text: "/clear" }])).resolves.toBeUndefined();
387387
});
388388

389+
it("degrades to the skill prefix when the skill catalog fails", async () => {
390+
// The parser runs on every message starting with "/", and its caller in
391+
// chat.handler awaits it outside any try block — a rejection here silently
392+
// drops the user's message instead of sending it.
393+
const listSkills = () => Promise.reject(new Error("engine unavailable"));
394+
395+
await expect(parseHostSlashCommand("/skill:review carefully", listSkills)).resolves.toEqual({
396+
name: "skill:review",
397+
args: "carefully",
398+
raw: "/skill:review carefully",
399+
skillName: "review",
400+
});
401+
await expect(parseHostSlashCommand("/plan on", listSkills)).resolves.toEqual({
402+
name: "plan",
403+
args: "on",
404+
raw: "/plan on",
405+
});
406+
await expect(parseHostSlashCommand("/unknown-thing", listSkills)).resolves.toBeUndefined();
407+
});
408+
389409
it("resolves a built-in skill invoked under its bare name", async () => {
390410
const listSkills = async () => [
391411
{ name: "gen-changesets", description: "", path: "/s", source: "builtin", type: "prompt" },

apps/vscode/webview-ui/src/components/ProvidersModal.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,9 @@ function AddProviderPanel({
318318
setSelected(entry);
319319
setUseEnvVar(false);
320320
setDefaultModel(undefined);
321+
// A key typed for one provider must not be submittable
322+
// under another provider's id.
323+
setApiKey("");
321324
}}
322325
className="w-full flex items-center gap-2 px-2.5 py-1.5 rounded hover:bg-muted/60 text-left"
323326
>
@@ -408,7 +411,15 @@ function AddProviderPanel({
408411

409412
{selected !== undefined && (
410413
<div className="flex items-center justify-between gap-2 px-3 py-2 border-t">
411-
<Button variant="ghost" size="sm" className="h-6 text-xs" onClick={() => setSelected(undefined)}>
414+
<Button
415+
variant="ghost"
416+
size="sm"
417+
className="h-6 text-xs"
418+
onClick={() => {
419+
setSelected(undefined);
420+
setApiKey("");
421+
}}
422+
>
412423
Back
413424
</Button>
414425
<Button size="sm" className="h-6 text-xs" disabled={!canSubmit || saving} onClick={() => void submit()}>

0 commit comments

Comments
 (0)