Skip to content

Keep catalog reads scoped to the request instance - #1061

Open
justadityaraj wants to merge 1 commit into
czlonkowski:mainfrom
justadityaraj:fix/1034-context-catalog
Open

Keep catalog reads scoped to the request instance#1061
justadityaraj wants to merge 1 commit into
czlonkowski:mainfrom
justadityaraj:fix/1034-context-catalog

Conversation

@justadityaraj

@justadityaraj justadityaraj commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem / Context A request context containing an n8n URL and MCP token but no Public API key is authoritative for the official MCP client. The catalog handler still resolved the Public API client before branching, however, so tag reads could fall back to the operator's environment instance. The same early lookup also replaced the shared same-instance hint with a generic configuration error for project requests without an environment client. Fixes #1034 ## Fix / Changes - Refuse tag reads with the shared context hint before resolving any Public API client. - Resolve the Public API client only inside the tags branch. - Let project requests continue through the existing project resolver and its official-MCP fallback. - Cover both residual context shapes with handler regressions. ## User impact Per-request catalog calls no longer read tags from a different, environment-configured n8n instance, and project failures consistently explain that x-n8n-key must accompany x-n8n-url. ## Verification - New focused regressions failed on current upstream main: the environment API getter was called for tags, and projects returned the generic setup error. - npx vitest run tests/unit/mcp/handlers-official-tools.test.ts --reporter=dot — 39 passed. - Relevant handler, exposure, and official-MCP integration selection — 66 passed, 2 environment-gated tests skipped. - npm run typecheck — passed. - npm run build — passed. - npm run test:unit executed the full unit set but did not exit on this Windows host after emitting all files; unrelated existing failures were confined to unavailable SQLite FTS5, POSIX shell/path assumptions, and Windows symlink privileges. Conceived by Romuald Członkowski - https://aiadvisors.pl/en

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@czlonkowski

Copy link
Copy Markdown
Owner

Reviewed the current fc0eba1 revision. The code change looks correct and I found no blocking correctness, security, or privacy issue.

The new guard in src/mcp/handlers-official-tools.ts:175-179 runs before getN8nApiClient(), so a URL + MCP-token context without x-n8n-key cannot read tags through the environment API. The projects path continues through the existing context-aware resolver. The added regressions cover both residual cases from #1034.

The only requested change is metadata/process: AGENTS.md requires the exact attribution Conceived by Romuald Członkowski - https://aiadvisors.pl/en in both the PR description and commit message; both currently use www.aiadvisors.pl/en. Please update the PR description and commit attribution, unless the maintainers explicitly decide that the final squash commit is sufficient for the commit-message requirement.

Local verification: 58 relevant unit tests and typecheck passed. The socket-bound wire tests were not rerun locally because the review sandbox cannot host their fake server; the full GitHub test workflow passed. Once the attribution is resolved, I am comfortable with this PR being merged.

@justadityaraj
justadityaraj force-pushed the fix/1034-context-catalog branch from fc0eba1 to bd28469 Compare September 3, 2026 20:35
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.

Remaining env-fallback reads on a url + MCP-token context without x-n8n-key

2 participants