Skip to content

feat(mcp)!: tools that decide a merge ask the person through the client - #691

Merged
vriesdemichael merged 8 commits into
nextfrom
cc/mcp-asks-620
Sep 26, 2026
Merged

vriesdemichael merged 8 commits into
nextfrom
cc/mcp-asks-620

Conversation

@vriesdemichael

Copy link
Copy Markdown
Owner

Implements #620 as decided in the preparation: tools that decide a merge ask the person through the MCP client, and --yolo goes.

What changes

  • Every tool is listed.
    • merge_pull_request, enable_auto_merge, disable_auto_merge, submit_pr_review, set_build_status and create_tag ask the person to confirm each call in the client before they run.
    • update_pull_request asks whenever a call sets the draft flag. Live-verified on 10.4.3: a draft cannot merge or have auto-merge set, and making one a draft cancels its auto-merge. A title-only update leaves the flag alone, so it runs without asking.
  • The confirmation is an elicitation with one required, unticked checkbox naming the target ("Merge feat: implement CLI commands for project, repo, browse, and commit/ref #42 into main"). The tool acts only on an accept.
    • A 2026-07-28 client gets it as an input request (multi round-trip). go-sdk sends it itself to a handshake-era client.
    • A client that cannot show one gets -32021 MissingRequiredClientCapability with {"requiredCapabilities":{"elicitation":{"form":{}}}}, whatever revision it speaks, and nothing reaches Bitbucket.
  • An answer accepts only the call it was asked about.
    • The request state is HMAC-signed with a per-process key and decoded strictly. It binds the tool, a digest of the scoped arguments, a 10-minute expiry, and a nonce used once. Tampered, re-encoded, cross-argument, replayed and state-less answers are refused.
    • A merge and an auto-merge are held to the pull request version the person was shown. One pushed to meanwhile gets Bitbucket's 409 instead of merging.
    • A decline or cancel that arrives after the expiry still stands. Only a late accept is refused, with a request to ask again.
  • Why the confirmation is in the handler. It is one wrapper that askingSpec applies, not code in the middleware, because go-sdk's bridge for handshake-era clients sits below the middleware.
  • Flags.
    • --read-only exposes only the read-only tools.
    • --yolo and --allow-writes are accepted, do nothing, and warn from the deprecation registry until v6.
    • bb ai mcp tools reports asks (always, never, when-setting-draft), prints a header row, and takes --read-only.
    • --safe-only, safe and exposure are deprecated the same way (ADR-084).
  • Hints. Every tool declares all four hints and a title, as the spec defines them, independent of asking. openWorldHint is false, since one Data Center instance is a closed domain; the code notes to flip it if bb ever serves Cloud.
  • Server. It advertises tools and nothing else: no deprecated logging, no listChanged. It sends instructions.
  • Audit. Records gain confirmation (accepted, declined, cancelled, unavailable). Any call the confirmation stops is status: denied, including one whose answer cannot be used, and one decision is one record.
  • Records.
    • ADR-098 records the decision and amends ADR-039, 061 and 062.
    • amended_by may now be a list, since ADR-039 is amended twice.
    • ADR-067 and AGENTS.md list the renamed and new guards.

Also fixed: a project key or slug could steer a request to another endpoint

Keys and slugs were written into request paths unescaped. Against 10.4.3, add_pr_comment with repo: "payments/pull-requests/7/merge?version=3#" POSTed to the merge endpoint, and Bitbucket merged the pull request. That is a tool that never asks doing what the asking tools exist to hold back.

ValidateRepository now refuses a key or slug containing / \ ? # %, whitespace or control characters, or equal to . or ... No real key or slug does. The services that build a path themselves escape each segment. This is its own commit, fix(repo), so it can be cherry-picked.

Tests

  • Unit, both protocol eras:
    • accept, decline, cancel and an unticked box;
    • -32021 and its data shape;
    • update_pull_request asking only when draft is set;
    • the form's shape;
    • forged, altered, re-encoded, cross-argument and replayed states;
    • late answers (a decline stands, an accept is refused);
    • a nonce kept to the last second its state is valid;
    • one audit record per decision, and denied for an unusable answer;
    • injected slugs refused before any request.
  • Guards, each broken first to see it fail:
    • TestToolsThatAskAreTheOnesThatDecideAMerge
    • TestReadOnlyToolsDoNotAsk
    • TestEveryToolDeclaresItsHintsAndTitle
    • each confirmation fix above
  • Live:
    • every tool that asks is listed, refused with -32021 for a client that cannot ask, and refused on decline, with every value read back unchanged;
    • the merge question names the target and the quoted title, and merges on accept;
    • a draft is refused before anyone is asked;
    • a pull request pushed to while the question is open is neither merged nor has auto-merge set: Bitbucket's stale-version 409 is asserted, and dropping the pin makes the pull request merge at once;
    • a title-only update of a draft runs without asking, and the pull request stays a draft;
    • the --read-only listing;
    • the conformance sweep with an accepting client.

