Uh oh!
There was an error while loading. Please reload this page.
fix(engine): a failed sys_organization read is no longer answered as an emptiness — the two genuinely-wrong read seams - #12864
Conversation
…le organization'
`SeedLoaderService.resolveSoleOrganizationId()` sat behind a bare `catch {}`
whose comment named ONE benign cause while the catch swallowed every cause. A
dropped connection, a timeout or a permission refusal all arrived at the caller
as `undefined` — the 'genuinely ambiguous' verdict — so `load()` stamped no
`organization_id` and every business seed row of that run landed org-less,
invisible afterwards under strict org-scoping, with the seed report's `errors`
field never touched.
Discriminate by error TYPE through the shared `isMissingTableError` predicate,
which is the repair PR #9817 already landed on the sibling probe
(`ObjectQL.probeInstallOrganizations`) with this exact shape. Only an
unprovisioned table is truthful emptiness; everything else propagates.
Part of #12852
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry…a retention window
`LifecycleService.loadGovernance()` filled `snapshot.tenantOverrides` — the
ADR-0057 3.2 per-tenant retention/expiry window set — behind a bare `catch {}`
whose comment named one benign cause while the catch swallowed every cause. On
any read failure the map came back EMPTY, which `reap()` and `archiveObject()`
read as 'this deployment has tuned no tenant': every tenant fell back to the
global window, so a tenant configured to retain LONGER had its rows expired
early. Nothing reported it — the snapshot has no field for an incomplete tenant
pass and the catch logged nothing.
Discriminate by error TYPE through the shared `isMissingTableError` predicate.
An unprovisioned `sys_organization` really is 'no tenant overrides', so a
single-tenant kernel is unchanged; every other cause aborts the sweep before any
policy is applied. For a deletion action, not acting on incomplete evidence is
the correct failure direction.
The abort is reported, not thrown: one `report.errors` entry per declared object
plus a warn. `sweep()`'s declared contract is that it never throws, and the
scheduler enters it as `void this.sweep()` where a rejection would be unhandled
— the objection #8906 recorded when it declined to rethrow from
`checkGovernance` one method below.
Part of #12853
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4DryAblation A reverts the seed-loader narrowing (packages/metadata-protocol/src/ seed-loader.ts, resolveSoleOrganizationId) back to a bare swallow, leaving the #12853 seam untouched. PREDICTION, committed before the mutation runs: - Direction: RED. Not 'fewer diagnostics' and not a reversal — the narrowing is the only thing that turns a non-benign read failure into a propagated error, and three pins assert exactly that. - seed-loader-sole-organization-read-failure.test.ts: 3 of 9 FAIL — 'a dropped connection surfaces that error and writes NO org-less rows', 'a permission refusal surfaces the same way', 'a missing COLUMN on an existing sys_organization stays loud (the superstring case)'. Each fails inside the rejection() helper: the load RESOLVES instead of rejecting, and the store carries an org-less widget row. - The other 6 in that file stay GREEN (two probe controls, the several-orgs control, both benign phrasings, and the pinned-organizationId non-effect): none of them reaches a non-benign throw. - objectql lifecycle-service.test.ts: 109 of 109 stay GREEN. Ablation A must red ONLY its own pins; a run that reds #12853's pins too proves neither card. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
Ablation B reverts the lifecycle narrowing (packages/objectql/src/lifecycle/ lifecycle-service.ts, loadGovernance's tenant scan) back to a bare swallow, leaving the #12852 seam untouched. The sweep()-side containment stays in place deliberately: without the throw it is unreachable, which is exactly what the pins must detect. PREDICTION, committed before the mutation runs: - Direction: RED. The narrowing is the only thing that stops a non-benign read failure being answered as an empty tenant-override map. - lifecycle-service.test.ts: 4 of 109 FAIL, all in the #12853 describe — 'a dropped connection aborts the sweep: nothing is read as a candidate and nothing is deleted' (the reaper reads a candidate page and deletes on the GLOBAL 30d window instead of the tenant's 90d, and report.errors is empty), 'a permission refusal aborts it the same way', 'a missing COLUMN on an existing sys_organization stays loud (the superstring case)', and 'the abort is REPORTED, never thrown — sweep() still resolves, and the next sweep runs' (first.errors is empty and box.deletes is not). - The other 105 stay GREEN, including this describe's two controls and the benign unprovisioned-table case: none of them reaches a non-benign throw. - metadata-protocol seed-loader-sole-organization-read-failure.test.ts: 9 of 9 stay GREEN. Ablation B must red ONLY its own pins. Ablation A already ran and matched its own prediction: 3 failed / 6 passed in the #12852 file, 109/109 green in this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
…e, and pin its engine double
Two gate families move on a NEW test file, and both were red before this:
- `pnpm check:where-matcher`: the fixture's matcher was an inline closure the
gate could not lift out of the file ('could not lift: ReferenceError: Cannot
access store before initialization'), so it landed as 1 UNJUDGED — and
'could not run' is a failure, not a pass. Lifted to a module-level
`matchesWhere(row, where)` with no free bindings; behaviour is identical
(flat equality, and a `$`-prefixed combinator is REFUSED rather than read as
a column name). Gate now: 310 discovered, 310 conforming, 194 by refusing,
0 unjudged.
- `pnpm check:engine-double-contract`: the file's engine double is pinned to
the producer's own dispatch predicates, but the shrink-only PINNED ledger did
not know the file, so the pins protected nothing. Recorded via
`node scripts/check-engine-double-contract.mjs --write` — 3 rows added
(delete/findOne/update), 0 lost, and no DEBT-baseline row was touched.
Part of #12852
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry📓 Docs Drift CheckThis PR changes 2 package(s): 7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 20 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 9409b7f1dfc9c98fc07ce8c33d14c326c2af112e && git checkout 9409b7f1dfc9c98fc07ce8c33d14c326c2af112e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 277948f7aea27a6f246bc73d95d8359086cca250 9b809594315c921e01e8655eef54a90ba9387d59 && git checkout -B drift-repro 277948f7aea27a6f246bc73d95d8359086cca250 && git merge --no-ff 9b809594315c921e01e8655eef54a90ba9387d59
node scripts/docs-audit/affected-docs.mjs --json 277948f7aea27a6f246bc73d95d8359086cca250
|
os-zhuang
commented
Aug 28, 2026
Reviewer-of-record notes on the drift rows above. The release-owned audit is the interesting part. 1. Release-owned pages — audited, none falsified, and v17 is better than neutral⛔ Read-only, but the check says they are still audited, so here it is.
and states the method:
⇒ These two seams are the same declared class, fixed by the same published method. The page is not falsified, and it does not claim exhaustiveness — it enumerates four
2. The seven hand-written rows are coarse-anchor noiseEvery row matched the bare class name Found nothing falsified. The nearest page, 3. |
os-zhuang
commented
Aug 28, 2026
Reviewer-of-record: option A stands. This is mechanism inside a ruled posture, not a re-opening of it — so it does not go to the maintainer.The seat raised where Why this is not the escalation triage armedTriage's stop-and-report trigger was explicit: if the implementer concludes the ops posture must be fail-open, do not switch — escalate. The seat concluded the opposite and implemented fail-closed. The trigger was correctly not pulled. Triage's ruled outcome was: governance loading fails ⇒ the sweep aborts ⇒ nothing is deleted on unverified evidence. Option A delivers that exactly — on the aborted path nothing is reaped at all, and the failure is reported through Why B is wrong, verified rather than arguedI checked the three facts the choice turns on, on
⇒ B does not mean "the sweep aborts." It means a transient database blip kills the process. That is a far heavier operational change than triage chose, it contradicts a declared invariant in at least two places, and it reverses a decision the file already recorded. ⛔ Not taken. On the fence that was not drawnThe dispatch said "narrow the catch so the sweep aborts" and stopped there. It should have said where the throw lands, because in a service whose scheduler calls it as Two other things worth recording⭐ The marker-discrimination trap was caught independently. The seat reports the narrowing as 5 → 4 counting the code form only, and states why the naive token count reads 6 → 6: the new comments mention the token they replace, so that marker's count does not move and cannot discriminate. That is the same hazard this lane hit earlier today from the other direction, found here without prompting. ⭐ Both cards carry a small factual imprecision, reported not absorbed: they place Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
os-zhuang
commented
Aug 28, 2026
Dequeued for |
Fixes#12853
Fixes#12852
Merged dispatch, as triage specified: the only two read seams graded genuinely wrong out of the eleven parameterless-
catchsites from one measurement. Same shape, same repair, same lane, two different package files — one PR, one commit per card. The other nine seams (graded correct-but-undeclared) are untouched, and so ischeckGovernance, which #8906 (already closed) covers.The two seams
Both are the same defect: a
catchwhose comment names ONE benign cause while thecatchswallows every cause, so a real outage is answered exactly like the benign case — and the value it answers with is the one the caller acts on.1.
packages/metadata-protocol/src/seed-loader.ts—resolveSoleOrganizationId()(#12852)Consequence: invisibility.
undefinedis the verdict the method's own JSDoc calls "genuinely ambiguous", soload()stamps noorganization_idand every BUSINESS seed row of the run lands org-less — invisible afterwards under strict org-scoping.SeedLoadResultcarries anerrorsfield this path never touches, so the operator sees a clean, successful seed.This is not a design question: the sibling probe across the engine boundary,
ObjectQL.probeInstallOrganizations, had the same shape and was repaired by PR #9817. This site was missed by that pass. The repair is that repair, copied:The JSDoc's "or when
sys_organizationis absent" stays true — an unprovisioned table can hold no organization — while every other cause propagates with its envelope intact.2.
packages/objectql/src/lifecycle/lifecycle-service.ts—loadGovernance()(#12853)Consequence: deletion.
snapshot.tenantOverridesis the ADR-0057 section 3.2 per-tenant retention/expiry window set. An empty map is indistinguishable from "this deployment has tuned no tenant", soreap()andarchiveObject()fall every tenant back to the global window — wrong in both directions, and the expensive direction is a tenant configured to retain LONGER having its rows expired early. Nothing reported it:GovernanceSnapshothas no field saying the tenant pass did not complete, and the catch logged nothing.Same narrowing. Operational posture changes, deliberately: a transient
sys_organizationoutage now costs a sweep instead of silently completing on the wrong window. For a deletion action, "do not act on incomplete evidence" is the correct failure direction — a log cannot bring back a reaped row, and the rows a deferred sweep leaves are still there for the next one.One implementation decision that is NOT in the dispatch fence — please rule on it
The fenced line is implemented verbatim. What the fence did not settle is where the throw lands, and the answer was not free:
sweep()'s declared contract is that it never throws. It is stated in three places in this file: the class header,LifecycleSweepReport.errors' own JSDoc ("Isolated per-object failures — the sweep itself never throws"), and lifecycle checkGovernance: a failed row-count probe silently drops the object from quota/growth alerting and fromnextCounts, losing the next sweep's growth baseline too (split from #8896) #8906's comment sixty lines below this seam, which declined to rethrow for exactly this reason: "the only caller issweep(), whose scheduler entry point isvoid this.sweep()— a throw would land as an unhandled rejection".start()really does enter it asvoid this.sweep()(twice: the initial timer and the interval), and this repo installs noprocessunhandledRejection handler. So an escaping rejection is not "the sweep aborts" — under Node's default it terminates the host on a transient database blip.So the throw is contained at the call site inside
sweep(): the sweep aborts before a single policy is applied, and the failure is reported throughreport.errors(one entry per declared object — that field means "a lifecycle policy did not get applied", and here none of them did) plus awarn. Triage's posture is delivered exactly — governance loading fails, the sweep aborts, nothing is deleted on unverified evidence — and no declared invariant is broken.I did not switch to the forbidden "log an error and continue" option: nothing is reaped on the aborted path. I also do not think the posture should be fail-open. If you would rather the rejection escape
sweep()and change the contract at all three declaration sites, that is a one-line revert of the containment and I will make it.Tests
New pins on each seam separately, each paired with positive controls on the same harness so a fixture that had stopped exercising the seam could not pass vacuously:
packages/metadata-protocol/src/seed-loader-sole-organization-read-failure.test.ts(new, 9 tests): one-org / no-org / several-org controls, a dropped connection and a permission refusal both surfacing with the injected error object identity intact and no rows written, both benign phrasings (SQLiteno such table, Postgres42P01) still seeding org-less, the Postgres missing-COLUMN superstring staying loud, and a non-effect (a pinnedconfig.organizationIdnever consults the probe).packages/objectql/src/lifecycle/lifecycle-service.test.ts(7 added, 109 total in file): two controls, the benign unprovisioned case still reaping on the global window, and — the pin this card exists for — a non-benign failure where no candidate row is even read and no delete is issued, with the tenant that would have been found configured to retain three times longer than the global window. Plus the contract pin: the abort is reported,sweep()still resolves, and the next sweep runs.Two independent ablations
Each prediction was committed empty before the mutation ran; each mutation was proven on disk with anchored greps in both directions plus a
git hash-objectchange; each restore was proven bygit hash-objectequal to the HEAD blob and an emptygit diff HEAD; both ran undertrap restore EXIT INT TERMwith absolute paths. Both files are loaded from source through their own relative specifiers, so no rebuild sits between the mutation and the run.Each ablation reds only its own pins.
Verification
Union re-run on the final commit
9b8095943, working tree clean:pnpm --filter @objectstack/metadata-protocol exec vitest run— 1993 passed, 10 skipped (145 files)pnpm --filter @objectstack/objectql exec vitest run— 4260 passed (246 files)pnpm --filter @objectstack/objectql typecheck— clean. Stated honestly:--listFilesshowslifecycle-service.tsin the program andlifecycle-service.test.tsNOT in it, so that reading says nothing about the new test code.metadata-protocoldeclares notypecheckscript at all; both are answered by the ledger below.pnpm lint(eslint . --no-inline-config, whole repo) — clean, 91spnpm check:type-check-debton a fully built workspace closure — "31 ledger entries re-measured, 1570 raw tsc errors total, none above its recorded number. surplus: none"node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, all green:changeset-gate-self-tests,cross-package-test-inputs,durability-log-level,objectql-double-limit,objectui-changeset,page-declaration-shape,published-files,slot-lookup,test-source-alias,type-source-resolution,nul-bytes,query-options-erasure,type-check-coverage,engine-double-contract,where-matcher, pluscheck-adr-0087-registration,check-changeset-no-major,check-ci-filter-parity,check-comment-mask-adoption,check-cross-package-test-inputs,check-empty-changeset,check-engine-split-ratio,check-plugin-teardown-shape,docs-audit/check-affected-docs,docs-audit/check-drift-comment,pm/release-rehearsal-clone --self-test.node scripts/pm/check-half-states.mjsrefuses with "PREREQUISITE NOT MET — the token in the environment is not a valid GitHub credential" (exit 3). Its CI spellingpnpm check:pm-half-statesis the self-test and passes (1515 cases).Two gates were red on the first run and are fixed in the third commit, both caused by the new test file rather than by either seam: the fixture's WHERE matcher could not be lifted by
check:where-matcher(an UNJUDGED verdict, which that gate treats as a failure, not a pass) and is now a module-level helper with no free bindings; and the file's engine double was pinned to the producer's dispatch predicates but absent from the shrink-only PINNED ledger, recorded with--write(3 rows added, 0 lost, no DEBT-baseline row touched).Scope, stated so it can be checked
git diff --statagainst the merge base is seven files: the two source files, two test files, two changesets, and the engine-double pinned ledger.catchnarrowed per file. Counting the CODE form only,5to4in each. Reported this way deliberately: the naivegrep -c 'catch {'reads6to6on both files, because the new comments mention the token they replace — a marker whose count does not move cannot discriminate.checkGovernanceis byte-identical between the merge base and HEAD (samegit hash-objectover the method body). lifecycle checkGovernance: a failed row-count probe silently drops the object from quota/growth alerting and fromnextCounts, losing the next sweep's growth baseline too (split from #8896) #8906 is not redone.Changesets
Both
patch, argued rather than defaulted. No exported signature, type, option or report field moves in either package, and the declared answer for every case the JSDoc describes is unchanged. The honest tension is that what a deployment observes on a failure path does change — a seed run that used to complete while writing invisible rows now fails loudly, and a sweep that used to complete silently now aborts and says so. That is the correction of a defect rather than a new capability, and the three landed repairs in this family (#8896, #8906, #9817) all shipped aspatch.Generated by Claude Code