Skip to content

feat(mcp): normalize impact and context parameter aliases#2462

Merged
4 commits merged into
abhigyanpatwari:mainfrom
electricsheephq:upstream/mcp-parameter-aliases
Jul 16, 2026
Merged

feat(mcp): normalize impact and context parameter aliases#2462
4 commits merged into
abhigyanpatwari:mainfrom
electricsheephq:upstream/mcp-parameter-aliases

Conversation

@100yenadmin

Copy link
Copy Markdown
Contributor

Summary

  • accept impact.target, impact.name, and impact.symbol
  • accept context.file_path and context.file
  • reject conflicting canonical and alias values before repository resolution
  • normalize once before single-repository or group dispatch

Validation

  • tsc --noEmit on current upstream
  • vitest run test/unit/tools.test.ts (32 passing)
  • vitest run test/unit/calltool-dispatch.test.ts (216 passing)
  • vitest run test/integration/local-backend-calltool.test.ts (34 passing against LadybugDB)
  • focused Prettier and diff checks

The older stacked fork PR had one unrelated typecheck job fail because its historical base lockfile omitted @huggingface/transformers; all runtime and platform jobs passed. This branch starts from current upstream and passes a clean local typecheck. Its own current-head CI is the remote acceptance gate.

Closes #2461

@vercel

vercel Bot commented Jul 13, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

@100yenadmin

Copy link
Copy Markdown
Contributor Author

Self-review follow-up at 3f3494fd:

  • rebased behavior onto current upstream main via a normal merge (no history rewrite)
  • removed the advertised top-level anyOf, which some MCP providers reject
  • moved the at-least-one impact target contract into runtime normalization
  • preserved valid target_uid-only impact calls
  • reject empty, non-string, and conflicting aliases before repository resolution
  • retain canonicalization before group forwarding

Focused proof: 287 dispatch/schema/real-index tests pass; typecheck and Prettier pass; ESLint reports zero errors (repository baseline warnings remain).

@100yenadmin

Copy link
Copy Markdown
Contributor Author

The current-head CI failures exposed an internal CLI compatibility case that the MCP-only tests did not cover: Commander materializes omitted optional flags as properties whose value is JavaScript undefined. Alias normalization treated those as explicitly supplied invalid values, so ordinary context CLI calls returned a validation error before repository resolution.

Fixed at d4576630: undefined optional alias keys are now treated as absent. Explicit nulls, non-strings, blanks, and conflicts still fail closed.

Focused proof:

  • 289 alias schema, dispatch, and real-index tests
  • exact previously failing cli-limit-e2e and skip-git-cli suites: 23/23 pass
  • build and npx tsc --noEmit
  • focused Prettier and ESLint: zero errors (pre-existing warning baseline only)

GitHub CI has restarted on the repaired exact head.

@github-actions

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
14553 14491 0 62 18s

✅ All 14491 tests passed

62 test(s) skipped — expand for details

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 80.69% 53938/66839 80.68% 📈 +0.0 🟢 ████████████████░░░░
Branches 67.71% 32928/48624 67.7% 📈 +0.0 🟢 █████████████░░░░░░░
Functions 86.72% 6214/7165 86.72% = 0.0 🟢 █████████████████░░░
Lines 84.22% 48110/57122 84.21% 📈 +0.0 🟢 ████████████████░░░░

📋 View full run · Generated by CI

@magyargergo magyargergo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The normalizer sits at the right chokepoint, after the group early return but before repo routing, and the test proving groupImpact receives the canonical target is exactly the coverage I wanted. Good catch on the Commander undefined flags regression, that chokepoint touches every CLI command and the dedicated test locks it in.

One design question and one behavior note inline. Merge order: conflicts with #2460 on tools.ts, whoever lands second rebases.

Comment thread gitnexus/src/mcp/tools.ts
Comment thread gitnexus/src/mcp/local/local-backend.ts
@magyargergo magyargergo closed this pull request by merging all changes into abhigyanpatwari:main in 2e2db8b Jul 16, 2026
pull Bot pushed a commit to AmirulAndalib/GitNexus that referenced this pull request Jul 16, 2026
@magyargergo

Copy link
Copy Markdown
Collaborator

Merged via #2511. I kept the runtime guard over anyOf; #2489 proposing to drop the api_impact combinator supports that call from the other direction.

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.

feat(mcp): normalize impact and context parameter aliases

2 participants