Skip to content

fix(service-settings): user-keyed loadRows includes tenant/global rows on the engine branch (#11228) - #11272

Merged
os-zhuang merged 1 commit into
mainfrom
fix/settings-loadrows-scope-11228
Aug 23, 2026
Merged

fix(service-settings): user-keyed loadRows includes tenant/global rows on the engine branch (#11228)#11272
os-zhuang merged 1 commit into
mainfrom
fix/settings-loadrows-scope-11228

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes#11228.

The defect

loadRows(namespace, userId) on the engine branch built where.user_id = userId, excluding every upper-scope row (they carry user_id NULL), while the in-memory branch's predicate includes them. Two consumers search the ONE result set a user-keyed load returns, so on every engine-bound deployment — i.e. every real one:

  1. read cascade: a user-scope key with no personal row fell straight through to the manifest default, silently ignoring persisted tenant/global values;
  2. Phase-2 locks: the upper-scope lock check found no locked tenant/global row, so a lock that should refuse user-scope writes never fired — a policy bypass, not just a stale read.

The fix

One WHERE clause: { namespace, $or: [{ user_id }, { scope: 'tenant' }, { scope: 'global' }] }, mirroring the memory predicate exactly. Both call sites (resolveKey / setMany's lock pre-flight) heal at once; getMany's grouped loads only ever get MORE rows, which its per-scope resolution already filters.

Why the suite was green over it

The shared fake engine matcher did bare field equality — the #4434 class (a double looser than the engine), in a WHERE clause — and would ALSO have silently matched nothing against the new $or. Both harnesses (this suite + settings-getmany.test.ts) now implement $or and THROW on any other combinator, per the WHERE-matcher gate convention. New suite pins: the global fallback, other-user exclusion, an engine/memory differential over three contexts, and the lock refusal on the engine path. 481/481 package tests green.

🤖 Generated with Claude Code

…s on the engine branch (#11228)
The engine branch built `where.user_id = userId`, which excludes every
upper-scope row (user_id NULL) — while the in-memory branch's predicate
includes them. Two consumers search the ONE result set a user-keyed load
returns, so on every engine-bound deployment:
1. resolveKey's user→tenant→global cascade fell straight through to the
manifest default whenever the user had no personal row, silently
ignoring persisted tenant/global values; and
2. the Phase-2 upper-scope lock check found no locked tenant/global row,
so a lock that should refuse user-scope writes never fired — a policy
bypass, not just a stale read.
The suite stayed green because the in-memory double answered correctly
(the #4434 class — a double looser than the engine — in a WHERE clause):
the shared fake matcher did bare field equality, so it would also have
silently matched nothing against the new $or. Both test harnesses now
implement $or and THROW on any other combinator, and the new suite pins
the fallback, the other-user exclusion, an engine/memory differential,
and the lock refusal on the engine path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@os-zhuang
os-zhuang enabled auto-merge August 23, 2026 08:11
@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 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 — 7 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 a2ec377078d41432358aac2a42cd74b87ed473f1packageMentionDocs.

Which tree this was computed on

This run read content/docs from a94cdf5338664dbcd32617d1ce0e38a3946644fb — the merge of head 89bd8d6707b1b21331aad6bba1224657911d8cff into base a2ec377078d41432358aac2a42cd74b87ed473f1, 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 a94cdf5338664dbcd32617d1ce0e38a3946644fb && git checkout a94cdf5338664dbcd32617d1ce0e38a3946644fb
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a2ec377078d41432358aac2a42cd74b87ed473f1 89bd8d6707b1b21331aad6bba1224657911d8cff && git checkout -B drift-repro a2ec377078d41432358aac2a42cd74b87ed473f1 && git merge --no-ff 89bd8d6707b1b21331aad6bba1224657911d8cff
node scripts/docs-audit/affected-docs.mjs --json a2ec377078d41432358aac2a42cd74b87ed473f1

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

@os-zhuang
os-zhuang added this pull request to the merge queueAug 23, 2026
Merged via the queue into main with commit 050d8d8Aug 23, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the fix/settings-loadrows-scope-11228 branch August 23, 2026 08:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

settings: loadRows engine branch drops tenant/global rows for user-scope reads — cascade dies at the user level on every engine-bound deployment

1 participant

@os-zhuang