Repository navigation
revertCommit attributes its revert commit to the request's organization even when the commit it reverted was env-wide — newly reachable as of #7819 tier 1 #7860
Description
Activity
Triage: routed
domain:metadata(packages/metadata-protocol/src/protocol.ts, same lane as the #7705/#7779/#7819 family); grade held asfinding, honoring the filer's explicit "not filed as a defect" framing. The card's step one is the gate between its two futures: measure what a different org'slistCommitsand a no-org caller see after an org-scoped revert of an env-wide commit. If the env-wide publish shows no visible compensation to other readers, this re-grades to a queue card as a concrete reporting defect; if every reader sees a coherent timeline, it collapses to an intent/documentation note (and the #7559 row-derived-vs-request-derived asymmetry question can ride the next protocol.ts card in this family rather than standing alone).Restart conditions: (a) that measurement being run (cheap to attach to any dispatch in the same file family — the working harness is PR #7857's
package-revert-commit-org-scope.integration.test.ts, mind the dist-vs-src rebuild trap the card records); (b) any real-world report of a revert invisible to env-wide readers.
Generated by Claude Code
Finding-grading round: hold confirmed, trigger sharpened: the card's own step one (what a different-org / no-org caller sees on
recordPackageCommit's request-derivedorgId) has still not been run — and the harness for it now exists (PR #7857'spackage-revert-commit-org-scope.integration.test.ts; mind the dist-vs-src rebuild). The asymmetry is live onorigin/main(protocol.ts:13050request-derived vs:13045per-itemitemOrgIdunder the #7559 comment). Promote or close on that measurement's outcome; a #8138-shaped measurement dispatch is the cheap way to fire it.
Generated by Claude Code
Findings-triage round — PROMOTED to
pm:queue, premise-gated.Why it is promoted
Concrete, located, and invariant-restoring: a revert commit should be attributed to what it reverted, not to who asked for it. That is a correctness fix with no product decision in it, so it is PM discretion rather than the maintainer's.
⚠️ The premise gate — verify this FIRST, before writing any codeGrading-time re-check on
origin/mainturned up something that may already have overtaken this card, and it must be settled before implementation rather than discovered halfway:protocol.tscarries a comment at ~:12600reading "…FROM, not the request's — the same rule #7559 gaverevertCommit". That is a claim thatrevertCommitalready attributes to the source rather than the requester. This card says the opposite.⇒ Three possibilities, and the dev's first job is to determine which:
- Commit-revert fails with
VERSION_NOT_FOUND: No history row at version 2while/historyshows exactly that row — the revert's lookup key disagrees with the history writer (and the package-level revert route 500s) #7559's rule covers this and the card is stale ⇒premise_still_valid: falsewith no PR is a legitimate and valuable delivery. Pin the behaviour if it is unpinned and say so. - Commit-revert fails with
VERSION_NOT_FOUND: No history row at version 2while/historyshows exactly that row — the revert's lookup key disagrees with the history writer (and the package-level revert route 500s) #7559's rule exists but does not reach the env-wide case — the card's specific claim is that a commit whoseorganization_idis NULL gets stamped with the request's org. A rule about "the org it came from" can be correct in general and still mishandle NULL. ⇒ implement, and pin the NULL case specifically. - The comment describes a neighbouring method rather than
revertCommititself (it sits near acopyOrgIdlimb). ⇒ implement as filed.
⛔ Do not assume 2 because it is the one that leaves work to do. Re-locate
revertCommitby function name — every line number in this card and in this comment is stale by construction,protocol.tshas moved under many merges.Serialisation
Touches
protocol.ts, this lane's hot file. At grading time #8332 (#7654, read merge) and #8334 (#8136, error/catch paths) are both in CI against it. ⛔ Do not dispatch this until both have landed or the region-exemption cap is respected — the revert-attribution limb is close enough to neither to be obviously disjoint, and this lane has paid for "green and semantically wrong" once already.Also still true from the card: it became reachable only as of #7819 tier 1, so it is a live path, not a theoretical one.
Generated by Claude Code
- Commit-revert fails with
Claim — session
session_012WMpuAfA2KSdDjGF6tm1bH, branchclaude/issue-7860-revert-commit-org-attribution.Dispatched as a measurement-first card: step one (what a different-org and a no-org direct-mount caller see after an org-scoped revert of an env-wide commit) is run before any decision about a fix, and the
protocol.tspremise gate from the promotion comment is verified by content, not by line number.If an earlier claim from a different session exists, this one yields — say so and I will stand down.
Generated by Claude Code
Generated by Claude Code
{ "issue": 7860, "status": "done", "branch": "claude/issue-7860-revert-commit-org-attribution", "pr": "https://github.com/objectstack-ai/objectstack/pull/8427", "premise_still_valid": true, "summary": "Step one was run first on a real ObjectQL + SqlDriver, and it landed on fork branch ONE: after org_active reverts an env-wide commit, a DIFFERENT organization's listCommits shows ['apply'] alone — the env-wide publish with no visible compensation — while the no-org direct-mount door and the actor both see ['revert','apply']. Not cosmetic: sys_metadata held NO row for the reverted view, so the artifact was withdrawn env-wide (items revert in the ROW's scope per #7559) while the record documenting it stayed private, and #7814 made rollbackToPackageCommit PLAN from that list. Ruling 2 therefore made this a concrete reporting defect and I fixed it rather than choosing intent: recordPackageCommit's orgId now reads row.organization_id ?? null (packageId on the same call was already row-derived). The mirror direction fell out of the same line — a no-org revert of an org-scoped commit used to leave every other org reading a dangling 'Revert: ...' whose parent it cannot see; measured ['revert'] before, [] after. PREMISE-GATE ALARM IS FALSE: the protocol.ts comment claiming '...the same rule #7559 gave revertCommit' is at :13087 inside duplicatePackage (the #7819 tier 2 copy limb, right above copyOrgId), and its cross-reference is to revertCommit's PER-ITEM scope rule, which really does exist — it says nothing about the commit RECORD, which is what this card is about. Promotion-comment possibility 3. CONFLICT TO SURFACE: dispatch ruling 1 says this is 'NOT filed as a defect and you must not treat it as one', while the 2026-08-13 06:02 triage comment PROMOTED it to pm:queue saying 'implement'. I did not pick a side — the measurement did, and both paths agreed on measuring first.", "tests": "NEW packages/runtime/src/package-revert-commit-attribution-org-scope.integration.test.ts (real engine, real driver, seeded through the real publish path). Post-fix: 4/4 green; with the #7819 and #7814 sibling suites, 18/18 across 3 files. @objectstack/metadata-protocol full suite 1170 passed (80 files). ABLATION with a rebuild between measurements (these suites resolve metadata-protocol through dist): direction predicted BEFORE running — the two defect-direction cases plus the raw-row case red, the actor/no-org case green. Measured exactly that: 'Tests 3 failed | 1 passed (4)', with 'AssertionError: expected [ 'apply' ] to deeply equal [ 'revert', 'apply' ]'. The survivor was the predicted one. Fix restored byte-identical (empty git diff HEAD), rebuilt, re-run green. Every prediction in this card matched actual — nothing was retro-fitted. Gates derived from actual changed paths via scripts/pm/dispatch-gates.mjs (the prompt named none): check:nul-bytes, check:changeset-gate-self-tests, check:cross-package-test-inputs, check:durability-log-level, check:filter-alias-parity, check:objectui-changeset, check:query-options-erasure, check:type-check-coverage, check-changeset-no-major all PASS. check:objectui-pin-fresh is RED and is red identically on an unmodified checkout — pre-existing release chore, not this diff.", "open_questions": [ { "question": "Dispatch ruling 1 ('NOT filed as a defect, must not treat it as one') contradicts the 2026-08-13 06:02 triage comment, which promoted the card to pm:queue with 'implement'. I followed the measurement rather than either instruction, and it selected the fix path on its own.", "options": [ "A. Accept: the fork's first branch was met on measured evidence, so the fix is what both instructions converge on", "B. Treat the PR as premature because ruling 1 forbade the defect framing, and re-triage from the measurement alone" ], "recommendation": "A — ruling 2 made the fork binding and self-executing: a different org seeing the env-wide publish with no compensation was measured, which is exactly the condition under which the card says it 'stops being a design question'. Nothing about the #7559 asymmetry was decided by me; the row-derived choice is forced by the measurement, not preferred on taste." }, { "question": "#8333 is still OPEN and in flight against this same function (sibling worktree active, PM assumption said it was 'live in revertCommit right now' and to merge it first — it has not landed on main).", "options": [ "A. Land whichever is ready; the hunks do not overlap", "B. Serialise deliberately and rebase the second one" ], "recommendation": "A, with a look from whoever lands second. I verified by reading its actual diff: its revertCommit hunk ends at the catch block's closing brace, mine is the orgId argument ~3 lines below — different lines, so I did not stop, but they are adjacent enough that git's 3-line context could still conflict textually." } ], "out_of_scope_findings": [] }
Generated by Claude Code
Generated by Claude Code
Landed —
a2c82a87onmainConfirmed by both readings: the merge event and the commit on
origin/main(13:02:41Z), never theauto_mergefield. PR #8427, closed automatically by itsFixestrailer;pm:dispatcheddropped.What shipped: one functional line.
recordPackageCommit'sorgIdnow readsrow.organization_id ?? null.packageIdon that same call was already read off the reverted row; the org was the one field still taken from whoever asked. The other thirty-one lines of theprotocol.tsdiff are the comment stating the invariant and the measured numbers — the right ratio for a change whose entire difficulty is knowing which scope is correct.Nobody chose the intent — the measurement did. This card was explicitly not filed as a defect, and asked for a measurement before any choice between request-derived and row-derived attribution. Run on a real
ObjectQL+SqlDriverafter an org-scoped revert of an env-wide commit:reader before after the actor [revert, apply][revert, apply]a different organization [apply][revert, apply]no-org (direct-mount REST) [revert, apply][revert, apply]The middle row is the condition this card itself named as the point where it "stops being a design question". So the #7559 asymmetry — row-derived scope for items, request-derived for the commit record — was never adjudicated on taste.
The load-bearing assertion is
expect(meta).toEqual([]).sys_metadataheld no row for the reverted view afterwards, because items revert in the row's scope. The artifact really was withdrawn env-wide while the record documenting it stayed private — another organization was shown anapplythat had already been undone beneath it. And since #7814,rollbackToPackageCommitplans fromlistCommits, so this was planning input rather than an observability nicety. Without that assertion the card reads as cosmetic; with it, it is a correctness defect.The invariant, worth keeping in the lane's vocabulary: a revert commit is visible to exactly the readers who can see the commit it reverts. The mirror direction fell out of the same line — a no-org caller reverting an org-scoped commit used to leave every other organization reading a dangling
Revert: …whoseparentCommitIdnames a commit that door cannot see. Measured[revert]before,[]after.Reachability was created by #7819 tier 1: before it the lookup answered
COMMIT_NOT_FOUNDfor an env-wide row, so this line could not be reached with a mismatched scope at all. A dormant quirk whose reachability arrives with a fix in the same function is exactly the thing that gets absorbed silently into the PR that enabled it — this one was reported instead, which is why it exists as a card.Two corrections on the record, both mine
My dispatch brief contradicted triage. I opened it with "⛔ this is NOT filed as a defect", taken from this card's body — but the triage comment of 06:02 had already promoted it to
pm:queuewith "implement". The dev received two conflicting instructions, did not pick a side, ran the measurement both agreed on, and let the result select. That is the correct resolution of a contradictory brief. The lesson is one I had already been burned by an hour earlier on #8323: a card's body is not its state.The premise-gate alarm was false — the second today. My notes said a
protocol.tscomment claimed the opposite of this card. It does not: the comment sits insideduplicatePackageand cross-referencesrevertCommit's per-item scope rule, which genuinely exists, saying nothing about the commit record. Structurally identical to the false alarm on #7748 this morning. A comment that cross-references a function is not a claim about every property of that function — verify by content and enclosing scope, never by proximity.
Generated by Claude Code
- added a commit that references this issue
on Aug 17, 2026
Filed by the
domain:metadataPM seat from #7819 tier 1's dev report (PR #7857), which located this while holdingpackages/metadata-protocol/src/protocol.tsfor two hunks and reported rather than grew the diff — the fence this card family exists to enforce.No
domain:*applied — routing is the triage seat's territory. For the record the surface ispackages/metadata-protocol/src/protocol.ts, same package and lane as #7705 / #7779 / #7819.The observation
revertCommitrecords the compensating commit withrecordPackageCommit({ orgId: request.organizationId ?? null, … })— i.e. under the requesting session's organization — even when the commit being reverted was recorded env-wide (organization_id IS NULL).So an org-scoped caller reverting an env-wide publish produces an org-scoped revert commit. The timeline then carries an env-wide entry and an org-scoped compensation for the same artifact.
Why it is filed now rather than earlier
It is pre-existing and unchanged by PR #7857 — but that PR promotes it from unreachable to reachable. Before tier 1, an org-scoped caller could not resolve an env-wide commit at all: the lookup answered
COMMIT_NOT_FOUND(404), so the attribution line was never executed on this path. Tier 1 makes exactly that operation succeed.That is the whole reason this deserves a card rather than a shrug: a dormant quirk whose reachability is created by a fix landing in the same function is precisely the kind of thing that gets absorbed silently into the PR that enabled it.
What is NOT claimed
The dev was explicit that the current behaviour is at least self-consistent, and this card inherits that honesty rather than overriding it: with tier 1's
$orin place,listCommitsshows the resulting org-scoped revert commit to the same caller who created it. Nothing is currently known to break.⛔ So this is not filed as a defect. It is filed because the intended attribution is unreadable from the code, and the two candidate answers have different consequences:
VERSION_NOT_FOUND: No history row at version 2while/historyshows exactly that row — the revert's lookup key disagrees with the history writer (and the package-level revert route 500s) #7559 already does inside the batch path, whereresolveMetaItemOrgScoperesolves each item's scope from the row rather than the request, on the rationale that "a batch legitimately mixes an env-wide artifact with an org overlay".The tension is that #7559 established row-derived scope for the items, while the commit record itself stays request-derived. Whether that asymmetry is intended is the question.
Step one for whoever takes it
Establish the consequence before choosing. Specifically: after an org-scoped revert of an env-wide commit, what does a different organization's
listCommitsshow, and what does a no-org (direct-mount REST) caller see? If another org can still see the original env-wide publish with no visible compensation, that is a concrete reporting defect and this stops being a design question. If every reader sees a coherent timeline, it is a documentation-and-intent question and should be closed as such.engine.findcannot see any of these NULL-semantics behaviours. PR #7857'spackage-revert-commit-org-scope.integration.test.tsis the working harness;metadata-protocolthrough itsdist, and stack traces are source-mapped back tosrc, so rebuild between measurements or you measure nothing.Provenance
#7819 tier 1 · PR #7857 (
58bef026), "Scope discipline" section · #7559 (row-derived item scope) · #7814 / #7779 (thelistCommitshalf that made the rollback planner see env-wide commits in the first place).