fix(spec,runtime,service-i18n): 让 dispatcher domain 和它们的服务契约描述同一个面 (#4127) - #4143
Merged
os-zhuang merged 1 commit intoJul 30, 2026
Merged
Conversation
…ice contracts describe the same surface (#4127) #4087 retired a `/storage` bridge that called `upload(key, data, options?)` as `upload(file, { request })`. Sweeping the other dispatcher domains against `packages/spec/src/contracts/*` found the mirror-image gap in three places: the call site and the implementation agreed, and the CONTRACT was the thing nobody had written. Each was worked around with `typeof x.foo === 'function'` — a duck-type is what "the contract does not cover this" looks like when nobody fixes the contract. Fixed at the contract (Prime Directive #12). Contracts: - `INotificationService` += `listInbox?` / `markRead?` / `markAllRead?` and `InboxQuery` / `InboxNotification` / `InboxListResult` / `MarkReadResult`. Three SDK-expressed routes rested on them, implemented by service-messaging, while the contract described only `send`. The dev stub implements exactly `send`/`sendBatch` BECAUSE it followed the contract — so the one implementation written to spec was the one the domain had to duck-type past. Optional, because a send-only provider (SMTP, Twilio, a Slack webhook) fills the slot legitimately without an inbox — a fact `handlerReady` cannot express, since the slot is serveable and only this capability is absent. - `II18nService` += `getFieldLabels?`. Both serving surfaces probed for it and both documented it as "optional on II18nService", which was untrue until now. - `IAutomationService` += `getFlowRuntimeStates?` and `FlowRuntimeState`. The dispatcher's inline cast declared `{ name, enabled, bound }` — a third copy of the shape, narrower than the engine returns, dropping the `status` / `triggerType` / `object` fields that say WHY a flow is unbound. Two runtime defects fell out of the same sweep, both on the legacy trigger route — the one `client.automation.trigger()` calls: - It passed the raw HTTP body to `execute(name, body)`, so the `{recordId, objectName, params}` translation never ran AND no caller identity was forwarded. A flow's default `runAs` is `'user'`, and a `runAs:'user'` run whose trigger resolved no user has its data operations REFUSED (#3760, fail-closed) — so that SDK method could not run a data-touching flow at all, while `POST /:name/trigger` could. service-automation's own comment claims "most trigger surfaces (REST action / trigger endpoint) already resolve the full envelope"; for this endpoint it was not. Both routes share one context builder now. - The `automationService.trigger(...)` probe it tried first is deleted. Nothing in the repo has ever implemented `trigger` on the automation slot and the contract never declared it, so the branch was unreachable everywhere and its `execute` "fallback" was the route. Declaring `trigger?` would have blessed a second name for `execute`. Call sites read the contract now instead of `as any` / inline re-declarations, so the next deviation is a compile error. service-i18n's probe loses two casts with it — one through `Record<string, unknown>`, one restating the signature; its build failed the moment the method became declared, which is the point. The test that pinned the dead `trigger` branch asserted `trigger` was called with three arguments no implementation takes — the same shape that kept #4087 green for months. It now pins the contract method and the translated context. Not included: `getConnectorDescriptors`, the fourth gap in #4127. It needs the `ConnectorDescriptor` / `ConnectorActionDescriptor` / `ConnectorOrigin` / `ConnectorState` cluster promoted from service-automation into `packages/spec`, which is its own change. Refs #4127 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015T5CeitwV1jXLvsL1A84tw
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 115 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 30, 2026 11:30
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
… connector registry it already serves (#4127) (#4150) The fourth and last dispatcher call site from #4127 calling a method its contract never declared. The first three shipped in #4143; this one was held back because the fix is a type MOVE, not a type addition — `ConnectorDescriptor` was declared in service-automation's engine, one IMPLEMENTATION of `IAutomationService`. A contract cannot name a type living inside its own implementation, so `getConnectorDescriptors` could not be declared until the type had a home in the spec. `IAutomationService` += `getConnectorDescriptors?()`. It is the sibling of `getActionDescriptors`, declared since ADR-0018: the two fill the flow designer's `connector_action` node together — node vocabulary from one, the connector / action / input pickers from the other. Only one was written down. `GET /automation/connectors` has served the other since ADR-0022 by probing for the method and re-typing its own result as `any` to filter on `?type=` — a filter on a field the type system did not know existed, one typo from silently matching nothing and answering an empty registry, which is also what the route legitimately returns when the method is absent, so the failure had no distinguishable symptom. Optional for the same reason `getActionDescriptors` is: a connector registry is a capability of the flow-engine implementation, not a property of every automation slot. A script-runner filling the slot has no connectors to describe and answers an empty registry rather than 404 — `handlerReady` does not apply, since the slot is serveable and only this capability is absent. `ConnectorDescriptor` / `ConnectorActionDescriptor` / `ConnectorOrigin` / `ConnectorState` move to `@objectstack/spec/integration`, beside the ADR-0097 provider contract, for the reason that file already states about itself: they are pure types, so a connector plugin — or a designer client, or the dispatcher — speaks about registered connectors depending only on the spec, with no runtime coupling to the engine. `ConnectorOrigin` is ADR-0097 §4 vocabulary and `ConnectorState` is #3017 vocabulary; neither was ever engine-private in meaning, only in location. Nothing is renamed and no shape changes. service-automation imports the four back and re-exports them from its index — same names, same entry point — so every existing importer compiles unchanged. `ConnectorState` joins that re-export, which it should have been in all along: it is a required field of the descriptor the index has always exported. The four additions are recorded in the `check:api-surface` snapshot (0 breaking, 4 added). The dispatcher's connector fixture had already drifted, which is the concrete cost: it declared `{ name, label, type, actions }` and omitted `origin` and `state` — both REQUIRED, and both the fields a designer reads to tell a live declarative instance from a plugin one (ADR-0097 §4), or a dispatchable connector from a degraded one listed honestly rather than hidden (#3017). Nothing caught it, because an undeclared return type cannot be checked against. The fixture is typed now, and a new test pins that origin / state / degradedReason survive the route rather than only name and type. Closes #4127
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
… not `any` (#4127) (#4168) #4127's most valuable item was the one it did not do: add a gate for the class. Its four contract gaps were found by a human sweeping the dispatcher by hand. A sweep is not repeatable, and this one was not complete. The root was one line — `getService(name: string): any` in domain-handler-registry.ts. Against `any`, a domain calling a method its contract declares and a domain calling a method nobody declares typecheck identically. That is what let #4087 ship a `/storage` handler passing two arguments no implementation takes, and what hid #4127's four. `CoreServiceContracts` is the slot -> contract ledger. `CoreServiceName` named the slots and `contracts/*` described them; nothing connected the two. It does now, and `getService<K>(name: K)` resolves through it, so a call outside the contract is a compile error at the call site. An entry is a claim, so entries are made only where the binding is evidenced: by the provider that registers the slot (service-storage -> file-storage; objectql -> data, whose own comment reads "ObjectQL implements IDataEngine"), or by dispatcher work that proved it (#4143/#4150 for automation, notification, i18n). `ui` is deliberately unmapped — the slot exists and domains/ui.ts serves it, but no IUiService was ever written. An unmapped slot resolves to `unknown`, not `any`, so it must be cast deliberately and the gap stays legible. Two findings within minutes of turning it on: - `/auth` called a method that does not exist. domains/auth.ts probed `authService.handler(request, response)`; `IAuthService` declares `handleRequest(request)` and `AuthManager` implements exactly that, with no `handler`. False on every deployment — #4143's dead `automation.trigger` again. #4127's sweep never mentions `/auth` in either its gap list or its "clean" list: the file the compiler flagged first is the one the human pass skipped. Not a live hole — the Hono adapter calls `handleRequest` itself and only falls through when no usable auth service answered — but reading the contract makes the branch reachable for the first time, so a host calling `handleAuth` directly WITH an auth service now gets it instead of mockAuthFallback's `mock_<uuid>` session. - `POST /analytics/sql` invoked an optional method unguarded. `generateSql?` is optional on IAnalyticsService — unlike `query`/`getMeta` beside it — so a provider without it answered a 500 from TypeError instead of saying the capability is absent. Answers `handled: false` now, the same 404 the entry gate already gives for absent analytics capability. `isServiceServeable` becomes a type guard (`svc is NonNullable<T>`). Every domain already calls it first on a resolved slot, so one predicate narrows away the `undefined` for the whole body — the null check and the capability check were always the same check. The test-side hole #4127 predicted, closed for this batch: THREE tests across two files mocked `{ handler }` for auth, including one whose subject was the resolution path, so it proved the lookup worked and nothing about the call. `ContractMock<T>` guards mock keys against the contract; signatures stay `unknown` so vi.fn() does not force everything back to `as any`. The automation mock's `trigger` stays as a labelled negative control outside the checked literal — a test asserting the route never calls it is the point. The 12 domains not calling `getService` are untouched. `resolveService`, which also takes non-CoreServiceName names like `protocol` and `objectql`, is left for a later batch rather than widened here. Refs #4127
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #4127(#4087 / PR #4112 的后续)。实现过程中有两处需要更正我在 issue 里的判断,都比原判断严重——见第 1 节。
1. 更正:
/automation那条不是"契约漏声明",是一个真实缺陷issue 里我把
trigger归类为"契约漏了声明,实现有"。核查后是反的:①
trigger全仓没有任何实现。 automation 槽上从来没有过trigger方法——只有 job 服务(IJobService.trigger)和 kernel 事件总线(ctx.trigger)有同名方法。domains/automation.ts里那个typeof automationService.trigger === 'function'守卫在任何部署上都是 false,真正跑的一直是它下面的 "fallback to execute"。② 于是真正的缺陷浮出来了:遗留 trigger 路由不转发调用者身份。 两条路由做同一件事,只有一条做对:
execute的第二参POST /automation/:name/triggerAutomationContext:{recordId, objectName, params}翻译 +userId/positions/permissions/tenantIdPOST /automation/trigger/:nameclient.automation.trigger()flow 的默认
runAs是'user',而 #3760 规定runAs:'user'的运行若没解析出用户,数据操作一律拒绝。这是 fail-closed,所以不是安全漏洞——但结果是 SDK 的automation.trigger()跑不了任何碰数据的 flow,而 UI 那条能跑。而且params翻译也没跑,flow 变量(params.recordId、<object>Id别名)无从解析。顺带一提,service-automation 自己的注释写着「大多数触发面(REST action / trigger endpoint)已经解析了完整的授权信封」——对这个 endpoint 而言这句话不成立。
修法:抽出
buildAutomationContext(),两条路由共用;删掉死的trigger探测。没有声明trigger?让探测"合法化"——那等于给execute祝福第二个名字,正是 Prime Directive #12 禁止的。③
/i18n也需要更正。 不是"契约没有、实现有",而是"契约没有、实现也没有,但两处代码注释都以为契约有"。dispatcher 和 service-i18n 都在探测getFieldLabels,两处注释都写着"它在II18nService上是可选的"——这句话在本 PR 之前不成立。这里选择声明而不是删除:它是一个真实的扩展点(provider 若把标签放在 bundle 表达不了的地方,比如翻译记忆库或按租户覆盖表,就实现它),两处探测本就是按扩展点写的。和trigger的区别在于:getFieldLabels是一个独立能力,trigger只是execute的重名。2. 契约改动(Prime Directive #12:修契约,不修调用点)
INotificationService+=listInbox?/markRead?/markAllRead?,以及InboxQuery/InboxNotification/InboxListResult/MarkReadResult。保持可选是关键判断:收件箱需要持久化存储,纯发送方(SMTP / Twilio / Slack webhook)合法地占这个槽却没有收件箱,而
handlerReady表达不了这件事——槽是可服务的,只是缺一个能力。所以探测保留,但它现在读的是已声明的可选能力,不是调用点凭空发明的方法。这里最值得记的一点:dev stub 只实现
send/sendBatch,恰恰因为它照着契约实现。契约漏了收件箱面,于是唯一按契约写的实现反而成了 domain 必须 duck-type 绕过的那个。II18nService.getFieldLabels?(理由见上)。IAutomationService.getFlowRuntimeStates?+FlowRuntimeState。dispatcher 原来用内联 cast 声明成{name, enabled, bound}——形状的第三份副本,而且比引擎实际返回的窄,丢掉了status/triggerType/object这三个恰好用来回答"为什么没绑定"的字段。3. 调用点改成读契约
domains/notifications.ts从as any换成INotificationService;三条路由各自探测自己的方法(它们分别可选,"有收件箱可读"从不蕴含"有读状态可写"——在方法被声明之前,入口那个探测是唯一挡在 list-only provider 和markRead的TypeError之间的东西)。/automation/_status读Pick<IAutomationService, 'getFlowRuntimeStates'>。这套改法当场自证了一次:service-i18n 的构建在方法变成"已声明"的瞬间就失败了,因为它那两个 cast(一个穿
Record<string, unknown>,一个内联重声明签名)只有在方法未声明时才需要。删掉即可——这正是 issue 里说的"让偏离变成编译错误"。4. 测试
钉死
trigger死分支的那个旧用例,断言的是trigger被以三个参数调用——没有任何实现接受这个签名。和让 #4087 绿了几个月的形状完全一样:mock handler 想要的样子,handler 当然永远同意。现在钉的是契约方法 + 翻译后的 context。新增:两条 trigger 路由产出完全相同的 context(含身份信封)、非契约的
trigger即使存在也不被调用、_status透传完整的FlowRuntimeState、以及契约层的 inbox provider 类型测试(含"纯发送方仍然合法"这一条)。5. 验证
@objectstack/runtime65 files / 923 tests 全绿@objectstack/spec270 files / 7032 tests、service-messaging137、service-i18n62 全绿gen:api-surface已重跑,新增 5 个 interface 导出,纯 additive)pnpm lint、check:route-envelope、check:doc-authoring、check:nul-bytes、check:error-code-casing、check:authz-resolver全过6. 没有做的
getConnectorDescriptors—— #4127 里的第四处。它需要把ConnectorDescriptor/ConnectorActionDescriptor/ConnectorOrigin/ConnectorState这一组从 service-automation 提升进packages/spec,是独立的一块(spec 导出 + 产物再生 + 单独 review),留给后续。无需迁移:新增的契约成员全部可选,现有实现照常有效;两处运行时修复只是让本来失败/降级的路由和它们能工作的孪生路由行为一致。
Generated by Claude Code