Skip to content

fix(mcp): guard isTestFilePath against nodes without a filePath#2565

Open
SAY-5 wants to merge 1 commit into
abhigyanpatwari:mainfrom
SAY-5:fix-istestfilepath-undefined-guard
Open

fix(mcp): guard isTestFilePath against nodes without a filePath#2565
SAY-5 wants to merge 1 commit into
abhigyanpatwari:mainfrom
SAY-5:fix-istestfilepath-undefined-guard

Conversation

@SAY-5

@SAY-5 SAY-5 commented Jul 18, 2026

Copy link
Copy Markdown

Summary

isTestFilePath crashed on undefined/null paths; this adds a guard so it returns false instead of throwing. Fixes #2547.

Motivation / context

The MCP trace tool fails with a raw TypeError: Cannot read properties of undefined (reading 'toLowerCase') while every other per-repo tool works against the same index. During the BFS in _traceImpl, filePath is decoded as row.filePath ?? row[4], which is undefined for node types that carry no file (Community, Process, etc.). That value flows straight into isTestFilePath(filePath), and filePath.toLowerCase() throws. A file-less node is not a test file, so the guard returns false and the BFS visits it normally.

Areas touched

  • gitnexus/ (CLI / core / MCP server)
  • gitnexus-web/ (Vite / React UI)
  • .github/ (workflows, actions)
  • eval/ or other tooling
  • Docs / agent config only (AGENTS.md, CLAUDE.md, .cursor/, llms.txt, etc.)

Scope & constraints

In scope

  • Widen the isTestFilePath parameter to string | null | undefined and return false for falsy input.

Explicitly out of scope / not done here

  • No change to how filePath is decoded at the two call sites; the guard lives in the helper so both BFS loops (impact and trace) are covered.
  • No new envelope/error-message shaping for trace beyond removing the crash.

Implementation notes

The guard is in the exported helper rather than at each call site so any current or future caller passing a file-less node gets consistent behavior.

Testing & verification

  • cd gitnexus && npm test (security suite, 18 passed)
  • cd gitnexus && npm run test:integration (if core/indexing/MCP paths changed)
  • cd gitnexus && npx tsc --noEmit
  • cd gitnexus-web && npm test (if web changed)
  • cd gitnexus-web && npx tsc -b --noEmit (if web changed)
  • Manual / Playwright E2E (note environment — see gitnexus-web/e2e/)

Added a regression case to security.test.ts: with the guard reverted it fails with the exact Cannot read properties of undefined (reading 'toLowerCase'); with the guard it passes.

Risk & rollout

Low risk. Behavior only changes for previously-throwing input; no index refresh needed.

Checklist

  • PR body meets repo minimum length (workflow may label short descriptions)
  • If AGENTS.md / overlays changed: headers, scope block, and changelog updated per project conventions
  • No secrets, tokens, or machine-specific paths committed

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
@SAY-5
SAY-5 requested a review from azizur100389 as a code owner July 18, 2026 21:34
@vercel

vercel Bot commented Jul 18, 2026

Copy link
Copy Markdown

@SAY-5 is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

@koriyoshi2041 koriyoshi2041 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I traced this against the reported BFS path in #2547. The guard is at the right boundary: trace can decode a file-less graph node, and treating a missing path as “not a test file” preserves traversal while removing the raw TypeError. The focused null/undefined/empty regression also covers the exact failure at the helper boundary, so I do not see a code blocker in this two-line change.

I would still wait for the normal CI Gate before merging. At the current head, GitHub only shows the PR labeler plus the unrelated Vercel fork-authorization failure; none of the repository typecheck/unit/integration jobs have run.

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.

trace tool throws TypeError: Cannot read properties of undefined (reading 'toLowerCase')

2 participants