Refs #620

🤖 Generated with Claude Code

vriesdemichael and others added 7 commits September 26, 2026 16:46
amended_by took one number, so a record amended twice could name only
one of the records that changed it, and the validator refused the other
end of the link. It takes a list now, as amends does, in the validator,
the page export and the test that reads the records.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every tool is exposed. merge_pull_request, enable_auto_merge,
disable_auto_merge, submit_pr_review, set_build_status and create_tag ask
the person to confirm each call through the MCP client before they run,
and update_pull_request asks when it changes the draft flag: a draft
cannot merge, and making one a draft cancels its auto-merge. The
confirmation is an elicitation with one required, unticked checkbox
naming the target, and the tool acts only on an accept. A client that
cannot show one gets -32021 MissingRequiredClientCapability, whatever
revision it speaks, and nothing reaches Bitbucket.

An answer accepts only the call it was asked about. The request state is
signed with a key drawn per process and holds the tool, a digest of the
scoped arguments, the pull request version a merge was shown, which the
merge is then held to, an expiry, and a nonce used once. The confirmation
runs in the handler, one wrapper that askingSpec applies, because
go-sdk's bridge for handshake-era clients sits below the middleware.

--read-only exposes only the read-only tools. --yolo and --allow-writes,
bb ai mcp tools --safe-only, and the tools listing's safe and exposure
fields do nothing and are deprecated until v6. The listing reports asks
and takes --read-only.

Every tool declares all four hints and a title, by what the specification
defines them to mean; openWorldHint is false, since one Data Center
instance is a closed domain. The server advertises tools and nothing
else, and sends instructions. The audit record adds confirmation, a
refused confirmation is status denied, and one decision is one record.

ADR-098 records the decision, amending ADR-039, ADR-061 and ADR-062.

BREAKING CHANGE: bb ai mcp serve lists every tool, and a client that
cannot show an elicitation gets error -32021 for the tools that ask
instead of not seeing them. --yolo no longer makes them run unasked.

Refs #620

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The AI page, llms.txt, enterprise hardening and the threat model said the
tools that decide a merge were withheld unless --yolo. They ask the
person through the client now, a client that cannot ask gets -32021, and
--read-only is the server for a client nobody trusts with that. The audit
section shows the confirmation field, and the threat model counts the
confirmation as the control for TB-5 and T-4, as strong as the client
that answers it.

Refs #620

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r --safe-only

The catalogue test held --safe-only to leaving out merge_pull_request.
Every tool is exposed now, so the deprecated flag lists them all, and
--read-only is the filter that leaves out the tools that write.

Refs #620

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ment

A project key or slug is written into the path of every request that acts
on a repository. A slug of "repo/pull-requests/7/merge?version=3#" sent
add_pr_comment's POST to the merge endpoint, and Bitbucket merged the pull
request (confirmed against 10.4.3). Through bb's MCP server, a tool that
never asks could do what the tools that ask exist to hold back.

ValidateRepository now refuses a key or slug holding a slash, a backslash,
?, #, %, whitespace or a control character, or one that is . or ..; no
real key or slug does. The services that build a path themselves escape
each segment as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l stand

Follow-ups from reviewing the confirmation:

- enable_auto_merge is held to the pull request version the person was
  shown, as merge_pull_request is. Setting auto-merge merges at once when
  nothing holds a pull request back, and without the pin one that moved
  after the question merged unseen (seen live); it now gets Bitbucket's 409.
- A decline or cancel that arrives after the confirmation expired stands.
  Only a late acceptance is refused, and the model is told to ask again.
- A request state has one spelling: it is decoded strictly, so another
  encoding of the same bytes is refused as malformed.
- An answered state's nonce is remembered for as long as the state can be
  accepted, to the second.
- A call refused because its answer cannot be used is audited as denied.
- -32021 names form elicitation, {"elicitation":{"form":{}}}, so a client
  that declared URL mode alone sees what it lacks.
- update_pull_request's asks value is when-setting-draft: it asks whenever
  a call sets the flag, whether or not the flag changes.
- bb ai mcp tools prints a header row.
- A title-only update of a draft is live-tested to run without asking and
  to leave the pull request a draft.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ere withheld

ADR-098 says a merge and an auto-merge are held to the version the person
was shown, that update_pull_request asks for a call that sets the draft
flag, and that any call the confirmation stops is audited as denied. The
v5.0.0 release notes name the four tools --yolo used to expose instead of
"the first four", which named the wrong ones, and add the version pin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread docs/release-notes/v5.0.0.md Outdated
* `bb <group> --describe` and `bb --describe` list their commands with what `--dry-run` does for each, rather than saying a group has nothing to describe. A path that names no command answers with `description.error`, exiting `0` under `--json`.
* There is no warning period: one document cannot carry the old description beside the new one.
5. **MCP tools that decide a merge ask you, through the client, instead of being withheld ([ADR-098](https://vriesdemichael.github.io/bitbucket-data-center-cli/latest/adr/098-mcp-tools-that-decide-a-merge-ask-the-person/), [#620](https://github.com/vriesdemichael/bitbucket-data-center-cli/issues/620)):**
* `bb ai mcp serve` exposes every tool. Merging, enabling or disabling auto-merge, submitting a review, reporting a build status, creating a tag, and setting a pull request's draft flag ask you to confirm each call in the MCP client, and run only when you accept. Before, merging, enabling auto-merge, submitting a review and reporting a build status were withheld unless the server started with `--yolo`, and then ran with nobody asked.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This piece reads too technical. What doea it mean for a user.
Adoption of a more modern permission system with elicitation.
Defaults to what previously was yolo mode (with the elicitation for permission on the client side instead of the server side). The non permissive option is opt in instead of the default.

Also this document has wonky encoding, fix while you are at it.

## Agent Instructions

Add a tool with mcp.AddTool through the toolSpec helper in internal/mcp/server.go. Define an input struct and an output struct; do not hand-write a schema. Fields without omitempty become required, fields with it become optional, and the jsonschema struct tag carries the description. The output struct always names its payload. A handler returning a collection returns struct{ Commits []T `json:"commits"` }, never a bare slice and never a generic items key. Name a single object too. A model reads a fresh tool result on every call with no parser holding a field path, so the name is what tells it what it is looking at. Return a Go error for a failure. The SDK packs it into the result content with IsError set, so there is no need to build an error result by hand, and no need to return a nil error alongside one. Add an entry to toolArguments in the client compatibility test under internal/mcp in the same change. TestEveryToolHasCallArguments fails without one, and without it the new tool is never called by any test. Do not add an enum to an input schema for a value the service layer normalises. Pinning the upper-case spelling of a role rejects "author", which works today. Put the permitted values in the field description instead. Enums are for vocabularies the service does not widen. Do not reintroduce a versioned envelope here. The CLI's --json surface has one for a different consumer; see the rationale.
DestructiveHint is no longer derived from a Safe classification: ADR-098. Declare every hint by its definition, with readOnly or writes. Add a tool with mcp.AddTool through the toolSpec helper in internal/mcp/server.go. Define an input struct and an output struct; do not hand-write a schema. Fields without omitempty become required, fields with it become optional, and the jsonschema struct tag carries the description. The output struct always names its payload. A handler returning a collection returns struct{ Commits []T `json:"commits"` }, never a bare slice and never a generic items key. Name a single object too. A model reads a fresh tool result on every call with no parser holding a field path, so the name is what tells it what it is looking at. Return a Go error for a failure. The SDK packs it into the result content with IsError set, so there is no need to build an error result by hand, and no need to return a nil error alongside one. Add an entry to toolArguments in the client compatibility test under internal/mcp in the same change. TestEveryToolHasCallArguments fails without one, and without it the new tool is never called by any test. Do not add an enum to an input schema for a value the service layer normalises. Pinning the upper-case spelling of a role rejects "author", which works today. Put the permitted values in the field description instead. Enums are for vocabularies the service does not widen. Do not reintroduce a versioned envelope here. The CLI's --json surface has one for a different consumer; see the rationale.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this timeless, don't give an update on what was, but state it as it is now

The v5.0.0 note on the MCP server leads with what changes for a user: the
server asks permission through elicitation in the client, offers every tool
by default as --yolo did, and --read-only is the opt-in lock-down. The note
is plain ASCII again; its one ellipsis showed up garbled in tools that read
the file as ANSI.

The lines ADR-039, ADR-061 and ADR-098 gained state the rules as they are
rather than what they replaced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vriesdemichael
vriesdemichael enabled auto-merge (rebase) September 26, 2026 18:38
@vriesdemichael
vriesdemichael merged commit b4257d0 into next Sep 26, 2026
20 checks passed
@vriesdemichael
vriesdemichael deleted the cc/mcp-asks-620 branch September 26, 2026 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant