Skip to content

Commit de68479

Browse files
fix(service-automation): a held connector instance logs its reason, and the connection refusal ends with one period (#29000)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X1nKKLC3bjaEUZnprGuxxth)_ Fixes #28939 Clause-②: no — log meta and one character of refusal text; no schema, accept set, error code or status moves. Metadata-protocol: none ## 维护者速读 - 改了什么:连接器实例因配置故障被挂起(平台自己拒绝服务的 `oauth2` 实例,例如 MCP 服务器走非回环地址的明文 http)时,启动日志那条 ERROR 现在在结构化字段 `reason` 里写明原因,不再写 `{"error":"undefined"}`;连接器连接接口(`GET`/`POST /api/v1/automation/connectors/NAME/connection`)返回的 400 拒绝句末尾从两个句号改为一个。 - 为什么改:运维排查被挂起的连接器时先看 ERROR 行,原来这里只有 "undefined",原因要到下一行 WARN 才有;管理员在界面和接口里看到的拒绝句以 ".." 结尾,这段文字会随版本发出。 - 风险与回滚:只动了日志字段和接口里多拼的一个句号。挂起判断、不重试的策略、WARN 行都没动;provider 抛错导致的挂起照旧把错误写在 `error` 字段(有对照测试)。回滚即还原本 PR。 - 先看:`packages/services/service-automation/src/plugin.ts` 的 `failDegradedConnectorInstance`(ERROR 行的第三个参数)与 `connectionDoorSubject`(去掉多拼的句号);新测试 `src/connector-held-fault-log-reason.test.ts`。 - 两分钟验证:`pnpm --filter @objectstack/service-automation exec vitest run src/connector-held-fault-log-reason.test.ts`,预期 5/5 通过(其中 4 例在 main 上为红)。 - 需要你做的:无,席位落地。 ## What changes - **The held-instance ERROR record carries its reason.** `failDegradedConnectorInstance` logged `describeThrownForLog(info.cause)` as the record's meta. The grant-refusal caller (an `oauth2` instance this server cannot serve) passes no thrown value, so the record rendered `{"error":"undefined"}`. The meta is now `{ reason: info.reason }` when there is no cause, the same sentence the husk stores as `degradedReason`, and `describeThrownForLog(info.cause)` when there is one (the provider-configuration-fault caller, unchanged). - **The refusal sentence ends once.** Every reason `oauth2NotServableReason` returns already ends with a period. `connectionDoorSubject` appended another (`${notServable}.`), so all three sentences the door can answer ended "..": plain-http MCP server, endpoint not over https, no endpoints to discover. The door now passes the reason as it is. The constants are unchanged. - **Unchanged.** The hold decision, the retry posture, the engine's WARN line (`degradedReason` meta), the error code `VALIDATION_ERROR` and the status `400`. ## Tests - New `connector-held-fault-log-reason.test.ts` (5 cases), with a real `ObjectLogger` in JSON format and stderr captured, the way `connector-fail-cause.test.ts` reads records: - the held plain-http MCP instance logs exactly one ERROR record whose `reason` equals the instance's degraded reason, with no `error` field, and no stderr line contains `"error":"undefined"`; - CONTROL: a held instance whose fault is a thrown provider error (degraded first, then a reload meets an `include` the server does not list) still logs that error in `error`; - for each of the three refusals, `beginConnectorConnection` and `getConnectorConnection` reject with `status: 400`, `code: 'VALIDATION_ERROR'`, and a message equal to the stored degraded reason, not ending "..". <details><summary>Measured on main, red first, ablations</summary> - Main `135054eb`, probe against the built `dist/` (declared `qa_mcp_oauth_plain`, `provider: 'mcp'`, transport `http://192.0.2.2:41520/api/v1/mcp`, `authorization_code`): the ERROR record was `{"level":"error","msg":"[Automation] failed to materialize connector instance 'qa_mcp_oauth_plain' via provider 'mcp' (ADR-0097): a configuration fault, so the instance is no longer retried. Fix its declaration; an edit or a restart re-attempts it.","error":"undefined"}`; the door answered `400 VALIDATION_ERROR` ending `… accepted only to a loopback host..`. The stored `degradedReason` (and the WARN line that carries it) ended with one period, so only the door doubled it. - Red first (new test on unfixed source): `4 failed | 1 passed (5)`: the log case (`reason` undefined) and the three door cases (message ends ".."); the CONTROL passed. - Green on head: `5 passed (5)`. - Ablation 1 (`scripts/ablation-replace.mjs`, meta back to `describeThrownForLog(info.cause)`): `1 failed | 4 passed (5)`, the log case only. Restored: blob `38cbee7e` == HEAD, `git diff HEAD` empty. - Ablation 2 (door back to `${notServable}.`): `3 failed | 2 passed (5)`, the three door cases only. Restored: blob `38cbee7e` == HEAD, `git diff HEAD` empty. - The test imports `./plugin.js` from `src/` (no `dist/` on its resolution path), so the ablations needed no rebuild. </details> <details><summary>Package suite, type check, lint</summary> - `pnpm --filter @objectstack/service-automation test`: `256 passed` files, `3286 passed` tests. - `pnpm --filter @objectstack/service-automation typecheck`: exit 0. - `eslint packages/services/service-automation --no-inline-config`: exit 0. </details> ## Acceptance notes - Another record has the same `{"error":"undefined"}` shape, outside this card's scope: `degradeConnectorInstance` (the backoff-retried degrade, not a held one) logs `describeThrownForLog(info.cause)`, and its "connection store unreadable" caller passes `cause: undefined`. Left alone per the order (diff kept to the held-instance branch and the constant); reported to the seat. - The issue said the stored `degradedReason` also ended "..". Measured: it ended with one period; only the door's answer doubled it. ## Verification Every local reading above is on head `7cfc27ff`. Derived gate families (`node scripts/pm/dispatch-gates.mjs --commands`, reconciled with `--ran`): 61 derived, 60 run with exit 0, 1 not measured (`check:dual-build-cjs-loads`, exit 3 PREREQUISITE NOT MET: not every package has a `dist/` in this checkout). `check-plugin-teardown-shape --self-test` first answered exit 3 (its pinned fixture commit was outside the shallow clone); after `git fetch --depth=1` of that commit it answered exit 0 (48 cases). `check:nul-bytes`: exit 0. The repo-wide lint, `test:repo` and the full farm are CI's. Full dogfood not run: no `!` in the title and no `(narrowing)` arm. Overlap: none of the 24 open PRs touches `packages/services/service-automation/src/plugin.ts`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5nw17JgWHWmJM5HZNT --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 97ca257 commit de68479

3 files changed

Lines changed: 242 additions & 4 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
fix(service-automation): a held connector instance's ERROR record carries its reason, and the connection door's refusal sentence ends with one period
6+
7+
Clause-②: no — log meta and one character of refusal text; no schema, accept set, error code or status moves
8+
Metadata-protocol: none
9+
10+
- **Log.** When a provider-bound connector instance is held as a configuration fault that the platform itself refuses (an `oauth2` instance this server cannot serve: an MCP server over plain `http://` off loopback, an endpoint not over https, no endpoints to discover), the `failed to materialize connector instance` ERROR record printed `{"error":"undefined"}`, because there was no thrown value to describe. It now carries the reason in its meta (`reason`), the same sentence the degraded instance stores. A fault that is a thrown provider error still logs that error in `error`, as before. The WARN line and the hold itself are unchanged.
11+
- **Retried instance, same record shape.** An `authorization_code` instance whose connections could not be read is degraded and retried on the backoff with no thrown value either; its `retrying with backoff` ERROR record printed `{"error":"undefined"}` too, and now carries `reason` the same way. A provider error that degrades an instance still rides `error`.
12+
- **Refusal text.** `GET`/`POST /api/v1/automation/connectors/NAME/connection` answered such an instance's `400 VALIDATION_ERROR` with the sentence followed by a second period ("… only to a loopback host.."). The door now answers the sentence as the instance's degraded reason states it, ending once.
Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// A held connector instance (a configuration fault: ADR-0097 does not retry
4+
// it) logs one ERROR record. When the fault is the platform's own refusal
5+
// rather than a thrown provider error (an `oauth2` instance this server cannot
6+
// serve: an MCP server over plain http off loopback, an endpoint not over
7+
// https), there is no thrown value to describe: the record carries the reason
8+
// itself in its meta, never the string "undefined". The same refusal sentence
9+
// answers the connection door's 400, and it ends its sentence once.
10+
//
11+
// The records are read through a REAL logger's bytes (JSON format, stderr),
12+
// as connector-fail-cause.test.ts does, not through a spy.
13+
14+
import { describe, it, expect, vi, afterEach } from 'vitest';
15+
import { LiteKernel } from '@objectstack/core';
16+
import type { ConnectorProviderFactory } from '@objectstack/spec/integration';
17+
import { ConnectorUpstreamUnavailableError } from '@objectstack/spec/integration';
18+
import { AutomationServicePlugin } from './plugin.js';
19+
import type { AutomationEngine } from './engine.js';
20+
import { ConnectorConnectionStore } from './connector-connections.js';
21+
22+
const flush = () => new Promise<void>((r) => setTimeout(r, 0));
23+
24+
afterEach(() => {
25+
vi.restoreAllMocks();
26+
});
27+
28+
const ADMIN = { tenantId: 'org_a', userId: 'usr_admin' };
29+
const MATERIALIZE_PREFIX = '[Automation] failed to materialize connector instance';
30+
31+
/** `provider: 'mcp'`, `authorization_code`, its transport over plain http to a non-loopback host. */
32+
function plainHttpMcp(name = 'qa_mcp_oauth_plain') {
33+
return {
34+
name,
35+
label: name,
36+
type: 'api',
37+
provider: 'mcp',
38+
providerConfig: { transport: { kind: 'http', url: 'http://192.0.2.2:41520/api/v1/mcp' } },
39+
auth: { type: 'oauth2', grant: 'authorization_code' },
40+
};
41+
}
42+
43+
/** An `oauth2` instance on `mcp` with every endpoint declared, its `authorizationUrl` plain http off loopback. */
44+
function plainAuthorizationUrl(name = 'plain_endpoint') {
45+
return {
46+
name,
47+
label: name,
48+
type: 'api',
49+
provider: 'mcp',
50+
providerConfig: { transport: { kind: 'http', url: 'https://mcp.example.com/mcp' } },
51+
auth: {
52+
type: 'oauth2',
53+
grant: 'authorization_code',
54+
clientId: 'client',
55+
tokenUrl: 'https://idp.example.com/token',
56+
authorizationUrl: 'http://idp.example.com/authorize',
57+
},
58+
};
59+
}
60+
61+
/** An `authorization_code` instance on a non-mcp provider declaring no endpoints or client. */
62+
function missingEndpoints(name = 'no_endpoints') {
63+
return {
64+
name,
65+
label: name,
66+
type: 'api',
67+
provider: 'mcp',
68+
providerConfig: { transport: { kind: 'stdio', command: 'dummy' } },
69+
auth: { type: 'oauth2', grant: 'authorization_code' },
70+
};
71+
}
72+
73+
/** Boot with the declared set swappable through `metadata:reloaded`, under a real JSON logger. */
74+
async function boot(initial: unknown[], factory: ConnectorProviderFactory) {
75+
const state = { declared: initial };
76+
let captured: any;
77+
const kernel = new LiteKernel({ logger: { level: 'error', format: 'json' } } as never);
78+
kernel.use(new AutomationServicePlugin());
79+
kernel.use({
80+
name: 'test.harness',
81+
type: 'standard' as const,
82+
version: '1.0.0',
83+
dependencies: ['com.objectstack.service-automation'],
84+
async init(ctx: any) {
85+
captured = ctx;
86+
ctx.registerService('objectql', {
87+
registry: { listItems: (type: string) => (type === 'connector' ? state.declared : []) },
88+
});
89+
ctx.getService('automation').registerConnectorProvider('mcp', factory, { credentialHandle: true });
90+
},
91+
async start() {},
92+
} as never);
93+
await kernel.bootstrap();
94+
await flush();
95+
return {
96+
kernel,
97+
engine: kernel.getService('automation') as AutomationEngine,
98+
reload: async (next: unknown[]) => {
99+
state.declared = next;
100+
await captured.trigger('metadata:reloaded');
101+
await flush();
102+
},
103+
};
104+
}
105+
106+
/** Everything written to stderr while `fn` runs, one entry per physical line. */
107+
async function captureStderr<T>(fn: () => Promise<T>): Promise<{ result: T; lines: string[] }> {
108+
const chunks: string[] = [];
109+
const spy = vi.spyOn(process.stderr, 'write').mockImplementation(((c: string | Uint8Array) => {
110+
chunks.push(String(c));
111+
return true;
112+
}) as never);
113+
let result: T;
114+
try {
115+
result = await fn();
116+
} finally {
117+
spy.mockRestore();
118+
}
119+
return { result, lines: chunks.join('').split('\n').filter((l) => l.length > 0) };
120+
}
121+
122+
type LoggedRecord = { level: string; msg: string; reason?: unknown; error?: unknown };
123+
124+
function heldRecordsOf(lines: string[], name: string): LoggedRecord[] {
125+
return lines
126+
.map((l) => JSON.parse(l) as LoggedRecord)
127+
.filter((r) => r.msg.startsWith(MATERIALIZE_PREFIX) && r.msg.includes(`'${name}'`));
128+
}
129+
130+
const neverCalled: ConnectorProviderFactory = () => {
131+
throw new Error('the factory of a held instance is never called');
132+
};
133+
134+
describe('a held connector instance logs its reason, and the refusal sentence ends with one period', () => {
135+
it('the ERROR record of a platform-refused instance carries the reason in its meta, never "undefined"', async () => {
136+
const { result: booted, lines } = await captureStderr(() => boot([plainHttpMcp()], neverCalled));
137+
const reason = booted.engine.getConnectorDegradedReason('qa_mcp_oauth_plain');
138+
expect(reason).toContain('transport `url` is not over https');
139+
140+
const records = heldRecordsOf(lines, 'qa_mcp_oauth_plain');
141+
expect(records, 'the held instance is reported exactly once').toHaveLength(1);
142+
expect(records[0].level).toBe('error');
143+
expect(records[0].reason).toBe(reason);
144+
expect(records[0].error, 'no thrown value, so no `error` field').toBeUndefined();
145+
expect(lines.join('\n')).not.toContain('"error":"undefined"');
146+
await booted.kernel.shutdown();
147+
});
148+
149+
it('CONTROL: a held instance whose fault is a thrown provider error still logs that error', async () => {
150+
// Degraded first (its upstream is down), then the retry meets a
151+
// provider configuration fault: the held path with a thrown cause.
152+
const upstream = { down: true };
153+
const factory: ConnectorProviderFactory = (ctx) => {
154+
if (upstream.down) throw new ConnectorUpstreamUnavailableError(`connector '${ctx.name}' could not reach its MCP server`);
155+
throw new Error(`connector-mcp provider: connector '${ctx.name}' providerConfig.include names 'nope', which the MCP server does not list.`);
156+
};
157+
const entry = {
158+
name: 'refuses_include',
159+
label: 'refuses_include',
160+
type: 'api',
161+
provider: 'mcp',
162+
providerConfig: { transport: { kind: 'http', url: 'https://mcp.example.com/mcp' }, include: ['nope'] },
163+
};
164+
const { kernel, engine, reload } = await boot([entry], factory);
165+
expect(engine.getConnectorDegradedReason('refuses_include')).toMatch(/could not reach/);
166+
167+
upstream.down = false;
168+
const { lines } = await captureStderr(() => reload([entry]));
169+
const records = heldRecordsOf(lines, 'refuses_include');
170+
expect(records).toHaveLength(1);
171+
expect(records[0].msg).toContain('a configuration fault, so the instance is no longer retried');
172+
expect(records[0].error).toBe(
173+
"connector-mcp provider: connector 'refuses_include' providerConfig.include names 'nope', which the MCP server does not list.",
174+
);
175+
await kernel.shutdown();
176+
});
177+
178+
it('the ERROR record of a retried instance whose connections could not be read carries the reason, never "undefined"', async () => {
179+
// A servable `authorization_code` instance whose connection store
180+
// answers with a failure: degraded and retried on the backoff, with no
181+
// thrown provider error to describe.
182+
vi.spyOn(ConnectorConnectionStore.prototype, 'listFor').mockRejectedValue(new Error('dummy-store-down-28939'));
183+
const entry = { ...plainAuthorizationUrl('unreadable_connections') };
184+
entry.auth = { ...entry.auth, authorizationUrl: 'https://idp.example.com/authorize' };
185+
const { result: booted, lines } = await captureStderr(() => boot([entry], neverCalled));
186+
const reason = booted.engine.getConnectorDegradedReason('unreadable_connections');
187+
expect(reason).toContain('could not be read');
188+
189+
const records = lines
190+
.map((l) => JSON.parse(l) as LoggedRecord)
191+
.filter((r) => r.level === 'error' && r.msg.includes("'unreadable_connections'"));
192+
expect(records, 'the degraded instance is reported exactly once').toHaveLength(1);
193+
expect(records[0].msg).toContain('retrying with backoff');
194+
expect(records[0].reason).toBe(reason);
195+
expect(records[0].error, 'no thrown value, so no `error` field').toBeUndefined();
196+
expect(lines.join('\n')).not.toContain('"error":"undefined"');
197+
await booted.kernel.shutdown();
198+
});
199+
200+
it.each([
201+
['an MCP server over plain http', plainHttpMcp()],
202+
['an endpoint not over https', plainAuthorizationUrl()],
203+
['no endpoints to discover', missingEndpoints()],
204+
] as const)('⛔ the connection door refuses %s with a 400 whose sentence ends with one period', async (_label, entry) => {
205+
const { kernel, engine } = await boot([entry], neverCalled);
206+
// The husk stores the same sentence the door answers.
207+
const stored = engine.getConnectorDegradedReason(entry.name);
208+
expect(stored).toMatch(/[^.]\.$/);
209+
for (const call of [
210+
() => engine.beginConnectorConnection({ connector: entry.name, context: ADMIN as never }),
211+
() => engine.getConnectorConnection({ connector: entry.name, context: ADMIN as never }),
212+
]) {
213+
const err = (await call().catch((e: unknown) => e)) as Error & { status?: number; code?: string };
214+
expect(err).toMatchObject({ status: 400, code: 'VALIDATION_ERROR' });
215+
expect(err.message).toBe(stored);
216+
expect(err.message).not.toMatch(/\.\.$/);
217+
}
218+
await kernel.shutdown();
219+
});
220+
});

‎packages/services/service-automation/src/plugin.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3574,7 +3574,9 @@ export class AutomationServicePlugin implements Plugin {
35743574
: 'instance registered degraded (no actions)') +
35753575
`; ${changed ? 're-checking' : 'retrying'} with backoff, attempt ${attempts}.`,
35763576
undefined,
3577-
describeThrownForLog(info.cause),
3577+
// No thrown value (the connection store could not be read): the
3578+
// reason rides `meta`, never `describeThrownForLog(undefined)`.
3579+
info.cause === undefined ? { reason: info.reason } : describeThrownForLog(info.cause),
35783580
);
35793581
}
35803582

