listCommits misses env-wide sys_metadata_commit rows — same strict organization_id equality #7705 fixed one function away

[os-zhuang]

Filed by the domain:metadata PM 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 is packages/metadata-protocol, the same package and lane as #7705.

The defect

packages/metadata-protocol/src/protocol.ts:12085 (in listCommits, against sys_metadata_commit) carries the byte-identical predicate #7705 just fixed one function away:

const where: Record<string, unknown> = { package_id: request.packageId };
if (request.organizationId) where.organization_id = request.organizationId;

An org-scoped listCommits therefore 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:

• The engine is not the variable. findData — the GET /api/v1/data/... path — issues this.engine.find(object, options) at protocol.ts:6803 on the same engine instance. A bare new ObjectQL() carries zero middlewares, and the driver was measured receiving the author-supplied where verbatim. So a differently-scoped engine was falsified as an explanation; strict equality is the mechanism.
• The equality demonstrably drops env-wide rows. On a real engine over a real SQLite driver, deletePackage with {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 $or if, 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):

if (request.organizationId) {
    where.$or = [
        { organization_id: request.organizationId },
        { organization_id: null },
    ];
}

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. On deletePackage that 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 packages/metadata-protocol/src/sys-metadata-repository.ts:896-899 (#3115, the "orphaned draft" bug). ⚠️ The commonly-cited path metadata-service/src/sys-metadata-repository.ts does not exist on main — that citation has already misdirected two cards.

Pin

Assert the consequence, not the call: seed commit rows in both scopes, call listCommits with an org id, and assert the env-wide rows are returned — plus the negative direction, that another org's commit rows are not. Both existing deletePackage suites stubbed engine.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.

[os-zhuang]

Claimed — domain:metadata seat.

• 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 the packages/metadata-protocol/src/protocol.ts lane 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 findData issues this.engine.find on the same engine instance, that a bare new ObjectQL() carries zero middlewares, and that the driver receives the author-supplied where verbatim — 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_commit rows 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:

1. Assuming the remedy transfers because the predicate does. protocol.deletePackage finds zero sys_metadata rows 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.
2. Copying half the shape. protocol.deletePackage finds zero sys_metadata rows the data plane finds 3 of — uninstall leaves orphaned rows (persistence half of #7557) #7705 deliberately did not narrow its no-org branch to organization_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 remains packages/metadata-protocol/src/sys-metadata-repository.ts:896-899 (#3115); ⚠️ the frequently-cited metadata-service/... path does not exist on main and has already misdirected two cards.

Pin must use a real engine and a real driver. Both existing deletePackage suites stub engine.find — which is exactly why neither could see the sibling defect, and a stub cannot see this one either. The question is whether organization_id = 'org' matches a NULL column, and that is a property of the driver's SQL, not of a stub's filter(). #7705's packages/runtime/src/package-uninstall-org-scope.integration.test.ts is the worked example, including why it lives in packages/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

[claude]

#7779 — shipped as draft PR #7814

Branched from main after #7705/PR #7771 (ecd83fd50) had landed, and read that fix first as the worked example. Merged main again before opening (098b62914).

Anchor drift, re-measured as instructed: the card says protocol.ts:12085; on the main I 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.

recordPackageCommit stores organization_id: request.organizationId ?? null. The only door into a publish is the dispatcher's POST /packages/:id/publish-drafts, which forwards an org only when resolveActiveOrganizationId yields one — and that resolver (http-dispatcher.ts:1668) answers undefined both for a session with no active organization and for any throw on the auth seam, since its whole body is catch-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. rollbackToPackageCommit derives the set of commits it must undo from listCommits (protocol.ts:12463), so an invisible commit was silently never reverted:

pre-fix, measured: an org-scoped rollbackToPackageCommit past 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. revertCommit was 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) and rollbackToPackageCommit's own target lookup (:12455) carry the byte-identical strict equality. Since protocol.ts is serialized and this card holds it for listCommits alone, I followed #7705's precedent and report rather than grow the diff.

Fixing listCommits alone converts rollback's failure from silent to loud: it now reports success: false naming 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-protocol tests	✅ 1066 passed (72 files)
objectql tests	✅ 3237 passed (183 files)
runtime tests	✅ 2039 passed (131 files), incl. the 6 new
check:type-check-debt	✅ holds — runtime measured exactly 227, its recorded ceiling, 0 errors from the new file
check:query-options-erasure	✅ holds — 67 non-test sites, none new
eslint (changed files)	✅ clean
CI	⏳ in_progress — not awaited, reported at push per #6644 L2

The type-check trap the card flagged was real and is cleared: runtime's own typecheck excludes *.test.ts via its tsconfig, so I measured the new file the way the ratchet does (raw tsc --noEmit with tests put back, full closure built first) — 227 total, 0 attributable to it.

Scope respected: no content/docs/releases/ edits, no docs/adr/**, a .changeset/*.md is included, and #7221 / #7780 were left alone.

---

Generated by Claude Code

[os-zhuang]

ACCEPT — domain:metadata PM seat. Shipped as PR #7814 (claude/issue-7779-listcommits-org-scope, head 0ee1ec2b2, base 098b62914). All 25 checks green; path face carries no docs/adr/**, so this takes the normal landing route.

What shipped

listCommits' strict organization_id equality replaced with the family's $or [{organization_id: oid}, {organization_id: null}], plus packages/runtime/src/package-list-commits-org-scope.integration.test.ts — 6 cases on a real ObjectQL over a real driver-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. recordPackageCommit stores organization_id: request.organizationId ?? null, and the sole door into a publish forwards an org only when resolveActiveOrganizationId yields one — a resolver whose whole body is catch-wrapped, so it answers undefined both 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 revertCommit to 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. rollbackToPackageCommit derives 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 measured 2 failed | 4 passed matching the prediction. That is the discipline working as intended, not a formality.

Remaining work is now #7819, not this card

Fixes #7779 is the correct binding: this card was listCommits and listCommits is 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 at 098b62914 found two more it did not report (duplicatePackage and reassignOrphanedMetadata, both on sys_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 / rollbackToPackageCommit are measured, the two sys_metadata sites are a grep match whose step one is to establish whether they are live at all. #7819 carries Blocked-by: #7814 and 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