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

.changeset/26702-mcp-discovery-refusal-marks-connection.md

@@ -0,0 +1,16 @@
+---
+'@objectstack/service-automation': patch
+---
+
+fix(service-automation): a borrowed token refused at `mcp` tool discovery marks its connection, not the client secret
+
+Clause-②: no
+Metadata-protocol: none
+
+An `mcp` instance with an `oauth2` `authorization_code` grant lists its tools with the token of its discovery organization's connection (ruled B on #26702). A discovery refusal of that connection-borrowed token is now attributed to the connection, not to the instance's client secret:
+
+- that organization's connection is marked `reconnect_required`, with a platform sentence naming the refusal (served as `statusReason` by the connection door and as the held instance's reason);
+- the client secret its `credentialRef` names is left as it is (it was marked `rejected`, which setting it again was the only way to clear);
+- the instance lists as the next connected organization, or is held "reconnect required" until a reconnect, and a reconnect by that organization lists again (before, the instance stayed held until a declaration edit or a restart).
+
+A reconnect that landed while the refused token was listing keeps its fresh grant: the connection is marked only while it is still the connect whose token was sent, and otherwise the instance retries on the backoff. Static credentials (`bearer`, `api-key`, `basic`) and tokens an instance mints itself (`client_credentials`, `app-credential`) keep the client secret's attribution, unchanged.

packages/connectors/connector-mcp/src/mcp-provider.ts

@@ -507,13 +507,17 @@ export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorP
     // the server's answer was given to that credential.
     //
     // A grant-bearing instance's borrowed token (`ctx.credentialHandle`)