@@ -3620,14 +3622,17 @@ export class AutomationServicePlugin implements Plugin {
36203622
);
36213623
}
36223624
}
3623-
// #5575 — the provider's message rides `meta`, never the message.
3625+
// #5575 — the provider's message rides `meta`, never the message. A
3626+
// platform refusal (an `oauth2` instance this server cannot serve) has
3627+
// no thrown value: its reason rides `meta` instead, so the record never
3628+
// renders `describeThrownForLog(undefined)`, the string "undefined".
36243629
ctx.logger.error(
36253630
`[Automation] failed to materialize connector instance '${info.name}' via provider '${info.provider}' (ADR-0097): ` +
36263631
'a configuration fault, so the instance is no longer retried. ' +
36273632
(info.hasLive ? 'The previously-materialized connector keeps serving. ' : '') +
36283633
'Fix its declaration; an edit or a restart re-attempts it.',
36293634
undefined,
3630-
describeThrownForLog(info.cause),
3635+
info.cause === undefined ? { reason: info.reason } : describeThrownForLog(info.cause),
36313636
);
36323637
}
36333638

@@ -3949,7 +3954,8 @@ export class AutomationServicePlugin implements Plugin {
39493954
);
39503955
}
39513956
const notServable = oauth2NotServableReason(name, { ...entry, provider: entry.provider as string }, auth, true);
3952-
if (notServable !== undefined) throw credentialDoorRefusal(`${notServable}.`, 'VALIDATION_ERROR', 400);
3957+
// The reason ends its own sentence: the door answers it as it is.
3958+
if (notServable !== undefined) throw credentialDoorRefusal(notServable, 'VALIDATION_ERROR', 400);
39533959
const organizationId = request?.context?.tenantId;
39543960
if (typeof organizationId !== 'string' || organizationId.length === 0) {
39553961
throw credentialDoorRefusal(

0 commit comments

Comments
 (0)