DMD-1701: RFC — merge request MCP tools (non-SOX) by martinsifra · Pull Request #612 · keboola/mcp-server

martinsifra · 2026-07-02T13:13:15Z

Description

Linear: DMD-1701

Change Type

Major (breaking changes, significant new features)
Minor (new features, enhancements, backward compatible)
Patch (bug fixes, small improvements, no new features)

RFC only — no code yet, so no change-type/version bump applies. This PR is for design review per CONTRIBUTING.md (new tools require an agreed RFC before implementation).

Summary

Adds feature_spec/branches_merge_requests_mcp/RFC.md — a design proposal for exposing the non-SOX Branches 2.0 merge-request flow (branches-merge-requests) through MCP tools, for the "Branches 2.0 - MCP integration" milestone.

Highlights:

Maps the nine Connection MR endpoints + state machine and the non-SOX specifics (0 default approvals, admin/share-only merge, async merge job).
Proposes a tiered toolset: Tier A MVP (get_merge_requests, get_merge_request, create_merge_request, request_merge_request_review, merge_merge_request, a publish_branch orchestrator, and read-only get_branches) + Tier B parity (approve, request_changes, update).
publish_branch is the "MCP simplifies the process" tool for less-technical AI-chat users: one call that find-or-creates the MR, checks conflicts, requests review, merges, and stops with a plain-language next_step at any gate needing a human.
Feature + role gating via the existing ToolsFilteringMiddleware; ID→name resolution and human-readable changelog summaries.

Scope decisions baked in: MR flow + read-only branch listing only — no branch create/switch (deferred; needs session-state/workspace architecture work). SOX flow explicitly out of scope.

Testing

Tested with Cursor AI desktop (Streamable-HTTP transports)

N/A — documentation/RFC only, no runtime changes.

Checklist

Self-review completed
Unit tests added/updated (if applicable)
Integration tests added/updated (if applicable)
Project version bumped according to the change type
Documentation updated (if applicable)

🤖 Generated with Claude Code

linear · 2026-07-02T13:13:18Z

DMD-1701

