Skip to content

Resolve document publish approvers from defaults#1471

Open
SachaProbo wants to merge 1 commit into
mainfrom
SachaProbo/mcp-publish-default-approvers
Open

Resolve document publish approvers from defaults#1471
SachaProbo wants to merge 1 commit into
mainfrom
SachaProbo/mcp-publish-default-approvers

Conversation

@SachaProbo

@SachaProbo SachaProbo commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

The MCP publishDocument tool required callers to pass approver_ids and to distinguish an omitted list (rejected) from an empty one (direct publish), a null-vs-empty subtlety that is awkward for an LLM to get right.

Drop approver_ids from the MCP tool and resolve a major publish's approvers from the document's default approvers instead: an approval is requested when the document has default approvers, otherwise the version is published directly. Default approvers are configured with addDocument or updateDocument.

Replace a document's default approvers on every major publish that supplies an explicit list, even when the list is empty, so a direct publish through the GraphQL API clears stale approvers instead of leaving them behind (previously the empty case skipped the update).

Expose the default-approver behaviour as a separate entry point, PublishVersionWithDefaultApprovers, that loads the defaults and delegates to PublishVersion. PublishVersion keeps its explicit-approver contract for the GraphQL API.

Require only the publish permission to publish a version, whether or not it opens an approval quorum, and drop the now-unused request-approval action.

Fold the publish steps into the publishMinor and publishMajor primitives shared by both the single and bulk publish paths, and drop the redundant InTx suffix from RequestApproval and emitDocumentEvent, which already take a transaction argument.


Summary by cubic

Major publishes in MCP now use a document’s default approvers: if defaults exist we open an approval quorum; if not, we publish directly. GraphQL still supports explicit approvers, and major publishes update the document’s default approvers (empty clears), with only the publish permission required.

Service +266 -165

  • Added PublishVersionWithDefaultApprovers in pkg/probo to load defaults and delegate to PublishVersion.
  • Major publishes now always update default approvers from the provided list (including empty).
  • Extracted publishMajor/publishMinor to handle single and bulk paths and emit version-published events.
  • Dropped request-approval action and its OAuth scope; publishing always authorizes with ActionDocumentVersionPublish.
  • Renamed RequestApprovalInTxRequestApproval and emitDocumentEventInTxemitDocumentEvent; updated all call sites, including generated document flows and bulk publish.

MCP +7 -18

  • publishDocument tool no longer accepts approver_ids; it resolves approvers from document defaults via PublishVersionWithDefaultApprovers.
  • Updated specification.yaml to reflect the simpler input and new default-approver behavior.

GraphQL API +1 -6

  • publishDocument now always authorizes with the publish action only.

Tests +107 -0

  • Added MCP e2e tests for approval quorum when defaults exist and direct publish when not.
  • Added console e2e test verifying that passing an explicit empty approver list on a major publish clears defaults and publishes.

Written for commit 6dfb33c. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found across 10 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread pkg/server/api/mcp/v1/specification.yaml
Comment thread pkg/probo/document_service.go Outdated
@SachaProbo SachaProbo force-pushed the SachaProbo/mcp-publish-default-approvers branch from 02652f9 to 52da1a0 Compare July 7, 2026 18:39
The MCP publishDocument tool required callers to pass approver_ids and
to distinguish an omitted list (rejected) from an empty one (direct
publish), a null-vs-empty subtlety that is awkward for an LLM to get
right.

Drop approver_ids from the MCP tool and resolve a major publish's
approvers from the document's default approvers instead: an approval is
requested when the document has default approvers, otherwise the version
is published directly. Default approvers are configured with addDocument
or updateDocument.

Replace a document's default approvers on every major publish that
supplies an explicit list, even when the list is empty, so a direct
publish through the GraphQL API clears stale approvers instead of
leaving them behind (previously the empty case skipped the update).

Expose the default-approver behaviour as a separate entry point,
PublishVersionWithDefaultApprovers, that loads the defaults and
delegates to PublishVersion. PublishVersion keeps its explicit-approver
contract for the GraphQL API.

Require only the publish permission to publish a version, whether or not
it opens an approval quorum, and drop the now-unused request-approval
action.

Fold the publish steps into the publishMinor and publishMajor primitives
shared by both the single and bulk publish paths, and drop the redundant
InTx suffix from RequestApproval and emitDocumentEvent, which already
take a transaction argument.

Signed-off-by: Sacha Al Himdani <[email protected]>
@SachaProbo SachaProbo force-pushed the SachaProbo/mcp-publish-default-approvers branch from 52da1a0 to 6dfb33c Compare July 7, 2026 18:57

@lukkor lukkor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

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.

2 participants