-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix(cli): hide mutating tools in read-only MCP mode #4248
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "trigger.dev": patch | ||
| --- | ||
|
|
||
| Hide profile switching and local dev-server process controls when the MCP server runs in read-only mode. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,9 @@ const WRITE_TOOLS = new Set([ | |
| startAgentChatTool.name, | ||
| sendAgentMessageTool.name, | ||
| closeAgentChatTool.name, | ||
| switchProfileTool.name, | ||
| startDevServerTool.name, | ||
| stopDevServerTool.name, | ||
|
Comment on lines
+51
to
+53
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
rg -n 'destructiveHint|readOnlyHint|WRITE_TOOLS' packages/cli-v3/src/mcp
fd -a -t f 'package.json|pnpm-lock.yaml|yarn.lock|package-lock.json' . \
-x rg -n '`@modelcontextprotocol/sdk`'Repository: triggerdotdev/trigger.dev Length of output: 945 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
sed -n '1,180p' packages/cli-v3/src/mcp/tools.tsRepository: triggerdotdev/trigger.dev Length of output: 3751 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('packages/cli-v3/src/mcp/tools.ts')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 180:
print(f"{i:4d}: {line}")
PYRepository: triggerdotdev/trigger.dev Length of output: 4501 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
rg -n 'switchProfileTool|startDevServerTool|stopDevServerTool|WRITE_TOOLS|destructiveHint|readOnlyHint' packages/cli-v3/src/mcp -A4 -B4Repository: triggerdotdev/trigger.dev Length of output: 6490 🌐 Web query:
💡 Result: In the Model Context Protocol (MCP), readOnlyHint and destructiveHint are properties within the ToolAnnotations schema used to describe tool behavior to clients [1][2]. These properties are designated as hints, meaning they provide guidance rather than guaranteed behavior, and clients should not rely on them when interacting with untrusted servers [2][3]. readOnlyHint The readOnlyHint property indicates whether a tool modifies its environment [4][2]. If set to true, the tool is considered read-only and does not modify the environment [4][1]. The default value is false [2]. destructiveHint The destructiveHint property indicates the nature of modifications performed by a tool when it is not read-only [1][2]. It is only meaningful when readOnlyHint is set to false [4][2]. If set to true, the tool may perform destructive updates to its environment, such as deleting or overwriting data [4][1][5]. If set to false, the tool performs only additive updates, such as creating or appending data [4][1][2]. The default value is true [1][2]. By distinguishing between destructive and additive updates, these hints allow clients to implement risk-aware interfaces, such as requiring human confirmation for tools marked with destructiveHint: true [1][5]. Other related annotations include idempotentHint (indicating if repeated calls have the same effect) and openWorldHint (indicating interaction with external entities) [1][5][2]. Citations:
🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for file in [
"packages/cli-v3/src/mcp/tools/profiles.ts",
"packages/cli-v3/src/mcp/tools/devServer.ts",
]:
print(f"\n=== {file} ===")
text = Path(file).read_text().splitlines()
for i, line in enumerate(text, 1):
if any(key in line for key in ["export const switchProfileTool", "export const startDevServerTool", "export const stopDevServerTool", "handler:", "respondWithError", "process", "profile", "server", "stop", "start"]):
start = max(1, i - 8)
end = min(len(text), i + 20)
for j in range(start, end + 1):
print(f"{j:4d}: {text[j-1]}")
print()
break
PYRepository: triggerdotdev/trigger.dev Length of output: 1934 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
targets = {
"packages/cli-v3/src/mcp/tools/profiles.ts": ["export const switchProfileTool", "handler:"],
"packages/cli-v3/src/mcp/tools/devServer.ts": ["export const startDevServerTool", "export const stopDevServerTool", "handler:"],
}
for file, markers in targets.items():
print(f"\n=== {file} ===")
lines = Path(file).read_text().splitlines()
for marker in markers:
for i, line in enumerate(lines, 1):
if marker in line:
start = max(1, i - 20)
end = min(len(lines), i + 80)
for j in range(start, end + 1):
print(f"{j:4d}: {lines[j-1]}")
print()
break
PYRepository: triggerdotdev/trigger.dev Length of output: 15717 Separate destructive hints from readonly filtering |
||
| ]); | ||
|
|
||
| export function registerTools(context: McpContext) { | ||
|
|
||
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.
🔍 Pre-existing: async handler errors silently escape the try/catch
The registered tool callback at
packages/cli-v3/src/mcp/tools.ts:116-121doesreturn tool.handler(...)withoutawait. Sincetool.handleris async (returns a Promise), any rejection from that promise will NOT be caught by the surroundingtry/catchblock — it will propagate as an unhandled rejection to the MCP framework instead of returning a structured error viarespondWithError. Addingawaitbeforetool.handler(...)would fix this. This is pre-existing (not introduced by this PR) but affects all tools including the three newly classified as write tools.(Refers to lines 116-121)
Was this helpful? React with 👍 or 👎 to provide feedback.