Skip to content

Commit c8c1fe1

Browse files
anandgupta42claude
andcommitted
fix: address PR review feedback — add logging and comments
- Add Log.Default.warn() to MCP and Skill catch blocks so failures are visible in debug logs instead of being silently swallowed - Add comment documenting MCP/Skills overwrite asymmetry (MCP can overwrite defaults by name, skills cannot due to guard clause) - Add comment noting resilience tests duplicate loading logic and may need updating if the real implementation changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 57c61af commit c8c1fe1

File tree

2 files changed

+15
-5
lines changed

2 files changed

+15
-5
lines changed

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import PROMPT_DISCOVER from "./template/discover.txt"
88
import PROMPT_REVIEW from "./template/review.txt"
99
import { MCP } from "../mcp"
1010
import { Skill } from "../skill"
11+
import { Log } from "../util/log"
1112

1213
export namespace Command {
1314
export const Event = {
@@ -108,6 +109,8 @@ export namespace Command {
108109
}
109110
// MCP and skill loading must not prevent default commands from being served.
110111
// Wrap each in try/catch so init, discover, review are always available.
112+
// Note: MCP prompts can overwrite defaults (by name), but skills cannot
113+
// (the `if (result[skill.name]) continue` guard preserves defaults over skills).
111114
try {
112115
for (const [name, prompt] of Object.entries(await MCP.prompts())) {
113116
result[name] = {
@@ -135,8 +138,10 @@ export namespace Command {
135138
hints: prompt.arguments?.map((_, i) => `$${i + 1}`) ?? [],
136139
}
137140
}
138-
} catch {
139-
// MCP prompt loading failed — continue with default commands
141+
} catch (e) {
142+
Log.Default.warn("MCP prompt loading failed, continuing with defaults", {
143+
error: e instanceof Error ? e.message : String(e),
144+
})
140145
}
141146

142147
// Add skills as invokable commands
@@ -154,8 +159,10 @@ export namespace Command {
154159
hints: [],
155160
}
156161
}
157-
} catch {
158-
// Skill loading failed — continue with default commands
162+
} catch (e) {
163+
Log.Default.warn("Skill loading failed, continuing with defaults", {
164+
error: e instanceof Error ? e.message : String(e),
165+
})
159166
}
160167

161168
return result

packages/altimate-code/test/command/command-resilience.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,10 @@ describe("Command module", () => {
133133
// ---------------------------------------------------------------------------
134134

135135
describe("Command loading resilience pattern", () => {
136-
// Simulate the command loading logic from command/index.ts
136+
// NOTE: These tests duplicate the loading logic from command/index.ts rather than
137+
// mocking the real MCP/Skill modules. This avoids complex module mocking but means
138+
// loadCommands() below could drift from the real implementation. If the loading
139+
// pattern in command/index.ts changes, these tests should be updated to match.
137140
async function loadCommands(opts: {
138141
mcpPrompts: () => Promise<Record<string, any>>
139142
skillAll: () => Promise<Array<{ name: string; description: string; content: string }>>

0 commit comments

Comments
 (0)