Skip to content

fix(core,rest): declare the getMany the localization resolver actually calls, and pin its leg count - #11677

Merged
huangyiirene merged 1 commit into
mainfrom
claude/issue-11222-localization-getmany-declarations
Aug 24, 2026
Merged

fix(core,rest): declare the getMany the localization resolver actually calls, and pin its leg count#11677
huangyiirene merged 1 commit into
mainfrom
claude/issue-11222-localization-getmany-declarations

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes#11222

The #10826 switch from three settings.get() calls to one settings.getMany()
left two declarations describing the old consumer, left the metric that switch
was calibrated on unmeasured, and left the resolution context unobservable by
any pin. Three items, one PR, spanning core + rest as triage designated.

1. Two declarations now say what the consumer actually calls

resolveLocalizationContext has called getMany since #10826. Two declarations
still said get was the one method consumed:

  • packages/core/src/security/resolve-authz-context.ts — the param JSDoc on
    ResolveLocalizationInput.settings.
  • packages/rest/src/rest-api-plugin.tsinterface SettingsReadSurface, whose
    prose read "The one method the platform consumes is get".

getMany is declared OPTIONAL, not required, and that is the load-bearing
choice: the branch is feature-detected, so an occupant without getMany still
answers correctly at three reads instead of one. Declaring it required would
over-state the contract as badly as omitting it under-stated it.

Nothing was broken and the reason is itself worth writing down, so both sites now
say it: the type is erased three times before the call site — ctx.getService is
a cast, rest-server.ts widens the provider's return to a bare promise, and the
resolver receives settings?: any — so settings.getMany is type-checked
nowhere. That is why CI was green without the declaration, and why these
interfaces are documentation for host authors rather than enforcement.

No fourth site. Verified: runtime's own lookup
(resolve-execution-context.ts) calls opts.getService('settings') untyped, and
plugin-auth / plugin-email name their own SettingsReadSurface but call no
getMany (grep: zero hits in both) — their narrower declarations are accurate
and deliberately untouched.

2. Legs, not calls — the card's own metric, now pinned

#10826's calibration is "a query-count fix, not a latency fix", because
cloud#1539 measured the three reads running in parallel: 3 queries but 1 leg,
and legs are the latency multiplier. Every pin in this file counts calls
(getMany.calls, gets === 3), so the leg count was pinned by nothing.

makeSettingsRig measures a leg: a row load that starts while nothing else
is in flight — the number of sequential round-trip waves. Three loads issued
inside one Promise.all all increment before any resumes past its first await,
so they count as one; three sequential awaits count as three.

Reverse-verified in both directions — a leg pin that has never been seen red
is not a pin:

  • Feature-detect disabled (if (false && typeof settings.getMany === 'function'))
    — the batched occupant falls to the per-key path:

    AssertionError: expected { queries: 3, legs: 1 } to deeply equal { queries: 1, legs: 1 }
    Tests 3 failed | 67 passed (70)
    

    That is the salvaged rig's original failure line, reproduced against the landed
    source: queries 3 -> 1, legs 1 -> 1.

  • The per-key Promise.all replaced with a sequential for loop — the exact
    regression class item 2 exists to catch:

    AssertionError: expected { queries: 3, legs: 3 } to deeply equal { queries: 3, legs: 1 }
    Tests 1 failed | 69 passed (70)
    

    69 of 70 stayed green. Every pre-existing call-count assertion — including
    gets === 3 and getMany.calls === 1 — passed while legs went 1 -> 3. The new
    leg pin is the only assertion in the file that can see it, which is precisely
    the gap being closed.

Both ablations restored via an EXIT INT TERM trap and confirmed on disk by
anchor grep counts before and after each mutation (Promise.all 2 -> 1,
marker 0 -> 1, and back). No rebuild leg applies: the test imports the resolver
by relative specifier (./resolve-authz-context.js), so vitest resolves source,
never dist/.

3. The resolution context, observable for the first time

The existing getMany double is declared (ns, keys)two parameters — so
it structurally cannot see the third argument. Drop sctx from the production
call today and all three pins stay green. The rig's double takes ctx, and the
new pin asserts the whole call shape: namespace, key list, and
{ tenantId: 'o1', userId: 'u1' }. It went red under ablation 1 alongside the
leg pin.

4. Partial-manifest all-or-nothing — split producer/consumer, not widened

Judgement called as the card invited. The all-or-nothing rule is
SettingsService.getMany's own contract (it validates every key up front and
throws UnknownKeyError for the whole call), so documenting the rule belongs on
the producer, in packages/services/service-settings — a domain:services card,
filed separately rather than silently widened into this PR's package set.

What is this caller's own and is recorded here: when getMany throws, this
resolver drops to the direct tenant-scoped $in read, which has no global
scope layer and no OS_LOCALIZATION_* env override
. The existing comment said
the fallback "reads the exact same three keys" without saying it reads them
through a shorter cascade. Degradation, never a wrong answer, and unreachable
against the in-repo localizationSettingsManifest, which declares all three keys.

Verification

pnpm --filter @objectstack/core exec vitest run src/security/resolve-authz-context.test.ts
Test Files 1 passed (1) · Tests 70 passed (70).
pnpm --filter @objectstack/rest typecheck — exit 0, script name echoed
(> @objectstack/rest@17.2.0 typecheck), so it is not the zero-match no-op.

Gates run at final HEAD 266eb589, each read from its own printed verdict line:
check:authz-resolver · check:cross-package-test-inputs ·
check:dispatcher-error-vocabulary · check:kernel-hook-pairs ·
check:published-files · check:slot-lookup · check:test-source-alias ·
check:type-source-resolution · check:query-options-erasure ·
check:engine-double-contract · check:where-matcher · check:nul-bytes ·
check:type-check-coverage · check-plugin-teardown-shape ·
docs-audit/check-affected-docs — all exit 0.

@objectstack/core's tsconfig is include: ["src/**/*"], exclude: [], so its
*.test.ts files are inside the tsc program the 98-error DEBT ratchet
freezes. Measured directly rather than assumed: tsc --noEmit -p on this branch
reports 98 errors, unchanged, and zero of them in
resolve-authz-context.test.ts.

Declared narrowing (repo-wide pnpm lint left to CI): eslint run with
--no-inline-config over the three changed files, --format json reporting
3 files linted, 0 errors, 0 warnings. The narrowing excludes nothing,
because this repo's eslint.config.mjs never enables type-aware linting — no
parserOptions.project, no typed @typescript-eslint rules, for any file
(stated and measured in the config's own comment) — so this diff cannot move the
verdict on a file it does not touch.

No changeset

Nothing here releases: no runtime path moves, no exported type changes,
SettingsReadSurface is not exported, and the rest is JSDoc, a code comment and
tests. skip-changeset applied per the workflow's own prescription for a PR that
declares no release of its own. Say the word and a patch changeset goes in
instead.

Scope

Three files exactly, as claimed. packages/plugins/plugin-auth and
plugin-email declare their own SettingsReadSurface and were deliberately left
alone. packages/core/src/security/auth-gate.ts (hold #7898's indexed path) was
not touched.


Generated by Claude Code

…lly calls, and pin its leg count
The #10826 switch from three `settings.get()` calls to one `settings.getMany()`
left two declarations describing the old consumer, and left the metric that
switch was calibrated on unmeasured.
- `ResolveLocalizationInput.settings`'s param JSDoc and `rest`'s
`SettingsReadSurface` both said `get` was the one method consumed. Neither is
type-checked at the call site (`getService` is a cast, `rest-server.ts` widens
to a bare promise, the resolver receives `settings?: any`), which is why CI was
green without them — the cost is a host author being told `getMany` is not
consumed. `getMany` is declared OPTIONAL: the branch is feature-detected and an
occupant without it still answers, at three reads instead of one.
- Legs, not calls: the existing pins count calls, so turning the per-key
fallback's `Promise.all` into a sequential loop would take legs 1 -> 3 with
every assertion still green. `makeSettingsRig` measures row loads that start
while nothing else is in flight — the sequential wave count cloud#1539 used.
- The `getMany` double now takes the third parameter, so the resolution context
it is called with is observable for the first time.
Item 4 (partial-manifest all-or-nothing) is recorded as this caller's
degradation; the rule itself is the settings service's contract.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VK8rFDtg8eREaxBGX99Csn
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 2 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 29 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 88b9d749a1eb76b53966b0081006d2c5bccb341dpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 6605f80194c95b44a72b3f4890a11ea0e4f62aac — the merge of head 266eb589ffefedc071cd00e451389a61f7025610 into base 88b9d749a1eb76b53966b0081006d2c5bccb341d, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6605f80194c95b44a72b3f4890a11ea0e4f62aac && git checkout 6605f80194c95b44a72b3f4890a11ea0e4f62aac
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 88b9d749a1eb76b53966b0081006d2c5bccb341d 266eb589ffefedc071cd00e451389a61f7025610 && git checkout -B drift-repro 88b9d749a1eb76b53966b0081006d2c5bccb341d && git merge --no-ff 266eb589ffefedc071cd00e451389a61f7025610
node scripts/docs-audit/affected-docs.mjs --json 88b9d749a1eb76b53966b0081006d2c5bccb341d

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] The localization getMany switch leaves two stale declarations, an unpinned leg count, and a resolution context no pin can see

2 participants

@huangyiirene@claude