Skip to content

fix(enrichment): don't crash on a missing optional mcp.json or skills/ dir#123

Open
mftnakrsu wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
mftnakrsu:fix/enrichment-optional-config-enoent
Open

fix(enrichment): don't crash on a missing optional mcp.json or skills/ dir#123
mftnakrsu wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
mftnakrsu:fix/enrichment-optional-config-enoent

Conversation

@mftnakrsu

Copy link
Copy Markdown

What

loadMcpTools and loadSkills (toolbox/enrichment/src/agent/tools.ts) crash kcagent enrich when their optional config inputs are absent.

Why

Both functions call fs.promises.stat() outside their try blocks:

const stat = await fs.promises.stat(mcpConfigPath); // rejects ENOENT if absent
if (!stat.isFile()) return [];                       // dead code for missing path
try { ... } catch { return []; }

fs.promises.stat rejects with ENOENT for a missing path, so a missing mcp.json / skills/ dir throws an uncaught ENOENT up through enrichCommand, aborting the run — and the !isFile()/!isDirectory() guards never get a chance to return [].

Change

Move both stat() calls inside the existing try blocks and treat ENOENT as "not configured" (return [] silently; other errors still warn). This matches the catch blocks' existing return-[] intent and keeps these inputs genuinely optional.

Verification

npx tsc --noEmit type-checks the changed file clean. The only diagnostics in a fresh checkout are pre-existing TS2307: Cannot find module 'kcmd' in agent.ts/command.ts from the unbuilt local file:../mdcode dependency — unrelated to this change.

🤖 Generated with Claude Code

…/ dir

loadMcpTools and loadSkills called fs.promises.stat() OUTSIDE their try blocks. fs.promises.stat rejects with ENOENT when the path is absent, so a missing (optional) mcp.json or skills/ directory threw an uncaught ENOENT up through enrichCommand and aborted 'kcagent enrich' — the !stat.isFile()/!stat.isDirectory() guards were dead code for that case. Move the stat() calls inside the existing try blocks and treat ENOENT as 'not configured' (return [] without a spurious warning); other errors still warn. Matches the catch blocks' existing return-[] intent.
@mftnakrsu mftnakrsu marked this pull request as ready for review June 20, 2026 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant