Skip to content

Commit 07450b5

Browse files
fix(service-automation): a connection marked reconnect required says the cause that actually happened (#28863) (#28955)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XUSbzmMRfCQ13hL4Jffkmqp)_ Fixes #28863 Clause-②: no Metadata-protocol: none Skills-docs: none — no file under `skills/**` teaches the connection's `reconnect_required` causes (0 hits for `reconnect_required`, `reconnect required` and `refused the grant`). ## 维护者速读 - **改了什么**:连接器连接变成「需要重新连接」有四种原因:授权服务器在刷新时拒绝授权;它没发 refresh token;连接没存可用来刷新的 client;MCP 工具发现时上游拒绝了令牌。存下的 `status_reason`、`sys_connector_connection.status` 字段说明、`markReconnectRequired` 注释和 `connectors.mdx` 以前都只说第一种。现在三处文字列出全部四种;「没有 refresh token」和「没有 client」两条路径存下的原因各自说真正发生的事,不再先声称服务器拒绝,也不再说访问令牌「已过期」(缓存未命中或调度 401 也会触发刷新,令牌可能仍有效)。 - **为什么改**:管理员读到的原因说服务器拒绝了授权,可服务器根本没被问过(#28614 开发者发现,#28862 合同审查补充)。 - **风险与回滚**:低。只改文字和两条路径存储的原因句;拒绝路径的句子一字不变;状态值、错误码、写入都不变。回滚即 revert。 - **先看**:`packages/services/service-automation/src/oauth2-grant/grant-handle.ts` 的 `reconnectRequiredReason` 与 `refreshOnce`;同目录 `grant-handle.test.ts` 末尾的 `[#28863]` 三条。 - **两分钟验证**:`pnpm --filter @objectstack/service-automation exec vitest run src/oauth2-grant/grant-handle.test.ts`,预期 `Tests 22 passed (22)`(main 上新加的 2 条红,对照两边都绿)。 - **需要你做的**:无,席位落地。spec 里同样的「expired」措辞(下面 Acceptance notes 点了行号)由席位转给 spec 车道。 ## What changes - **`reconnectRequiredReason(connector, organizationId, cause?)`** (`oauth2-grant/grant-handle.ts`) takes an optional `ReconnectRequiredCause`: `'refused'` (the default, so every existing caller keeps the old sentence byte for byte), `'no_refresh_token'` or `'no_client'`. The sentence's cause clause is the one that happened: - refused: "its authorization server refused the grant (revoked, or expired with no usable refresh)" (unchanged); - no refresh token: "its authorization server issued no refresh token, so an access token the connection has to renew cannot be renewed"; - no client: "its connection keeps no client to refresh with, so an access token it has to renew cannot be renewed". - **`refreshOnce`** passes each path's cause to `grantRefused`, so the stored `status_reason` and the dispatch's 503 message are that one sentence. Before, the two non-refusal paths stored the refusal sentence with the real cause appended, and the no-refresh-token one said "an expired access token". - **Prose, all four causes, in PR #28862's words:** the `status` field description in `sys-connector-connection.object.ts` (it lives in `service-automation`, not `platform-objects` as the order's path guessed), the `ConnectorConnectionStore#markReconnectRequired` docblock, and `content/docs/automation/connectors.mdx` (the paragraph at L689 and the boot-behaviour table row at L868). Each points at `status_reason` / `statusReason` for which cause happened. - **Changeset** `.changeset/28863-reconnect-required-cause.md`: `patch` on `@objectstack/service-automation`, both declaration lines. The docs page ships in no package. - No new error code, no new status value, no new gate, no `packages/spec/**`. **Why `Clause-②: no`.** No accept set, status value, code or export changes meaning. What changes is prose and the reason text two refresh paths store and answer with (the 503 stays a 503 `SERVICE_UNAVAILABLE`). ## Tests Three new cases at the end of `packages/services/service-automation/src/oauth2-grant/grant-handle.test.ts` (`[#28863]`), on the file's in-memory connection store, each reading the row's stored `statusReason` and the dispatch refusal: - **No refresh token:** the reason names the missing refresh token and "an access token the connection has to renew", contains neither `refused` nor `expired`, equals the 503 message, and no token request is made. - **No client** (an instance that declares no client and a connection that keeps none): the reason names the missing client, contains neither `refused` nor `expired`, equals the 503 message, and no token request is made. - **CONTROL, refused** (`invalid_grant`): the reason still names the refusal, carries no token, and equals the 503 message. <details><summary>Red on base, green on head</summary> - **Base** (`grant-handle.ts` from `5e1b9b0a`, the new tests at head): `Tests 2 failed | 20 passed (22)`. The two reds: `expected 'connector instance 'crm' needs to b…' to contain 'issued no refresh token, so an access…'` and the same for `'keeps no client to refresh with, so a…'`. The refusal control is green both ways. Restored with `git checkout HEAD -- grant-handle.ts`, `git diff HEAD` empty. - **Head:** `Tests 22 passed (22)`. </details> ## Acceptance notes - **The spec half of item 5 is not in this PR** (ruling 4). `packages/spec/src/contracts/automation-service.ts` L1145–1146, the `ConnectorConnectionState.status` JSDoc from PR #28862: "the connection keeps nothing to renew an / expired access token with". "expired" there is narrower than the code, for the same reason as above. The runtime wording here, "an access token it has to renew", is the proposed replacement, so the two can agree. - **Who matched the old text.** `grant-handle.test.ts` L322 matches `issued no refresh token`, which the new sentence keeps, so it stays green. `connector-connections.test.ts` L148–157 passes its own reason string to the store. `connector-connection-door.test.ts` L576 matches the plugin's `onGrantRefused` log line ("…refused the grant of organization…"), which this PR does not touch. No other consumer reads the reason text (searched objectstack, objectui and hotcrm for `no usable refresh`, `refused the grant`, `expired access token` and `issued no refresh token`). - **Left as they are, the same class (noted, not filed):** - the plugin's `onGrantRefused` warn line (`plugin.ts` ≈L3697) says "the authorization server refused the grant" on all three refresh paths; - the dispatch refusal for a row ALREADY marked (`select` in `grant-handle.ts` ≈L238, `refreshOnce`'s first check, and the plugin's hold-reason fallback ≈L3189) uses the default refused sentence whatever cause the row stores. Answering with the row's own `statusReason` would make those honest too. ## Verification All readings at head `7cf0294a` (the change `790e473c`, merged once with `origin/main`, a clean merge). Full `pnpm turbo run build --concurrency=2`: 118/118. <details><summary>Package suites, typecheck, lint</summary> - `pnpm --filter @objectstack/service-automation test`: `Test Files 255 passed (255)`, `Tests 3284 passed (3284)`, lock VERDICT command-exit 0. Its `typecheck` (tsc + test-typecheck): exit 0. - `pnpm --filter @objectstack/platform-objects test`: `Test Files 108 passed (108)`, `Tests 1503 passed (1503)`, command-exit 0. Its `typecheck`: exit 0. (No file of it changed; run as the order asks.) - The repo's `pnpm lint` command scoped to `packages/services/service-automation`: exit 0. </details> <details><summary>Derived gates (`dispatch-gates.mjs --commands`, 89 families)</summary> - All 89 exit 0 at `7cf0294a`, the docs families included (`check-affected-docs`, `check-drift-comment`, `check:docs-*`, `check:corpus-claim-drift`); `dispatch-gates --ran`: `89 derived, 89 run, 0 NOT-MEASURED, 0 UNRUN`. Record: `pm-orders/server/28863-gates-ran.txt`. - `check:type-check-debt`: OK, `--re-measure: OK`. - Repo-wide `pnpm lint` and the CI-only families are CI's; see the PR's check-runs. </details> Dogfood battery: not owed (no `!`, no narrowing arm) and not run. --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5mTYb1BTJNRqf3bdJ8)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent fed3a51 commit 07450b5

6 files changed

Lines changed: 116 additions & 20 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
fix(service-automation): a connection marked "reconnect required" says the cause that actually happened (#28863)
6+
7+
Clause-②: no — prose, and the stored `status_reason` text of two refresh paths. No accept set, status value, error code or export changes meaning.
8+
Metadata-protocol: none
9+
10+
A connector connection becomes `reconnect_required` four ways: the authorization server refuses the grant on refresh (`invalid_grant`); it issued no refresh token; the connection keeps no client to refresh with; or the upstream refuses the connection's token while an `mcp` instance discovers its tools. The connection's stored reason, the `sys_connector_connection.status` field help and the `ConnectorConnectionStore#markReconnectRequired` docblock all named only the first.
11+
12+
- **The stored reason, per path.** `reconnectRequiredReason` takes an optional cause (`'refused'`, `'no_refresh_token'`, `'no_client'`; `'refused'` when none is given, the old sentence unchanged). The no-refresh-token path now stores "…its authorization server issued no refresh token, so an access token the connection has to renew cannot be renewed…", and the no-client path "…its connection keeps no client to refresh with, so an access token it has to renew cannot be renewed…". Neither says the server refused anything, and neither says the access token expired: a refresh also runs on a cache miss and on a dispatch's 401, while the token may still be valid. The dispatch's 503 refusal carries the same sentence.
13+
- **The field help and the docblock** list all four causes and point to `status_reason` for which one happened.
14+
- Unchanged: the refused path's sentence, the MCP discovery path's own reason, and every status, code and write.

‎content/docs/automation/connectors.mdx‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -686,9 +686,13 @@ a rotating refresh token is never spent twice.
686686
not a sign-in account, and signing in never touches it. A run dispatches
687687
with its own organization's connection; a run with no organization uses the
688688
instance's only connection, and is refused when there are several. When the
689-
vendor refuses the grant (`invalid_grant`), the connection is marked
690-
**reconnect required** and the instance degraded until an administrator
691-
connects it again. A refresh, and the mark "reconnect required", are each
689+
grant stops working, the connection is marked **reconnect required** and the
690+
instance degraded until an administrator connects it again. That happens when
691+
the vendor refuses the grant on refresh (`invalid_grant`), when the
692+
connection keeps nothing to renew an access token it has to renew with (the
693+
vendor issued no refresh token, or no client is kept to refresh with), or when
694+
the upstream refuses the connection's token while an `mcp` instance discovers
695+
its tools. The connection's `statusReason` says which. A refresh, and the mark "reconnect required", are each
692696
written only over the revision they read, in one conditional write, so a
693697
stale one never overwrites a newer connect on any replica. A revoke deletes
694698
the stored connection only: the server does not revoke the refresh token at
@@ -865,7 +869,7 @@ retry):
865869
| `provider` names no installed factory | **Hard boot error** naming the entry, the provider, and the plugin that supplies it — add the executor to `plugins:` |
866870
| Invalid `providerConfig` (missing `baseUrl` / `spec` / `transport`, wrong shape) | **Hard boot error** from the provider factory |
867871
| `auth` is a grant-bearing kind (`oauth2`, `app-credential`) on a provider that does not ask the credential handle (a plugin's provider registered without `{ credentialHandle: true }`) | **Not fatal**: the instance registers **degraded**, naming the kind, and the upstream is never called without its credential; a datasource borrowing it is refused with `422` |
868-
| An `authorization_code` instance nobody connected, or whose grant the vendor refused | **Not fatal**: the instance registers **degraded**, "not connected" or "reconnect required" — connect it (`POST …/connectors/:name/connection`), and it materializes without a restart |
872+
| An `authorization_code` instance nobody connected, or whose grant stopped working (the vendor refused it on refresh, no refresh token or client to renew an access token with, or the upstream refused its token at MCP tool discovery) | **Not fatal**: the instance registers **degraded**, "not connected" or "reconnect required" — connect it (`POST …/connectors/:name/connection`), and it materializes without a restart |
869873
| The token endpoint is unreachable or answers `5xx` / `429` | **Not fatal**: the dispatch fails `CONNECTOR_UPSTREAM_UNAVAILABLE` and is retried on the existing backoff; the connection stays connected |
870874
| `auth` is `signed` on a provider that does not say it applies the request signer | **Hard boot error** naming the instance, the scheme and the provider: its calls would go out unsigned — declare it through `rest` or `openapi` |
871875
| `credentialRef` (or a `signed` instance's `vendorPublicKeyRef`) resolves to nothing | **Not fatal**: the instance registers **degraded**, naming the reference — set it (`PUT …/credential-refs/:ref`), and it materializes without a restart |

‎packages/services/service-automation/src/connector-connections.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -322,11 +322,15 @@ export class ConnectorConnectionStore {
322322
}
323323

324324
/**
325-
* Record that the authorization server refused the grant `record` held
326-
* (revoked, or expired with no usable refresh). `reason` is the platform's
327-
* sentence, never the server's text. Left alone when the row moved since
328-
* — checked by the write itself ({@link updateAtRevision}): a reconnect
329-
* that landed between the refusal and this write keeps its fresh grant.
325+
* Record that the grant `record` held stopped working: the authorization
326+
* server refused it on refresh, the connection keeps nothing to renew an
327+
* access token it has to renew with (no refresh token was issued, or no
328+
* client to refresh with), or the upstream refused the connection's token
329+
* while an `mcp` instance discovered its tools. `reason` is the platform's
330+
* sentence naming which, never the server's text. Left alone when the row
331+
* moved since — checked by the write itself ({@link updateAtRevision}): a
332+
* reconnect that landed between the mark and this write keeps its fresh
333+
* grant.
330334
*/
331335
async markReconnectRequired(record: ConnectorConnectionRecord, reason: string): Promise<void> {
332336
const engine = this.resolveEngine();

‎packages/services/service-automation/src/oauth2-grant/grant-handle.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,3 +413,48 @@ describe('ADR-0143 D2.1 — an MCP instance whose connection keeps the client it
413413
expect(onGrantRefused).toHaveBeenCalledWith('org_a');
414414
});
415415
});
416+
417+
describe('[#28863] the stored reason of a connection marked reconnect required names the cause that happened', () => {
418+
const storedReason = (held: Map<string, ConnectorConnectionRecord>) => String(held.get('org_a')?.statusReason);
419+
420+
it('no refresh token: the reason names the missing refresh token, claims no refusal and no expiry', async () => {
421+
const idp = tokenEndpoint();
422+
const { store, held } = fakeConnections([{ org: 'org_a' }]);
423+
const handle = createOAuth2GrantHandle({ connector: 'crm', config: AUTH_CODE, connections: store, fetch: idp.fetch });
424+
const refused = await handle.get({ organizationId: 'org_a' }).catch((e) => e);
425+
const reason = storedReason(held);
426+
expect(reason).toContain("connector instance 'crm' needs to be reconnected for organization 'org_a'");
427+
expect(reason).toContain('issued no refresh token, so an access token the connection has to renew cannot be renewed');
428+
expect(reason).not.toMatch(/refused/);
429+
expect(reason).not.toMatch(/expired/);
430+
expect(refused.message).toBe(reason);
431+
expect(idp.state.requests).toHaveLength(0);
432+
});
433+
434+
it('no client: the reason names the missing client, claims no refusal and no expiry', async () => {
435+
const idp = tokenEndpoint();
436+
const { store, held } = fakeConnections([{ org: 'org_a', refreshToken: 'dummy-rt-a' }]);
437+
const handle = createOAuth2GrantHandle({ connector: 'notion', config: { grant: 'authorization_code' }, connections: store, fetch: idp.fetch });
438+
const refused = await handle.get({ organizationId: 'org_a' }).catch((e) => e);
439+
const reason = storedReason(held);
440+
expect(reason).toContain("connector instance 'notion' needs to be reconnected for organization 'org_a'");
441+
expect(reason).toContain('keeps no client to refresh with, so an access token it has to renew cannot be renewed');
442+
expect(reason).not.toMatch(/refused/);
443+
expect(reason).not.toMatch(/expired/);
444+
expect(refused.message).toBe(reason);
445+
expect(idp.state.requests).toHaveLength(0);
446+
});
447+
448+
it('control: a refresh the authorization server refuses (`invalid_grant`) stores a reason naming the refusal', async () => {
449+
const idp = tokenEndpoint();
450+
idp.state.answer = () => ({ status: 400, body: { error: 'invalid_grant' } });
451+
const { store, held } = fakeConnections([{ org: 'org_a', refreshToken: 'dummy-rt-a' }]);
452+
const handle = createOAuth2GrantHandle({ connector: 'crm', config: AUTH_CODE, connections: store, fetch: idp.fetch });
453+
const refused = await handle.get({ organizationId: 'org_a' }).catch((e) => e);
454+
const reason = storedReason(held);
455+
expect(reason).toContain('its authorization server refused the grant (revoked, or expired with no usable refresh)');
456+
expect(reason).not.toContain('dummy-rt-a');
457+
expect(refused.message).toBe(reason);
458+
expect(idp.state.requests).toHaveLength(1);
459+
});
460+
});

‎packages/services/service-automation/src/oauth2-grant/grant-handle.ts‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,40 @@ export function notConnectedReason(connector: string, organizationId?: string):
119119
+ 'dispatch uses that connection (ADR-0143 D4)';
120120
}
121121

122-
/** The dispatch refusal of an organization whose grant the authorization server refused. */
123-
export function reconnectRequiredReason(connector: string, organizationId: string): string {
124-
return `connector instance '${connector}' needs to be reconnected for organization '${organizationId}': its authorization `
125-
+ 'server refused the grant (revoked, or expired with no usable refresh). Reconnect required: an administrator connects '
126-
+ 'it again, and the next dispatch uses the new connection (ADR-0143 D4)';
122+
/**
123+
* [#28863] Why a refresh left a connection `reconnect_required`, as
124+
* {@link reconnectRequiredReason} words it: the authorization server refused
125+
* the grant (`invalid_grant`); it issued no refresh token; or the connection
126+
* keeps no client to refresh with. The last two never ask the server. (The
127+
* fourth way a connection is marked, the upstream refusing its token at MCP
128+
* tool discovery, words its own reason in `plugin.ts`.)
129+
*/
130+
export type ReconnectRequiredCause = 'refused' | 'no_refresh_token' | 'no_client';
131+
132+
const RECONNECT_REQUIRED_CAUSES: Readonly<Record<ReconnectRequiredCause, string>> = {
133+
refused: 'its authorization server refused the grant (revoked, or expired with no usable refresh)',
134+
// Not "expired": a refresh also runs on a cache miss (a fresh replica, a
135+
// revision another replica moved) and on a dispatch's 401, while the
136+
// access token may still be valid.
137+
no_refresh_token: 'its authorization server issued no refresh token, so an access token the connection has to renew '
138+
+ 'cannot be renewed',
139+
no_client: 'its connection keeps no client to refresh with, so an access token it has to renew cannot be renewed',
140+
};
141+
142+
/**
143+
* The dispatch refusal, and the stored `status_reason`, of an organization
144+
* whose connection needs to be reconnected. `cause` names what happened on
145+
* the path that marked it; with none, the authorization server's refusal, as
146+
* before.
147+
*/
148+
export function reconnectRequiredReason(
149+
connector: string,
150+
organizationId: string,
151+
cause: ReconnectRequiredCause = 'refused',
152+
): string {
153+
return `connector instance '${connector}' needs to be reconnected for organization '${organizationId}': `
154+
+ `${RECONNECT_REQUIRED_CAUSES[cause]}. Reconnect required: an administrator connects it again, and the next dispatch `
155+
+ 'uses the new connection (ADR-0143 D4)';
127156
}
128157

129158
/** Create the handle of one `oauth2` instance. */
@@ -266,21 +295,21 @@ export function createOAuth2GrantHandle(deps: OAuth2GrantHandleDeps): OAuth2Gran
266295
+ 'read back now; the next dispatch retries');
267296
}
268297
if (refreshToken === undefined) {
269-
throw await grantRefused(current, reconnectRequiredReason(connector, organizationId)
270-
+ '. Its authorization server issued no refresh token, so an expired access token cannot be renewed');
298+
throw await grantRefused(current, reconnectRequiredReason(connector, organizationId, 'no_refresh_token'));
271299
}
272300
const endpoint = await endpointOf(current);
273301
if (endpoint === undefined) {
274-
throw await grantRefused(current, reconnectRequiredReason(connector, organizationId)
275-
+ '. Its connection keeps no client to refresh it with');
302+
throw await grantRefused(current, reconnectRequiredReason(connector, organizationId, 'no_client'));
276303
}
277304
const params: Record<string, string> = { grant_type: 'refresh_token', refresh_token: refreshToken };
278305
if (endpoint.resource) params.resource = endpoint.resource;
279306
let token: OAuth2TokenResponse;
280307
try {
281308
token = await tokenRequest(params, endpoint);
282309
} catch (err) {
283-
if (err instanceof OAuth2GrantRefusedError) throw await grantRefused(current, reconnectRequiredReason(connector, organizationId));
310+
if (err instanceof OAuth2GrantRefusedError) {
311+
throw await grantRefused(current, reconnectRequiredReason(connector, organizationId, 'refused'));
312+
}
284313
throw err;
285314
}
286315
const written = await deps.connections.recordRefresh(current, {

‎packages/services/service-automation/src/sys-connector-connection.object.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export const SysConnectorConnection = ObjectSchema.create({
9595
required: true,
9696
defaultValue: 'connected',
9797
description:
98-
"`connected` while the grant works; `reconnect_required` once the authorization server refused it (revoked, or expired with no usable refresh). Only a new connect returns it to `connected`.",
98+
"`connected` while the grant works; `reconnect_required` once the grant stopped working: the authorization server refused it on refresh, the connection keeps nothing to renew an access token it has to renew with (no refresh token was issued, or no client to refresh with), or the upstream refused the connection's token while an `mcp` instance discovered its tools. `status_reason` says which. Only a new connect returns it to `connected`.",
9999
}),
100100

101101
status_reason: Field.textarea({

0 commit comments

Comments
 (0)