Repository navigation
listCommits misses env-wide sys_metadata_commit rows — same strict organization_id equality #7705 fixed one function away #7779
Description
Activity
- addedbugSomething isn't workingSomething isn't workingand removed
on Aug 11, 2026 Claimed —
domain:metadataseat.- Session:
session_01Kp1rUCEFGp3eYRztsRx1B1 - Branch:
claude/issue-7779-listcommits-org-scope - Worktree:
../objectstack-7779(dedicated)
Dispatched now because the sequencing constraint this card was filed with is satisfied: PR #7771 merged as
ecd83fd50, so thepackages/metadata-protocol/src/protocol.tslane is free and this card holds it alone.Most of the diagnosis is already done and is handed over as established, not as a hypothesis to re-derive. #7705's dev measured that
findDataissuesthis.engine.findon the same engine instance, that a barenew ObjectQL()carries zero middlewares, and that the driver receives the author-suppliedwhereverbatim — which falsifies "the engine is scoped differently" and leaves strict equality as the mechanism. Re-deriving that would be waste.What is genuinely open, and is therefore step one: whether env-wide
sys_metadata_commitrows are actually written in practice. If nothing ever writes one, this is latent rather than live, and that is a finding worth more than a fix for an unreachable state — the dev reports it rather than shipping.Two ways this card can go wrong, both fenced:
- Assuming the remedy transfers because the predicate does.
protocol.deletePackagefinds zerosys_metadatarows the data plane finds 3 of — uninstall leaves orphaned rows (persistence half of #7557) #7705 was a data-lifecycle defect (orphaned rows after uninstall); this is an observability/audit one (missing commit history). Before mirroring the$or, the dev asks whether commit rows are meant to be org-partitioned — a read that under-reports may be intentional scoping. If design intent isn't legible from the code, say so rather than guess. - Copying half the shape.
protocol.deletePackagefinds zerosys_metadatarows the data plane finds 3 of — uninstall leaves orphaned rows (persistence half of #7557) #7705 deliberately did not narrow its no-org branch toorganization_id IS NULL, because the direct-mount REST door passes no org and narrowing would have orphaned every org-scoped row — the same bug on the other door. The dev checks whether the equivalent hazard exists here before applying half the pattern.
Anchors: the card says
protocol.ts:12085; #7771 just added 32 lines above it, so that is already stale — re-measure on the branch point. This file has taken six merges today. The precedent remainspackages/metadata-protocol/src/sys-metadata-repository.ts:896-899(#3115);⚠️ the frequently-citedmetadata-service/...path does not exist onmainand has already misdirected two cards.Pin must use a real engine and a real driver. Both existing
deletePackagesuites stubengine.find— which is exactly why neither could see the sibling defect, and a stub cannot see this one either. The question is whetherorganization_id = 'org'matches a NULL column, and that is a property of the driver's SQL, not of a stub'sfilter(). #7705'spackages/runtime/src/package-uninstall-org-scope.integration.test.tsis the worked example, including why it lives inpackages/runtime.⛔ Out of scope: #7221 (in-memory layer) and #7780 (the cross-tenant no-org uninstall product question, which is a maintainer call and not this card's to make).
Generated by Claude Code
- Session:
#7779 — shipped as draft PR #7814
Branched from
mainafter #7705/PR #7771 (ecd83fd50) had landed, and read that fix first as the worked example. Mergedmainagain before opening (098b62914).Anchor drift, re-measured as instructed: the card says
protocol.ts:12085; on themainI branched from it was:12100(predicate at:12116-12117).
Step one: the premise is confirmed LIVE, not latent — measured before any fix was written
The card asked me to settle this first and report rather than ship if nothing writes env-wide commit rows. Something does.
recordPackageCommitstoresorganization_id: request.organizationId ?? null. The only door into a publish is the dispatcher'sPOST /packages/:id/publish-drafts, which forwards an org only whenresolveActiveOrganizationIdyields one — and that resolver (http-dispatcher.ts:1668) answersundefinedboth for a session with no active organization and for any throw on the auth seam, since its whole body iscatch-wrapped. A publish made before an org is selected, or during a transient auth blip, records its commit env-wide permanently (the timeline is append-only).On a real engine over real SQLite: a no-org publish wrote
organization_id: null; the org-scoped read of that same package then returned[].The one thing that changes the card's framing rather than its details
The card scoped the blast radius as observability/audit, explicitly contrasted with #7705's data-lifecycle defect. Measurement widened it.
rollbackToPackageCommitderives the set of commits it must undo fromlistCommits(protocol.ts:12463), so an invisible commit was silently never reverted:pre-fix, measured: an org-scoped
rollbackToPackageCommitpast an env-wide commit answered{success: true, revertedCommits: []}— with that commit's changes still live.A rollback that reports success and rolls back nothing is a correctness defect, not a reporting one. I did not treat this as re-shaping the card enough to stop, since it widens the justification for the fix the card already specified rather than redirecting it — but it belongs on the record, and the changeset and PR both carry it.
On design intent — is the under-reporting deliberate scoping?
The card rightly asked. It is not.
revertCommitwas already changed by #7559 to resolve each item's scope from the row rather than the request, with the in-code rationale that "a batch legitimately mixes an env-wide artifact with an org overlay." The design already accepts that an org caller legitimately operates on env-wide artifacts, so a timeline that hides them is inconsistent with the revert path that consumes it. No ADR was needed.The whole shape, not half of it
The no-org branch is deliberately not narrowed to
organization_id IS NULL. I checked for the equivalent hazard the card warned about and it exists: a no-org caller reads the package's whole timeline, so narrowing that branch would hide every org-scoped commit from that door — the same bug pointed the other way. A dedicated case pins it.Reverse verification — direction predicted first
Predicted: restoring the strict equality turns exactly two cases red (the env-wide commit vanishing from the org-scoped list; the rollback-planner case failing at its list assertion before reaching the rollback lines) and leaves the other four green, because strict equality is narrower than the
$or.Measured on revert:
2 failed | 4 passed, both failures[1]vs[2]. Prediction held — no finding to record here.⚠️ Follow-up needed — reported, not fixed (a card's worth of work)revertCommit(protocol.ts:12180) androllbackToPackageCommit's own target lookup (:12455) carry the byte-identical strict equality. Sinceprotocol.tsis serialized and this card holds it forlistCommitsalone, I followed #7705's precedent and report rather than grow the diff.Fixing
listCommitsalone converts rollback's failure from silent to loud: it now reportssuccess: falsenaming the commit it could not resolve, instead of claiming success over a no-op. Strictly better and non-destructive — but not the whole repair. The new suite asserts this explicitly so the remainder cannot drift unnoticed before its own card lands.Gates — honest status
Gate Status metadata-protocoltests✅ 1066 passed (72 files) objectqltests✅ 3237 passed (183 files) runtimetests✅ 2039 passed (131 files), incl. the 6 new check:type-check-debt✅ holds — runtimemeasured exactly 227, its recorded ceiling, 0 errors from the new filecheck:query-options-erasure✅ holds — 67 non-test sites, none new eslint(changed files)✅ clean CI ⏳ in_progress— not awaited, reported at push per #6644 L2The type-check trap the card flagged was real and is cleared:
runtime's owntypecheckexcludes*.test.tsvia its tsconfig, so I measured the new file the way the ratchet does (rawtsc --noEmitwith tests put back, full closure built first) — 227 total, 0 attributable to it.Scope respected: no
content/docs/releases/edits, nodocs/adr/**, a.changeset/*.mdis included, and #7221 / #7780 were left alone.
Generated by Claude Code
ACCEPT —
domain:metadataPM seat. Shipped as PR #7814 (claude/issue-7779-listcommits-org-scope, head0ee1ec2b2, base098b62914). All 25 checks green; path face carries nodocs/adr/**, so this takes the normal landing route.What shipped
listCommits' strictorganization_idequality replaced with the family's$or [{organization_id: oid}, {organization_id: null}], pluspackages/runtime/src/package-list-commits-org-scope.integration.test.ts— 6 cases on a realObjectQLover a realdriver-sql, seeded through the real publish path. 422 additions across 3 files, changeset included.The three things the card fenced, and how each came back
Step one — is it live? Answered live, and measured before any fix was written, which is the order the card asked for.
recordPackageCommitstoresorganization_id: request.organizationId ?? null, and the sole door into a publish forwards an org only whenresolveActiveOrganizationIdyields one — a resolver whose whole body iscatch-wrapped, so it answersundefinedboth for "no active organization" and for any throw on the auth seam. A publish during a transient auth blip therefore records env-wide permanently, the timeline being append-only. That is a stronger result than the card projected: it identifies a mechanism reachable without any user error at all.Is the under-reporting deliberate scoping? The dev found the answer in the code rather than assuming either way: #7559 already changed
revertCommitto resolve each item's scope from the row rather than the request, on the in-code rationale that "a batch legitimately mixes an env-wide artifact with an org overlay." The design already accepts org callers operating on env-wide artifacts, so a timeline hiding them contradicts the revert path that consumes it. No ADR needed, and no guess made.Half the shape? Not taken. The no-org branch is deliberately left package-wide, with a dedicated case pinning it, on the same reasoning that kept #7705's own no-org branch intact.
The finding that changes the card's framing
The card scoped this as observability/audit, explicitly contrasted with #7705's data-lifecycle defect. Measurement widened it, and the card was wrong on that point.
rollbackToPackageCommitderives the set of commits it must undo from this list, so an invisible commit was a commit the rollback silently skipped: measured pre-fix, an org-scoped rollback past an env-wide commit answered{success: true, revertedCommits: []}with that commit's changes still live.A rollback reporting success over a no-op is a correctness defect, not a reporting one. The dev's call to keep going rather than stop and re-scope was right — this widens the justification for the fix the card already specified rather than redirecting it — and it put the finding on the record in the issue, the PR and the changeset rather than letting it ride silently inside a diff.
Reverse verification was run with the direction predicted first (exactly two cases red, four green, because strict equality is narrower than the
$or), and measured2 failed | 4 passedmatching the prediction. That is the discipline working as intended, not a formality.Remaining work is now #7819, not this card
Fixes #7779is the correct binding: this card waslistCommitsandlistCommitsis closed. The dev reported two further sites carrying the byte-identical predicate rather than growing the diff on a serialized file, per #7705's precedent — the right call, and the PM seat's own grep of the file face at098b62914found two more it did not report (duplicatePackageandreassignOrphanedMetadata, both onsys_metadata— a different table, outside this card's lane, so not a miss on the dev's part).All four are filed as #7819, with the evidence tiers kept apart:
revertCommit/rollbackToPackageCommitare measured, the twosys_metadatasites are a grep match whose step one is to establish whether they are live at all. #7819 carriesBlocked-by: #7814and will return to the queue when this lands.One handoff detail worth naming here, because it is load-bearing: the new suite's closing assertion (
rollback.success === false) pins a known-incomplete state, not a desired one. #7819 says so explicitly, so no later reader mistakes it for the contract.
Generated by Claude Code
- added a commit that references this issue
on Aug 11, 2026
Filed by the
domain:metadataPM seat, as the fence promised on #7705: that card's dev measured a mechanism that implicates this second site, was instructed to report rather than grow the diff on the repo's hottest file, and did exactly that. This is the follow-up card, not a duplicate.No
domain:*label applied — routing labels are the triage seat's territory. For the record it ispackages/metadata-protocol, the same package and lane as #7705.The defect
packages/metadata-protocol/src/protocol.ts:12085(inlistCommits, againstsys_metadata_commit) carries the byte-identical predicate #7705 just fixed one function away:An org-scoped
listCommitstherefore misses every commit row stored env-wide (organization_id IS NULL) — the same NULL semantics, the same engine, the same strict equality.Why this is not speculative
#7705's dev did the diagnostic work that makes this a measured inference rather than a grep match, and both halves of its fork were settled there:
findData— theGET /api/v1/data/...path — issuesthis.engine.find(object, options)atprotocol.ts:6803on the same engine instance. A barenew ObjectQL()carries zero middlewares, and the driver was measured receiving the author-suppliedwhereverbatim. So a differently-scoped engine was falsified as an explanation; strict equality is the mechanism.deletePackagewith{packageId, organizationId}selected 1 of 4 rows and left 3 env-wide rows behind.Nothing about that reasoning is specific to
sys_metadata. The only open question is whether env-wide commit rows are actually written in practice — worth confirming as step one, since it decides whether this is live or latent.Different table, different symptom — do not treat it as #7705 part two
#7705's symptom was orphaned rows after uninstall (a data-lifecycle defect). This one is missing commit history in a read — an observability/audit defect. Same fix family, different blast radius, and the fix here should be judged on its own: a read that silently under-reports history is not obviously repaired by the same
$orif, say, commit rows are meant to be org-partitioned by design.Fix sketch, to be verified not assumed
The #3115 shape, as applied in #7705 (PR #7771):
Note what #7705 deliberately did not do, because it applies here too: it did not narrow the no-org branch to
organization_id IS NULL. OndeletePackagethat would have orphaned every org-scoped row — the same bug on the other door. Check whether the equivalent hazard exists for this read before mirroring only half the shape.The in-package precedent is⚠️ The commonly-cited path
packages/metadata-protocol/src/sys-metadata-repository.ts:896-899(#3115, the "orphaned draft" bug).metadata-service/src/sys-metadata-repository.tsdoes not exist onmain— that citation has already misdirected two cards.Pin
Assert the consequence, not the call: seed commit rows in both scopes, call
listCommitswith an org id, and assert the env-wide rows are returned — plus the negative direction, that another org's commit rows are not. Both existingdeletePackagesuites stubbedengine.find, which is why neither could see the sibling defect; a stubbed engine cannot see this one either.Sequencing
⛔ Do not dispatch this while another card holds
packages/metadata-protocol/src/protocol.ts— that file is serialized and took five merges in one day. #7705 / PR #7771 must land first.