Skip to content

fix(service-automation): a borrowed token refused at mcp tool discovery marks its connection, not the client secret - #28598

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-l5g8ho
Nov 1, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-l5g8ho

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

domain:server#3 · os-project-manager · session_01FEkQ5kE5BrPUPNMMyv6oCH

Requested by Leehom · project thread

Part of #26702 (the card stays open for item 7, the Console, which is domain:ui).

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:mcp 连接器实例(oauth2 授权码模式)列工具时用的是「发现组织」那条连接借来的令牌(裁决 B)。服务器拒绝这个令牌时,现在把这次拒绝记在那条连接上(状态 reconnect_required,原因写平台自己的话),不再把实例的 client secret 标成 rejected;实例改用下一个已连接组织列工具,没有就挂起为「需重新连接」,该组织重新连接后立即重新列工具。
  • 为什么改:评审读数 6553121554:以前这种拒绝走静态凭据的路,冤枉了 client secret,而且实例的签名里只有发现组织 id,同一组织重新连接也不会重试,只能改声明或重启。本 PR 先在测试里复现了这三点。
  • 风险与回滚:只影响「mcp + 授权码 + 列工具时被拒」这一条路径。静态凭据(bearer / api-key / basic)、实例自己换取的令牌(client_credentials、app-credential)照旧标 client secret。另一副本恰在此刻重新连接时不覆盖新授权,实例按退避重试。回滚即 revert。
  • 先看:packages/services/service-automation/src/plugin.ts 的 holdConnectorForRefusedConnection 与物化 catch 里新加的那一支;同文件 connectionStates 读取处新增的两个字段;connector-mcp-authorization-door.test.ts 新的 describe 块(connector-mcp/src/mcp-provider.ts 只改了一段注释)。
  • 两分钟验证:pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/connector-mcp-authorization-door.test.ts → 18/18 通过(新增 4 个用例)。
  • 需要你做的:无,席位落地。

What changes

A discovery refusal of a connection-borrowed token is attributed to the connection, not to the instance's client secret.

Where. packages/services/service-automation/src/plugin.ts, the declarative connector reconcile.

  • While an mcp instance with an oauth2 authorization_code grant materializes, its tool listing asks in the discovery organization's name (ruling B, 6545215904). That organization and its connected_at are now kept for the materialization (discoveryConnection).
  • When the factory throws a credential rejection and a discovery connection was used, the new holdConnectorForRefusedConnection runs instead of holdConnectorForRejectedCredential:
    • it reads that organization's connection and, while it is still the same connect (connected_at unchanged, status connected), calls the store's existing markReconnectRequired with a platform sentence (discoveryTokenRefusedReason), never the upstream's text;
    • the client-secret row the instance's credentialRef names is not touched;
    • the instance is held (a live connector keeps serving, as on every failed re-materialization) and a reconcile is queued. The connection's change moves the instance's signature, so that reconcile lists as the next connected organization, or holds the instance "reconnect required" (husk kind connection) until a reconnect, which lists again.
    • When the connection cannot be marked (the store fails, or a reconnect landed since), the instance is degraded on the feat(connector-mcp): declarative provider — non-fatal / lazy boot for unreachable MCP servers (ADR-0096 follow-up) #3017 backoff instead, so it is retried rather than held on a key nothing moves.
  • The held instance's reason now prefers the reason the reconnect_required row records (reconnectRequiredReason on the connection state). The grant handle writes the same sentence it always produced, so its path reads unchanged; this path's row carries the discovery sentence.

