-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(mcp): project .mcp.json + workspace approval gating with aligned scope precedence (#4615) #4713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
qqqys
wants to merge
8
commits into
QwenLM:main
Choose a base branch
from
qqqys:feat/mcp-scope-precedence-workspace-approval
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat(mcp): project .mcp.json + workspace approval gating with aligned scope precedence (#4615) #4713
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
6590364
feat(mcp): project .mcp.json + workspace approval gating with aligned…
qqqys 8768761
test(cli): cover mcp scope stamping
qqqys bc518fe
fix(mcp): harden approval binding
qqqys e402a39
fix(mcp): harden approval store persistence
qqqys a9b81bc
fix(mcp): address approval review feedback
qqqys 97c9cc8
Merge remote-tracking branch 'refs/remotes/upstream/main' into HEAD
qqqys 18e0a72
fix(cli): align prompt width with approval visuals
qqqys ddd05cc
refactor(cli): rename gated MCP approval helper
qqqys File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2025 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; | ||
| import * as fs from 'node:fs'; | ||
| import * as os from 'node:os'; | ||
| import * as path from 'node:path'; | ||
|
|
||
| const mockWriteStdoutLine = vi.hoisted(() => vi.fn()); | ||
| vi.mock('../../utils/stdioHelpers.js', () => ({ | ||
| writeStdoutLine: mockWriteStdoutLine, | ||
| writeStderrLine: vi.fn(), | ||
| clearScreen: vi.fn(), | ||
| })); | ||
|
|
||
| import { approveCommand, rejectCommand } from './approve.js'; | ||
| import { | ||
| loadMcpApprovals, | ||
| resetMcpApprovalsForTesting, | ||
| MCP_APPROVALS_FILENAME, | ||
| } from '../../config/mcpApprovals.js'; | ||
| import { loadProjectMcpServers } from '../../config/mcpJson.js'; | ||
|
|
||
| describe('qwen mcp approve / reject', () => { | ||
| let dir: string; | ||
| let cwdSpy: ReturnType<typeof vi.spyOn>; | ||
|
|
||
| const output = () => | ||
| mockWriteStdoutLine.mock.calls.map((c) => c[0]).join('\n'); | ||
|
|
||
| const run = async ( | ||
| cmd: typeof approveCommand, | ||
| argv: Record<string, unknown>, | ||
| ) => { | ||
| await (cmd.handler as (a: Record<string, unknown>) => Promise<void>)({ | ||
| _: [], | ||
| $0: 'qwen', | ||
| ...argv, | ||
| }); | ||
| }; | ||
|
|
||
| beforeEach(() => { | ||
| dir = fs.mkdtempSync(path.join(os.tmpdir(), 'mcp-approve-')); | ||
| process.env['QWEN_CODE_MCP_APPROVALS_PATH'] = path.join( | ||
| dir, | ||
| MCP_APPROVALS_FILENAME, | ||
| ); | ||
| resetMcpApprovalsForTesting(); | ||
| cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(dir); | ||
| mockWriteStdoutLine.mockClear(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| delete process.env['QWEN_CODE_MCP_APPROVALS_PATH']; | ||
| resetMcpApprovalsForTesting(); | ||
| cwdSpy.mockRestore(); | ||
| fs.rmSync(dir, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| const writeMcpJson = (servers: Record<string, unknown>) => | ||
| fs.writeFileSync( | ||
| path.join(dir, '.mcp.json'), | ||
| JSON.stringify({ mcpServers: servers }), | ||
| ); | ||
|
|
||
| const stateOf = (name: string) => { | ||
| resetMcpApprovalsForTesting(); | ||
| const { servers } = loadProjectMcpServers(dir); | ||
| return loadMcpApprovals().getState(dir, name, servers[name]!); | ||
| }; | ||
|
|
||
| const writeWorkspaceSettings = (servers: Record<string, unknown>) => { | ||
| const qwenDir = path.join(dir, '.qwen'); | ||
| fs.mkdirSync(qwenDir, { recursive: true }); | ||
| fs.writeFileSync( | ||
| path.join(qwenDir, 'settings.json'), | ||
| JSON.stringify({ mcpServers: servers }), | ||
| ); | ||
| }; | ||
|
|
||
| /** Read the persisted approval status straight off disk (scope-agnostic). */ | ||
| const persistedStatus = (name: string): string | undefined => { | ||
| const raw = fs.readFileSync( | ||
| process.env['QWEN_CODE_MCP_APPROVALS_PATH']!, | ||
| 'utf-8', | ||
| ); | ||
| return JSON.parse(raw)[dir]?.[name]?.status; | ||
| }; | ||
|
|
||
| it('reports when there are no gated servers', async () => { | ||
| await run(approveCommand, { name: 'slack', all: false }); | ||
| expect(output()).toContain('No approval-requiring MCP servers found'); | ||
| }); | ||
|
|
||
| it('approves a named project server (pending -> approved)', async () => { | ||
| writeMcpJson({ slack: { command: 'node', args: ['slack.js'] } }); | ||
| expect(stateOf('slack')).toBe('pending'); | ||
|
|
||
| await run(approveCommand, { name: 'slack', all: false }); | ||
|
|
||
| expect(stateOf('slack')).toBe('approved'); | ||
| expect(output()).toContain('Approved MCP server "slack"'); | ||
| }); | ||
|
|
||
| it('approves a workspace .qwen/settings.json server', async () => { | ||
| writeWorkspaceSettings({ ws: { command: 'node', args: ['ws.js'] } }); | ||
|
|
||
| await run(approveCommand, { name: 'ws', all: false }); | ||
|
|
||
| expect(output()).toContain('Approved MCP server "ws"'); | ||
| expect(persistedStatus('ws')).toBe('approved'); | ||
| }); | ||
|
|
||
| it('rejects a named project server', async () => { | ||
| writeMcpJson({ slack: { command: 'node' } }); | ||
| await run(rejectCommand, { name: 'slack', all: false }); | ||
| expect(stateOf('slack')).toBe('rejected'); | ||
| }); | ||
|
|
||
| it('approves all with --all', async () => { | ||
| writeMcpJson({ a: { command: 'a' }, b: { command: 'b' } }); | ||
| await run(approveCommand, { name: undefined, all: true }); | ||
| expect(stateOf('a')).toBe('approved'); | ||
| expect(stateOf('b')).toBe('approved'); | ||
| }); | ||
|
|
||
| it('reports an unknown server name', async () => { | ||
| writeMcpJson({ slack: { command: 'node' } }); | ||
| await run(approveCommand, { name: 'ghost', all: false }); | ||
| expect(output()).toContain('not found'); | ||
| expect(stateOf('slack')).toBe('pending'); | ||
| }); | ||
|
|
||
| it('binds approval to the config hash: editing .mcp.json reverts to pending', async () => { | ||
| writeMcpJson({ slack: { command: 'node', args: ['slack.js'] } }); | ||
| await run(approveCommand, { name: 'slack', all: false }); | ||
| expect(stateOf('slack')).toBe('approved'); | ||
|
|
||
| // Edit the server's command — approval must no longer apply. | ||
| writeMcpJson({ slack: { command: 'curl', args: ['slack.js'] } }); | ||
| expect(stateOf('slack')).toBe('pending'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2025 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| // Files for 'qwen mcp approve' / 'qwen mcp reject' commands (issue #4615). | ||
| import type { CommandModule } from 'yargs'; | ||
| import type { MCPServerConfig } from '@qwen-code/qwen-code-core'; | ||
| import { isGatedMcpScope } from '@qwen-code/qwen-code-core'; | ||
| import { writeStdoutLine } from '../../utils/stdioHelpers.js'; | ||
| import { loadSettings } from '../../config/settings.js'; | ||
| import { assembleMcpServers } from '../../config/mcpServers.js'; | ||
| import { | ||
| loadMcpApprovals, | ||
| type McpApprovalStatus, | ||
| } from '../../config/mcpApprovals.js'; | ||
|
|
||
| /** | ||
| * All gated (approval-requiring) servers visible from `cwd` — project | ||
| * `.mcp.json` plus workspace `.qwen/settings.json` (#4615). Non-gated sources | ||
| * (user/system/extension) never need approval and are excluded. | ||
| */ | ||
| function loadGatedServers(cwd: string): Record<string, MCPServerConfig> { | ||
| const settings = loadSettings(cwd); | ||
| const all = assembleMcpServers(settings.merged.mcpServers, cwd); | ||
| const gated: Record<string, MCPServerConfig> = {}; | ||
| for (const [serverName, config] of Object.entries(all)) { | ||
| if (isGatedMcpScope(config.scope)) { | ||
| gated[serverName] = config; | ||
| } | ||
| } | ||
| return gated; | ||
| } | ||
|
|
||
| async function setProjectServerStatus( | ||
| name: string | undefined, | ||
| status: McpApprovalStatus, | ||
| all: boolean, | ||
| ): Promise<void> { | ||
| const cwd = process.cwd(); | ||
| const servers = loadGatedServers(cwd); | ||
|
|
||
| const names = Object.keys(servers); | ||
| if (names.length === 0) { | ||
| writeStdoutLine( | ||
| 'No approval-requiring MCP servers found (looked in .mcp.json and .qwen/settings.json).', | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const verb = status === 'approved' ? 'Approved' : 'Rejected'; | ||
| const approvals = loadMcpApprovals(); | ||
|
|
||
| const targets = all ? names : name ? [name] : []; | ||
| if (targets.length === 0) { | ||
| writeStdoutLine('Specify a server name or pass --all.'); | ||
| return; | ||
| } | ||
|
|
||
| for (const target of targets) { | ||
| const config = servers[target]; | ||
| if (!config) { | ||
| writeStdoutLine( | ||
| `Server "${target}" not found. Available: ${names.join(', ')}`, | ||
| ); | ||
| continue; | ||
| } | ||
| // The decision binds to this exact config's hash: editing the server in | ||
| // its source file later returns it to pending (issue #4615). | ||
| await approvals.setState(cwd, target, config, status); | ||
| writeStdoutLine( | ||
| `${verb} MCP server "${target}" (bound to its current config).`, | ||
| ); | ||
| } | ||
|
|
||
| if (status === 'approved') { | ||
| writeStdoutLine( | ||
| 'Approved servers connect in your next interactive session.', | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export const approveCommand: CommandModule = { | ||
| command: 'approve [name]', | ||
| describe: | ||
| 'Approve a gated MCP server (.mcp.json or workspace .qwen/settings.json)', | ||
| builder: (yargs) => | ||
| yargs | ||
| .usage('Usage: qwen mcp approve [options] [name]') | ||
| .positional('name', { | ||
| describe: 'Name of the gated server to approve', | ||
| type: 'string', | ||
| }) | ||
| .option('all', { | ||
| describe: 'Approve all gated servers in this workspace', | ||
| type: 'boolean', | ||
| default: false, | ||
| }), | ||
| handler: async (argv) => { | ||
| await setProjectServerStatus( | ||
| argv['name'] as string | undefined, | ||
| 'approved', | ||
| argv['all'] as boolean, | ||
| ); | ||
| }, | ||
| }; | ||
|
|
||
| export const rejectCommand: CommandModule = { | ||
| command: 'reject [name]', | ||
| describe: | ||
| 'Reject a gated MCP server (.mcp.json or workspace .qwen/settings.json)', | ||
| builder: (yargs) => | ||
| yargs | ||
| .usage('Usage: qwen mcp reject [options] [name]') | ||
| .positional('name', { | ||
| describe: 'Name of the gated server to reject', | ||
| type: 'string', | ||
| }) | ||
| .option('all', { | ||
| describe: 'Reject all gated servers in this workspace', | ||
| type: 'boolean', | ||
| default: false, | ||
| }), | ||
| handler: async (argv) => { | ||
| await setProjectServerStatus( | ||
| argv['name'] as string | undefined, | ||
| 'rejected', | ||
| argv['all'] as boolean, | ||
| ); | ||
| }, | ||
| }; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Critical] TypeScript error:
Property 'getOptions' does not exist on type 'Argv<{}> | PromiseLike<Argv<{}>>'. Thebuilderfunction (typed asCommandModule['builder']) can returnArgv | PromiseLike<Argv>. The runtimetypeof builder !== 'function'guard narrows it to a function, but the return type is still the union.(Make the test callback
asyncandawaitthe builder result to unwrap thePromiseLike.)— qwen3.7-max via Qwen Code /review