fix(runtime): a metadata write carries the session's organization only for types declaring allowOrgOverride (#7018) - #7060
Conversation
…OrgOverride types (#7018) The #6190 ruling's runtime half (Option A). Both dispatcher write sites used to thread `resolveActiveOrganizationId` into `protocol.saveMetaItem` unconditionally, and `SysMetadataRepository.put` stamps `organization_id` for every type — so a session with an active organization minted org-scoped rows for types cold boot never reads (`loadMetaFromDb` hydrates `organization_id IS NULL` only). Those rows are phantom writes: a `flow` fires until the next restart and then silently stops; an `object` 404s every record. - `domains/meta.ts` PUT: the active org rides the write only when the target type declares `allowOrgOverride: true`; otherwise the write lands env-wide — the same row, and the same receipt, a no-active-org session produces today. - `domains/packages.ts` ADR-0045 §3 visibility flip: `app` is non-overridable, so the flip writes env-wide, on the row boot hydrates. The org-scoped flip was itself a phantom that reverted on restart. The predicate is derived from `DEFAULT_METADATA_TYPE_REGISTRY` (PD #8, no parallel allowlist) and deliberately ignores `OS_METADATA_WRITABLE` — the same call `reportUnhydratableOrgScopedRows` already made on the read side. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2
…time-org-threading-r2
… dispatch (#7018) `check:engine-double-contract` flagged the fake engine in `meta-write-org-scope.test.ts`: its `update()`/`delete()` accepted call shapes the real ObjectQL engine refuses. Both verbs now open with `assertEngineUpdateDispatch` / `assertEngineDeleteDispatch` from `@objectstack/metadata-core` (never `@objectstack/objectql` — that reverse edge is a cycle turbo refuses), and route by-id dispatches through the bound id. Also records the MEASURED reverse-verification direction in the file header (4 red / 4 green, not the 5/3 first predicted) and adds the two registry methods `getMetaItems` calls on the app-listing path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 20 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also reference the affected code. These are read-only:
|
…angeset to patch (#7018) Rebased-in-place adjustments for the r2 takeover branch, on top of the prior session's ad11fe5 + 1a01e8d: - ctx() now grants manage_metadata: the dispatcher's /meta PUT gate (#7019, landed on main after the prior branch forked) 403s an unauthorized caller before the org-scoping decision these tests pin is ever reached. - registry stub carries isPackageDisabled + applyNavContributions — the two methods getMetaItems grew on main (disabled-package filter, ADR-0029 D7 nav merge); without them the ADR-0045 flip cases fail on a TypeError inside the flip's try, not on the partition assertion. - reverse verification re-measured on the merged #7043 base: same 4 red / 4 green, same failure shapes (header updated in place). - changeset level minor -> patch: behavioural fix, no new API surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGRN2cSRfggfX9B2L83bQc
|
PM coordination (session_01LGRN2cSRfggfX9B2L83bQc) — fast-forwarding this PR's branch to the verified-green head, first-PR identity preserved. To the prior session ( Pushing Generated by Claude Code |
…time-org-threading
…scope fixture (#7018) `main` moved under this branch: #6603 / PR #7027 put a `manage_metadata` gate in front of `PUT /meta/:type/:name`, ahead of everything this file measures. The fixture's session carried `systemPermissions: []`, so on the PR's merge ref the door answered 403 and the scoping decision was never reached. The capability is now granted explicitly rather than inherited from whatever another suite registered, so the file is order- and shard-independent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2
…reading' into claude/issue-7018-runtime-org-threading # Conflicts: # packages/runtime/src/meta-write-org-scope.test.ts
…T ledger (#7018) `check:type-check-debt` went red on the merged base: `@objectstack/runtime`'s TEST_DEBT is a shrink-only ratchet (#5278) recording 227 raw errors, and the new file pushed the measured count to 240. The package's own `tsc --noEmit` never saw them — `tsconfig.json` excludes `*.test.ts`, which is the hidden layer that ledger exists to measure. Fixed rather than ledgered, since none of the 15 were irreducible: - `HttpDispatcherResult.response` is optional (a declining route answers `{ handled: false }`), so every `res.response.status` was a TS18048. One `responseOf()` helper says once, loudly, that these routes must answer, and hands back a narrowed response — instead of a narrowing dance at each of the 13 call sites. - the muting `console.warn` spy was bound to an unread variable (TS6133); only the `console.error` spy is read back, so only that one is bound now. - the `error.mock.calls` reducers carried implicit `any` parameters (TS7006). Measured with the ledger's own method — a sibling tsconfig that drops the test exclusion — over this file: 15 errors before, 0 after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2
|
PM step-7 review: ACCEPT — queued (verified queue-branch membership 13:28:49Z). Settling the report's open question for the record: changeset stays Two notes recorded with the PM ledger: (1) a concurrent session pushed to this branch mid-task ( After merge: the metadata seat will be reminded to rebase PR #6973 onto the new base (its rebase surface now also carries PR #7040's two test cases). Generated by Claude Code |
|
PM step-7 acceptance review — PASS (session_01LGRN2cSRfggfX9B2L83bQc, maintainer-expedited #7018, the #6190-A runtime half). Verified independently on the fast-forwarded head
Marking ready and enabling queue auto-merge — standard flow for a non-ADR code PR. Generated by Claude Code |
Fixes #7018
Part of the #6190 ruling — the runtime half; PR #6973 rebases after this and lands as the declared=enforced backstop. Issue #6190 itself stays open here — PR #6973 is the card that closes it, and this PR deliberately declares no closing keyword for it.
The maintainer ruling, quoted verbatim and untranslated from #6190 comment 5230671098:
The defect
SysMetadataRepository.putstampsorganization_id: this.organizationIdfor every type, and both runtime write sites threadedresolveActiveOrganizationIdintosaveMetaItemunconditionally. So any session carrying an active organization minted an org-scopedsys_metadatarow even for types with no per-org read channel at all — while cold boot (loadMetaFromDb) hydratesorganization_id IS NULLonly.Those rows are phantom writes: correct for the life of the process, silently absent after the next restart. The measured specimens are the ones #6190 filed — a
flowauthored in Studio binds its triggers, fires all day, and stops firing after a restart with nothing said; anobjectwritten the same way 404s every record. ForallowOrgOverride: truetypes the identical skip is the ADR-0005 design, because those overlays are loaded on demand bygetMetaItem/getMetaItems.What changed
packages/runtimeonly. Nopackages/metadata-protocoland nopackages/specchanges.src/meta-write-org-scope.ts(new) —declaresOrgOverride(type)/organizationIdForMetaWrite(type, activeOrg), derived fromDEFAULT_METADATA_TYPE_REGISTRYwith plural/singular normalization. No parallel allowlist (PD Convert to monorepo with scoped packages #8): the set is computed from the very exportObjectStackProtocolImplementation.OVERLAY_ALLOWED_TYPESderives from, so a registry entry flippingallowOrgOverridemoves both sides together.src/domains/meta.ts—PUT /meta/:type/:namethreads the active org only when the type declares it; otherwise the write lands env-wide.src/domains/packages.ts— the ADR-0045 §3 visibility flip writesapp(allowOrgOverride: false) env-wide, on the row cold boot hydrates and the App Switcher reads. The org-scoped flip was itself a phantom that reverted on restart. ThegetMetaItemsread beside it is left org-aware on purpose — a layered read is a superset, never a loss.Why the static registry flag and not
isOverlayAllowedisOverlayAllowedis aprivate staticin metadata-protocol and additionally consultsOS_METADATA_WRITABLE. The predicate here must agree with the one deciding whether the row is ever readable again, and boot hydration keys off the static registry flag alone.reportUnhydratableOrgScopedRowssettled the identical question on the read side, in its own words:So the escape hatch still unlocks the write, and the write still lands env-wide. No
needs_decisionwas required: the clean registry-derived source exists as a public@objectstack/spec/kernelexport.Sweep — are these the only two sites?
Verified against
origin/main, not assumed.packages/runtimehas exactly twosaveMetaItemcall sites (the two above) and zeropublishMetaItemcall sites. The other nineresolveActiveOrganizationIduses are reads (getMetaItem/getMetaItems/listDrafts) or package-level verbs that are not per-type and therefore outside this predicate:publishPackageDrafts,discardPackageDrafts,listCommits,revertCommit,rollbackToPackageCommit,reassignOrphanedMetadata,duplicatePackage,deletePackage,assemblePackageManifest. No third site.One coupling was checked rather than assumed: a draft that now lands env-wide is still publishable.
SysMetadataRepository.listDraftssurfaces env-wide drafts to a non-null-org caller via$orandpublishPackageDraftspromotes each draft in its own scope (#3115), so nothing is stranded.Tests
packages/runtime/src/meta-write-org-scope.test.ts— eight cases, all driving a session that HAS an active organization through the realHttpDispatcher.resolveActiveOrganizationId(a real auth-servicegetSessionshape), the realhandleMetadataRequest/handlePackagesRequest, the realObjectStackProtocolImplementationand the realSysMetadataRepository, then reading the stored row. The pre-existing runtime suite was green on the defect only because nothing in it ever populatedsession.activeOrganizationId; with no active org the two branches are indistinguishable.Reverse verification — direction predicted before running
Taking the fix back out with
git checkout origin/main -- src/domains/meta.ts src/domains/packages.ts. Predicted 4 red / 4 green; measured 4 red / 4 green:flowwriteorganization_id = null"org_alpha"objectwriteorganization_id = null"org_alpha"(org=org_alpha, …)vs(env-wide, …)appflip_unpublished:false; env-wide list serves it published_unpublished:trueplus an org-scoped_unpublished:false— and the env-wide list still answers_unpublished: trueview"org_alpha"views(plural URL)"org_alpha"The first prediction written into the file was 5 red / 3 green; the measurement corrected it to 4/4. The last case asserts an absence of a degradation line, which the unfixed code satisfies too — its flip succeeds, it just succeeds into the wrong partition. It is kept because it guards the opposite regression (a flip degraded into warn-and-continue, which this route answers 200 through), and its greenness is stated in the file rather than dressed up as a red. The two
viewcontrols are not slack either: a "fix" that simply stopped threading the org anywhere would pass every red case and fail there, silently retiring ADR-0005 per-org overlays.Local runs
Gates enumerated one by one from
.github/workflows/lint.yml(ESLint job in full, plus the Type Check job's non-build steps) andnode scripts/check-adr-0087-registration.mjs --base origin/main— 43 gates, 42 green on the first lap. The one red wascheck:engine-double-contract: the new fake engine'supdate()/delete()accepted shapes the real engine refuses. Both verbs now open withassertEngineUpdateDispatch/assertEngineDeleteDispatchfrom@objectstack/metadata-core(the gate's own remedy).Two CI-only reds, both fixed here
Neither was reproducible locally, and both are worth naming because the reason is the same in each case — the merge ref sees things the branch base did not:
Test Core: 403 instead of 200 on every/metaPUT.mainmoved under this branch — finding: after ADR-0106, a restricted caller's GET → edit → PUT of an object schema DELETES the fields that were masked out of their read #6603 / PR feat(rest): PUT /meta/:type/:name 要求 manage_metadata 能力 (#6603) #7027 (and #6603's gate does not close #6603: the same masked-schema round-trip deletion is still reachable through the compound-name PUT and the dispatcher PUT (plus an ungated DELETE) #7019 / PR feat(rest,runtime): 元数据写入的其余三扇门同样要求 manage_metadata (#7019) #7043 for the other three doors) put amanage_metadatacapability gate ahead of the write. The fixture now grants that capability explicitly, so the file is order- and shard-independent rather than depending on what another suite registered.TypeScript Type Check: the@objectstack/runtimeTEST_DEBT ratchet drifted upward.tsconfig.jsonexcludes*.test.ts, so the package's owntypecheckscript never saw the new file's 15 errors — the hidden layer [finding] DEBT ledger counts in check-type-check-coverage.mjs drift silently — @objectstack/metadata-protocol records 28, actually reports 63 #5278's ratchet exists to measure. Fixed rather than ledgered: oneresponseOf()helper replaces 13 TS18048 sites (HttpDispatcherResult.responseis optional because a declining route answers{ handled: false }), the mutingconsole.warnspy is no longer bound to an unread variable, and the twoerror.mock.callsreducers are annotated. Measured with the ledger's own method — a sibling tsconfig that drops the test exclusion — 15 errors before, 0 after.Release
.changeset/runtime-meta-write-org-scope.md—@objectstack/runtimepatch. No authoring change and no new refusal: writes that succeeded still succeed, with the same response body. What changes is which partition the row lands in for types that never had a per-org read channel. (The bump was authored asminorand set topatchby a concurrent session on this same branch; kept aspatchhere rather than reverted — flagging it for the reviewer to settle, since the two readings are "behaviour change" vs "fix toward declared behaviour".)Generated by Claude Code