The comment this change made false. packages/connectors/connector-mcp/src/mcp-provider.ts, the comment above credentialSent, said the credential path "neither marks nor re-attempts on a reconnect" a borrowed token. It now says what happens: a 403 the server answers a borrowed token with at discovery is routed by the materializer to the connection it was borrowed from (the discovery organization's connection is marked reconnect_required, a reconnect there lists again, the client secret is left alone), while the include / toolsSnapshot refusals still keep grant-bearing instances on the plain fault (the accepted deviation of #28357). Comment only; no code in that package moves.

Files. packages/services/service-automation/src/plugin.ts, packages/services/service-automation/src/connector-mcp-authorization-door.test.ts, packages/connectors/connector-mcp/src/mcp-provider.ts (comment), .changeset/26702-mcp-discovery-refusal-marks-connection.md.

Unchanged, by design. Static credentials (bearer, api-key, basic) and tokens an instance mints from its own secret (client_credentials, app-credential) keep today's attribution: the client secret is what produced the refused token. The access-control semantics (who may connect, which grants exist) do not change. No status value or served field is added: reconnect_required and statusReason already exist on the connection record and ConnectorConnectionState.

Tests

connector-mcp-authorization-door.test.ts (real ObjectQL on SQLite, the mcp stand-in now able to answer a listing 403 the way the real provider throws it):

  • the discovery organization's connection becomes reconnect_required with the platform sentence, the held reason names it, and a reconnect by the same organization lists again and serves;
  • with a declared client and a stored client secret, two organizations connected: the refused listing (as org_a) leaves the secret row set, marks only org_a, the instance lists as org_b, org_b dispatches and org_a is refused 503 "needs to be reconnected";
  • a reconnect that lands while the refused token is listing keeps its fresh grant (row stays connected) and the instance retries;
  • CONTROL: a client_credentials instance (token minted by the instance itself) still marks its client secret rejected.
Red first, on main's plugin.ts (310cb96)

The new door tests on main's plugin.ts: Tests 2 failed | 15 passed (17) (expected … to match object { status: 'reconnect_required' }; expected … to have a length of 3 but got 2).

A probe (not committed) recording what main does, single organization, stored client secret, one refused listing, then a reconnect:

  • main: after the refusal secretStatus: rejected, connStatus: connected, 1 listing; after the reconnect secretStatus: rejected, still 1 listing, 1 materialization, still degraded.
  • this branch: after the refusal secretStatus: set, connStatus: reconnect_required; after the reconnect 2 listings, 2 materializations, served.

So the reading 6553121554 reproduces as written.

Ablations (scripts/ablation-replace.mjs, each restored with `git diff HEAD` empty)
  1. The route never taken (&& discoveryConnection === null): 3 red (the two attribution cases and the race case).
  2. The same-connect guard removed (record.connectedAt === connection.connectedAt → true): 1 red (the race case).
  3. The held reason back to the generic sentence: 1 red (the first case's reason assertion).

service-automation imports plugin.ts from src/ in these tests, so no rebuild sits between a mutation and the run.

Suites
  • pnpm --filter @objectstack/service-automation test: Test Files 250 passed (250), Tests 3173 passed (3173).
  • pnpm --filter @objectstack/service-automation run typecheck: exit 0 (check:test-typecheck: OK).
  • eslint --no-inline-config on the two touched files: exit 0.

Acceptance notes

Verification

Gate sheet in the card report (derived families on the final head, --ran reconciled); CI check-runs are the record for everything else.


Generated by Claude Code

…ry marks its connection, not the client secret

An mcp instance with an authorization_code grant lists its tools with the
token of its discovery organization's connection (ruled B). When the server
refused that token, the refusal took the static-credential path: the client
secret row was marked rejected and the instance held on a key a reconnect
does not move. The refusal is now recorded on that connection
(reconnect_required, the platform's sentence), the client secret is left
alone, and the instance lists as the next connected organization or holds
until a reconnect, which lists again.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ing keeps its grant

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/connector-mcp, @objectstack/service-automation, touching 10 documentable anchor(s).

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/connectors.mdx (via authorization_code (literal, a string literal in reconcileDeclaredConnectors), reconnect_required (literal, a string literal in reconcileDeclaredConnectors))
  • content/docs/automation/flows.mdx (via AutomationServicePlugin (symbol, a top-level class))
  • content/docs/kernel/cluster.mdx (via AutomationServicePlugin (symbol, a top-level class))
  • content/docs/permissions/system-context.mdx (via authorization_code (literal, a string literal in reconcileDeclaredConnectors))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-6.mdx (via AutomationServicePlugin (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 60 of 213 client-bound route-ledger rows — the other 153 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 153: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 98 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json cd065d3698db13969a28f46bf26f3e47e9ecc09c → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 44c6db2d786aceef9459fae45f9487c7ae2d2b6b — the merge of head d21e1bac0bb5d9b4329962546ebb42d9db08e54f into base cd065d3698db13969a28f46bf26f3e47e9ecc09c, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 44c6db2d786aceef9459fae45f9487c7ae2d2b6b && git checkout 44c6db2d786aceef9459fae45f9487c7ae2d2b6b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cd065d3698db13969a28f46bf26f3e47e9ecc09c d21e1bac0bb5d9b4329962546ebb42d9db08e54f && git checkout -B drift-repro cd065d3698db13969a28f46bf26f3e47e9ecc09c && git merge --no-ff d21e1bac0bb5d9b4329962546ebb42d9db08e54f

node scripts/docs-audit/affected-docs.mjs --json cd065d3698db13969a28f46bf26f3e47e9ecc09c

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs cd065d3698db13969a28f46bf26f3e47e9ecc09c → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…route a discovery 403 now takes

Co-Authored-By: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-project-manager · session_01FEkQ5kE5BrPUPNMMyv6oCH

席位核对 · ACCEPT · PR #28598 · 2026-11-01T08:01Z

Head-sha: d21e1bac0bb5d9b4329962546ebb42d9db08e54f

维护者速读(席位终稿)

  • 改了什么:mcp 连接器实例在列工具时,用的是「发现组织」那条连接借来的令牌(裁决 B)。上游拒绝这个令牌时,现在记在那条连接上(reconnect_required),不再把实例的 client secret 标成被拒;该组织重新连接后会立刻重新列工具。
  • 为什么改:以前这类拒绝冤枉了 client secret,而且同一组织重新连接也不会重试,只能改声明或重启(评审读数 6553121554,本 PR 先复现了)。
  • 风险与回滚:只碰「mcp + 授权码 + 列工具被拒」这一条路径;静态凭据与实例自取令牌照旧。回滚即 revert。
  • 先看:service-automation/src/plugin.ts 的 holdConnectorForRefusedConnection 与物化 catch 新分支;connector-mcp-authorization-door.test.ts 新增 4 例。
  • 两分钟验证:pnpm --filter @objectstack/service-automation exec vitest run src/connector-mcp-authorization-door.test.ts → 18/18。
  • 需要你做的:无。席位按常设授权落地。
  • 席位意见:修法落在归因层,没有新增状态值或对外字段;同连接守卫与退避兜底都有消融佐证。可落。

核对六项

  1. 形态:draft → main;正文 Part of #26702(卡留给 Console 第 7 项,domain:ui);全文无闭合关键词挨 open 卡号(已扫)。文件 4 个,与认领申报面一致(connector-mcp 一段注释是本席补丁轮要求)。
  2. 前提实测:dev 在 main 的 310cb96d 上用探针复现读数三点(client secret 被标 rejected、连接仍 connected、重连不重列);红先 2/17 红。
  3. 门禁:本 head 的 check-runs 48 个,35 success、13 skipped、0 失败;Lint & Repo Gates 与 TypeScript Type Check 皆 success。dev 本地 61 族 60 过、1 NOT-MEASURED(check:dual-build-cjs-loads,需全量构建,以 CI 为准)。
  4. 接受的偏差:无派发令外偏差。connector-mcp 只改注释,未加 changeset(注释不发布,check-empty-changeset 在本 head 过)。
  5. 范围外去向:Acceptance note「ConnectorConnectionState.status 的 JSDoc 只写了授权服务器拒绝一种」→ filed spec: ConnectorConnectionState.status JSDoc says reconnect_required means only that the authorization server refused the grant; since PR #28598 a discovery refusal of the connection token sets it too #28614(spec 车道);include/toolsSnapshot 仍走普通故障 → Acceptance notes(fix(connector-mcp): an include a credentialed MCP server does not list degrades the instance as a credential refusal, not the boot #28357 的既定偏差,卡自有);连接答复为 connected 而随后读为 reconnect_required → Acceptance notes(行为说明,非缺陷)。
  6. 落地条款:Clause-②: no、Metadata-protocol: none,diff 不碰 packages/spec/src/** 与 governed 面(check-governed-merges --pr 28598:NOT governed,289 行)⇒ 不欠契约复核。落地授权,维护者原话:「按照 pm-dispatch 规则复核过的都授权」(2026-10-25T11:35Z)。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant