specs/012-patch-branch-refactor/spec.md
Ref: Size: 10.8 KiB
# Feature Specification: Patch-as-Branch Refactor
**Feature Branch**: `012-patch-branch-refactor`
**Created**: 2026-03-21
**Status**: Draft
**Input**: User description: "Refactor patches to be branches with a review layer. Currently patches store a head commit OID and base ref name in the DAG, duplicating what git branches already track. Instead, a patch should BE a branch — the patch DAG becomes purely the review layer on top of normal git branches."
## User Scenarios & Testing *(mandatory)*
### User Story 1 - Create a Patch from a Branch (Priority: P1)
A developer has been working on a feature branch. They want to submit it for review. They run a create command that associates their existing branch with a review conversation. The system records the branch name and target base, and the review DAG is created. The branch itself is untouched — it's still a normal git branch they can push, rebase, and update as usual.
**Why this priority**: This is the foundational change — without branch-based patch creation, nothing else works.
**Independent Test**: Create a branch with commits, run patch create referencing that branch, verify the patch is listed and the branch is unmodified.
**Acceptance Scenarios**:
1. **Given** a developer has a feature branch with commits ahead of main, **When** they create a patch referencing that branch, **Then** a review conversation is created and the patch appears in the patch list showing the branch name and base.
2. **Given** a developer creates a patch from a branch, **When** they view the patch, **Then** it shows the branch name, base ref, number of commits ahead, and whether it can be fast-forward merged.
3. **Given** a developer has no local branch (e.g., they have a detached HEAD or a commit OID), **When** they create a patch, **Then** the system creates a branch for them automatically and associates it with the review.
4. **Given** a patch already exists for a branch, **When** the developer tries to create another patch for the same branch, **Then** the system rejects it with an error (one active patch per branch).
---
### User Story 2 - View Patch Staleness and Diff via Git (Priority: P2)
A reviewer wants to see the current state of a patch. The system uses git's own tools to show the diff between the branch and its base, detect how many commits the base has moved ahead, and whether the patch would merge cleanly. No custom diff logic — just git.
**Why this priority**: Reviewers need accurate, up-to-date information to make decisions. Leveraging git ensures this is always correct.
**Independent Test**: Create a patch, advance the base branch, then run patch show and verify it reports the patch as outdated with the correct commit count.
**Acceptance Scenarios**:
1. **Given** a patch exists and the base branch has not moved, **When** the reviewer views the patch, **Then** it shows the patch is up-to-date.
2. **Given** a patch exists and the base branch has moved 3 commits ahead, **When** the reviewer views the patch, **Then** it shows "outdated: base is 3 commits ahead" or similar.
3. **Given** a patch exists, **When** the reviewer requests the diff, **Then** the system shows the diff between the branch and the merge-base with the target branch (the same as `git diff base...branch`).
4. **Given** a patch exists and the branch has been rebased or force-pushed, **When** the reviewer views the patch, **Then** the diff reflects the current state of the branch, not a stale snapshot.
---
### User Story 3 - Merge a Patch as a Normal Git Merge (Priority: P3)
A maintainer approves a patch and wants to merge it. The system performs a standard git merge of the patch branch into the base branch — the same operation as `git merge <branch>`. After merging, the patch is marked as merged in the review DAG.
**Why this priority**: Merge is the final step in the review lifecycle, and using git's native merge ensures compatibility with all existing git workflows.
**Independent Test**: Create a patch, approve it, merge it, verify the branch is merged into the base and the patch status is updated.
**Acceptance Scenarios**:
1. **Given** a patch is approved, **When** the maintainer merges it, **Then** the patch branch is merged into the base branch using a standard git merge.
2. **Given** a patch merge would result in conflicts, **When** the maintainer attempts to merge, **Then** the system reports the conflicts and does not complete the merge.
3. **Given** a patch is merged, **When** anyone views the patch, **Then** it shows status "merged" and the merge commit.
4. **Given** a patch is merged, **When** listing patches, **Then** it no longer appears in the default (open) list but is visible with the `--all` flag.
---
### User Story 4 - Revise a Patch by Updating the Branch (Priority: P4)
A developer receives review feedback and pushes new commits to their branch (or rebases it). The system detects the branch has changed and the review conversation continues against the updated code. No explicit "revise" command is needed — the branch IS the patch, so updating the branch updates the patch.
**Why this priority**: This simplifies the workflow significantly — developers just use normal git operations (commit, push, rebase) and the review stays attached.
**Independent Test**: Create a patch, push a new commit to the branch, verify patch show reflects the new commit without any explicit revise command.
**Acceptance Scenarios**:
1. **Given** a patch exists for a branch, **When** the developer pushes new commits to that branch, **Then** the patch diff and status automatically reflect the updated branch state.
2. **Given** a patch exists and the developer rebases the branch, **When** the reviewer views the patch, **Then** the diff shows the rebased state.
3. **Given** a patch has existing reviews, **When** the branch is updated, **Then** the existing review comments remain attached to the patch conversation.
---
### User Story 5 - Backward Compatibility with Existing Patches (Priority: P5)
Existing patches that were created with the old model (storing head_commit OID) should continue to work. The system should be able to read and display old-format patches while all new patches use the branch-based model.
**Why this priority**: Migration safety — existing data must not be broken.
**Independent Test**: Load a repository with old-format patches, verify they still appear in patch list and can be viewed/commented on.
**Acceptance Scenarios**:
1. **Given** a repository has patches created with the old format (head_commit OID), **When** listing patches, **Then** old patches appear alongside new branch-based patches.
2. **Given** an old-format patch exists, **When** viewing it, **Then** the system shows the stored head commit and diff as before.
3. **Given** an old-format patch exists, **When** attempting to merge it, **Then** the system uses the stored head commit for the merge (old behavior).
---
### Edge Cases
- What happens when the branch referenced by a patch is deleted? The patch shows an error status ("branch not found") but the review conversation is preserved.
- What happens when two patches target the same base branch? Both are allowed — they are independent branches with independent reviews.
- What happens when a patch branch is merged outside of git-collab (e.g., via `git merge` directly)? The patch status should ideally detect this, but at minimum the review conversation is preserved.
- What happens when the user runs `patch create` without specifying a branch? The system uses the current branch (HEAD) if it's not the base branch, or errors if HEAD is the base branch.
- What happens with the existing `patch revise` command? It becomes optional/deprecated — updating the branch is the primary way to revise. The command can remain for explicitly recording a revision event in the DAG.
- What happens with `patch import`? Imported patches create a branch from the imported commits, then attach a review DAG to that branch.
## Requirements *(mandatory)*
### Functional Requirements
- **FR-001**: System MUST associate patches with git branches rather than storing commit OIDs in the review DAG.
- **FR-002**: System MUST use git's native merge-base and rev-list to determine patch staleness relative to the base branch.
- **FR-003**: System MUST compute patch diffs using git's diff between the branch and the merge-base with the target (three-dot diff).
- **FR-004**: System MUST perform patch merges as standard git merges of the branch into the base.
- **FR-005**: System MUST preserve the review conversation (comments, reviews, inline comments) independently of branch state changes.
- **FR-006**: System MUST support reading and displaying patches created with the old format (head_commit OID).
- **FR-007**: System MUST automatically reflect branch updates (new commits, rebases) in patch status and diff without requiring an explicit revise command.
- **FR-008**: System MUST prevent creating multiple active patches for the same branch.
- **FR-009**: System MUST detect and report when a patch branch has been deleted.
- **FR-010**: System MUST default to the current branch when no branch is specified during patch creation.
- **FR-011**: System MUST create a branch automatically when a patch is created from a detached HEAD or commit OID.
### Key Entities
- **Patch**: A review conversation attached to a git branch. Contains a branch name, target base ref, and a DAG of review events. The branch itself holds the code changes.
- **Review Event**: An entry in the review DAG — comments, reviews (with verdicts), inline comments. These are independent of the branch state and persist across rebases and force-pushes.
## Success Criteria *(mandatory)*
### Measurable Outcomes
- **SC-001**: All existing patch workflows (create, list, show, comment, review, merge, close) continue to function with the new branch-based model.
- **SC-002**: Patch diff always reflects the current state of the branch, not a stale snapshot.
- **SC-003**: Patch staleness is accurately reported using git's own rev-list, showing exact commit counts.
- **SC-004**: Existing old-format patches remain readable and functional after the refactor.
- **SC-005**: Developers can update a patch by simply pushing to the branch — no explicit revise step required.
- **SC-006**: Patch merge uses standard git merge, producing the same result as `git merge <branch>`.
## Assumptions
- The branch name is stored in the PatchCreate event in the review DAG, replacing (or supplementing) the head_commit field.
- The review DAG (collab refs) remains the same structure — only the PatchCreate and PatchRevise event payloads change.
- Sync continues to work — branch refs are synced via normal git push/fetch, and collab refs are synced as before.
- The `patch revise` command is not removed but becomes optional — it can be used to explicitly record a revision event with a message.
- The `patch import` command creates a branch from imported commits before attaching the review DAG.