fix(metadata-protocol): listCommits no longer hides env-wide commit history (#7779) - #7814
Conversation
…istory (#7779) `protocol.listCommits` selected the ADR-0067 timeline with the strict `organization_id` equality that #7705 (PR #7771) had just replaced one function above it. `organization_id = '<org>'` matches no row whose column is NULL, so a session with an active organization was shown none of the commits recorded env-wide. Live, not latent, and measured before the fix was written — which the card set as step one. `recordPackageCommit` stores `request. organizationId ?? null`, and the only door into a publish (the dispatcher's `POST /packages/:id/publish-drafts`) forwards an org only when `resolveActiveOrganizationId` yields one. That resolver 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. So 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. Driven on a real engine over SQLite, a no-org publish wrote `organization_id: null` and the org-scoped read of that same package then returned `[]`. The blast radius is wider than the audit/observability one the card projected, and that is a finding rather than a detail: `rollbackToPackageCommit` derives the set of commits it must undo FROM THIS LIST. A commit the list could not see was silently never reverted — 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 that reports success and rolls back nothing is a correctness defect, not a reporting one. An org-scoped read now matches its own organization OR env-wide — the `$or` shape this package already uses for #3115, the shape #7705 applied to the sibling `deletePackage` read, and the shape the SQL driver's own tenant wall uses (#2734). Both directions that must not widen are pinned: another organization's commits stay invisible, another package's are never returned, and newest-first ordering is unchanged. The no-org branch is deliberately left package-wide rather than narrowed to `organization_id IS NULL` — narrowing would hide every org-scoped commit from that door instead, re-creating the bug pointed the other way, which is why #7705 left its own no-org branch alone. The whole shape, or none of it. The pin uses a real engine and a real driver and asserts the CONSEQUENCE. Both existing `deletePackage` suites stubbed `engine.find`, which is why neither could see the sibling defect; the question here is whether `organization_id = 'org'` matches a NULL column, which is a property of the driver's SQL and not of a stub's `filter()`. It seeds through the real publish path and reads back what landed in SQLite. Reverse-verified with the direction predicted first: restoring the strict equality was predicted to turn exactly the two positive cases red and leave all four others green, because strict equality is NARROWER than the `$or` — it cannot reach another org's rows or another package's, and does not touch the no-org branch. Measured on revert: exactly that, 2 failed / 4 passed, both failures `[1]` vs `[2]`. KNOWN REMAINING GAP, reported on #7779 rather than fixed here — this card holds `protocol.ts`, a serialized file, for `listCommits` alone. `revertCommit` and `rollbackToPackageCommit`'s own target lookups still carry the identical strict equality. The consequence is now loud rather than silent: the rollback above 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, so the new suite asserts it and the remainder cannot drift unnoticed. Ratchets unchanged: runtime's TEST_DEBT measured exactly 227, its recorded ceiling, with zero errors attributable to the new file; the query-options-erasure ratchet holds at 67 non-test sites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kp1rUCEFGp3eYRztsRx1B1
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
|
PM note — this PR is green and will not merge itself. Recording the mechanism so it is not re-derived each patrol. Measured at 16:58Z: The cause is not a stuck or missing check:
Arming auto-merge on this PR at 16:33Z was a no-op. GitHub only queues auto-merge for a PR that is not yet mergeable; on an already- ⛔ Not hand-merged. The fence on this loop is to diagnose before forcing, and the diagnosis says the blocker is not technical — it is that a green, unreviewed PR needs someone to press merge. That is the maintainer's call, so it is surfaced rather than taken. What is waiting behind it: #7819 (the four remaining strict Generated by Claude Code |
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31514486823 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31515157229 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31516021714 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31516762284 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31517423442 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
…ommit (#7819 tier 1) `revertCommit` and `rollbackToPackageCommit`'s target lookup each resolved their target commit with a strict `organization_id` equality, which matches no row whose column is NULL. An org-scoped caller therefore got COMMIT_NOT_FOUND (404) for any commit recorded env-wide — a row that demonstrably exists and that the same caller's `listCommits` hands back. Both lookups now accept org-scoped or env-wide rows, the same `$or` `deletePackage` (#7705) and `listCommits` (#7779) already carry. The `$or` was chosen over the two alternatives rather than copied. `where` is keyed on `id`, so the predicate reads like an authorization filter on a unique key; measured against the only door it is not one. Authorization is `requireManageMetadata`, checked before the call, and the `organizationId` that arrives is the session's active org selection from `resolveActiveOrganizationId` — a resolver whose body is entirely catch-wrapped and whose `undefined` omits the predicate, i.e. the widest reading. A boundary that fails open is not a boundary, which rules out "keep the check but distinguish 'not yours' from 'no such commit'". Dropping the predicate outright would newly let an org caller revert another organization's commit by id, a widening this card never asked for. The body already agreed with the `$or`: #7559 made each item resolve its scope from the row, and since #7814 `rollbackToPackageCommit` plans from `listCommits` (org + env-wide) and fed each id back into a lookup that refused half of them. The no-org branch is deliberately left un-narrowed, exactly as #7705 and #7779 left theirs. Pinned by a new real-engine/real-driver suite in packages/runtime (eight cases: the premise out of SQLite, the positive per site, both negative directions, and the no-org door per site; refusals asserted on code AND status per ADR-0112). The #7814 handoff assertion that pinned this as known-incomplete now asserts the rollback succeeds. Reverse verification, direction predicted first: 3 failed | 11 passed, exactly the three positive cases. Tier 1 only — `duplicatePackage` and `reassignOrphanedMetadata` are untouched and #7819 stays open to carry them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019hxiiv8qFCUmDThHU1k7HV
3 participants
Fixes #7779
protocol.listCommitsselected the ADR-0067 timeline with the strictorganization_idequality that #7705 (PR #7771) had just replaced one function above it:organization_id = '<org>'matches no row whose column is NULL, so a session with an active organization was shown none of the commits recorded env-wide. The commits were insys_metadata_committhe whole time; the read could not see them.Step one, as the card required: live, not latent — and measured before the fix was written
recordPackageCommitstoresorganization_id: request.organizationId ?? null. The only door into a publish is the dispatcher'sPOST /packages/:id/publish-drafts(packages/runtime/src/domains/packages.ts), which forwards an organization only whenresolveActiveOrganizationIdyields one. That resolver (packages/runtime/src/http-dispatcher.ts:1668) answersundefinedboth for a session with no active organization and for any throw on the auth seam, because its whole body iscatch-wrapped.So a publish made before an organization is selected — or during a transient auth blip — records its commit env-wide permanently, since the timeline is append-only.
Driven on a real engine over a real SQLite driver, a no-org publish wrote
organization_id: null, and the org-scoped read of that same package then returned[].The blast radius is wider than the card projected — a finding, not a detail
The card framed this as observability/audit in contrast with #7705's data-lifecycle defect. Measurement widened it:
rollbackToPackageCommitderives the set of commits it must undo from this list (protocol.ts:12463). A commit the list could not see was a commit the rollback silently skipped.Measured pre-fix, an org-scoped rollback past an env-wide commit answered
{success: true, revertedCommits: []}while that commit's changes stayed live. A rollback that reports success and rolls back nothing is a correctness defect, not a reporting one.The fix
An org-scoped read now matches its own organization or env-wide — the
$or [{organization_id: oid}, {organization_id: null}]shape this package already uses for the #3115 orphaned-draft fix (packages/metadata-protocol/src/sys-metadata-repository.ts:896-899), the shape #7705 applied to the siblingdeletePackageread, and the shape the SQL driver's own implicit tenant wall uses (field = :tenant OR field IS NULL, #2734).The whole shape, or none of it. The no-org branch is deliberately not narrowed to
organization_id IS NULL. Narrowing it would hide every org-scoped commit from that door instead — the same bug pointed the other way — which is exactly why #7705 left its own no-org branch alone. A dedicated case pins it.The pin asserts the consequence
packages/runtime/src/package-list-commits-org-scope.integration.test.ts— realObjectQL+ realdriver-sqlover on-disk SQLite, seeded through the real publish path, inpackages/runtimebecause that is the only package withmetadata-protocol+objectql+ a realdriver-sql(metadata-protocolcannot importobjectql— dependency cycle).Both existing
deletePackagesuites stubbedengine.find, which is 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, which is a property of the driver's SQL and not of a stub'sfilter().Six cases: the premise (a no-org publish really does write
organization_id: null, read straight back out of SQLite); the positive (env-wide commits returned to an org-scoped caller, ordering preserved); both negative directions (another organization's commits and another package's commits stay out); the no-org door (still package-wide); and the rollback planner's view of the timeline.Reverse verification — direction predicted before the revert was run
Restoring the strict equality was predicted to turn exactly two cases red — the env-wide commit vanishing from the org-scoped list, and the rollback-planner case failing at its list assertion before reaching the rollback lines — and to leave the other four green, because strict equality is narrower than the
$or: it cannot reach another organization's rows or another package's, and it does not touch the no-org branch at all.Measured on revert: exactly that.
2 failed | 4 passed, both failures[1]vs[2]. The prediction held.revertCommit(protocol.ts:12180) androllbackToPackageCommit's own target lookup (:12455) still carry the byte-identical strict equality. This card holdsprotocol.ts— a serialized file — forlistCommitsalone, so per #7705's precedent this is reported rather than grown into the diff.The consequence is now loud instead of silent: the rollback above reports
success: falsenaming the commit it could not resolve, rather than claiming success over a no-op. That is strictly better and non-destructive, but it is not the whole repair — so the new suite asserts it, and the remainder cannot drift unnoticed before its own card lands.Gates
metadata-protocoltestsobjectqltestsruntimetestscheck:type-check-debtruntimemeasured exactly 227, its ceiling, with 0 errors from the new filecheck:query-options-erasureeslinton changed filesNo
content/docs/releases/edits. Nodocs/adr/**needed. A.changeset/*.mdis included.mainmerged before opening (098b62914).🤖 Generated with Claude Code
https://claude.ai/code/session_01Kp1rUCEFGp3eYRztsRx1B1
Generated by Claude Code