Skip to content

Commit d92c72d

Browse files
os-zhuangclaude
andauthored
fix(lint,runtime,core): 槽查找护栏认出分体声明形态 —— 越清扫越显得干净的那个盲区 (#4251) (#4373)
* fix(lint,runtime,core): slot-lookup guard sees the split-declaration form (#4251) The three selectors from #4321 all key off the erasure and the lookup being in ONE expression. Split them and every selector misses: let ql: any; try { ql = ctx.getService('objectql'); } catch { /* optional */ } Selector 1 needs the call inside the declarator (this one has no init), selector 2 needs `as`, selector 3 needs a type argument. The contract is erased exactly as in `const ql: any = ctx.getService(...)`. Why this could not wait for the batches: the baseline's monotonicity check means a file that leaves the grandfather list can never be re-added, so every batch converted more of this shape from "grandfathered" into "lint covers this file and says nothing" -- B2 alone moved plugin-security/security-plugin.ts into that state. A ratchet that reports a cleaner number the more you sweep is the #4342 failure wearing different clothes. It is a RULE, not a fourth selector, and that is the finding. esquery can match AssignmentExpression:has(CallExpression[...]), but cannot tell which declaration the assigned identifier resolves to -- so it would equally flag the correctly-typed form this work line exists to produce (8 such sites today). That needs scope analysis: cheap, no type information, but a rule. Verified: flags all 16 real sites, none of the 8 lookalikes. Baseline 140 -> 169 sites, file count unchanged at 37 -- 29 were already inside grandfathered files and simply invisible. 16 more could not be grandfathered and are typed here. No baseline key added. Verified: ratchet holds (169/37, none new, no files added vs c54c822); eslint clean on every changed file; tsc on the three DEBT packages (runtime/core/cloud-connection) shows 0 errors in touched files and identical whole-package counts vs clean HEAD (18/91/13); runtime 974/68, core 437/27, platform-objects 254/8, plugin-security 677/32. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(platform-objects): FreshDatastoreEngine extends MigrationFlagEngine (#4251) CI caught what my local run could not: `platform-objects` has no `typecheck` script (type-check coverage DEBT), so its dts build is the only gate, and I had run only its tests. The named surface introduced for the `objectql` lookup declared the two attestation members but not the engine shape `attestFreshDatastore` takes, so the call no longer typechecked. Extending `MigrationFlagEngine` is the right fix rather than widening the call: that interface IS the contract the helper states, and the surface's job is to add ObjectQL's attestation members on top of it. Verified: platform-objects build (CJS/ESM/DTS) succeeds, 254 tests / 8 files pass, eslint clean; core / runtime / cloud-connection / plugin-security dts builds re-run and all succeed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 38182ff commit d92c72d

11 files changed

Lines changed: 353 additions & 44 deletions

File tree

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
---
2+
"@objectstack/core": patch
3+
"@objectstack/runtime": patch
4+
"@objectstack/platform-objects": patch
5+
"@objectstack/plugin-security": patch
6+
"@objectstack/cloud-connection": patch
7+
---
8+
9+
fix(lint,runtime,core): the slot-lookup guard sees the split-declaration form — the shape that made the ratchet look cleaner the more it was used (#4251)
10+
11+
The three selectors from #4321 all key off the erasure and the lookup being in
12+
ONE expression. Split them and every selector misses:
13+
14+
```ts
15+
let ql: any;
16+
try { ql = ctx.getService('objectql'); } catch { /* optional */ }
17+
```
18+
19+
Selector 1 needs the call inside the declarator (this declarator has no init),
20+
selector 2 needs `as`, selector 3 needs a type argument. The contract is erased
21+
exactly as in `const ql: any = ctx.getService(…)`.
22+
23+
**Why this could not wait for the batches.** The baseline's monotonicity check
24+
means a file that leaves the grandfather list can never be re-added. So every
25+
batch converted more of this shape from "grandfathered" into "lint covers this
26+
file and says nothing" — B2 alone moved `plugin-security/security-plugin.ts`
27+
into that state. A ratchet that reports a cleaner number the more you sweep is
28+
the #4342 failure wearing different clothes, and the fix only gets more
29+
expensive per batch shipped.
30+
31+
**It is a rule, not a fourth selector, and that is the whole finding.** esquery
32+
can match `AssignmentExpression:has(CallExpression[…])`, but it cannot tell
33+
which declaration the assigned identifier resolves to — so it would equally
34+
flag the correctly-typed form this work line exists to produce (`let
35+
i18nService: II18nService | undefined; i18nService = …`, 8 such sites today in
36+
runtime/app-plugin.ts, service-automation and metadata-protocol). Resolving the
37+
identifier needs SCOPE analysis. That is cheap and needs no type information, so
38+
this stays out of the typed-lint pass the KNOWN RESIDUAL still waits on — but it
39+
is a rule, and the earlier "just one more selector" estimate was wrong.
40+
41+
Verified against exactly that: the rule flags all 16 real sites and none of the
42+
8 correctly-typed lookalikes.
43+
44+
**Scale.** The baseline goes 140 → **169 sites** with the file count unchanged
45+
at 37: 29 sites were already inside grandfathered files and simply invisible.
46+
16 more could NOT be grandfathered (12 in files earlier batches had cleared, 3
47+
in files never listed, 1 the regex sweep had missed) and are typed here —
48+
`runtime/app-plugin.ts` ×5, `core/fallbacks/authored-translation-sync.ts` ×2,
49+
`plugin-security/security-plugin.ts` ×2, `cloud-connection/{runtime-config,
50+
marketplace-proxy}-plugin.ts` ×3, `platform-objects/src/plugin.ts` ×2,
51+
`runtime/http-dispatcher.ts`, `runtime/domains/ai.ts`. No baseline key was
52+
added; the key set still only shrinks.
53+
54+
Contracts where they exist (`IAIService`, `IJobService`, `IMetadataService`,
55+
`II18nService`, `IDataEngine`, `IHttpServer`), named local surfaces where they
56+
do not — `AppEngineSurface`, `SecurityEngineSurface`, `RawAppHost`,
57+
`EnvRegistrySurface`, `FreshDatastoreEngine`, `AuthoredTranslationSink`. Two of
58+
those record something worth naming: `IHttpServer` has no `getRawApp()` (the
59+
contract is framework-agnostic and the raw app is Hono's own handle), and
60+
ObjectQL's `_defaultBodyRunner` / `_defaultActionRunner` have no public reader
61+
at all — the engine attaches them via `(this as any)` and publishes nothing,
62+
while `getHookMetricsRecorder()` exists for exactly that question about the
63+
metrics recorder. Declared rather than laundered through `any`, and filed.

eslint.config.mjs

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,98 @@ const SLOT_LOOKUP_UNSWEPT = Object.keys(JSON.parse(
115115
readFileSync(new URL('./scripts/slot-lookup-baseline.json', import.meta.url), 'utf8'),
116116
));
117117

118+
// [#4251] The FOURTH erasure shape: the declaration and the lookup split apart.
119+
//
120+
// let ql: any;
121+
// try { ql = ctx.getService('objectql'); } catch { /* optional */ }
122+
//
123+
// The contract is erased exactly as in `const ql: any = ctx.getService(…)`, and
124+
// all three selectors below miss it: selector 1 needs the call inside the
125+
// declarator (here the declarator has no init), selector 2 needs `as`, selector
126+
// 3 needs a type argument. 23 sites repo-wide used it, 12 of them in files no
127+
// longer grandfathered — i.e. lint covered them and said nothing. Worse, that
128+
// number GREW with every batch: sweeping a file removes it from the baseline,
129+
// and the baseline's monotonicity check means it can never be re-added, so each
130+
// batch converted more of this shape from "grandfathered" into "silently clean".
131+
// A ratchet that looks cleaner the more you use it is the #4342 failure again.
132+
//
133+
// This is a RULE and not a fourth selector because esquery cannot do it. A
134+
// selector can match `AssignmentExpression:has(CallExpression[…])`, but it
135+
// cannot tell which declaration the assigned identifier resolves to — so it
136+
// would equally flag the correctly-typed form this whole work line is trying to
137+
// produce (`let i18nService: II18nService | undefined; i18nService = …`, 8 such
138+
// sites today, in runtime/app-plugin.ts and service-automation among others).
139+
// Resolving the identifier to its declaration needs SCOPE analysis, which is
140+
// cheap and needs no type information — so this stays out of the typed-lint
141+
// pass that the KNOWN RESIDUAL below still waits on.
142+
const slotLookupPlugin = {
143+
rules: {
144+
'no-any-assignment': {
145+
meta: {
146+
type: 'problem',
147+
docs: { description: 'Ban assigning a service-lookup result to an `any`-declared variable.' },
148+
schema: [],
149+
messages: { erased: SLOT_LOOKUP_ANY_MESSAGE },
150+
},
151+
create(context) {
152+
const lookupNames = new Set(SLOT_LOOKUPS.split('|'));
153+
const uncontracted = new RegExp(`^(${UNCONTRACTED_SLOTS})$`);
154+
155+
/** The slot-lookup call inside `node`, or null. Mirrors the selectors' `:has`. */
156+
const findLookupCall = (node) => {
157+
let found = null;
158+
const walk = (n) => {
159+
if (found || !n || typeof n.type !== 'string') return;
160+
if (
161+
n.type === 'CallExpression' &&
162+
n.callee?.type === 'MemberExpression' &&
163+
lookupNames.has(n.callee.property?.name)
164+
) {
165+
// Same exemption channel as the selectors: the slot name is read
166+
// off a literal argument, so an UNCONTRACTED_SLOTS lookup is
167+
// legitimately `any` and must not be reported.
168+
const exempt = n.arguments.some(
169+
(a) => a?.type === 'Literal' && uncontracted.test(String(a.value)),
170+
);
171+
if (!exempt) { found = n; return; }
172+
}
173+
for (const key of Object.keys(n)) {
174+
if (key === 'parent') continue;
175+
const child = n[key];
176+
if (Array.isArray(child)) child.forEach(walk);
177+
else if (child && typeof child.type === 'string') walk(child);
178+
}
179+
};
180+
walk(node);
181+
return found;
182+
};
183+
184+
/** True when `name` resolves, in scope, to a variable declared `: any`. */
185+
const declaredAny = (name, node) => {
186+
let scope = context.sourceCode.getScope(node);
187+
for (; scope; scope = scope.upper) {
188+
const variable = scope.variables.find((v) => v.name === name);
189+
if (!variable) continue;
190+
return variable.defs.some(
191+
(d) => d.node?.id?.typeAnnotation?.typeAnnotation?.type === 'TSAnyKeyword',
192+
);
193+
}
194+
return false;
195+
};
196+
197+
return {
198+
AssignmentExpression(node) {
199+
if (node.left.type !== 'Identifier') return;
200+
if (!findLookupCall(node.right)) return;
201+
if (!declaredAny(node.left.name, node)) return;
202+
context.report({ node, messageId: 'erased' });
203+
},
204+
};
205+
},
206+
},
207+
},
208+
};
209+
118210
export default [
119211
{
120212
files: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}'],
@@ -259,7 +351,12 @@ export default [
259351
parser: tsParser,
260352
parserOptions: { ecmaVersion: 'latest', sourceType: 'module' },
261353
},
354+
plugins: { 'slot-lookup': slotLookupPlugin },
262355
rules: {
356+
// The split-declaration form (#4251) — see `slotLookupPlugin`. Reports the
357+
// SAME message as the three selectors below, so `check:slot-lookup` counts
358+
// all four shapes without knowing there are four.
359+
'slot-lookup/no-any-assignment': 'error',
263360
'no-restricted-syntax': ['error',
264361
{
265362
// `const svc: any = await deps.resolveService('auth', env)`

packages/cloud-connection/src/marketplace-proxy-plugin.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,21 @@ import {
3333
publicMarketplaceKeyForApiPath,
3434
} from './marketplace-public-url.js';
3535

36+
import type { IHttpServer } from '@objectstack/spec/contracts';
37+
38+
/**
39+
* The `http-server` slot plus the one member this plugin needs from it.
40+
*
41+
* [#4251] `IHttpServer` is a real contract (`@objectstack/spec/contracts`) and
42+
* this slot is bound to it — but `getRawApp()` is NOT on it, deliberately: the
43+
* contract is framework-agnostic and the raw app is the framework's own handle
44+
* (Hono here). So the escape is one named member returning `any`, scoped to the
45+
* one thing that genuinely cannot be typed framework-agnostically, instead of
46+
* the whole lookup collapsing to `any`. The probe below is what runs when a
47+
* host serves the slot with an adapter that exposes no raw app.
48+
*/
49+
type RawAppHost = IHttpServer & { getRawApp?(): any };
50+
3651
const MARKETPLACE_PREFIX = '/api/v1/marketplace';
3752

3853
/**
@@ -181,9 +196,9 @@ export class MarketplaceProxyPlugin implements Plugin {
181196
manifest?.register?.(MARKETPLACE_BROWSE_UI_BUNDLE);
182197
} catch { /* no manifest service */ }
183198

184-
let httpServer: any;
199+
let httpServer: RawAppHost | undefined;
185200
try {
186-
httpServer = ctx.getService('http-server');
201+
httpServer = ctx.getService<RawAppHost>('http-server');
187202
} catch {
188203
ctx.logger?.warn?.('[MarketplaceProxyPlugin] http-server not available — marketplace routes not mounted');
189204
return;

packages/cloud-connection/src/runtime-config-plugin.ts

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,31 @@
4141

4242
import type { Plugin, PluginContext } from '@objectstack/core';
4343
import { resolveCloudUrl } from './cloud-url.js';
44+
import type { IHttpServer } from '@objectstack/spec/contracts';
45+
46+
/**
47+
* The `http-server` slot plus the one member this plugin needs from it.
48+
*
49+
* [#4251] `IHttpServer` is a real contract (`@objectstack/spec/contracts`) and
50+
* this slot is bound to it — but `getRawApp()` is NOT on it, deliberately: the
51+
* contract is framework-agnostic and the raw app is the framework's own handle
52+
* (Hono here). So the escape is one named member returning `any`, scoped to the
53+
* one thing that genuinely cannot be typed framework-agnostically, instead of
54+
* the whole lookup collapsing to `any`. The probe below is what runs when a
55+
* host serves the slot with an adapter that exposes no raw app.
56+
*/
57+
type RawAppHost = IHttpServer & { getRawApp?(): any };
58+
59+
/**
60+
* The `env-registry` slot's hostname resolvers — see the call site for both
61+
* spellings. Async by contract: the consumer awaits the result, and declaring
62+
* it `unknown` would only have moved the erasure one line down.
63+
*/
64+
interface EnvRegistrySurface {
65+
resolveByHostname?(hostname: string): Promise<any>;
66+
resolveHostname?(hostname: string): Promise<any>;
67+
}
68+
4469

4570
/**
4671
* Feature-flag overrides a host's distribution policy can derive per request.
@@ -171,9 +196,9 @@ export class RuntimeConfigPlugin implements Plugin {
171196

172197
start = async (ctx: PluginContext): Promise<void> => {
173198
ctx.hook('kernel:ready', async () => {
174-
let httpServer: any;
199+
let httpServer: RawAppHost | undefined;
175200
try {
176-
httpServer = ctx.getService('http-server');
201+
httpServer = ctx.getService<RawAppHost>('http-server');
177202
} catch {
178203
ctx.logger?.warn?.('[RuntimeConfigPlugin] http-server not available — runtime/config not mounted');
179204
return;
@@ -194,8 +219,14 @@ export class RuntimeConfigPlugin implements Plugin {
194219
// kernel router uses (env-registry). Falls back to the static
195220
// payload when the host doesn't map to any env (e.g. a marketing
196221
// root or a CLI-served single-env runtime).
197-
let envRegistry: any = null;
198-
try { envRegistry = ctx.getService('env-registry'); } catch { /* not mounted (file/CLI mode) */ }
222+
// [#4251] `env-registry` has no written contract, so the two
223+
// hostname resolvers this reads are declared here rather than
224+
// erased. Both spellings are probed below because the slot has two
225+
// providers that disagree — declaring them is what makes that
226+
// disagreement visible instead of a pair of `?.` guesses.
227+
let envRegistry: EnvRegistrySurface | null = null;
228+
try { envRegistry = ctx.getService<EnvRegistrySurface>('env-registry') ?? null; }
229+
catch { /* not mounted (file/CLI mode) */ }
199230

200231
// Merge the distribution's feature overrides over the static base.
201232
// Arbitrary keys returned by the host pass through verbatim — the

packages/core/src/fallbacks/authored-translation-sync.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
*/
4545

4646
import { LEGACY_OBJECT_FIRST_KEYS } from '@objectstack/spec/system';
47+
import type { IDataEngine } from '@objectstack/spec/contracts';
4748

4849
import { deepMerge } from './memory-i18n.js';
4950

@@ -63,6 +64,22 @@ interface MinimalCtx {
6364
*/
6465
const OWNER_PROP = '__authoredTranslationSyncOwner';
6566

67+
/**
68+
* The `i18n` slot as this wirer uses it: the authored-layer replace seam, plus
69+
* the ownership marker stamped on the instance.
70+
*
71+
* [#4251] `replaceAuthoredTranslations` is NOT on `II18nService` — it is the
72+
* authored-overlay seam service-i18n grew for this sync, and every call site
73+
* probes for it. `OWNER_PROP` is this module's own marker, stamped on whatever
74+
* object occupies the slot so two wirers cannot both drive one instance.
75+
* Declared here rather than erased to `any` so both facts stay legible: what
76+
* the slot must supply, and what this module writes onto it.
77+
*/
78+
interface AuthoredTranslationSink {
79+
replaceAuthoredTranslations(layer: Record<string, unknown>): void;
80+
[OWNER_PROP]?: symbol;
81+
}
82+
6683
// Deliberately narrow (language + optional script/region only): item names
6784
// are snake_case, so a permissive multi-segment pattern would classify names
6885
// like `my_custom_strings` as locales.
@@ -157,8 +174,8 @@ export function wireAuthoredTranslationSync(ctx: MinimalCtx): void {
157174
if (typeof ctx.hook !== 'function') return;
158175

159176
const token = Symbol('authored-translation-sync');
160-
const resolveOwnedI18n = (): any | null => {
161-
let i18n: any;
177+
const resolveOwnedI18n = (): AuthoredTranslationSink | null => {
178+
let i18n: AuthoredTranslationSink | undefined;
162179
try { i18n = ctx.getService('i18n'); } catch { return null; }
163180
if (!i18n || typeof i18n.replaceAuthoredTranslations !== 'function') return null;
164181
const current = i18n[OWNER_PROP];
@@ -176,7 +193,7 @@ export function wireAuthoredTranslationSync(ctx: MinimalCtx): void {
176193
const run = chain.then(async () => {
177194
const i18n = resolveOwnedI18n();
178195
if (!i18n) return;
179-
let engine: any;
196+
let engine: IDataEngine | undefined;
180197
try { engine = ctx.getService('objectql'); } catch { return; }
181198
if (!engine || typeof engine.find !== 'function') return;
182199
const layer = await readAuthoredTranslationLayer(engine, ctx.logger);

packages/platform-objects/src/plugin.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,20 @@ import { SetupAppTranslations } from './apps/translations/index.js';
44
import { MetadataFormsTranslations } from './metadata-translations/index.js';
55
import { SysMigration } from './system/sys-migration.object.js';
66
import { SysSecret } from './system/sys-secret.object.js';
7-
import { attestFreshDatastore } from './system/migration-flag.js';
7+
import { attestFreshDatastore, type MigrationFlagEngine } from './system/migration-flag.js';
8+
import type { II18nService } from '@objectstack/spec/contracts';
9+
10+
/**
11+
* The `objectql` slot's fresh-datastore attestation seam.
12+
*
13+
* [#4251] Not on `IDataEngine` — these are ObjectQL's own migration-flag
14+
* accessors, and the probe at the call site is what runs when the slot holds an
15+
* engine without them. Declared narrow and named instead of erased to `any`.
16+
*/
17+
interface FreshDatastoreEngine extends MigrationFlagEngine {
18+
wasDatastoreCreatedFromEmpty?(): boolean;
19+
invalidateDataMigrationFlags?(): void;
20+
}
821

922
/**
1023
* `PlatformObjectsPlugin`
@@ -95,7 +108,9 @@ export class PlatformObjectsPlugin {
95108
// service-storage). A store that was found rather than created attests
96109
// nothing and keeps producing evidence by scan.
97110
ctx?.hook?.('kernel:ready', async () => {
98-
let engine: any;
111+
// [#4251] The fresh-datastore attestation seam is ObjectQL's own, not
112+
// `IDataEngine`'s; declared here rather than erased to `any`.
113+
let engine: FreshDatastoreEngine | undefined;
99114
try {
100115
engine = ctx.getService?.('objectql');
101116
} catch {
@@ -120,7 +135,7 @@ export class PlatformObjectsPlugin {
120135
});
121136

122137
ctx?.hook?.('kernel:ready', async () => {
123-
let i18n: any;
138+
let i18n: II18nService | undefined;
124139
try {
125140
i18n = ctx.getService?.('i18n');
126141
} catch {

0 commit comments

Comments
 (0)