fix(diff): make POST /diffs functional with repo-addressed IRIs#270
Open
blake-regalia wants to merge 2 commits into
Open
fix(diff): make POST /diffs functional with repo-addressed IRIs#270blake-regalia wants to merge 2 commits into
blake-regalia wants to merge 2 commits into
Conversation
Diff creation was broken end-to-end by several compounding defects:
- the route never set diffId, so every reference to the diff resolved to
the never-URN placeholder
- the request body was filtered against the lock prefix ("morl") instead
of the diff prefix
- the base conditions required a commit in the request path (there is
none on the diffs container), failing every request with 404
- genDiffUpdate ignored its diffTriples and conditions parameters
entirely: user-supplied metadata was silently dropped, no conditions
(permit, ref validation, uniqueness) were enforced at update time, and
?__mms_policy stayed unbound so the transaction lacked
mms:appliedPolicy — which made the validation construct unmatchable
and every request fail even after a successful write
- cleanup deleted mt: while the transaction was created as mt:diff,
leaking a transaction record per request
Diffs are now minted at {repo}/diffs/{id} (Slug header or generated),
consistent with every other resource; the commit-pair scheme's sha256 is
retained as the mms:id and etag. The internal commit pipeline continues
to mint its own hash-derived IRIs. Responds 201 with a Location header.
Adds end-to-end tests: diff between a lock and a branch (including ins
graph content), extra user metadata (exercising serializePairs),
generated ids, and transaction cleanup.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
genDiffUpdate unconditionally inserts the mms:Diff metadata and an AutoDiffOwner policy before the delta is known (its WHERE ends in a guaranteed-solution union), but the empty-delta path of diffAndFinalizeCommit returns before the commit is created — so every no-op update (e.g. an idempotent client re-submitting the same change) permanently accreted a dangling mms:Diff referencing a commit that does not exist, plus an orphaned policy. Delete the diff metadata and the transaction's created policy in the empty-delta path before responding. Adds a test asserting that after a no-op update, no mms:Diff references a nonexistent commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
POST /orgs/{org}/repos/{repo}/diffshas never worked. Compounding defects: the route never setdiffId(everymord:reference resolved to the never-URN), the body was filtered against the lock prefix, the base conditions required a commit in the request path (404 on every request),genDiffUpdateignored both itsdiffTriplesandconditionsparameters (user metadata dropped; no permit/uniqueness/ref validation at update time;mms:appliedPolicynever written, making the validation construct unmatchable so even successful writes returned errors), and cleanup deletedmt:while the transaction was created asmt:diff. There were zero diff tests.Separately, the internal commit pipeline's empty-delta path (an idempotent client re-submitting the same update) permanently accreted a dangling
mms:Diffreferencing a never-created commit plus an orphanedAutoDiffOwnerpolicy, on every no-op update.Fix
c97ee0e— diffs are minted at repo-addressed IRIs{repo}/diffs/{id}(Slug header or generated), consistent with every other resource; the commit-pair sha256 is retained asmms:idand etag. The internal pipeline keeps its hash-derived IRIs.genDiffUpdatenow actually embeds its conditions and user metadata. Responds 201 + Location.e393c26— the empty-delta path deletes the diff metadata and the transaction's created policy before responding.Tests
New
DiffCreateTest(5 tests): diff between a lock and a branch including ins-graph content verification, extra user metadata, generated ids, transaction cleanup, and the no-op-update leak invariant (nomms:Diffmay reference a nonexistent commit). Full suite green (399/399).Dependencies
Stacked on the review-findings branch (this PR's base): diff creation with user metadata requires the
serializePairsseparator fix from that branch. Retarget todevelopafter it merges.Out of scope (needs design)
/diff/{diffId}/queryremains non-functional: it resolves refs viamms:commit, which diffs don't carry, and the intended query semantics (ins/del graph union? named graphs?) are undefined. Left untouched pending a decision.🤖 Generated with Claude Code