Skip to content

Commit c48cd2c

Browse files
fix(service-automation)!: a connector pull whose input breaks the action's declared inputSchema is refused before dispatch (#28140)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DNxhDE3NbjyqbvDsmL1Aox7)_ Fixes #28040 Clause-②: yes (narrowing) Metadata-protocol: none Skills-docs: updated here — `content/docs/automation/jobs.mdx` ("How a run is recorded") and `packages/spec/docs/SYNC_ARCHITECTURE.md` ("How a pull runs") name the new refusal; `skills/**`: none — `git grep -n -i "connectorSource\|pullConnectorSource\|inputSchema" -- skills` has 1 hit, `skills/objectui/guides/plugin-development.md:113` (`ComponentInputSchema`, unrelated), so no skill teaches the pull door. ## 维护者速读 - 改了什么:连接器拉取(job 的 `pull`)在调用连接器动作前,先用动作声明的 `inputSchema` 校验将要发送的输入(含水位参数);不合格即以 `VALIDATION_ERROR` 400 拒绝并点名参数,上游零调用。 - 为什么改:#27995 只给 `connector_action` 流程节点加了这道校验,拉取入口仍把缺必填参数的请求发到远端(OpenAPI 路径占位符被原样发送)。 - 风险与回滚:收窄(BREAKING,patch 级):原本"侥幸成功"的拉取若输入不满足声明的 schema 会转为 job 运行 `failed`;回滚即 revert 本 PR。节点的行为与报错文字逐字节不变。 - 先看:`packages/services/service-automation/src/connector-pull.ts`(新增校验段)、`src/connector-input-schema.ts`(从节点原样搬出的校验器)、`src/builtin/connector-nodes.ts`(改为调用共享校验器)。 - 两分钟验证:`pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/connector-pull.test.ts`,预期 23/23 通过,其中 `[#28040]` 组 5 条。 - 需要你做的:无,席位落地。 ## What changes - **One checker, two doors.** The `inputSchema` judgment the `connector_action` node gained in #28035 (#27995) moves out of `builtin/connector-nodes.ts` into `src/connector-input-schema.ts`, unchanged byte for byte (the moved block was diffed against `origin/main`'s, `IDENTICAL`). The module exports `findConnectorActionInputSchema` (the descriptor read the node made), `judgeConnectorInput` (the violations joined as the node joined them, capped at five plus `; and N more`) and `TEXT_FOR_SCALAR_HINT`. It is not re-exported from the package entry. - **The node** calls the shared helper. Its refusal text is assembled from the same pieces in the same order, so `code`, `error` and the cast hint are byte-identical; its existing pins (`connector-input-schema.test.ts`, untouched) stay green. - **The pull door** (`pullConnectorSource`, the door a job's `pull` runs) judges the input it is about to send, after the watermark parameter is added and before the handler is called. A failing input throws `ConnectorPullError` with `code: 'VALIDATION_ERROR'`, `status: 400` (the pair the door already uses for its other input faults) and `reason: 'input_schema_mismatch'`, and the message names each parameter by dotted path, says nothing was pulled, and points at `GET /api/v1/automation/connectors`. Nothing is called upstream and nothing is written. - **Public TypeScript surface** (why `Clause-②: yes (narrowing)`, re-derived from the diff rather than the order's `no (narrowing)`): `ConnectorPullRefusalReason` gains `'input_schema_mismatch'`, and `ConnectorPullRegistry` now extends `ConnectorInputSchemaSource`, which requires `getConnectorDescriptors()`. The automation engine (what `AutomationServicePlugin` passes) already has it; a hand-built registry adds it. - Changeset `.changeset/28040-connector-pull-input-schema.md`: `patch`, BREAKING banner, both declaration lines, ADR-0087 `not-required (no-migration-prescription)` marker, FROM / TO / fix. ## Why Prime Directive #10: a rule holds at every call site. Triage 6521041814 graded the pull door as the sibling call site of #27995, to be judged by the same check through one helper and refused before dispatch, naming the parameter. ## Tests - **Re-measure on `main` (premise holds).** The three refusal pins added here, run against `origin/main`'s `connector-pull.ts` (`8cb8c7f4`), fail with `the pull must refuse, not resolve: expected undefined to be an instance of ConnectorPullError`: the pull resolved, so it called the action. `Tests 3 failed | 20 passed (23)`. - **New pins** (`src/connector-pull.test.ts`, `[#28040]` group, through the shipped `pullConnectorSource` with a counting handler): 1. A missing required parameter (`query.since`) → `VALIDATION_ERROR` / 400 / `input_schema_mismatch`, the message names `'query.since'`, the handler is called 0 times, nothing is written. 2. A missing required section → names it with the parameters it requires (`missing required 'query' (with 'query.since')`), 0 calls. 3. A wrong type (`query.limit: '50'` against `integer`) → refused naming it, 0 calls; text is not converted. 4. The input judged is the input sent: a required `query.since` supplied only by the watermark pulls (1 call, with the watermark value). 5. A valid input still pulls (1 call) and writes the record. Every existing pull pin passes with no `inputSchema` declared, which is the "no schema ⇒ pull what you are given" leg. - **Ablation** (`node scripts/ablation-replace.mjs`, on the committed fix): `if (verdict)` → `if (verdict && false /* ABLATION */)` in `connector-pull.ts`, landed on disk (anchor 1 → 0, marker `grep -c` = 1, blob `6b358231` → `349fc8e6`). Exactly the three refusal pins turned red (`Tests 3 failed | 20 passed (23)`); restored, `blob == HEAD (6b35823) and git diff HEAD is empty`. The tests import `./connector-pull.js` from source, so no `dist/` leg applies. ## Acceptance notes - A required parameter that only the watermark supplies is absent on a first pull (nothing stored yet), so that pull is now refused. The changeset says so and gives the fix (give it a first value in `connectorSource.input`). - The `rest` provider's `request` action declares no `required` and its declared types match what a pull writes, so the shipped `rest` pull (the integration pin) is unaffected; `openapi` actions are where the check bites. - The refusal comes after the watermark read and the object-schema read (both reads, no writes), because the judged input includes the watermark value. Nothing is called upstream and nothing is written. - Out of scope, seen on the dogfood battery: `packages/qa/dogfood/test/api-action-door-forward.dogfood.test.ts:178` expects the `ACTION_DISABLED` message to contain `sys_metadata_activation`, but #27964 (`3e0fd7a8f7`, an ancestor of `origin/main`: `merge-base --is-ancestor` exit 0) rewrote that refusal as a plain sentence. The pin is red on `main`; this diff touches no file on that path. Reported to the seat for filing. ## Verification All local readings are at head `02de39cc91`. <details><summary>Package test + typecheck (service-automation), connector-openapi dispatch pin</summary> - `pnpm --filter @objectstack/service-automation build` → `VERDICT command-exit 0` - `pnpm --filter @objectstack/service-automation typecheck` → exit 0 (includes `check:test-typecheck`: `0 file(s) / 0 error(s)`) - `pnpm --filter @objectstack/service-automation test` → `Test Files 227 passed (227)`, `Tests 2881 passed (2881)`, `VERDICT command-exit 0` - `pnpm --filter @objectstack/connector-openapi exec vitest run --maxWorkers=2 src/input-schema-dispatch.test.ts` (#28035's end-to-end node pin) → `Tests 4 passed (4)` - Focused: `connector-pull.test.ts`, `builtin/connector-input-schema.test.ts`, `builtin/connector-nodes.test.ts`, `connector-pull.integration.test.ts`, `connector-pull-service-door.test.ts` → `Test Files 5 passed (5)`, `Tests 64 passed (64)` </details> <details><summary>Derived gate families (dispatch-gates) + pnpm lint</summary> - `node scripts/pm/dispatch-gates.mjs --commands` derived 92 families from 7 paths vs merge base `8cb8c7f46`. - All 92 exit 0. Three first ran into a prerequisite (exit 3) and were re-run green once it was met: `check-plugin-teardown-shape --self-test` (after `git fetch --unshallow`), `check:skill-examples` and `check:dual-build-cjs-loads` (after the full build that `check:type-check-debt` runs). `check:type-check-debt` first queued behind the dogfood run (exit 99, not measured) and passed on re-run: `124/127 workspace packages type-checked, 0 in the DEBT ledger`. - `node scripts/pm/dispatch-gates.mjs --ran` → `92 derived famil(ies) accounted for — 92 run, 0 NOT-MEASURED`; `pnpm lint` recorded outside the derivation. - `pnpm lint` → exit 0. - No exported symbol is renamed or removed (the moved helpers were module-private), so `check-platform-checklist --changed-from` is not owed. </details> <details><summary>Dogfood battery (the narrowing arm owes it)</summary> - Closure `pnpm --filter '@objectstack/dogfood^...' build` → exit 0; `pnpm --filter @objectstack/dogfood test` under the lock → `VERDICT command-exit 1`, `Test Files 1 failed | 357 passed | 3 skipped (361)`, `Tests 1 failed | 2882 passed | 20 skipped (2903)`. - The one failure is `api-action-door-forward.dogfood.test.ts > the switch holds: ban_user OFF answers 409 ACTION_DISABLED…`: `expected 'This action is turned off for this de…' to contain 'sys_metadata_activation'`. That is the stale pin on `main` named under Acceptance notes, not a consequence of this diff (no automation or connector path). </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (No schema touched.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: only `docs/SYNC_ARCHITECTURE.md` (not in `files[]`, no generator reads it), so nothing to regenerate. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English (the maintainer summary is Chinese by the seat's convention). --- _Generated by [Claude Code](https://claude.ai/code/session_016WV4cxUdTtczoG6TtcncPa)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 412908c commit c48cd2c

7 files changed

Lines changed: 350 additions & 127 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
fix(service-automation)!: a connector pull whose input breaks the read action's declared `inputSchema` is refused before the action is called, with `VALIDATION_ERROR` naming each parameter (#28040)
6+
7+
Clause-②: yes (narrowing)
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export, type or error code is removed or renamed: `MappingSchema.connectorSource` and `ConnectorActionSchema.inputSchema` keep their shapes, and the refusal reuses the existing `VALIDATION_ERROR` code the pull door already answers its other input faults with. What narrows is the run-time dispatch of the connector pull: a call the action's own declared schema says is wrong is no longer sent upstream. Nothing stored is rewritten or re-judged at rest, so there is nothing for `objectstack migrate meta` to rewrite and no tombstone. -->
11+
12+
**BREAKING** (an accept-set narrowing at run time), shipped as `patch`: nothing a metadata author writes changes shape (`Metadata-protocol: none`), so the level follows the kind, and the banner and the ADR-0087 disposition stay.
13+
14+
**FROM.** `pullConnectorSource` (the door a `job`'s `pull` runs) built the action's input from `connectorSource.input` plus the watermark parameter and called the connector's handler with it unchecked. The check the `connector_action` flow node gained in #27995 did not cover this door, so a pull whose input missed a parameter the action declares `required` still reached the remote API; for an OpenAPI `getPet`-style action the path placeholder was sent literally.
15+
16+
**TO.** The input the pull is about to send, with the watermark parameter added, is judged against the action's `inputSchema` by the same check the `connector_action` node runs (one shared helper, so the two doors judge alike: `required`, `type` with OpenAPI 3.0 `nullable`, descent through `properties` and `items`; keywords outside that subset never refuse). A failing input throws `ConnectorPullError` with `code: 'VALIDATION_ERROR'`, `status: 400` and the new `reason: 'input_schema_mismatch'`, naming each problem by its dotted path (`missing required 'query.since'`, `'query.limit' must be integer, got string`); a text value where a number or boolean is declared adds a hint to write it as that JSON type (no CEL runs on this door, so there is no cast to suggest). The action is not called and nothing is written, so the job run is recorded `failed` and its `retryPolicy` applies. An action that declares no `inputSchema` pulls whatever it is given, as before. The `connector_action` node's behaviour and messages are unchanged.
17+
18+
**TypeScript.** `ConnectorPullRefusalReason` gains `'input_schema_mismatch'`, and `ConnectorPullRegistry` now requires `getConnectorDescriptors()` (where the action's `inputSchema` is read). The automation engine already has it; a hand-built registry passed to `pullConnectorSource` adds it.
19+
20+
**Who sees it.** A `mapping` whose `connectorSource.input` omits a parameter the read action declares `required`, or sends one of a type the action does not declare: its pull now fails before the call instead of the remote answering it. A required parameter that only the watermark supplies is absent on the first pull (nothing is stored yet), so that pull is refused too.
21+
22+
**Fix.** Make `connectorSource.input` satisfy the action's `inputSchema`, which `GET /api/v1/automation/connectors` lists: supply every required parameter, and write each one as the declared type (`{ query: { limit: 50 } }`, not `{ query: { limit: '50' } }`). The watermark parameter is sent as the value stored in its target field, so the action must declare that parameter with the stored field's type (a text or datetime field ⇒ `string`, the ISO form a pull sends for a date), and must not declare it `required` unless `connectorSource.input` also gives it a first value.

‎content/docs/automation/jobs.mdx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,8 @@ export const OrdersPullJob = defineJob({
260260
[Data sync is defined on the target](https://github.com/objectstack-ai/objectstack/blob/main/packages/spec/docs/SYNC_ARCHITECTURE.md#data-sync-is-defined-on-the-target)
261261
for the watermark and the one-response limit.
262262
- **How a run is recorded.** A pull the platform refuses — the mapping is gone,
263-
the connector is degraded, the upstream answered `ok: false` — rejects, so the
263+
the connector is degraded, the input breaks the read action's declared
264+
`inputSchema`, the upstream answered `ok: false` — rejects, so the
264265
run is `failed` and the `retryPolicy` applies. A pull whose rows the import
265266
runner refused completes as `degraded`, with the counts as its reason; retrying
266267
would refuse the same rows. Otherwise the run is `success`, including a pull

‎packages/services/service-automation/src/builtin/connector-nodes.ts‎

Lines changed: 11 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { defineActionDescriptor } from '@objectstack/spec/automation';
55
import type { AutomationEngine, ConnectorActionContext } from '../engine.js';
66
import { refuseNode } from '../guard-refusal.js';
77
import { describeHold } from '../plugin-extension-api.js';
8+
import { findConnectorActionInputSchema, judgeConnectorInput, TEXT_FOR_SCALAR_HINT } from '../connector-input-schema.js';
89
import { resolveValueSlotMap } from './value-slot-map.js';
910

1011
/**
@@ -128,38 +129,24 @@ export function registerConnectorNodes(engine: AutomationEngine, ctx: PluginCont
128129
// hint: the EVALUATED input is judged against it here, after the
129130
// value slots resolved and before the handler is called, so a call
130131
// missing a required parameter (or carrying one of the wrong type)
131-
// fails this step instead of reaching the remote API, whose answer
132-
// to a malformed call is the remote's to choose. Read from the
133-
// designer-facing descriptor — `inputSchema` is projected from the
134-
// stored def unchanged, and it is the very schema an author picks
135-
// the action by on `GET /connectors`, so what is enforced is what
136-
// was shown. A `runtime` failure, not a guard: the input can come
132+
// fails this step instead of reaching the remote API. The judgment
133+
// is `connector-input-schema.ts`'s, the one the pull door runs too
134+
// (#28040). A `runtime` failure, not a guard: the input can come
137135
// from run data, so a `fault` edge may handle it.
138-
const inputSchema = engine
139-
.getConnectorDescriptors()
140-
.find((d) => d.name === cfg.connectorId)
141-
?.actions.find((a) => a.key === cfg.actionId)?.inputSchema;
142-
const violations: InputSchemaViolations = { lines: [], textForScalar: false };
143-
collectInputSchemaViolations(inputSchema, input, '', violations);
144-
if (violations.lines.length > 0) {
145-
const shown = violations.lines.slice(0, MAX_NAMED_VIOLATIONS);
146-
const more = violations.lines.length - shown.length;
136+
const verdict = judgeConnectorInput(
137+
findConnectorActionInputSchema(engine, cfg.connectorId, cfg.actionId),
138+
input,
139+
);
140+
if (verdict) {
147141
return {
148142
success: false,
149143
code: 'VALIDATION_ERROR',
150144
error:
151145
`connector_action '${node.id}': input for '${cfg.connectorId}.${cfg.actionId}' `
152-
+ `does not match the action's inputSchema — ${shown.join('; ')}`
153-
+ (more > 0 ? `; and ${more} more` : '')
146+
+ `does not match the action's inputSchema — ${verdict.violations}`
154147
+ `. Nothing was sent. Fix connectorConfig.input so it satisfies the inputSchema `
155148
+ `GET /api/v1/automation/connectors lists for this action.`
156-
// Text is never converted here: the executor cannot tell
157-
// a provider that serializes a section to strings from
158-
// one that sends JSON, so the cast is the author's.
159-
+ (violations.textForScalar
160-
? ` A text value for an integer, number or boolean parameter is not converted: `
161-
+ `cast it in a CEL envelope with int(), double() or bool().`
162-
: ''),
149+
+ (verdict.textForScalar ? TEXT_FOR_SCALAR_HINT : ''),
163150
};
164151
}
165152

@@ -196,103 +183,3 @@ export function registerConnectorNodes(engine: AutomationEngine, ctx: PluginCont
196183

197184
ctx.logger.info('[Connector] 1 built-in node executor registered (connector_action)');
198185
}
199-
200-
/** What one judgment found: each violation as a sentence, and whether any was text where a number or boolean is declared. */
201-
interface InputSchemaViolations {
202-
lines: string[];
203-
textForScalar: boolean;
204-
}
205-
206-
/** How many violations one refusal names before it summarises the rest as a count. */
207-
const MAX_NAMED_VIOLATIONS = 5;
208-
209-
/** JSON Schema `type` names this check can judge; any other name leaves the value unjudged. */
210-
const JUDGED_TYPES = new Set(['string', 'number', 'integer', 'boolean', 'object', 'array', 'null']);
211-
212-
function isPlainRecord(value: unknown): value is Record<string, unknown> {
213-
if (value === null || typeof value !== 'object' || Array.isArray(value)) return false;
214-
const proto = Object.getPrototypeOf(value);
215-
return proto === Object.prototype || proto === null;
216-
}
217-
218-
/** The value's JSON kind, or `undefined` for one with no JSON spelling to judge (a `Date`, a `Map`, …). */
219-
function jsonKindOf(value: unknown): string | undefined {
220-
if (value === null) return 'null';
221-
if (Array.isArray(value)) return 'array';
222-
if (isPlainRecord(value)) return 'object';
223-
if (typeof value === 'bigint') return 'number';
224-
if (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') return typeof value;
225-
return undefined;
226-
}
227-
228-
function matchesType(type: string, value: unknown, kind: string): boolean {
229-
if (type === 'integer') return typeof value === 'bigint' || (typeof value === 'number' && Number.isInteger(value));
230-
return type === kind;
231-
}
232-
233-
/**
234-
* Collect where `value` breaks `schema` (#27995), each as a sentence naming the
235-
* parameter by its dotted path. Judges a deliberate SUBSET of JSON Schema —
236-
* `required`, `type` (with OpenAPI 3.0 `nullable`), and descent through
237-
* `properties` and `items` — because the schemas reaching here are not ours:
238-
* an OpenAPI document's operation (which may keep an unresolved `$ref`), an
239-
* MCP server's `tools/list` entry (any draft), or a code connector's literal.
240-
* A keyword outside the subset (`$ref`, `oneOf`/`anyOf`/`allOf`, `enum`,
241-
* `format`, bounds, `additionalProperties`, …) never refuses a call: the
242-
* remote still judges those, as it did before. A missing piece is the narrow
243-
* direction on purpose — a call this check refuses is one the declared
244-
* schema itself says is wrong under every draft. `null` on an optional
245-
* property counts as absent (see the property loop), and text is never
246-
* coerced to the number or boolean a parameter declares.
247-
*/
248-
function collectInputSchemaViolations(schema: unknown, value: unknown, path: string, out: InputSchemaViolations): void {
249-
if (!isPlainRecord(schema)) return;
250-
const kind = jsonKindOf(value);
251-
if (kind === undefined) return;
252-
const declared = typeof schema.type === 'string' ? [schema.type] : Array.isArray(schema.type) ? schema.type : [];
253-
const types = declared.filter((t): t is string => typeof t === 'string');
254-
const judgeable = types.length > 0 && types.length === declared.length && types.every((t) => JUDGED_TYPES.has(t));
255-
if (
256-
judgeable
257-
&& !(kind === 'null' && schema.nullable === true)
258-
&& !types.some((t) => matchesType(t, value, kind))
259-
) {
260-
out.lines.push(`'${path || 'input'}' must be ${types.join(' or ')}, got ${kind}`);
261-
if (kind === 'string' && types.some((t) => t === 'integer' || t === 'number' || t === 'boolean')) {
262-
out.textForScalar = true;
263-
}
264-
return;
265-
}
266-
if (kind === 'object') {
267-
const record = value as Record<string, unknown>;
268-
const properties = isPlainRecord(schema.properties) ? schema.properties : undefined;
269-
const required = new Set(
270-
Array.isArray(schema.required) ? schema.required.filter((k): k is string => typeof k === 'string') : [],
271-
);
272-
for (const key of required) {
273-
if (!Object.prototype.hasOwnProperty.call(record, key) || record[key] === undefined) {
274-
const at = path ? `${path}.${key}` : key;
275-
// A missing required SECTION names the parameters it requires
276-
// too, so the first refusal already carries the parameter.
277-
const sub = properties?.[key];
278-
const inner = isPlainRecord(sub) && Array.isArray(sub.required)
279-
? sub.required.filter((k): k is string => typeof k === 'string').map((k) => `'${at}.${k}'`)
280-
: [];
281-
out.lines.push(`missing required '${at}'${inner.length > 0 ? ` (with ${inner.join(', ')})` : ''}`);
282-
}
283-
}
284-
if (properties) {
285-
for (const [key, sub] of Object.entries(properties)) {
286-
if (!Object.prototype.hasOwnProperty.call(record, key) || record[key] === undefined) continue;
287-
// `null` on an OPTIONAL property is read as absent: a provider
288-
// that serializes a section drops it, so the wire equals
289-
// omission, which the schema permits. On a REQUIRED property
290-
// it is still judged — it would be sent as a placeholder.
291-
if (record[key] === null && !required.has(key)) continue;
292-
collectInputSchemaViolations(sub, record[key], path ? `${path}.${key}` : key, out);
293-
}
294-
}
295-
} else if (kind === 'array' && isPlainRecord(schema.items)) {
296-
(value as unknown[]).forEach((item, i) => collectInputSchemaViolations(schema.items, item, `${path}[${i}]`, out));
297-
}
298-
}

0 commit comments

Comments
 (0)