fix(spec,runtime): resolveService 也返回槽的契约,core 槽上的 : any 逃逸清零 (#4127) - #4176
Merged
os-zhuang merged 1 commit intoJul 30, 2026
Merged
Conversation
…and the `: any` escapes on core slots are gone (#4127) Batch 2 of the #4127 gate. #4168 typed `getService` — easy, because every one of its call sites already passed a `CoreServiceName`. `resolveService` is the mixed one, and it is where the remaining `any` lived. Overloads split it exactly where the evidence does. A `CoreServiceName` resolves to the slot's contract; anything else keeps `any`: - Core slots, however written. 17 call sites address a core slot with a bare literal (`'metadata'` x10, `'automation'` x3, `'auth'` x3, `'ai'`) rather than `CoreServiceName.enum.*` — the same slot addressed two ways. Both resolve to the contract now, with no edit to the call sites. - Everything else — `protocol` (x22), `objectql` (x9), `mcp`, `kernel-resolver`, `security`, `scope-manager`. Real services with no `CoreServiceName` entry and no written contract. They keep `any` rather than being given a shape here that nothing verifies: that `any` is where the ledger honestly ends, and writing those contracts is its own change. The typing was being erased at three call sites, and that is the actual finding. `const x: any = await deps.resolveService('auth', ...)` defeats all of this — the annotation wins and #4168's work does nothing there. Sweeping for the pattern found three on core slots: `/mcp` x2 — two more undeclared methods. The domain calls `authService?.getMcpResourceUrl?.()` and `?.getMcpResourceMetadataUrl?.()`. AuthManager implements both (plugin-auth uses them internally); IAuthService declared neither. Call site and implementation agree, the contract is the thing nobody wrote. The `: any` + optional-chaining combination made this WORSE than the earlier gaps, not better: invisible to the type system AND accidentally safe. An absent method returns `undefined`, so the skill route silently fell back to deriving an MCP URL from the request host — a real disagreement between the auth service's canonical value and the derived one would have looked like normal operation. The whole point of `getMcpResourceUrl` is that it comes off the auth `basePath` so the two CANNOT disagree about the API prefix; the route's own comment says "the auth service owns the canonical value". Both declared optional: an auth provider without MCP/OAuth support fills the slot legitimately, and `getMcpResourceMetadataUrl` returning `null` (OAuth track off) stays distinct from the method being absent. `/packages` x1 — `const metadata: any = await deps.getService(...metadata)`, feeding `new SeedLoaderService(ql, metadata, ...)`. Annotation dropped; it typechecks against IMetadataService now. Its neighbours `protocol` and `ql` keep their `any` for the honest reason above. No other core-slot lookup is annotated away — the sweep is exhaustive over domains/*.ts. `api-surface.json` is unchanged: the two additions are interface MEMBERS, not new exports. 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 2 package(s): 113 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 14:10
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…Name` (#4127) (#4202) Batch 3 of the #4127 gate, after #4168 (`getService`) and #4176 (`resolveService`). Three slots — `security`, `shareLinks`, `objectql` — each had a written contract, a provider registering them, and call sites already inside the contract. The only missing link was that the slot name was not a `CoreServiceName` member, so nothing could connect them and all three sat behind `as any`. The ledger extends past the enum rather than the enum growing. The two answer different questions, and conflating them is what left these untyped: `CoreServiceName` answers "what happens at boot when this slot is empty?" — it sits beside `ServiceCriticality` and drives startup orchestration and discovery, so adding a member changes runtime behaviour and is effectively permanent. The ledger answers "what shape occupies this slot?" — pure type information. These three need only the second, so `ServiceSlotContracts extends CoreServiceContracts` adds them there and `resolveService` keys on `keyof ServiceSlotContracts`. Zero runtime effect. If one is later promoted to a genuine core service, its entry moves up and nothing else changes; a test asserts these keys are NOT enum members, so that migration cannot happen silently. Evidence before an entry, as always: `plugin-security` registers `security` and `ISecurityService`'s own doc names that registration; `plugin-sharing` registers `ShareLinkService`, which declares `implements IShareLinkService`; and `objectql` is an ALIAS of `data` — `packages/objectql`'s plugin registers the same instance under both names two lines apart, so one object was resolving as `IDataEngine` through one name and `any` through the other. `protocol` (22 call sites) and `mcp` have no written contract and stay unmapped. Turning it on found four things, all on the `/security` admin surface: 1. Request input reached the security service unvalidated. `?status=` was `String(query.status)` — any string — handed to a method whose contract declares exactly three values, and from there into the query's `where` clause. Not an injection (the `where` is structured, never interpolated), but `?status=garbage` matched no row and returned an empty list, which reads as "there are no suggestions" rather than "that is not a status". Now a 400. 2. A test pinned that bug as expected behaviour. The existing case asserted `status: 'open'` — not one of the three declared values — reached the service and returned 200. It proved the delegate carried A FILTER and nothing about that filter being a status. Same shape as batch 1's `auth.handler` mocks: coverage in appearance, a wrong contract in substance. 3. and 4. Two writes could not prove they had a caller. `confirmAudienceBindingSuggestion`/`dismissAudienceBindingSuggestion` declare `callerContext: SecurityContext` non-optionally — deliberately, since the read beside them declares it optional — and the domain passed a possibly-`undefined` execution context. This was NOT a live hole, and the distinction matters: with no execution context `shouldDenyAnonymous` already denied, because it sees no `userId`/`isSystem` and its allowlist arm needs a non-empty `path` this seam never passes, so it fell through to `return true`. What it never did was narrow `ec` itself — it only read `ec?.userId`. Checking `ec` directly is behaviour-preserving and makes the invariant legible to the compiler and the next reader. The `?status=` rejection is the one BEHAVIOUR CHANGE: an unknown status was a silent empty list and is now a 400 naming the accepted values. The accepted set is a `Record` keyed on the contract type, so adding a status to the contract leaves a key missing and renaming one leaves a key excess — either fails to compile, where a plain array would have drifted silently. Documented on the permission-sets page, where the endpoint is described. Refs #4127
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
… which found the project-membership gate not gating (#4127) (#4214) Batch 4 of the #4127 gate. #4168/#4176/#4202 made a slot lookup return the slot's contract. Nothing protected that: an `any` annotation on the RESULT switches the checking back off for that call site, silently, with no test failing and no visual difference from code that has it. Three such sites already existed and were found by grep — the same unrepeatable sweep this work replaced. The rule bans `: any` / `as any` on a `resolveService` / `getService` / `getRequestKernelService` result. Slots with no written contract (`protocol`, `mcp`, `kernel-resolver`, `scope-manager`) are exempted BY NAME, CENTRALLY, in eslint.config.mjs — not by inline disables, because `pnpm lint` runs `--no-inline-config` and ignores those on purpose. The effect is the one worth having: a deliberate gap is a reviewed line in one file, a careless one is a build failure, and they stop looking identical in the code. ITS FIRST RUN FOUND A LIVE FAIL-OPEN. `enforceProjectMembership` read the session as `authService?.api?.getSession?.(...)` with no `getApi()` fallback — the only one of the codebase's three `.api` readers without it. `plugin-auth` registers `AuthManager`, which has NO `.api` member at all. So the read yielded `undefined`, `userId` stayed unset, and the function returned at its "anonymous — upstream auth will decide" line BEFORE ever querying `sys_environment_member`. A signed-in non-member passed the gate, on every deployment with project scoping on — which is where the flag defaults to true. Anonymous callers were still denied elsewhere (#2567/#3963), so this was specifically the signed-in non-member case. The existing test for that gate mocked auth as `{ api: { getSession } }` — the legacy shape the shipped provider does not have — so it was green throughout. That is the FOURTH test in this work line found encoding a contract nobody implements, after batch 1's three `auth.handler` mocks and batch 3's `status: 'open'`. The new test uses the `getApi()` shape and fails against the pre-fix code. Also found by the rule, all the same #4127 shape (implemented, called, undeclared) and all now declared: IAuthService gains `api`, `getApi`, `isAuthGateActive` and `verifyMcpAccessToken`; IMetadataService gains `load` and `loadDiagnosed`. `getApi`'s return type is the EVIDENCED SUBSET — `getSession({headers})` and the three fields callers read — not a re-declaration of better-auth's handle, which belongs to that library. And the pattern's real root: the lookup facade returning `any` was re-declared in THREE places. Batches 1-3 typed `DomainHandlerDeps` and left `ActionExecutionDeps` and resolve-execution-context's `ResolveOptions` still saying `any` — so the copy that stayed untyped was the way around all the others, and it is where the auth reads lived. All three are typed now. Completing the interface: `getRequestKernelService` gets the same overload split (its one caller resolves the same `objectql` slot the `resolveService` fallback beside it does, so the two arms of one expression had different types), and share-links' `getEngine` loses a `Promise<any>` return annotation — a THIRD erasure syntax after `: any` and `as any`, and one this AST rule cannot see. That residual is documented in the config. `getObjectQL` STAYS `any`, deliberately, with the reason recorded: it exists to reach ObjectQL's surface beyond IDataEngine (`registry`, `executeAction`), which has no contract. Typing it IDataEngine would be the comfortable-looking lie. The auth and metadata contract doc pages are brought back in step with the interfaces — the auth page still called itself "intentionally minimal" while missing six members, two of them from batch 2. Refs #4127
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。#4127 gate 的第二批(第一批是 #4168)。
#4168 先动
getService,因为它简单 —— 所有调用点本来就传CoreServiceName。resolveService是混杂的那个,剩下的any都在这里。1. 重载按证据切开三类
core 名字解析为槽的契约,其余保持
any:'metadata'×10、'automation'×3、'auth'×3、'ai'用裸字符串;另有 ~16 处用CoreServiceName.enum.*'protocol'×22、'objectql'×9、'mcp'、'kernel-resolver'、'security'、'scope-manager'any同一个槽此前被两种写法寻址(
'metadata'vsCoreServiceName.enum.metadata),现在两种都受检。非 core 那些是真实存在的服务,但既不在
CoreServiceName里、也没有写过契约。它们的any就是账本诚实的边界 —— 在这里给它们编一个没有任何东西验证的形状,正是这条工作线要消灭的东西。写它们的契约是另一件事。2. 真正的发现:类型在三个调用点被抹掉了
const x: any = await deps.resolveService('auth', …)—— 注解赢,#4168 做的一切在那里完全无效。按这个模式扫,core 槽上有三处:/mcp×2 —— 又是两个未声明的方法domain 调的是
authService?.getMcpResourceUrl?.()和?.getMcpResourceMetadataUrl?.()。AuthManager两个都实现了(auth-manager.ts:2771/:2796,plugin-auth 内部也在用),IAuthService一个都没声明。调用点和实现一致,缺的是契约 —— #4127 的标准形状。但
: any+ 可选链的组合让这处比之前几个更糟,不是更好:它让调用既对类型系统不可见、又碰巧安全。方法不存在时返回undefined,于是 skill 路由静默退回到「从请求 host 推导 MCP URL」—— 也就是说,auth 服务的规范值和推导值之间一旦真的不一致,看起来会和正常运行一模一样。而
getMcpResourceUrl存在的全部意义,就是它由 auth 的basePath推出,让两者不可能在 API prefix 上分歧 —— 路由自己的注释写着「the auth service owns the canonical value」。两个都声明为 optional:没有 MCP/OAuth 支持的 auth 提供方合法地填这个槽;而
getMcpResourceMetadataUrl返回null(OAuth 轨道关闭 —— 内嵌 AS 未启用,或 origin 不满足 OAuth 2.1 传输规则,fail-closed)与「方法根本不存在」保持语义区分。/packages×1const metadata: any = await deps.getService(…metadata),喂给new SeedLoaderService(ql, metadata, …)。去掉注解,现在对着IMetadataService校验。它旁边的protocol和ql按上面的理由保留any。没有其他 core 槽查找被注解抹掉 —— 这一遍扫描对
domains/*.ts是穷尽的。验证
@objectstack/runtime@objectstack/spectsc --noEmitpnpm lintcheck:*门禁api-surface.json下一步
剩下的是给
protocol/objectql/mcp/security/shareLinks这些槽写契约(security有ISecurityService但槽名不在CoreServiceName里,是另一种形状的缺口),以及packages.ts(38 处as any)和meta.ts(15 处)里与服务查找无关的那些as any。🤖 Generated with Claude Code
https://claude.ai/code/session_015T5CeitwV1jXLvsL1A84tw
Generated by Claude Code