AI-3607

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Consolidate get_merge_requests + get_merge_request into one tool (no IDs = list all, IDs = batched details) and align get_branches; keep action tools single-target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Target is server-forced to the default branch and the source can only be the current session branch, so both are resolved internally; error when on the default/production branch. Cross-branch MRs deferred to Tier C. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Nothing in the MR flow consumes a branch list (create_merge_request resolves branches internally, get_project_info already surfaces the current branch), so get_branches has no concrete use until branch create/switch lands (Tier C). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- Finalize tool set + renames (submit_merge_request_for_review, reject_merge_request); drop update_merge_request, cancel (no endpoint), get_branches (Tier C) - Three-axis gating (feature / role / session-branch); merge kept branch-only - Conflict resolution flow: per-config three-way diff + rebase, on a dev-branch session - Status object (blocked_reason / approvals / next_step) driving X/Y chat flows - Document verified backend facts: atomic merge, live conflicts, rebase keeps approvals, branch locked only in in_merge, no cancel endpoint Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- Add explicit MR-id convention: required for approve/reject, optional (resolved from session branch, validated when given) for branch-only tools; resolves the table vs. chat-flow inconsistency - Branch resolution: _resolve_branch_context returns only one branch, so a new resolve_branch_pair helper is needed (new code, not a plain reuse) - Async merge: no job-polling helper exists (jobs.py only creates/reads); the await loop must be written, following the workspace.py pattern - Clarify get_merge_requests is a simplified variant of get_configs, not a 1:1 mirror - Cover the above in the unit-test list Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Collapsing create->review->merge into one call would hide the review gate (user never sees the changelog before production), duplicate merge_merge_request as a second destructive merge path, and orchestrate only the trivial case since it must stop at conflicts and missing approvals anyway. Final toolset: 8 tools. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Replace prose descriptions with a normative contract: explicit params/defaults for all 8 tools and full Pydantic result models (MergeRequestStatus, MergeResult, ConfigDiff, ResolveConflictResult, ...). Consistency fixes surfaced by writing the signatures out: - resolve_config_conflict's merge_request_id is a guard only, never sent to the rebase endpoint (which is addressed by component+config on the session branch) - spell out rebase argument rules (full replacement, rows required, delete=tombstone) - flow X is two steps: list mode returns summaries without status by design - drop the duplicated pseudo-shape of the status object; the model is the source of truth Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Verified every model against the real PHP OpenAPI schemas; fixes found: - resolve_config_conflict was missing the REQUIRED `version` param (default-branch version to re-anchor onto) and the full content field set; delete = send only `version`, omitting all content fields (per RebaseRequest) - ConflictRef now mirrors the conflicts endpoint: adds `message` + version identifiers, drops `name` (not returned there; would cost a lookup per conflict) - ConfigDiff sides are ConfigurationVersionResponse snapshots (version/isDeleted + nested ConfigurationDiffData), not raw dicts; flattened for the agent - MergeRequest gains merged_by (merge.mergerName) - Document provenance (which fields are the API's vs. our derived status object) and the deliberate field translations (null->pending, ''->None, dropped externalId) - Flag ChangelogSummary as UNVERIFIED: the API types changeLog as a bare object, so the assumed keys need a real payload before the humanizer is written Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Verified the real changeLog shape in Model_Row_MergeRequest::updateChangeLog: {"configurations":[{componentId, configurationId, lastVersionIdentifier, isDeleted}]} - The assumed addedConfigs/modifiedConfigs/deletedConfigs keys were wrong; there is no added-vs-modified distinction, so an "N added, M modified" summary is not derivable. ChangelogSummary and the humanizer are dropped in favour of a 1:1 pass-through (changed_configurations) the agent narrates on demand. - It is populated at request-review / skip-review, not at merge, so the list is empty while the MR is in development. Documented. - MergeResult carries no change list (merge returns a Job, not the MR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…nature gaps Role gating (the serious one) — resolved the opposite way to the review's proposal, after checking StorageRouteGuard::canAccessInMergeRequestsProject: the guard is a strict whitelist and #[MergeRequestsAllowedRoles] is [ADMIN, SHARE] on every non-SOX write. REVIEWER/DEVELOPER live in #[ProtectedBranchAllowedRoles] (SOX only). So the Background bullet was wrong, not the gating; widening create to developer would only surface tools that 403. Corrected, with the persona consequence named. Also: - Add the normative status derivation table (state x conflicts x approvals -> mergeable/blocked_reason/action_required), define not_ready and wait_for_merge, and fix precedence (conflicts before needs_approval) - Document request costs: create = POST + 1 /conflicts; detail = 1 + 2N; list = 1 - resolve_config_conflict: is_disabled must be tri-state (bool | None); theirs is guaranteed present for a conflicting config, explicit failure otherwise - Note auto_merge='none' is a real API value; job_id is a string per JobResponse; state filtering is client-side - Correct the "every state-changing tool returns MergeRequestStatus" overstatement - Flow X from a production session ends in a handoff Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tomasfejfar · 2026-08-07T06:52:19Z

+
+# ---- Author / promotion — dev-branch session only, admin/share ----------------------
+async def create_merge_request(
+    title: str,


I am not sure how exactly the branch session is supposed to work given the MCP is sessionless. But generally, I think it would be best if part of the tool course was the branch ID where ever the merge request ID is not present. That's essentially what the documentation suggests for sessionless that you pass the session context. as parameters to the tool calls.

tomasfejfar · 2026-08-07T06:53:13Z

+    auto_merge_at: str | None = None,               # ISO-8601; required iff auto_merge='scheduled'
+) -> MergeRequestDetail: ...                        # costs +1 /conflicts call — see Resolution Strategy §2
+
+async def submit_merge_request_for_review(


Why is there a separate submit merge request for review tool? To me it's updating the merge request state, no?

tomasfejfar · 2026-08-07T06:54:06Z

+) -> MergeResult: ...                               # awaits the job; refusal -> MergeResult.status
+
+# ---- Conflict resolution — dev-branch session only, admin/share ---------------------
+async def get_merge_request_conflicts(


Why is there a separate tool to get the conflicts? It could be part of the merge requests detail. Also it's GET. Why is it limited to admin and share roles?

tomasfejfar · 2026-08-07T06:55:06Z

+    merge_request_id: int | None = None,            # MR-id convention
+) -> MergeRequestConflictsOutput: ...               # conflict list + three-way diff per config
+
+async def resolve_config_conflict(


What exactly does the conflict resolution do? My assumption would be that you see the conflicts and you update the configuration so that there is no conflict. Why does it need a specific Endpoint.

tomasfejfar · 2026-08-07T06:57:26Z

+ActivityEventType  = Literal['review_requested', 'approved', 'changes_requested',
+                             'merged', 'canceled']
+
+class Reviewer(BaseModel):


I assume the ID is a user ID, right?

tomasfejfar · 2026-08-07T06:59:36Z

+class ConflictRef(BaseModel):               # exactly what GET /merge-request/{id}/conflicts returns
+    component_id: str
+    configuration_id: str
+    message: str                            # backend's human-readable conflict description — reuse it


What exactly does this mean? How exactly will it look?

tomasfejfar · 2026-08-07T07:00:39Z

+    created_at: str
+    merged_at: str | None
+    merged_by: str | None                   # `merge.mergerName`; set for system merges too
+    links: list[Link]


What are those links?

tomasfejfar · 2026-08-07T07:00:59Z

+    merged_by: str | None                   # `merge.mergerName`; set for system merges too
+    links: list[Link]
+
+class MergeRequestDetail(MergeRequest):


I think this could show the conflict in line.

tomasfejfar · 2026-08-07T07:01:57Z

+class MergeRequestsDetailOutput(BaseModel):
+    merge_requests: list[MergeRequestDetail]
+
+class ConfigVersionSnapshot(BaseModel):     # = ConfigurationVersionResponse


I don't think the MCP is able to fetch a specific version currently, so this will need to be added.

tomasfejfar · 2026-08-07T07:04:04Z

+    # including it would cost an extra fetch. If the user asks what was merged, the agent
+    # re-reads get_merge_requests(merge_request_ids=[id]).
+
+class ResolveConflictResult(BaseModel):


I don't get the resolve conflict endpoint again.

tomasfejfar

#Agentic review

Presented semi-verbatim as guidelines for your agent to verify against ground truth. Might be wrong.

2. A successful merge deletes the source dev branch

MergeDevBranchJob.php:180-188 enqueues DevBranchDelete after publish. That drops the branch's
buckets, table data, files and workspaces.

The session that issued the merge dies with it — clients/client.py:104-111 then raises
Branch "<id>" not found on the next call. This is the happy path, every time. The RFC did
not mention it anywhere: not in the tool description, not in the result model, not in the chat
flows.

The required-approvals count is readable only via #[ManageRoute]
GET /manage/projects/{id}/metadata (RequiredApprovalsCountProvider). It is never serialized
into an MR response, so a Storage token cannot read it. Approvals.required had no source.

1. `POST …/rebase` accepts no `rows` — and a rows-only payload is read as a delete

RebaseRequest.php:52-68 defines CONTENT_PARAMS = [name, description, configuration, changeDescription, isDisabled]. There is no rows.

Two compounding facts make this worse than a dropped field:

allowExtraFields: true (RebaseRequest.php:110) — a rows key is accepted and silently
discarded. No 400, no warning.
isDeleteResolution() (RebaseRequest.php:168-176) classifies a payload carrying no
content param as a delete. So {version: N, rows: [...]} — the exact shape the RFC
specified — writes a tombstone.

Also: on a keep, name is required, and a partial keep defaults configuration to '{}'.

Consequence: Phase 1 rebase is config-only. Any row work done in the dev branch is replaced
by the default branch's rows, silently. The RFC as written would have shipped a data-loss path
labelled "(write)".

JobResponse.id is documented string but emitted as int.

token_role in ('admin', 'share') hides every MR write from OAuth sessions. The existing
flow gate has an explicit or is_oauth carve-out (mcp.py:403, :475) precisely for this.
Without a matching carve-out, the in-platform Kai persona cannot use any of the new tools.

Feasibility & risk

Critical

SessionStateMiddleware strips branch_id for every */list (mcp.py:214-217), so
is_client_using_main_branch() is always True at list time. A list-time branch filter would
hide the entire promotion path in every session. The branch axis can only be evaluated at
call time.
"Hide" and "deny" are separate code paths that have already drifted in this codebase.
Approvals.required has no reachable source — and is redundant with state.
No integration-test project exists that can legally run merge/conflict scenarios.
OAuth persona lockout (concurs with codebase alignment).
Request-cost figures are wrong and are prescribed as unit-test assertions.

Important

_await_storage_job needs concrete specs: 2 s interval, 120 s cap, terminal set, never raise.
unwrap_results is all-or-nothing; use process_concurrently.
Cap merge_request_ids.
branch_from_name is unfillable for canceled MRs.
get_merge_request_conflicts had a role contradiction against the fact-check findings.
Sequencing: the read path is the real first slice.

Fresh perspective

Critical

Merge deletes the branch and kills the session.
Merge-from-development reaches production in two calls.
The rebase contract is wrong twice — no rows, and delete is inferred from absence.
The LLM should not author the merged config when the server can. An explicit
resolution: 'ours'|'theirs'|'custom'|'delete' enum removes the whole class of
hallucinated-config bugs for the common cases.
CONFIG_DIFF_PREVIEW_TAG exists in the repo and the RFC does not use it.
create_merge_request 404s on the repeat scenario (one MR per branch, ever).

Enforcement lives in authorize_tool_call (mcp.py:424), which is also reused by
preview.py:300 — a gate added there affects both.

ProjectLinksManager._url() branch-prefixes unconditionally.

A Storage-job poller already exists at workspace.py:765-791; the RFC proposed writing one.

Models need Field(description=…) + AliasChoices to match repo conventions.

generate_tool_docs.py needs a category entry.

creator_name has no resolution path (no user-listing endpoint).

Fresh perspective

Critical

Merge deletes the branch and kills the session.
Merge-from-development reaches production in two calls.
The rebase contract is wrong twice — no rows, and delete is inferred from absence.
The LLM should not author the merged config when the server can. An explicit
resolution: 'ours'|'theirs'|'custom'|'delete' enum removes the whole class of
hallucinated-config bugs for the common cases.
CONFIG_DIFF_PREVIEW_TAG exists in the repo and the RFC does not use it.
create_merge_request 404s on the repeat scenario (one MR per branch, ever).

Important

Make merge_request_id required on merge — an implicit "the current branch's MR" default on
the single irreversible operation is the wrong default.
Deep-link the handoff to the UI.
Rename version → rebase_onto_version (it is a default-branch version, which reads as the
opposite).
Rename reject_merge_request → request_changes_on_merge_request (the backend transition is
not a rejection).
reviewer_ids is unusable — no user-listing tool exists to obtain ids.
submit_for_review emails every project member.
List mode is unpaginated.
Add an error-taxonomy table.

Nothing states what merging does not promote: config state, keboola.sandboxes, tables,
buckets, files, triggers, schedules.

needs SOX hard guard, because SOX reads the wrong approvals key, and would report mergeable=true on an MR that needs 2 approvals.

stateless_http=True means no elicitation exists — destructiveHint plus the tool
description is the only confirmation convention available.
State the project-scope confinement explicitly.
Pin the polling numbers.
Cap merge_request_ids.

Project standards

Critical

Four inventory-coupled tests in tests/test_server.py break mechanically when tools are added.
Gating tests belong in tests/test_mcp.py::TestToolsFilteringMiddleware, and the branch axis
needs a new sibling test with inverted polarity (allowed off the default branch).
Per-tool annotations are unspecified.
Integtest infrastructure is a five-part precedent (project, env var, CI secret, README rows,
fixtures) and none of it was in Scope.
JsonDict unusable.

Important

Field(description=…) needed on ~90 model fields.
State the model layering.
The poller already exists.
ProjectFeature is not what gates.
Draft the docstrings.
Register the tag.
@tool_errors(recovery_instructions=…).

= tuple() not = ().
The tag name should be plural.
isort missing from the tox list.

Open decisions the review surfaced (not the author's alone)

OD1 — does the write gate lock out the OAuth in-platform Kai persona?
The gate token_role in ('admin', 'share') is correct for a Storage token. It is unknown what
verify_token()['admin']['role'] returns for a production OAuth bearer session. If it is not
admin/share, Kai cannot use any MR write tool. The existing flow gate solved this with
or is_oauth. Blocking fact to establish first: the actual role value on a production
OAuth session.

OD2 — Phase 1 rebase cannot carry rows.
Either ship resolve_config_conflict with a client-side refusal when
ours.rows != theirs.rows (option A — narrower, safe, useful for config-only conflicts), or
defer the tool entirely until the backend supports row reconciliation (option B). Needs the
Phase-2 timeline from the Connection team — an external conversation.

OD3 — no integration-test project exists that can legally run merge/conflict scenarios.
Needs a dedicated, locked, feature-enabled project plus env var, CI secret and README rows.

martinsifra force-pushed the ms/merge-requests/dmd-1701 branch from 5be5734 to ebebad9 Compare August 3, 2026 10:57

martinsifra and others added 4 commits August 3, 2026 14:07

DMD-1701: add RFC for merge request MCP tools (non-SOX)

4b881fa

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

martinsifra force-pushed the ms/merge-requests/dmd-1701 branch from ebebad9 to 9cad742 Compare August 3, 2026 15:26

martinsifra and others added 7 commits August 4, 2026 11:12

tomasfejfar reviewed Aug 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

DMD-1701: RFC — merge request MCP tools (non-SOX) - #612

DMD-1701: RFC — merge request MCP tools (non-SOX)#612
martinsifra wants to merge 11 commits into
mainfrom
ms/merge-requests/dmd-1701

martinsifra commented Jul 2, 2026

Uh oh!

linear Bot commented Jul 2, 2026 •

edited by linear-code Bot

Loading

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar Aug 7, 2026

Uh oh!

tomasfejfar left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

martinsifra commented Jul 2, 2026

Description

Change Type

Summary

Testing

Checklist

Uh oh!

linear Bot commented Jul 2, 2026 • edited by linear-code Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomasfejfar left a comment

Choose a reason for hiding this comment

2. A successful merge deletes the source dev branch

1. POST …/rebase accepts no rows — and a rows-only payload is read as a delete

Feasibility & risk

Fresh perspective

Fresh perspective

Project standards

Open decisions the review surfaced (not the author's alone)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

linear Bot commented Jul 2, 2026 •

edited by linear-code Bot

Loading

1. `POST …/rebase` accepts no `rows` — and a rows-only payload is read as a delete