feat(relayfile): fork rebase primitive (recover from parent_moved)#372
Conversation
Store-layer half of Phase 2 (gap #3 in docs/multi-agent-collaboration-assessment.md). HTTP handler and SDK surface land in a follow-up commit. A fork that hits parent_moved today has no recovery except discard and redo from scratch, even when only one of its touched paths actually collided. RebaseFork re-pins the fork against current live state instead: - Paths whose BaseRevision already matches live are silently re-pinned (a no-op in practice, but upgrades a legacy pre-Phase-1 entry to a modern per-path one once proven unchanged). - Paths that genuinely diverged are reported as RebaseConflict{Path, ForkType, LiveExists, LiveRevision, LiveContentPreview} — NOT auto-mutated. A subsequent commit still correctly fails for exactly those paths. - A legacy overlay entry (BaseRevision == "", persisted by a pre-Phase-1 binary) is always reported as a conflict, since there's no reliable per-path base to compare it against. Resolution mechanism: WriteForkFile and DeleteForkFile now accept an IfMatch equal to the LIVE PARENT's current revision (not just the fork's own overlay revision) for a path already touched in the fork — this is how a caller explicitly resolves a reported conflict, citing the LiveRevision RebaseFork returned. Accepting it resets that path's BaseRevision to the cited value, via a new resolvedBaseRevision parameter on writeForkOverlayLocked (nil for normal edits, which keep the existing pin-at-first-touch behavior). Known gap, not solved by this commit: resolving a conflict on a path the fork itself DELETED (not wrote) by writing new content isn't supported the same way, since the merged view reports it as not-existing and the live revision isn't reachable through the normal IfMatch check for a create. Deleting again (citing the live revision) or discarding the whole fork remain the paths for that specific case. Tests: TestRebaseForkNoConflicts, TestRebaseForkReportsGenuineConflictWithoutMutating, TestRebaseForkResolveConflictByRewritingWithLiveRevision, TestRebaseForkResolveConflictByDeletingWithLiveRevision, TestRebaseForkLegacyEntryAlwaysConflicts.
HTTP + SDK half of Phase 2 (gap #3 in docs/multi-agent-collaboration-assessment.md), built on the store-layer RebaseFork primitive. - POST /v1/workspaces/{workspaceId}/forks/{forkId}/rebase, handleRebaseFork following the same error-handling conventions as handleCommitFork/ handleDiscardFork (fork_expired, not_found, bad_request, internal_error). - openapi/relayfile-v1.openapi.yaml: RebaseResponse and RebaseConflict schemas, matching the Go JSON tags and the truncatePreview 4000-char cap. - TypeScript SDK: rebaseFork() method, RebaseForkInput/RebaseForkResponse/ RebaseConflict types, exported from index.ts. - Fixes a real pre-existing gap while touching this code: throwForError had no branch for parent_moved (fell through to a generic thrown error) despite every other fork-relevant error code having one. Adds a proper ParentMovedError class carrying currentRevision, now directly relevant since rebaseFork() is the recovery path for it. New tests: TestForkRebaseClean, TestForkRebaseReportsGenuineConflict, TestForkRebaseResolveThenCommit (HTTP-level, exercising the full rebase -> resolve-by-rewrite-citing-live-revision -> commit flow), plus SDK-level rebaseFork and ParentMovedError tests in client.test.ts. Co-authored-by: Codex <noreply@openai.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
Phase 2 (RebaseFork, PR #372) and Phase 3 (Store.MergeFile symbol-level merge, PR #373) both landed and merged since this doc was last updated. Renumbers the remaining gaps, updates the verdict/recommendation sections, and notes what's explicitly still out of scope (mount rollout, non-Go languages) per the user's Phase 3 scope decision.
1 participant
Summary
Phase 2 of the goal-B fix series (gap #3 in
docs/multi-agent-collaboration-assessment.md). Builds on #370/#371 (both merged).A fork that hits
parent_movedhas had no recovery except discard-and-redo from scratch, even when only one of its touched paths actually collided. This adds aRebaseForkprimitive that re-pins a fork against current live state instead of forcing a hard restart.What changed
Store layer:
Store.RebaseFork(workspaceID, forkID): paths whoseBaseRevisionalready matches live are silently re-pinned (a no-op in practice, but upgrades a legacy pre-Phase-1 entry to a modern per-path one once proven unchanged). Paths that genuinely diverged are reported asRebaseConflict{Path, ForkType, LiveExists, LiveRevision, LiveContentPreview}— not auto-mutated. A subsequent commit still correctly fails for exactly those paths.BaseRevision == "", persisted by a pre-Phase-1 binary) is always reported as a conflict — there's no reliable per-path base to compare it against.WriteForkFile/DeleteForkFilenow accept anIf-Matchequal to the live parent's current revision (not just the fork's own overlay revision) for an already-touched path — this is how a caller explicitly resolves a reported conflict, citing theliveRevisionRebaseForkreturned. Accepting it resets that path'sBaseRevisionto the cited value.HTTP + SDK:
POST /v1/workspaces/{workspaceId}/forks/{forkId}/rebase,handleRebaseForkfollowing the existing fork-handler error conventions.RebaseResponse/RebaseConflictschemas.rebaseFork()method, matching types, exported fromindex.ts.throwForErrornever had a branch forparent_moved(fell through to a generic error) despite every other fork-relevant error code having one — added a properParentMovedErrorclass carryingcurrentRevision, directly relevant now thatrebaseFork()is the recovery path for it.Known gap, not solved here: resolving a conflict on a path the fork itself deleted (not wrote) by writing new content isn't supported the same way — the merged view reports it as not-existing, so the live revision isn't reachable through the normal create
If-Matchcheck. Deleting again (citing the live revision) or discarding the whole fork remain the paths for that specific case.Multi-agent workflow
Split as planned: I (Claude) implemented and tested the store-layer primitive directly; Codex (running on
sf-mac-mini, viacodex exec) built the HTTP handler, OpenAPI spec, and TypeScript SDK surface on top of it, from a detailed spec referencing the store-layer commit. Same pattern as #371: Codex's sandbox couldn't push (no git write access, DNS blocked) or run tests needing loopback binding or a full npm install — I picked those up directly, reviewed every diff line-by-line, and ran full independent validation outside that sandbox.Test plan
TestForkRebaseClean,TestForkRebaseReportsGenuineConflict,TestForkRebaseResolveThenCommit— the last exercises the full rebase → resolve → commit flow through the real HTTP API)go build ./...,go vet ./..., fullgo test ./...— all green, run directly onsf-mac-minioutside Codex's sandbox (the suite Codex's sandbox couldn't complete due toEPERMon loopback binding)scripts/check-contract-surface.sh— passingnpm test(229/229 passing),npx tsc --noEmit— clean (after building the@relayfile/coreworkspace dependency Codex's constrained environment hadn't built)sf-mac-mini, fork created + written from this laptop, genuine colliding write against the same live path issued fromfinn-mac-mini,RebaseForkcalled from the laptop correctly reports the conflict with the right live revision and content preview, resolved by re-writing citing that revision, committed successfully, final content verified correct — all over the real Tailscale network across three distinct physical machines.🤖 Generated with multi-agent assistance (Claude implementing the store layer + reviewing/verifying, Codex implementing the HTTP/SDK layer) as part of the goal-B initiative.