Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions .changeset/slot-lookup-fourth-shape.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
---
"@objectstack/core": patch
"@objectstack/runtime": patch
"@objectstack/platform-objects": patch
"@objectstack/plugin-security": patch
"@objectstack/cloud-connection": patch
---

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)

The three selectors from #4321 all key off the erasure and the lookup being in
ONE expression. Split them and every selector misses:

```ts
let ql: any;
try { ql = ctx.getService('objectql'); } catch { /* optional */ }
```

Selector 1 needs the call inside the declarator (this declarator 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, and the fix only gets more
expensive per batch shipped.

**It is a rule, not a fourth selector, and that is the whole finding.** esquery
can match `AssignmentExpression:has(CallExpression[…])`, but it cannot tell
which declaration the assigned identifier resolves to — so it would equally
flag the correctly-typed form this work line exists to produce (`let
i18nService: II18nService | undefined; i18nService = …`, 8 such sites today in
runtime/app-plugin.ts, service-automation and metadata-protocol). Resolving the
identifier needs SCOPE analysis. That is cheap and needs no type information, so
this stays out of the typed-lint pass the KNOWN RESIDUAL still waits on — but it
is a rule, and the earlier "just one more selector" estimate was wrong.

Verified against exactly that: the rule flags all 16 real sites and none of the
8 correctly-typed lookalikes.

**Scale.** The baseline goes 140 → **169 sites** with the file count unchanged
at 37: 29 sites were already inside grandfathered files and simply invisible.
16 more could NOT be grandfathered (12 in files earlier batches had cleared, 3
in files never listed, 1 the regex sweep had missed) and are typed here —
`runtime/app-plugin.ts` ×5, `core/fallbacks/authored-translation-sync.ts` ×2,
`plugin-security/security-plugin.ts` ×2, `cloud-connection/{runtime-config,
marketplace-proxy}-plugin.ts` ×3, `platform-objects/src/plugin.ts` ×2,
`runtime/http-dispatcher.ts`, `runtime/domains/ai.ts`. No baseline key was
added; the key set still only shrinks.

Contracts where they exist (`IAIService`, `IJobService`, `IMetadataService`,
`II18nService`, `IDataEngine`, `IHttpServer`), named local surfaces where they
do not — `AppEngineSurface`, `SecurityEngineSurface`, `RawAppHost`,
`EnvRegistrySurface`, `FreshDatastoreEngine`, `AuthoredTranslationSink`. Two of
those record something worth naming: `IHttpServer` has no `getRawApp()` (the
contract is framework-agnostic and the raw app is Hono's own handle), and
ObjectQL's `_defaultBodyRunner` / `_defaultActionRunner` have no public reader
at all — the engine attaches them via `(this as any)` and publishes nothing,
while `getHookMetricsRecorder()` exists for exactly that question about the
metrics recorder. Declared rather than laundered through `any`, and filed.
97 changes: 97 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,98 @@ const SLOT_LOOKUP_UNSWEPT = Object.keys(JSON.parse(
readFileSync(new URL('./scripts/slot-lookup-baseline.json', import.meta.url), 'utf8'),
));

// [#4251] The FOURTH erasure shape: the declaration and the lookup split apart.
//
// let ql: any;
// try { ql = ctx.getService('objectql'); } catch { /* optional */ }
//
// The contract is erased exactly as in `const ql: any = ctx.getService(…)`, and
// all three selectors below miss it: selector 1 needs the call inside the
// declarator (here the declarator has no init), selector 2 needs `as`, selector
// 3 needs a type argument. 23 sites repo-wide used it, 12 of them in files no
// longer grandfathered — i.e. lint covered them and said nothing. Worse, that
// number GREW with every batch: sweeping a file removes it from the baseline,
// and the baseline's monotonicity check means it can never be re-added, so each
// batch converted more of this shape from "grandfathered" into "silently clean".
// A ratchet that looks cleaner the more you use it is the #4342 failure again.
//
// This is a RULE and not a fourth selector because esquery cannot do it. A
// selector can match `AssignmentExpression:has(CallExpression[…])`, but it
// cannot tell which declaration the assigned identifier resolves to — so it
// would equally flag the correctly-typed form this whole work line is trying to
// produce (`let i18nService: II18nService | undefined; i18nService = …`, 8 such
// sites today, in runtime/app-plugin.ts and service-automation among others).
// Resolving the identifier to its declaration needs SCOPE analysis, which is
// cheap and needs no type information — so this stays out of the typed-lint
// pass that the KNOWN RESIDUAL below still waits on.
const slotLookupPlugin = {
rules: {
'no-any-assignment': {
meta: {
type: 'problem',
docs: { description: 'Ban assigning a service-lookup result to an `any`-declared variable.' },
schema: [],
messages: { erased: SLOT_LOOKUP_ANY_MESSAGE },
},
create(context) {
const lookupNames = new Set(SLOT_LOOKUPS.split('|'));
const uncontracted = new RegExp(`^(${UNCONTRACTED_SLOTS})$`);

/** The slot-lookup call inside `node`, or null. Mirrors the selectors' `:has`. */
const findLookupCall = (node) => {
let found = null;
const walk = (n) => {
if (found || !n || typeof n.type !== 'string') return;
if (
n.type === 'CallExpression' &&
n.callee?.type === 'MemberExpression' &&
lookupNames.has(n.callee.property?.name)
) {
// Same exemption channel as the selectors: the slot name is read
// off a literal argument, so an UNCONTRACTED_SLOTS lookup is
// legitimately `any` and must not be reported.
const exempt = n.arguments.some(
(a) => a?.type === 'Literal' && uncontracted.test(String(a.value)),
);
if (!exempt) { found = n; return; }
}
for (const key of Object.keys(n)) {
if (key === 'parent') continue;
const child = n[key];
if (Array.isArray(child)) child.forEach(walk);
else if (child && typeof child.type === 'string') walk(child);
}
};
walk(node);
return found;
};

/** True when `name` resolves, in scope, to a variable declared `: any`. */
const declaredAny = (name, node) => {
let scope = context.sourceCode.getScope(node);
for (; scope; scope = scope.upper) {
const variable = scope.variables.find((v) => v.name === name);
if (!variable) continue;
return variable.defs.some(
(d) => d.node?.id?.typeAnnotation?.typeAnnotation?.type === 'TSAnyKeyword',
);
}
return false;
};

return {
AssignmentExpression(node) {
if (node.left.type !== 'Identifier') return;
if (!findLookupCall(node.right)) return;
if (!declaredAny(node.left.name, node)) return;
context.report({ node, messageId: 'erased' });
},
};
},
},
},
};

export default [
{
files: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}'],
Expand Down Expand Up @@ -259,7 +351,12 @@ export default [
parser: tsParser,
parserOptions: { ecmaVersion: 'latest', sourceType: 'module' },
},
plugins: { 'slot-lookup': slotLookupPlugin },
rules: {
// The split-declaration form (#4251) — see `slotLookupPlugin`. Reports the
// SAME message as the three selectors below, so `check:slot-lookup` counts
// all four shapes without knowing there are four.
'slot-lookup/no-any-assignment': 'error',
'no-restricted-syntax': ['error',
{
// `const svc: any = await deps.resolveService('auth', env)`
Expand Down
19 changes: 17 additions & 2 deletions packages/cloud-connection/src/marketplace-proxy-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,21 @@ import {
publicMarketplaceKeyForApiPath,
} from './marketplace-public-url.js';

import type { IHttpServer } from '@objectstack/spec/contracts';

/**
* The `http-server` slot plus the one member this plugin needs from it.
*
* [#4251] `IHttpServer` is a real contract (`@objectstack/spec/contracts`) and
* this slot is bound to it — but `getRawApp()` is NOT on it, deliberately: the
* contract is framework-agnostic and the raw app is the framework's own handle
* (Hono here). So the escape is one named member returning `any`, scoped to the
* one thing that genuinely cannot be typed framework-agnostically, instead of
* the whole lookup collapsing to `any`. The probe below is what runs when a
* host serves the slot with an adapter that exposes no raw app.
*/
type RawAppHost = IHttpServer & { getRawApp?(): any };

const MARKETPLACE_PREFIX = '/api/v1/marketplace';

/**
Expand Down Expand Up @@ -181,9 +196,9 @@ export class MarketplaceProxyPlugin implements Plugin {
manifest?.register?.(MARKETPLACE_BROWSE_UI_BUNDLE);
} catch { /* no manifest service */ }

let httpServer: any;
let httpServer: RawAppHost | undefined;
try {
httpServer = ctx.getService('http-server');
httpServer = ctx.getService<RawAppHost>('http-server');
} catch {
ctx.logger?.warn?.('[MarketplaceProxyPlugin] http-server not available — marketplace routes not mounted');
return;
Expand Down
39 changes: 35 additions & 4 deletions packages/cloud-connection/src/runtime-config-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,31 @@

import type { Plugin, PluginContext } from '@objectstack/core';
import { resolveCloudUrl } from './cloud-url.js';
import type { IHttpServer } from '@objectstack/spec/contracts';

/**
* The `http-server` slot plus the one member this plugin needs from it.
*
* [#4251] `IHttpServer` is a real contract (`@objectstack/spec/contracts`) and
* this slot is bound to it — but `getRawApp()` is NOT on it, deliberately: the
* contract is framework-agnostic and the raw app is the framework's own handle
* (Hono here). So the escape is one named member returning `any`, scoped to the
* one thing that genuinely cannot be typed framework-agnostically, instead of
* the whole lookup collapsing to `any`. The probe below is what runs when a
* host serves the slot with an adapter that exposes no raw app.
*/
type RawAppHost = IHttpServer & { getRawApp?(): any };

/**
* The `env-registry` slot's hostname resolvers — see the call site for both
* spellings. Async by contract: the consumer awaits the result, and declaring
* it `unknown` would only have moved the erasure one line down.
*/
interface EnvRegistrySurface {
resolveByHostname?(hostname: string): Promise<any>;
resolveHostname?(hostname: string): Promise<any>;
}


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

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

// Merge the distribution's feature overrides over the static base.
// Arbitrary keys returned by the host pass through verbatim — the
Expand Down
23 changes: 20 additions & 3 deletions packages/core/src/fallbacks/authored-translation-sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
*/

import { LEGACY_OBJECT_FIRST_KEYS } from '@objectstack/spec/system';
import type { IDataEngine } from '@objectstack/spec/contracts';

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

Expand All @@ -63,6 +64,22 @@ interface MinimalCtx {
*/
const OWNER_PROP = '__authoredTranslationSyncOwner';

/**
* The `i18n` slot as this wirer uses it: the authored-layer replace seam, plus
* the ownership marker stamped on the instance.
*
* [#4251] `replaceAuthoredTranslations` is NOT on `II18nService` — it is the
* authored-overlay seam service-i18n grew for this sync, and every call site
* probes for it. `OWNER_PROP` is this module's own marker, stamped on whatever
* object occupies the slot so two wirers cannot both drive one instance.
* Declared here rather than erased to `any` so both facts stay legible: what
* the slot must supply, and what this module writes onto it.
*/
interface AuthoredTranslationSink {
replaceAuthoredTranslations(layer: Record<string, unknown>): void;
[OWNER_PROP]?: symbol;
}

// Deliberately narrow (language + optional script/region only): item names
// are snake_case, so a permissive multi-segment pattern would classify names
// like `my_custom_strings` as locales.
Expand Down Expand Up @@ -157,8 +174,8 @@ export function wireAuthoredTranslationSync(ctx: MinimalCtx): void {
if (typeof ctx.hook !== 'function') return;

const token = Symbol('authored-translation-sync');
const resolveOwnedI18n = (): any | null => {
let i18n: any;
const resolveOwnedI18n = (): AuthoredTranslationSink | null => {
let i18n: AuthoredTranslationSink | undefined;
try { i18n = ctx.getService('i18n'); } catch { return null; }
if (!i18n || typeof i18n.replaceAuthoredTranslations !== 'function') return null;
const current = i18n[OWNER_PROP];
Expand All @@ -176,7 +193,7 @@ export function wireAuthoredTranslationSync(ctx: MinimalCtx): void {
const run = chain.then(async () => {
const i18n = resolveOwnedI18n();
if (!i18n) return;
let engine: any;
let engine: IDataEngine | undefined;
try { engine = ctx.getService('objectql'); } catch { return; }
if (!engine || typeof engine.find !== 'function') return;
const layer = await readAuthoredTranslationLayer(engine, ctx.logger);
Expand Down
21 changes: 18 additions & 3 deletions packages/platform-objects/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,20 @@ import { SetupAppTranslations } from './apps/translations/index.js';
import { MetadataFormsTranslations } from './metadata-translations/index.js';
import { SysMigration } from './system/sys-migration.object.js';
import { SysSecret } from './system/sys-secret.object.js';
import { attestFreshDatastore } from './system/migration-flag.js';
import { attestFreshDatastore, type MigrationFlagEngine } from './system/migration-flag.js';
import type { II18nService } from '@objectstack/spec/contracts';

/**
* The `objectql` slot's fresh-datastore attestation seam.
*
* [#4251] Not on `IDataEngine` — these are ObjectQL's own migration-flag
* accessors, and the probe at the call site is what runs when the slot holds an
* engine without them. Declared narrow and named instead of erased to `any`.
*/
interface FreshDatastoreEngine extends MigrationFlagEngine {
wasDatastoreCreatedFromEmpty?(): boolean;
invalidateDataMigrationFlags?(): void;
}

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

ctx?.hook?.('kernel:ready', async () => {
let i18n: any;
let i18n: II18nService | undefined;
try {
i18n = ctx.getService?.('i18n');
} catch {
Expand Down
Loading
Loading