fix(service-storage): stamp sys_file with the acting organization, and backfill the rows that were never stamped (#12745) - #12929
Conversation
📓 Docs Drift CheckThis PR changes 1 package(s): 54 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 7 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 497e3bf2c1ec179b68f756db8b46c33ffbc85f14 && git checkout 497e3bf2c1ec179b68f756db8b46c33ffbc85f14
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6228c1f0ef3aad1946763b543e9fb207b8bf7de2 4dd420968ada19e56c52f11e518f0fb6308eda8d && git checkout -B drift-repro 6228c1f0ef3aad1946763b543e9fb207b8bf7de2 && git merge --no-ff 4dd420968ada19e56c52f11e518f0fb6308eda8d
node scripts/docs-audit/affected-docs.mjs --json 6228c1f0ef3aad1946763b543e9fb207b8bf7de2
|
Fixes #12745
Implements the maintainer ruling of 2026-08-28 — A with backfill (「12745 A回,其他同意。」): stamp
sys_fileforward and backfill the rows that were never stamped. Both halves are here; the fork the card deliberately left open is not re-litigated.Draft: this seat does not land it.
MEASURED — re-derived on this branch's base, not inherited from the card
Every claim below was re-run on
origin/mainataef1b7e64(the branch's merge base), file and line named.grep -c context packages/services/service-storage/src/metadata-store.ts→ 0sys_fileis tenancy-ENABLEDgrep -n "tenancy" packages/services/service-storage/src/objects/system-file.object.ts→ exit 1, no match ⇒isTenancyDisabled()readsfalsepackages/objectql/src/registry.ts:450applySystemFields: "Theorganization_idCOLUMN is provisioned unconditionally (subject only to the explicit opt-outs above) — its existence no longer depends on the global multi-tenant flag."packages/services/service-storage/src/storage-routes.ts:330and:435—owner_id: session?.userId, ten lines below eachcreateFilepackages/drivers/driver-sql/src/sql-driver.tsapplyTenantScope:builder.where(field, String(tenantId)).orWhereNull(field)packages/plugins/plugin-security/src/tenant-layer.ts:101— "{ organization_id: ORG_ID }→ theisolatedwall, AND-composed unconditionally";computeTenantLayer0Filterreturns{ organization_id: input.organizationId }packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts:40-44: "Layer 0's strictorganization_id = :tenantAND-composes over the driver's compatibility arm and the conjunction is the strict equality alone."singleposture is inertcomputeTenantLayer0Filteropens withif (!postureEnforcesWall(input.tenancyPosture)) return null;⇒ The ruling's rationale still holds on the current ref: forward-only stamping would have made new files org-walled and existing NULL-org files invisible to every principal. That split is what the backfill prevents.
Half 1 — forward stamping
The channel that was missing is a context, not a column.
createFile(rec, context?)takes a new optionalStorageWriteContext({ organizationId }) and hands the engine{ context: { tenantId } }. The platform's own insert-side chokepoint does the rest:context.tenantId→ObjectQLEngine.buildDriverOptions(packages/objectql/src/engine.ts:3577) →DriverOptions.tenantId→SqlDriver.injectTenantOnInsert(packages/drivers/driver-sql/src/sql-driver.ts:11771) → the object's tenant column.⛔ The column is deliberately NOT written onto the payload. Two questions a metadata store must not answer for itself are already answered there: whether this object has a tenant column (
resolveTenantField→null⇒ nothing is stamped) and whether an explicit value on the row wins (it does, "explicit values are never overwritten"). Re-deciding either one package away from the schema is how a stamp starts failing on an install that opted the object out. It also silences the[tenant-audit]warning this door has been raising on every walled deployment — the warning naming exactly this defect.Where the organization comes from.
buildAuthSessionResolvernow reportssession.session.activeOrganizationId— the platform's existing spelling, the same oneHttpDispatcher(packages/runtime/src/http-dispatcher.ts:1116) anddispatcher-plugin.ts:1808read — with the flattened shape as a fallback.⛔ No membership fallback.
marketplace-install-local-plugin'sresolveActiveOrgIdfalls back to the caller's firstsys_organization_memberrow; its own doc says that is a SCOPING read for a seed. Here the value becomes a wall, and a file stamped from a guessed membership is a file its uploader can no longer see from the organization they were actually acting in. No active organization ⇒ no stamp — the pre-#12745 behaviour, reported by the backfill rather than invented at the door.Files:
metadata-store.ts(createFile+StorageWriteContext),storage-routes.ts(both upload doors,StorageUploadSession),storage-service-plugin.ts(the session bridge),index.ts(the two new types).Half 2 — the backfill
packages/services/service-storage/src/backfill-sys-file-organizations.ts, following the tree's precedent (plugin-approvals/src/backfill-platform-row-organizations.ts): one-off, idempotent, dry-run first, not exported from the package index and not shipped indist.Subject derivation — the two holder channels, and "exactly one". A
sys_file's organization is its HOLDER's. There are exactly two holder channels in the tree and the sweep reads both: the exclusive field reference (ref_object/ref_id, ADR-0104 D3 wave 2) and everysys_attachmentjoin row (file_id→parent_object/parent_id, deliberately many-to-one). A row is stamped only when every named holder answered and they all answered the same organization.⛔ Anything else stays NULL and is REPORTED, never guessed — including the case that reads like a majority vote: one holder in an organization beside another holder with none is AMBIGUOUS, because stamping the answer would aim the invisibility at the other holder instead. The uploader (
owner_id) is not a subject: a user may belong to many organizations, so deriving from them would invent an answer the data does not carry.⭐ The residual-NULL count is the deliverable, not the leftovers.
report.totals.residualNullplus a per-reason breakdown (noSubject,subjectNotFound,subjectNotOrganizationScoped,subjectHasNoOrganization,ambiguousSubjects,unaddressable) is on the report object AND printed byformatSysFileOrganizationBackfillReport— for the dry run as well as the applied run, which is what makes the dry run a decision document rather than a preview. Those rows stay invisible under a wall; the ruling makes that residue the maintainer's to see.Dry run and rollback.
runSysFileOrganizationBackfill(engine)defaults todryRun: trueand writes nothing.applySysFileOrganizationBackfill(engine, plan)takes the plan a human read rather than a fresh scan, and each write carries only{ id, organization_id }— nothing else on the row is touched, so the undo is "write NULL back to exactly these ids", which the dry run names.Idempotency is by construction, not by a guard: the scan is
WHERE organization_column IS NULLand every write fills that column. Pinned by running it twice, not by prose.⛔ Scope:
sys_fileonly. The precedent requires a maintainer order per table; the ruling is that order for this one table.SYS_FILE_BACKFILL_OBJECTis a single constant, not a target list, andsys_upload_session— same package, same NULL column — is deliberately not swept (filed separately, see below).INFERRED — one deliberate divergence from the precedent, stated as such
The precedent resolves its organization column through
@objectstack/metadata-core'screateRecordOrganizationResolver. This sweep does not, and instead resolves the column locally increateWallOrganizationResolver, mirroringSqlDriver.computeTenantFieldlimb for limb (ADR-0066 opt-out → declaredtenancy.tenantField→ injectedorganization_id). Two reasons, the second load-bearing:tenancy.organizationField, a key scope-pinned by the spec: audit stamping needs a read-neutral organization declaration —tenancy.tenantFieldcannot servesys_api_keywithout walling the credential table (#8707 remainder) #8778 ruling (widened by name on cloud#1395) to exactly three platform-row writers — audit, approvals, automation runs. A fourth consumer needs its own maintainer ruling (packages/spec/src/data/object.zod.ts:598-638).sys_api_keyis the shipped object where the two diverge on purpose — a credential table that must stay unwalled (tenancy.enabled: false, [finding] API keys carry no organization — under the isolated posture a minted key reads no org data at all (no leak, but the key surface is inert) #8287) while recording an organization underactive_organization_id. Stamping a file from that column would wall the file into an organization its holder is not walled into. Pinned in both directions by a test.This divergence is INFERRED from the two sources above, not ruled. If a maintainer prefers the shared resolver, the change is one function.
Verification
All figures below were taken at the branch head
882a2fe04, working tree clean.Suite —
pnpm --filter @objectstack/service-storage test:26 of those are new (11 forward, 15 backfill).
Gate union, re-derived live on the ACTUAL changed set with⚠️ It named no contract family — the dispatch's clause-②
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(9 paths, three-dot against the merge base) and every named family run.nograding holds. Each gate's own judgment line, not a bare exit code:check:engine-double-contract→OK — 695 pinned, 134 in the DEBT ledger, 3 exempt.check:where-matcher→✓ where-matcher conformance holds: 310 matcher(s) discovered, 310 answer the combinator battery correctly or refuse it loudly (194 refuse).check:type-check-debt→check-type-check-coverage --re-measure: OK — 31 ledger entr(ies) re-measured in 225.6s, 1570 raw tsc error(s) total, none above its recorded number.check:type-check-coverage→OK — 65/78 workspace packages type-checked (plus the root), 13 in the DEBT ledgercheck:i18n→OK (9 package(s) — all bundles in sync, no undeclared authoring keys).check:cross-package-test-inputs→OK: 20 package(s) read outside themselves, all declaredcheck:objectql-double-limit→OK ... 288 double(s) graded ... none new.check:slot-lookup→✓ slot-lookup ratchet holds ... none new.check:query-options-erasure→✓ query-options-erasure ratchet holds ... none new.check:published-files,check:route-envelope,check:test-source-alias,check:type-source-resolution,check:page-declaration-shape,check:i18n-stale-fill,check:changeset-gate-self-tests,check:objectui-changeset,check:nul-bytes,check:agent-test-spelling,check:bash32-floor,check:cli-command-ids,check:entry-guard,check:parse-guard,check:pnpm-filter-targets,check:watch-hint-literal— all exit 0check-ci-filter-parity,check-comment-mask-adoption,check-cross-package-test-inputs,check-plugin-teardown-shape,check-adr-0087-registration,check-changeset-no-major,check-empty-changeset,docs-audit/check-affected-docs,docs-audit/check-drift-comment— all exit 0Repo-wide lint, not narrowed:
pnpm lint(eslint . --no-inline-config) exits 0 with no findings, 58s.node scripts/pm/check-half-states.mjsreturnsPREREQUISITE NOT MET — the token in the environment is not a valid GitHub credential(exit 3). Nothing was swept, so it is neither a pass nor a fail. Its self-test siblingpnpm check:pm-half-statespasses (1515 cases).@objectstack/service-storagedeclares notypecheckscript —pnpm --filter ... run typecheckwould match nothing and exit 0, which is not a measurement. It is covered instead byDEBT['@objectstack/service-storage']inscripts/check-type-check-coverage.mjs, and the ratchet's--re-measureruns the realtsc. That ratchet caught +5 new errors from the first draft of the new test file (four TS2835 missing.jsextensions, one TS7006). They were fixed, not ledgered: the entry sits at its recorded 51 and a directtsc --noEmit -p tsconfig.jsonnow reports 0 errors in either new file. The package'stsconfig.jsonhas no*.test.tsexclusion, and the positive control is stronger than a file listing — tsc reported those five errors AT lines in the new test file, so it demonstrably compiled it.Ablation — the pins can go red
All three legs mutate a module the test imports relatively, from source (no
exportsresolution, nodist/involved), so no rebuild leg applies. Each mutation was confirmed on disk bygit hash-object, never by an editor's exit code; each restore usedgit checkout HEAD -- ABSOLUTE_PATH(never bare), undertrap ... EXIT INT TERMon a path rooted atgit rev-parse --show-toplevel, and was verified by hash equality to the HEAD blob plus an emptygit diff HEADand an emptygit status --porcelain.metadata-store.ts: drop the options bag —engine.insert('sys_file', full), the pre-#12745 shape. Blob199c0331→425060dc; anchor 1 → 0, injected 1199c0331distinct.length === 1 && answered.length === subjects.length→distinct.length >= 1(first answer wins). Blobbecbe0a9→ce0e072cbecbe0a9if (options.dryRun !== false)→if (options.dryRun === true)(writes unless asked not to). Blobbecbe0a9→7e203af9becbe0a9Compatibility
createFile's new parameter is optional, andStorageRoutesOptions.resolveSessiononly WIDENS its return type ({ userId? }→{ userId?, organizationId? }), so existing resolvers and callers keep compiling and keep their current behaviour. A changeset is included (minoron@objectstack/service-storage).Out of scope, filed not fixed
sys_upload_sessioncarries the identical shape — notenancykey,createSessioninserts with no context atmetadata-store.ts:349— and is deliberately untouched: the ruling authorisessys_filealone, and the precedent requires a maintainer order per table. Recorded as #12928, unassigned and ungraded, with the lifetime difference (transient, TTL-reaped) that may well change the answer there.When this lands, the #11611 thread wants pointing at the ruling so the customer's operations planning is updated — the ruling asks for that and it is a PM action, not a code change.
Generated by Claude Code
Generated by Claude Code