fix(tools): resolve {{ENV_VAR}} references in user-only params for copilot tool executions#5679
Conversation
…pilot tool executions Chat-invoked integration tools with API-key auth have been broken since the mothership tool-dispatch rewrite (#4090) removed the orchestrator's env reference resolution. Agents pass {{VAR}} references (the VFS exposes env var names only), and the literal placeholder was sent to the provider, producing auth failures like Sentry's 401 Invalid token. Resolution is deliberately narrower than the removed deep resolver: only whole-value references on params declared visibility user-only, gated to copilot executions, resolving against the same personal+workspace env merge workflow runs use. LLM-writable params (urls, headers, bodies) never resolve, so references cannot be used to extract secrets. Missing variables fail fast with an actionable error instead of a provider-side 401.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview This PR adds Tests add a mock env tool and a suite covering resolution, exclusions, errors, non-copilot passthrough, and no mutation of caller params. Reviewed by Cursor Bugbot for commit 728917c. Configure here. |
Greptile SummaryThis PR restores env variable reference handling for copilot tool executions. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(tools): fail fast on env references ..." | Re-trigger Greptile |
…red executor resolver Replaces the hand-rolled exact-match regex with resolveEnvVarReferences (allowEmbedded: false), the same resolver used by workflow runs, MCP config, and webhooks — one set of reference semantics instead of two that can drift. Behavior is identical; all existing tests pass unchanged.
… personal-only scope errors Addresses review: a copilot execution missing userId now errors explicitly instead of forwarding the literal placeholder upstream, and a missing-variable error without a workspace context explains that only personal variables are in scope there (matching workflow-run resolution semantics).
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 728917c. Configure here.
Summary
401 Invalid token) — broken since the mothership tool-dispatch rewrite (improvement(mothership): restructured stream, tool structures, code typing, file write/patch/append tools, timing issues #4090) removed the orchestrator's env reference resolution{{VAR}}references (the workspace VFS exposes env var names, never values); the literal placeholder was being sent to the provider as the bearer tokenresolveCopilotEnvReferencesin the tool executor: resolves whole-value{{VAR}}references on params declaredvisibility: 'user-only', gated to copilot executions onlygetEffectiveDecryptedEnv— the same personal+workspace merge workflow runs use, so Chat and canvas behave identicallyType of Change
Testing
tools/index.test.tssuite (87 tests) passing; typecheck and lint cleansentry_projects_list401s via/api/mothership/chatwith the literal placeholder in params)Checklist