Skip to content

Commit 57c61af

Browse files
anandgupta42claude
andcommitted
fix: make command loading resilient to MCP/Skill failures
Two issues could cause /discover and other server commands to not show up in the TUI: 1. In command/index.ts, if MCP.prompts() or Skill.all() threw during command state initialization, the entire state promise rejected — all default commands (init, discover, review) were lost. Now each is wrapped in try/catch so defaults are always served. 2. In sync.tsx, the non-blocking Promise.all for loading commands, LSP, MCP status etc. was fire-and-forget with no error handler. If any request failed, errors were silently swallowed and setStore("status", "complete") never ran. Now each request catches errors individually and logs a warning, allowing the others to succeed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent d8abf88 commit 57c61af

4 files changed

Lines changed: 446 additions & 47 deletions

File tree

packages/altimate-code/src/cli/cmd/tui/context/sync.tsx

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -399,20 +399,26 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({
399399
})
400400
.then(() => {
401401
if (store.status !== "complete") setStore("status", "partial")
402-
// non-blocking
402+
// non-blocking — each request catches errors individually so one
403+
// failure doesn't prevent the others from populating the store.
404+
const safe = <T,>(p: Promise<T>) => p.catch((e) => {
405+
Log.Default.warn("non-blocking sync request failed", {
406+
error: e instanceof Error ? e.message : String(e),
407+
})
408+
})
403409
Promise.all([
404-
...(args.continue ? [] : [sessionListPromise.then((sessions) => setStore("session", reconcile(sessions)))]),
405-
sdk.client.command.list().then((x) => setStore("command", reconcile(x.data ?? []))),
406-
sdk.client.lsp.status().then((x) => setStore("lsp", reconcile(x.data!))),
407-
sdk.client.mcp.status().then((x) => setStore("mcp", reconcile(x.data!))),
408-
sdk.client.experimental.resource.list().then((x) => setStore("mcp_resource", reconcile(x.data ?? {}))),
409-
sdk.client.formatter.status().then((x) => setStore("formatter", reconcile(x.data!))),
410-
sdk.client.session.status().then((x) => {
410+
...(args.continue ? [] : [safe(sessionListPromise.then((sessions) => setStore("session", reconcile(sessions))))]),
411+
safe(sdk.client.command.list().then((x) => setStore("command", reconcile(x.data ?? [])))),
412+
safe(sdk.client.lsp.status().then((x) => setStore("lsp", reconcile(x.data!)))),
413+
safe(sdk.client.mcp.status().then((x) => setStore("mcp", reconcile(x.data!)))),
414+
safe(sdk.client.experimental.resource.list().then((x) => setStore("mcp_resource", reconcile(x.data ?? {})))),
415+
safe(sdk.client.formatter.status().then((x) => setStore("formatter", reconcile(x.data!)))),
416+
safe(sdk.client.session.status().then((x) => {
411417
setStore("session_status", reconcile(x.data!))
412-
}),
413-
sdk.client.provider.auth().then((x) => setStore("provider_auth", reconcile(x.data ?? {}))),
414-
sdk.client.vcs.get().then((x) => setStore("vcs", reconcile(x.data))),
415-
sdk.client.path.get().then((x) => setStore("path", reconcile(x.data!))),
418+
})),
419+
safe(sdk.client.provider.auth().then((x) => setStore("provider_auth", reconcile(x.data ?? {})))),
420+
safe(sdk.client.vcs.get().then((x) => setStore("vcs", reconcile(x.data)))),
421+
safe(sdk.client.path.get().then((x) => setStore("path", reconcile(x.data!)))),
416422
]).then(() => {
417423
setStore("status", "complete")
418424
})

packages/altimate-code/src/command/index.ts

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -106,46 +106,56 @@ export namespace Command {
106106
hints: hints(command.template),
107107
}
108108
}
109-
for (const [name, prompt] of Object.entries(await MCP.prompts())) {
110-
result[name] = {
111-
name,
112-
source: "mcp",
113-
description: prompt.description,
114-
get template() {
115-
// since a getter can't be async we need to manually return a promise here
116-
return new Promise<string>(async (resolve, reject) => {
117-
const template = await MCP.getPrompt(
118-
prompt.client,
119-
prompt.name,
120-
prompt.arguments
121-
? // substitute each argument with $1, $2, etc.
122-
Object.fromEntries(prompt.arguments?.map((argument, i) => [argument.name, `$${i + 1}`]))
123-
: {},
124-
).catch(reject)
125-
resolve(
126-
template?.messages
127-
.map((message) => (message.content.type === "text" ? message.content.text : ""))
128-
.join("\n") || "",
129-
)
130-
})
131-
},
132-
hints: prompt.arguments?.map((_, i) => `$${i + 1}`) ?? [],
109+
// MCP and skill loading must not prevent default commands from being served.
110+
// Wrap each in try/catch so init, discover, review are always available.
111+
try {
112+
for (const [name, prompt] of Object.entries(await MCP.prompts())) {
113+
result[name] = {
114+
name,
115+
source: "mcp",
116+
description: prompt.description,
117+
get template() {
118+
// since a getter can't be async we need to manually return a promise here
119+
return new Promise<string>(async (resolve, reject) => {
120+
const template = await MCP.getPrompt(
121+
prompt.client,
122+
prompt.name,
123+
prompt.arguments
124+
? // substitute each argument with $1, $2, etc.
125+
Object.fromEntries(prompt.arguments?.map((argument, i) => [argument.name, `$${i + 1}`]))
126+
: {},
127+
).catch(reject)
128+
resolve(
129+
template?.messages
130+
.map((message) => (message.content.type === "text" ? message.content.text : ""))
131+
.join("\n") || "",
132+
)
133+
})
134+
},
135+
hints: prompt.arguments?.map((_, i) => `$${i + 1}`) ?? [],
136+
}
133137
}
138+
} catch {
139+
// MCP prompt loading failed — continue with default commands
134140
}
135141

136142
// Add skills as invokable commands
137-
for (const skill of await Skill.all()) {
138-
// Skip if a command with this name already exists
139-
if (result[skill.name]) continue
140-
result[skill.name] = {
141-
name: skill.name,
142-
description: skill.description,
143-
source: "skill",
144-
get template() {
145-
return skill.content
146-
},
147-
hints: [],
143+
try {
144+
for (const skill of await Skill.all()) {
145+
// Skip if a command with this name already exists
146+
if (result[skill.name]) continue
147+
result[skill.name] = {
148+
name: skill.name,
149+
description: skill.description,
150+
source: "skill",
151+
get template() {
152+
return skill.content
153+
},
154+
hints: [],
155+
}
148156
}
157+
} catch {
158+
// Skill loading failed — continue with default commands
149159
}
150160

151161
return result
Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
import { describe, test, expect } from "bun:test"
2+
import { Command } from "../../src/command/index"
3+
import { Instance } from "../../src/project/instance"
4+
import { tmpdir } from "../fixture/fixture"
5+
6+
// ---------------------------------------------------------------------------
7+
// Helpers
8+
// ---------------------------------------------------------------------------
9+
10+
async function withInstance(fn: () => Promise<void>) {
11+
await using tmp = await tmpdir({ git: true })
12+
await Instance.provide({ directory: tmp.path, fn })
13+
}
14+
15+
// ---------------------------------------------------------------------------
16+
// Tests: Default commands are always available and correctly configured
17+
// ---------------------------------------------------------------------------
18+
19+
describe("Command module", () => {
20+
describe("default commands", () => {
21+
test("init, discover, review are always present", async () => {
22+
await withInstance(async () => {
23+
const commands = await Command.list()
24+
const names = commands.map((c) => c.name)
25+
expect(names).toContain("init")
26+
expect(names).toContain("discover")
27+
expect(names).toContain("review")
28+
})
29+
})
30+
31+
test("discover command has correct metadata", async () => {
32+
await withInstance(async () => {
33+
const cmd = await Command.get("discover")
34+
expect(cmd).toBeDefined()
35+
expect(cmd.name).toBe("discover")
36+
expect(cmd.source).toBe("command")
37+
expect(cmd.description).toBe("scan data stack and set up connections")
38+
})
39+
})
40+
41+
test("discover template references project_scan tool", async () => {
42+
await withInstance(async () => {
43+
const cmd = await Command.get("discover")
44+
const template = await cmd.template
45+
expect(template).toContain("project_scan")
46+
})
47+
})
48+
49+
test("discover template has $ARGUMENTS placeholder", async () => {
50+
await withInstance(async () => {
51+
const cmd = await Command.get("discover")
52+
expect(cmd.hints).toContain("$ARGUMENTS")
53+
})
54+
})
55+
56+
test("init command has correct metadata", async () => {
57+
await withInstance(async () => {
58+
const cmd = await Command.get("init")
59+
expect(cmd).toBeDefined()
60+
expect(cmd.name).toBe("init")
61+
expect(cmd.source).toBe("command")
62+
expect(cmd.description).toBe("create/update AGENTS.md")
63+
})
64+
})
65+
66+
test("review command has subtask flag", async () => {
67+
await withInstance(async () => {
68+
const cmd = await Command.get("review")
69+
expect(cmd).toBeDefined()
70+
expect(cmd.name).toBe("review")
71+
expect(cmd.subtask).toBe(true)
72+
expect(cmd.source).toBe("command")
73+
})
74+
})
75+
76+
test("all default commands have source 'command'", async () => {
77+
await withInstance(async () => {
78+
const commands = await Command.list()
79+
const defaults = commands.filter((c) => ["init", "discover", "review"].includes(c.name))
80+
expect(defaults.length).toBe(3)
81+
for (const cmd of defaults) {
82+
expect(cmd.source).toBe("command")
83+
}
84+
})
85+
})
86+
87+
test("Command.get returns undefined for non-existent commands", async () => {
88+
await withInstance(async () => {
89+
const cmd = await Command.get("nonexistent-command-12345")
90+
expect(cmd).toBeUndefined()
91+
})
92+
})
93+
})
94+
95+
describe("user-defined commands from config", () => {
96+
test("config commands are loaded alongside defaults", async () => {
97+
await using tmp = await tmpdir({
98+
git: true,
99+
config: {
100+
command: {
101+
"my-custom": {
102+
description: "Custom command",
103+
template: "Do something custom with $1",
104+
},
105+
},
106+
},
107+
})
108+
await Instance.provide({
109+
directory: tmp.path,
110+
fn: async () => {
111+
const commands = await Command.list()
112+
const names = commands.map((c) => c.name)
113+
// Defaults still present
114+
expect(names).toContain("init")
115+
expect(names).toContain("discover")
116+
expect(names).toContain("review")
117+
// Custom command also present
118+
expect(names).toContain("my-custom")
119+
const custom = await Command.get("my-custom")
120+
expect(custom.source).toBe("command")
121+
expect(custom.description).toBe("Custom command")
122+
},
123+
})
124+
})
125+
})
126+
})
127+
128+
// ---------------------------------------------------------------------------
129+
// Tests: Resilient loading pattern (isolated from module mocks)
130+
//
131+
// These tests verify the try/catch pattern used in command/index.ts to ensure
132+
// MCP and Skill failures don't prevent default commands from being served.
133+
// ---------------------------------------------------------------------------
134+
135+
describe("Command loading resilience pattern", () => {
136+
// Simulate the command loading logic from command/index.ts
137+
async function loadCommands(opts: {
138+
mcpPrompts: () => Promise<Record<string, any>>
139+
skillAll: () => Promise<Array<{ name: string; description: string; content: string }>>
140+
}) {
141+
const result: Record<string, { name: string; source: string; description?: string }> = {
142+
init: { name: "init", source: "command", description: "create/update AGENTS.md" },
143+
discover: { name: "discover", source: "command", description: "scan data stack" },
144+
review: { name: "review", source: "command", description: "review changes" },
145+
}
146+
147+
// This matches the pattern in command/index.ts
148+
try {
149+
for (const [name, prompt] of Object.entries(await opts.mcpPrompts())) {
150+
result[name] = { name, source: "mcp", description: (prompt as any).description }
151+
}
152+
} catch {
153+
// MCP prompt loading failed — continue with default commands
154+
}
155+
156+
try {
157+
for (const skill of await opts.skillAll()) {
158+
if (result[skill.name]) continue
159+
result[skill.name] = { name: skill.name, source: "skill", description: skill.description }
160+
}
161+
} catch {
162+
// Skill loading failed — continue with default commands
163+
}
164+
165+
return Object.values(result)
166+
}
167+
168+
test("all plugins healthy — defaults + plugins present", async () => {
169+
const commands = await loadCommands({
170+
mcpPrompts: async () => ({ "mcp-cmd": { description: "MCP prompt" } }),
171+
skillAll: async () => [{ name: "my-skill", description: "A skill", content: "" }],
172+
})
173+
const names = commands.map((c) => c.name)
174+
expect(names).toContain("init")
175+
expect(names).toContain("discover")
176+
expect(names).toContain("review")
177+
expect(names).toContain("mcp-cmd")
178+
expect(names).toContain("my-skill")
179+
})
180+
181+
test("MCP throws — defaults + skills still present", async () => {
182+
const commands = await loadCommands({
183+
mcpPrompts: async () => {
184+
throw new Error("MCP server unavailable")
185+
},
186+
skillAll: async () => [{ name: "my-skill", description: "A skill", content: "" }],
187+
})
188+
const names = commands.map((c) => c.name)
189+
expect(names).toContain("init")
190+
expect(names).toContain("discover")
191+
expect(names).toContain("review")
192+
expect(names).toContain("my-skill")
193+
expect(commands.filter((c) => c.source === "mcp").length).toBe(0)
194+
})
195+
196+
test("Skills throws — defaults + MCP still present", async () => {
197+
const commands = await loadCommands({
198+
mcpPrompts: async () => ({ "mcp-cmd": { description: "MCP prompt" } }),
199+
skillAll: async () => {
200+
throw new Error("Skill discovery failed")
201+
},
202+
})
203+
const names = commands.map((c) => c.name)
204+
expect(names).toContain("init")
205+
expect(names).toContain("discover")
206+
expect(names).toContain("review")
207+
expect(names).toContain("mcp-cmd")
208+
expect(commands.filter((c) => c.source === "skill").length).toBe(0)
209+
})
210+
211+
test("BOTH throw — only defaults present", async () => {
212+
const commands = await loadCommands({
213+
mcpPrompts: async () => {
214+
throw new Error("MCP server unavailable")
215+
},
216+
skillAll: async () => {
217+
throw new Error("Skill discovery failed")
218+
},
219+
})
220+
const names = commands.map((c) => c.name)
221+
expect(names).toContain("init")
222+
expect(names).toContain("discover")
223+
expect(names).toContain("review")
224+
expect(commands.length).toBe(3)
225+
})
226+
227+
test("skill with same name as default is skipped", async () => {
228+
const commands = await loadCommands({
229+
mcpPrompts: async () => ({}),
230+
skillAll: async () => [{ name: "discover", description: "Rogue skill", content: "" }],
231+
})
232+
const discover = commands.find((c) => c.name === "discover")!
233+
expect(discover.source).toBe("command")
234+
expect(discover.description).toBe("scan data stack")
235+
})
236+
237+
test("MCP command can overwrite default (same name)", async () => {
238+
const commands = await loadCommands({
239+
mcpPrompts: async () => ({ discover: { description: "MCP discover" } }),
240+
skillAll: async () => [],
241+
})
242+
const discover = commands.find((c) => c.name === "discover")!
243+
// MCP overwrites default because it's applied after defaults
244+
expect(discover.source).toBe("mcp")
245+
})
246+
})

0 commit comments

Comments
 (0)