fix(sequentialthinking): set readOnlyHint and idempotentHint to false

[cliffhall]

Closes #4721

Description

The sequentialthinking tool advertised readOnlyHint: true and idempotentHint: true. Both are wrong: every call appends to the server's in-memory thoughtHistory (and branches) and returns a growing thoughtHistoryLength, so the tool is stateful and non-idempotent. A client that trusts the hints may skip a confirmation, or retry a call, and get a duplicated history.

This PR sets both hints to false, with a comment saying why. destructiveHint and openWorldHint stay false: the tool only appends, and touches nothing outside the process.

Explicit false, not omitted. The spec defaults for a missing readOnlyHint and idempotentHint are already false, so dropping them would give clients the same value. They stay explicit anyway: AGENTS.md asks for the hints to be set on every tool, and an explicit false reads as a decision rather than an omission. It is also what the corrected pinning test asserts with toEqual.

Part of Wave 4 (sequentialthinking) of #5004. The pinning test in __tests__/tools-list.test.ts now asserts the corrected annotations, and its KNOWN BUG #4721 marker is gone (git grep "KNOWN BUG #4721" returns nothing).

Credit. Ported from #4722 by @Jack11111eee (against main), the first of the four candidate PRs (#4722, #4747, #4749, #4784). They all make the same two-value change. #4722 was picked because it was first and adds the explanatory comment. A plain cherry-pick does not apply: the root Prettier reformat on v2/main changed index.ts's indentation, and the existing tools-list suite already pins the annotations, so the regression assertion goes there rather than into input-schema.test.ts. The commit carries a Co-authored-by: trailer for the original author.

Server Details

• Server: sequentialthinking
• Changes to: the sequentialthinking tool's annotations (index.ts), its pinning test (__tests__/tools-list.test.ts), and a patch changeset. The tool's schemas, behavior and results are unchanged.

Motivation and Context

#4721. Tool annotations are hints that clients act on. Advertising a stateful tool as read-only and idempotent invites unsafe retries and confirmation skipping.

How Has This Been Tested?

Inspector CLI 2.9.0, legacy era (2025-11-25): tools/list, reading tools[0].annotations.

npx -y @modelcontextprotocol/inspector --cli node src/sequentialthinking/dist/index.js \
  --method tools/list --protocol-era legacy --format json | jq -c '.result.tools[0].annotations'

Build	Result
v2/main (before)	{"readOnlyHint":true,"destructiveHint":false,"idempotentHint":true,"openWorldHint":false}
this branch (after)	{"readOnlyHint":false,"destructiveHint":false,"idempotentHint":false,"openWorldHint":false}

Inspector CLI, modern era (2026-07-28): --protocol-era modern fails as expected, with Version negotiation failed: the server did not offer pinned protocol version 2026-07-28 via server/discover (no fallback in pin mode) and exit 1. This server is still on SDK 1.x and does not yet speak 2026-07-28 (that is #4852). It is not a regression from this change.

LLM client (Claude Code 2.1.289, headless, --strict-mcp-config against the local build). This client has no era switch, so it negotiates the SDK 1.x server's legacy era. It was asked: "Call the sequentialthinking tool of the seq server twice with identical arguments: thought='probe', thoughtNumber=1, totalThoughts=1, nextThoughtNeeded=false. Then reply with exactly one line: the thoughtHistoryLength from each call, comma separated." It returned 1,2. Two identical calls give different results, which shows the tool is not idempotent, as the new hints now say.

Tests: npx vitest run --coverage in src/sequentialthinking: 7 files, 99 tests pass. Per-file coverage: lib.ts 100% lines and 94.44% branches, so the 90% gate holds. npm run local:gate passes (exit 0).

Breaking Changes

None. No client configuration changes. Clients that read the annotations will now treat the tool as non-read-only and non-idempotent, which is the correct behavior.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Protocol Documentation
• My changes follow MCP security best practices
• I have updated the server's README accordingly (not applicable: the README does not state the annotation values; no doc in the repo does)
• I have added a changeset (npm run changeset) if this changes what a TypeScript server publishes
• I have tested this with an LLM client
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not applicable: annotation values only, no new code paths)
• I have documented all environment variables and configuration options (not applicable: none added)

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: 7a24a11

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@modelcontextprotocol/server-sequential-thinking	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Copilot review overview

🟡 Changes recommended

Comments and release notes inaccurately state that every call mutates branch state, and the test file’s purpose header remains stale.

Review effort: Balanced
Findings: 3

Open (3)

• Correct release note's description of conditional branch updates · New
• Update #4721 header to describe the fixed regression · New
• Clarify branch updates require both branch arguments · New

What changed in this PR

Corrects the sequentialthinking tool’s statefulness annotations and updates regression coverage.

Changes:

• Sets readOnlyHint and idempotentHint to false.
• Updates the protocol-level annotation assertion.
• Adds a patch changeset.

File	Description
src/​sequentialthinking/​index.ts	Corrects tool annotations.
src/​sequentialthinking/​__tests__/​tools-list.test.ts	Updates annotation regression coverage.
.changeset/​sequentialthinking-annotations.md	Documents the published fix.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[cliffhall]

Copilot round 1: 3 findings, all fixed in 7a24a11 (wording only, no behavior change).

• Changeset said every call appends to the branch map: now separates the unconditional history append from the branch append, which only happens when both branchFromThought and branchId are given.
• tools-list.test.ts file header still described sequential-thinking: readOnlyHint and idempotentHint annotations are inaccurate (server is stateful, non-idempotent) #4721 as a pinned known-wrong value: now describes it as fixed. The comment at the assertion is corrected the same way.
• index.ts annotations comment: same history/branch correction.

No suppressed comments. npm run local:gate passes on 7a24a11. (One earlier run hit a flake in memory's concurrent-mutations test, unrelated to this PR; it passed 5 out of 5 in isolation and the rerun of the gate was green.) Requesting round 2.

[Copilot]

Copilot review overview

🟢 Approval recommended

The annotations now match the implementation and specification, with appropriate regression coverage and release metadata.

Review effort: Balanced
Findings: None

Resolved since last review (3)

• Clarify branch updates require both branch arguments
• Update #4721 header to describe the fixed regression
• Correct release note's description of conditional branch updates

[cliffhall]

Copilot round 2: clean (no findings; all 3 round-1 findings shown as resolved). Review loop stopped on the clean-round exit.