-    // reaches the server too, and is deliberately not counted here: the
-    // credential path a refusal below takes holds and marks the instance's
-    // `credentialRef` row, which for such an instance is its client secret —
-    // a value the listing never depended on — while the token that was sent
-    // belongs to an organization's connection, which that path neither marks
-    // nor re-attempts on a reconnect. Until the materializer has a route for
-    // it, such an instance keeps the plain fault.
+    // reaches the server too, and is deliberately not counted here. A 403 the
+    // server answers that token with at discovery (thrown above as
+    // `ConnectorCredentialRejectedError`; a 401 is refreshed once, then a 503)
+    // is routed by the materializer to the
+    // connection the token was borrowed from: for an `authorization_code`
+    // instance, the discovery organization's connection is marked
+    // `reconnect_required` and a reconnect there lists again, while the
+    // instance's `credentialRef` (its client secret) is left alone (#26702).
+    // The `include` / `toolsSnapshot` refusals below are not routed that way:
+    // with the borrowed token not counted here, such an instance keeps the
+    // plain fault for them (the accepted deviation of #28357).
     const credentialSent = transport.kind === 'http' && sendsInstanceAuth(ctx.auth);
     if (includeList) {
       // An entry naming a tool the server does not list is refused after the

packages/services/service-automation/src/connector-mcp-authorization-door.test.ts

@@ -23,7 +23,12 @@ import { SqlDriver } from '@objectstack/driver-sql';
 import { SysSecret } from '@objectstack/platform-objects';
 import { callWithBorrowedCredential } from '@objectstack/types';
 import type { CryptoContext, CryptoHandle, ICryptoProvider } from '@objectstack/spec/contracts';
-import type { Connector, ConnectorProviderContext, ConnectorProviderFactory } from '@objectstack/spec/integration';
+import {
+    ConnectorCredentialRejectedError,
+    type Connector,
+    type ConnectorProviderContext,
+    type ConnectorProviderFactory,
+} from '@objectstack/spec/integration';
 
 import { AutomationServicePlugin } from './plugin.js';
 import type { AutomationEngine } from './engine.js';
@@ -149,14 +154,28 @@ function notion(auth: Record<string, unknown> = {}, transportUrl?: string) {
  * the real provider's `tools/list` does: an ask that names no organization.
  */
 function mcpStandIn(lists = false) {
-    const state = { materialized: [] as ConnectorProviderContext[], sent: [] as string[], listedWith: [] as string[], refuse: new Set<string>() };
+    const state = {
+        materialized: [] as ConnectorProviderContext[],
+        sent: [] as string[],
+        listedWith: [] as string[],
+        refuse: new Set<string>(),
+        /** What the next listings answer, in order (then `200`): a `403` is thrown as the real provider throws it. */
+        listingAnswers: [] as number[],
+        /** Run before a listing answers (e.g. another replica's reconnect landing meanwhile). */
+        beforeListingAnswer: undefined as undefined | (() => Promise<void>),
+    };
     const factory: ConnectorProviderFactory = async (ctx) => {
         state.materialized.push(ctx);
         if (lists) {
-            await callWithBorrowedCredential(ctx.credentialHandle!, async (auth) => {
-                state.listedWith.push(auth.type === 'bearer' ? auth.token : '');
-                return { status: 200 };
+            const listed = await callWithBorrowedCredential(ctx.credentialHandle!, async (auth) => {
+                const token = auth.type === 'bearer' ? auth.token : '';
+                state.listedWith.push(token);
+                await state.beforeListingAnswer?.();
+                return { status: state.listingAnswers.shift() ?? 200 };
             }, {});
+            if (listed.status === 403) {
+                throw new ConnectorCredentialRejectedError(`connector-mcp provider: connector '${ctx.name}' was refused by its MCP server (HTTP 403)`, 403);
+            }
         }
         return {
             def: {
@@ -385,6 +404,87 @@ describe('ADR-0143 D4, ruled B — the connection an mcp instance lists its tool
     });
 });
 
+describe('ADR-0143 D4 — a borrowed token refused at tool discovery is its connection\'s, not the client secret\'s', () => {
+    const SECRET_REF = 'NOTION_CLIENT_SECRET';
+    const STORED_SECRET = 'dummy-stored-secret-26702';
+    const OPERATOR = { userId: 'usr_admin' } as never;
+    const secretRow = async (engine: AutomationEngine) =>
+        (await engine.listConnectorCredentialRefs()).find((r) => r.ref === SECRET_REF);
+    const connectionOf = (engine: AutomationEngine, as: { userId: string; tenantId: string }) =>
+        engine.getConnectorConnection({ connector: 'notion', context: as as never });
+
+    it('the discovery organization\'s connection needs reconnecting; a reconnect there lists again', async () => {
+        const { engine, provider, connect, call } = await boot([notion()], { lists: true });
+        provider.state.listingAnswers.push(403);
+        await (await connect()).done();
+        expect(provider.state.listedWith).toHaveLength(1);
+
+        // The connection whose token was refused is the one marked, with the platform's sentence.
+        await vi.waitFor(async () => expect(await connectionOf(engine, ADMIN_A)).toMatchObject({
+            status: 'reconnect_required',
+            statusReason: expect.stringContaining('refused the token of this connection while it listed its tools (HTTP 403)'),
+        }));
+        await vi.waitFor(() => expect(engine.getConnectorDegradedReason('notion')).toContain("needs to be reconnected for organization 'org_a'"));
+        expect(engine.getConnectorDegradedReason('notion')).toContain('while it listed its tools (HTTP 403)');
+        expect(engine.resolveConnectorAction('notion', 'search')).toBeUndefined();
+
+        // A reconnect by the same organization re-attempts discovery, and the instance serves.
+        await (await connect()).done();
+        await vi.waitFor(() => expect(engine.getConnectorDegradedReason('notion')).toBeUndefined());
+        expect(provider.state.listedWith).toHaveLength(2);
+        expect(await connectionOf(engine, ADMIN_A)).toMatchObject({ status: 'connected' });
+        expect(await call()).toMatchObject({ status: 200 });
+    });
+
+    it('the stored client secret stays `set`; the instance lists as the next organization, each dispatch keeps its own', async () => {
+        const { engine, provider, connect } = await boot([notion({ clientId: 'notion-app', credentialRef: SECRET_REF })], { lists: true });
+        await engine.setConnectorCredential({ ref: SECRET_REF, value: STORED_SECRET, context: OPERATOR });
+        await (await connect(undefined, ADMIN_A)).done();
+        await (await connect(undefined, ADMIN_B)).done();
+        expect(provider.state.materialized).toHaveLength(1);
+
+        // The secret is set again: the instance materializes again, listing as org_a (the earliest), which is refused.
+        provider.state.listingAnswers.push(403);
+        await engine.setConnectorCredential({ ref: SECRET_REF, value: `${STORED_SECRET}-2`, context: OPERATOR });
+        await vi.waitFor(() => expect(provider.state.materialized).toHaveLength(3));
+        await vi.waitFor(() => expect(engine.getConnectorDegradedReason('notion')).toBeUndefined());
+
+        expect(await secretRow(engine)).toMatchObject({ status: 'set' });
+        expect(await connectionOf(engine, ADMIN_A)).toMatchObject({ status: 'reconnect_required' });
+        expect(await connectionOf(engine, ADMIN_B)).toMatchObject({ status: 'connected' });
+        const dispatch = (tenantId: string) =>
+            engine.resolveConnectorAction('notion', 'search')!({}, { automation: { tenantId } } as never);
+        expect(await dispatch('org_b')).toMatchObject({ status: 200 });
+        await expect(dispatch('org_a')).rejects.toMatchObject({ status: 503, message: expect.stringContaining("needs to be reconnected for organization 'org_a'") });
+    });
+
+    it('a reconnect that landed while the refused token was listing keeps its fresh grant; the instance retries', async () => {
+        const { engine, provider, connect, ql } = await boot([notion()], { lists: true });
+        provider.state.listingAnswers.push(403);
+        // Another replica's reconnect: the row is a new connect by the time the refusal is recorded.
+        provider.state.beforeListingAnswer = async () => {
+            provider.state.beforeListingAnswer = undefined;
+            const [row] = await rowsOf(ql);
+            await ql.update(CONNECTOR_CONNECTION_OBJECT, { id: row.id, connected_at: '2999-01-01T00:00:00.000Z' }, { context: SYSTEM } as never);
+        };
+        await (await connect()).done();
+        expect(provider.state.listedWith).toHaveLength(1);
+        expect(await connectionOf(engine, ADMIN_A)).toMatchObject({ status: 'connected' });
+        expect(engine.getConnectorDegradedReason('notion')).toContain("refused the token of organization 'org_a' while it listed its tools (HTTP 403); it retries");
+    });
+
+    it('CONTROL: a client_credentials token the instance mints itself keeps the client secret\'s attribution', async () => {
+        const { engine, provider } = await boot([notion({
+            grant: 'client_credentials', clientId: 'notion-app', tokenUrl: `${mcp.base}/auth/token`, credentialRef: SECRET_REF,
+        })], { lists: true });
+        provider.state.listingAnswers.push(403);
+        await engine.setConnectorCredential({ ref: SECRET_REF, value: STORED_SECRET, context: OPERATOR });
+        await vi.waitFor(() => expect(provider.state.listedWith).toHaveLength(1));
+        await vi.waitFor(async () => expect(await secretRow(engine)).toMatchObject({ status: 'rejected' }));
+        expect(engine.getConnectorDegradedReason('notion')).toContain('HTTP 403');
+    });
+});
+
 describe('ADR-0143 D2.1 — the pre-registered clientId form', () => {
     it('a declared clientId is used as is: no registration, the exchange authenticates with the declared secret', async () => {
         const { connect, ql } = await boot([notion({ clientId: 'hubspot-app', credentialRef: 'HUBSPOT_CLIENT_SECRET' })]);