[dan-petty]

A role server and a client choose tools by the read-only flag, so a tool with an option that writes must not be read-only. The change marks pr_check_readiness, roadmap_refine, roadmap_run and ci not read-only. Each command keeps its option. The schemas of scan_injection and tf_plan drop the file options sarif and out, and both tools stay read-only. A new test reads the published schema of every tool. It fails on a read-only tool whose option description holds the verb resolve, write, delete or create. Other tools stay open in #3642 (tools that write local state), #3644 (help text of tf_notify_plan) and #3480 (path decisions).

Closes #3235

Change: openspec/changes/archive/2026-11-05-3235-read-only-mcp-tools-have-no-write-option/
Elapsed: 3.61s

[dan-petty]

Test results

Fail-to-pass run (uv run devops ci fail-to-pass --base origin/main): 7 tests fail on the base and 7 pass. The new test file tests/ai/mcp/test_mcp_read_only_write_options.py holds all 14.

The 7 tests that fail on the base:

• test_no_read_only_tool_publishes_a_write_option
• test_a_tool_that_keeps_a_writing_option_is_not_read_only for pr_check_readiness, roadmap_refine, roadmap_run and ci
• test_a_read_only_tool_leaves_out_an_option_that_writes_a_file for scan_injection (sarif) and tf_plan (out)

Full gate (uv run devops ci, on the final code): 18,802 tests pass and 4 fail with 1 error. All other checks pass: lint, format, typecheck, security, docs, openspec, build, deps and the rest.

The 4 failures and the 1 error read the state of the host. They fail the same way on a clean main checkout at 66a8a63, with none of these changes:

• tests/crypto/test_ssh.py::test_get_ssh_key_prefix_resolution
• tests/telemetry/test_config_env_overrides.py::test_find_project_config_path_skips_directory (a stray /tmp/config.yaml on the host)
• tests/commands/test_devcontainer_install_tools_dry_run.py::test_mark_only_commands_under_global_dry_run (the host has the devcontainer tool)
• tests/ai/test_tool_executor_contract.py::test_the_direct_executor_refuses_a_network_profile_before_it_starts_the_tool[direct] (the host has Popeye), with its teardown error

Open issues already own them (#3585, #3590, #3593, #3594, #3597 and #3025). This change touches none of them.

Review findings

A review of the first version gave five findings. I checked each one.

1. Criterion 1 not met for other tools. Partly right.
   • roadmap_run.confirm and ci.fix are true writes. Both tools are now not read-only, and the new test covers them.
   • tf_notify_plan.pr_number is a wrong finding. devops tf notify-plan sends no request. It prints the comment and puts the PR number in its payload. The tool is read-only. Its help says that it posts, and issue fix(tf): tf notify-plan says it posts a comment and posts nothing #3644 owns that text.
   • ai_gateway_failover.reset and review_export_feedback.output are true. So are ai_quiesce, ai_resume and ai_failover, which the review did not list. Each command writes local state or a file, and each has the class no-external-request. The requirement mcp-read-only-hint-agrees-with-no-external-request and its ratchet test force readOnlyHint true. Dropping one option would leave the whole command as a write. Issue fix(mcp): tools that write local state are flagged read-only #3642 decides what the class means for the flag. The proposal and the tasks.md follow-up list say that criterion 1 is met for the tools that the rule and the review found.
2. tf_plan.out is an unconfined write path. Right. The tool now leaves out out and stays read-only, like scan_injection.sarif. The spec, the test, the design and docs/MCP_TOOLS.md follow. scan_injection.suppress and review_export_feedback.output stay as before. Issue fix(mcp): the path audit finds path options by type and by name #3480 owns both.
3. No follow-up bullet. Right. tasks.md names fix(mcp): tools that write local state are flagged read-only #3642, fix(tf): tf notify-plan says it posts a comment and posts nothing #3644 and fix(mcp): the path audit finds path options by type and by name #3480 under ## Workflow follow-up.
4. "Four flags change" was wrong. Right. With the new rows, four flags change against main: pr_check_readiness, roadmap_refine, roadmap_run and ci. The text is correct now.
5. Long sentences. Right. I split the long sentences in the proposal, the design, the tasks, the spec and the test docstring.