docs/superpowers/plans/2026-09-06-mr04-mr-lifecycle.md

v1.6.1
gitbay-ios/docs/superpowers/plans/2026-09-06-mr04-mr-lifecycle.md rendered · source · history · blame · raw

409 lines · 20222 bytes

MR 4: Merge request lifecycle Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Close four parity rows at once — draft, ready, retarget, request a review, and stacked merge requests.

Architecture: All four are already on the wire and none needs a server change. mr show and mr list carry draft, review_requests, stacked_on and stacked as omitempty fields the iOS models simply do not decode yet. So this is decoding plus four write actions on a screen that already has a write path.

Tech Stack: Swift 6, default MainActor isolation, iOS 26.5, SwiftUI, Swift Testing, @Observable.

Spec: docs/superpowers/specs/2026-09-06-ios-parity-design.md

Global Constraints

  • Swift 6 language mode, default MainActor isolation. Wire models are nonisolated structs.
  • Swift Testing only — never XCTest.
  • gitbayTests is hermetic and offline; network goes through StubProtocol.
  • New files under gitbay/ and gitbayTests/ need no project.pbxproj edit.
  • The label model is IssueLabel, never Label.
  • Never mention Claude, LLMs or AI in commits, comments, or the merge request. No Co-Authored-By trailer.
  • Never commit to main.

The commands, verbatim from the registry:

mr draft    <owner/name> <n>
mr ready    <owner/name> <n>
mr retarget <owner/name> <n> <branch>
mr review request <owner/name> <n> [--add <user>]... [--remove <user>]...
mr create   <target owner/name> --source [owner/name:]<branch> --target <branch>
            --title <t> [--body <b> | --file -] [--format md|org] [--draft]
repo refs   <owner/name>

Note mr review request is a three-word path — ["mr", "review", "request", repo, n]. The app already sends ["mr", "review", verdict, ...] for approvals, so getting this wrong silently posts a review instead of requesting one.

The wire fields, from internal/control/mr.go:337-357 — all omitempty, all currently undecoded by iOS:

Draft          bool       `json:"draft,omitempty"`
ReviewRequests []string   `json:"review_requests,omitempty"`
StackedOn      *stackRef  `json:"stacked_on,omitempty"`   // {number, title}
Stacked        []stackRef `json:"stacked,omitempty"`      // [{number, title}]

mr list rows carry draft and stacked_on; mr show carries all four.

Build and test command:

xcodebuild -project gitbay.xcodeproj -scheme gitbay \
  -destination 'platform=iOS Simulator,name=iPhone 17' \
  test -only-testing:gitbayTests

Baseline: 258 passed, 0 failed, 1 skipped (LiveInstanceTests — expected).


File Structure

File Responsibility
gitbay/MRs/MRModels.swift (modify) Decode draft, review_requests, stacked_on, stacked
gitbay/MRs/MRDetailViewModel.swift (modify) setDraft, retarget, requestReview, removeReviewRequest, branch list
gitbay/MRs/MRCreateViewModel.swift (modify) --draft on create
gitbayTests/MRLifecycleTests.swift (create) Every test in this plan
gitbay/Views/MRs/MRView.swift (modify) Draft badge, stack section, reviewer rows, the three actions
gitbay/Views/MRs/MRListView.swift (modify) Draft badge, "stacked on" hint

Task 1: Decode the four fields

Files: Modify gitbay/MRs/MRModels.swift; test in gitbayTests/MRLifecycleTests.swift

Interfaces produced:

  • MergeRequest.draft: Bool (defaulting false when absent) and MergeRequest.stackedOn: StackRef?
  • MRDetail.draft: Bool, .reviewRequests: [String], .stackedOn: StackRef?, .stacked: [StackRef]
  • nonisolated struct StackRef: Decodable, Sendable, Hashable, Identifiable — number: Int64, title: String, id is number

The subtlety this task exists to get right. All four keys are omitempty, so absence is the common case: a non-draft merge request has no draft key at all, not "draft": false. Bool is not Optional, so a plain let draft: Bool fails to decode the ordinary merge request. Either declare them optional and expose non-optional accessors, or write init(from:) with decodeIfPresent and a default. The tests below pin the absent case for every field, because that is what production sends most of the time.

draft is a flag, not a fifth state: state stays "open". A draft merge request is open but not asking — it does not merge and does not appear in a review queue. isOpen must keep meaning what it means.

  • Step 1: Write the failing tests

Create gitbayTests/MRLifecycleTests.swift:

import Foundation
import Testing
@testable import gitbay

private func decodeDetail(_ json: String) throws -> MRDetail {
    let decoder = JSONDecoder()
    decoder.dateDecodingStrategy = .iso8601
    return try decoder.decode(MRDetail.self, from: Data(json.utf8))
}

private func decodeRow(_ json: String) throws -> MergeRequest {
    let decoder = JSONDecoder()
    decoder.dateDecodingStrategy = .iso8601
    return try decoder.decode(MergeRequest.self, from: Data(json.utf8))
}

private let plainDetail = """
    {"number":7,"title":"a change","state":"open","author":"cmc",\
    "source":"feat","target_ref":"main","head_sha":"abc123",\
    "created_at":"2026-09-01T00:00:00Z"}
    """

struct MRLifecycleDecodingTests {

    /// Every one of these keys is omitempty. The ordinary merge request
    /// carries none of them, so absence must decode, not throw.
    @Test func anOrdinaryMergeRequestHasNoneOfTheNewKeys() throws {
        let mr = try decodeDetail(plainDetail)
        #expect(mr.draft == false)
        #expect(mr.reviewRequests.isEmpty)
        #expect(mr.stackedOn == nil)
        #expect(mr.stacked.isEmpty)
    }

    @Test func aDraftDecodesAndStaysOpen() throws {
        let mr = try decodeDetail("""
            {"number":7,"title":"a change","state":"open","draft":true,"author":"cmc",\
            "source":"feat","target_ref":"main","head_sha":"abc123",\
            "created_at":"2026-09-01T00:00:00Z"}
            """)
        #expect(mr.draft)
        // Draft is a flag, not a fifth state.
        #expect(mr.state == "open")
        #expect(mr.isOpen)
    }

    @Test func reviewRequestsDecode() throws {
        let mr = try decodeDetail("""
            {"number":7,"title":"a change","state":"open","author":"cmc",\
            "review_requests":["rae","sam"],\
            "source":"feat","target_ref":"main","head_sha":"abc123",\
            "created_at":"2026-09-01T00:00:00Z"}
            """)
        #expect(mr.reviewRequests == ["rae", "sam"])
    }

    @Test func bothHalvesOfAStackDecode() throws {
        let mr = try decodeDetail("""
            {"number":7,"title":"a change","state":"open","author":"cmc",\
            "stacked_on":{"number":6,"title":"the one below"},\
            "stacked":[{"number":8,"title":"one above"},{"number":9,"title":"another"}],\
            "source":"feat","target_ref":"main","head_sha":"abc123",\
            "created_at":"2026-09-01T00:00:00Z"}
            """)
        #expect(mr.stackedOn?.number == 6)
        #expect(mr.stackedOn?.title == "the one below")
        #expect(mr.stacked.map(\.number) == [8, 9])
    }

    @Test func aListRowCarriesDraftAndStackedOn() throws {
        let row = try decodeRow("""
            {"number":7,"title":"a change","state":"open","draft":true,"author":"cmc",\
            "stacked_on":{"number":6,"title":"below"},\
            "source":"feat","target_ref":"main","head_sha":"abc123",\
            "created_at":"2026-09-01T00:00:00Z"}
            """)
        #expect(row.draft)
        #expect(row.stackedOn?.number == 6)
    }

    @Test func aPlainListRowHasNeither() throws {
        let row = try decodeRow("""
            {"number":7,"title":"a change","state":"open","author":"cmc",\
            "source":"feat","target_ref":"main","head_sha":"abc123",\
            "created_at":"2026-09-01T00:00:00Z"}
            """)
        #expect(row.draft == false)
        #expect(row.stackedOn == nil)
    }
}
  • Step 2: Run to verify failure.
  • Step 3: Implement. Add StackRef to MRModels.swift, add the four fields to MRDetail and the two to MergeRequest, and extend both CodingKeys. Use whichever of the two approaches above is cleaner, but the non-optional accessors (draft: Bool, reviewRequests: [String], stacked: [StackRef]) are what Tasks 2 and 3 consume — do not push optionality onto callers.
  • Step 4: Run the tests. Expect PASS and the full suite green.
  • Step 5: Commit — git commit -m "Decode draft, review requests and the merge request stack"

Task 2: The four write actions

Files: Modify gitbay/MRs/MRDetailViewModel.swift and gitbay/MRs/MRCreateViewModel.swift; test in gitbayTests/MRLifecycleTests.swift

Interfaces produced:

  • MRDetailViewModel.setDraft(_ draft: Bool) async — mr draft when true, mr ready when false
  • MRDetailViewModel.retarget(to branch: String) async
  • MRDetailViewModel.requestReview(from user: String) async / .removeReviewRequest(_ user: String) async
  • MRDetailViewModel.branches: [String] and func loadBranches() async — from repo refs, for the retarget picker
  • MRCreateViewModel gains a draft: Bool input that appends --draft

Follow the existing perform(argv:stdin:) in MRDetailViewModel — it already sets working, clears actionError, runs, and reloads. Do not restructure it.

mr review request is a three-word path. The argv is ["mr", "review", "request", repoPath, number, "--add", user]. The existing approve path sends ["mr", "review", verdict, ...], so a two-word mistake here posts a review rather than requesting one — silently, with a 200. The tests below assert the exact argv for this reason.

repo refs returns {branches: [{name, sha}], tags: [...]}. RepoRefs and RepoRef already exist and RefsViewModel.load already reads this command — reuse both models rather than writing a second pair.

  • Step 1: Write the failing tests — append to gitbayTests/MRLifecycleTests.swift:
@MainActor
struct MRLifecycleActionTests {

    private func loaded() async throws -> (MRDetailViewModel, StubProtocol.Box) {
        let box = StubProtocol.box()
        let client = GitbayClient(
            instance: try GitbayInstance(url: "https://gitbay.org"),
            token: "test-token",
            session: box.session()
        )
        box.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"data":\(plainDetail),"exit_code":0}
            """))
        let model = MRDetailViewModel(client: client, repoPath: "krz/gitbay", number: 7)
        await model.load()
        return (model, box)
    }

    private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] {
        let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any])
        return try #require(body["argv"] as? [String])
    }

    private func ok(_ box: StubProtocol.Box) {
        box.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"exit_code":0}
            """))
        box.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"data":\(plainDetail),"exit_code":0}
            """))
    }

    @Test func markingDraftAndReadyAreDifferentCommands() async throws {
        let (model, box) = try await loaded()
        ok(box)
        await model.setDraft(true)
        var write = try #require(box.seen.first { $0.method == "POST" })
        #expect(try argvOf(write) == ["mr", "draft", "krz/gitbay", "7"])

        let (model2, box2) = try await loaded()
        ok(box2)
        await model2.setDraft(false)
        write = try #require(box2.seen.first { $0.method == "POST" })
        #expect(try argvOf(write) == ["mr", "ready", "krz/gitbay", "7"])
    }

    @Test func retargetPassesTheBranchPositionally() async throws {
        let (model, box) = try await loaded()
        ok(box)
        await model.retarget(to: "release")
        let write = try #require(box.seen.first { $0.method == "POST" })
        #expect(try argvOf(write) == ["mr", "retarget", "krz/gitbay", "7", "release"])
    }

    /// `mr review request` is a THREE-word path. Two words posts a
    /// review instead — silently, with a 200.
    @Test func requestingAReviewUsesTheThreeWordPath() async throws {
        let (model, box) = try await loaded()
        ok(box)
        await model.requestReview(from: "rae")
        let write = try #require(box.seen.first { $0.method == "POST" })
        #expect(try argvOf(write)
            == ["mr", "review", "request", "krz/gitbay", "7", "--add", "rae"])
    }

    @Test func removingAReviewRequestUsesRemoveNotAdd() async throws {
        let (model, box) = try await loaded()
        ok(box)
        await model.removeReviewRequest("rae")
        let write = try #require(box.seen.first { $0.method == "POST" })
        #expect(try argvOf(write)
            == ["mr", "review", "request", "krz/gitbay", "7", "--remove", "rae"])
    }

    @Test func branchesComeFromRepoRefs() async throws {
        let (model, box) = try await loaded()
        box.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"data":{"branches":[{"name":"main","sha":"a"},\
            {"name":"release","sha":"b"}],"tags":[]},"exit_code":0}
            """))
        await model.loadBranches()
        #expect(model.branches == ["main", "release"])
    }

    @Test func aRefusalSurfacesAndDoesNotReload() async throws {
        let (model, box) = try await loaded()
        box.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"error":"cannot retarget a merged merge request",\
            "exit_code":1}
            """))
        await model.retarget(to: "release")
        #expect(model.actionError?.isEmpty == false)
        #expect(model.working == false)
    }
}

@MainActor
struct MRCreateDraftTests {

    @Test func creatingAsADraftAppendsTheFlag() async throws {
        let box = StubProtocol.box()
        let client = GitbayClient(
            instance: try GitbayInstance(url: "https://gitbay.org"),
            token: "test-token",
            session: box.session()
        )
        box.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"data":{"number":7},"exit_code":0}
            """))
        let model = MRCreateViewModel(client: client, repoPath: "krz/gitbay")
        model.draft = true
        _ = await model.create(source: "feat", target: "main", title: "a change", body: "")

        let write = try #require(box.seen.first { $0.method == "POST" })
        let body = try #require(
            try JSONSerialization.jsonObject(with: write.body) as? [String: Any]
        )
        let argv = try #require(body["argv"] as? [String])
        #expect(argv.contains("--draft"))
    }

    @Test func creatingWithoutDraftOmitsTheFlag() async throws {
        let box = StubProtocol.box()
        let client = GitbayClient(
            instance: try GitbayInstance(url: "https://gitbay.org"),
            token: "test-token",
            session: box.session()
        )
        box.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"data":{"number":7},"exit_code":0}
            """))
        let model = MRCreateViewModel(client: client, repoPath: "krz/gitbay")
        _ = await model.create(source: "feat", target: "main", title: "a change", body: "")

        let write = try #require(box.seen.first { $0.method == "POST" })
        let body = try #require(
            try JSONSerialization.jsonObject(with: write.body) as? [String: Any]
        )
        #expect((body["argv"] as? [String])?.contains("--draft") == false)
    }
}

MRCreateViewModel.create(source:target:title:body:) is the existing signature and the tests above match it. Add draft as a settable property rather than a fifth parameter, so the call sites do not all change.

  • Step 2: Run to verify failure.
  • Step 3: Implement the five methods and the draft input.
  • Step 4: Run the tests.
  • Step 5: Commit — git commit -m "Draft, retarget and review requests on a merge request"

Task 3: The UI

Files: Modify gitbay/Views/MRs/MRView.swift and gitbay/Views/MRs/MRListView.swift

No unit tests — UI. Verification is the build plus the suite staying green.

On the list row: a GBChip("draft", .secondary) when mr.draft, and a quiet "stacked on !N" line when mr.stackedOn != nil. Match the row's existing density; do not add a line to every row.

On the detail screen:

  • Draft badge in the header beside the state.
  • A Stack section when stackedOn != nil || !stacked.isEmpty, listing both directions — what this is stacked on, and what is stacked on it — each row a NavigationLink(value: MRRoute.mr(repo:number:)). Say which direction each is; "stacked on !6" and "!8 is stacked on this" read differently and the user needs to know which.
  • A Reviewers section listing reviewRequests, each removable, with a field to add one. Reuse the add/remove shape IssueView already uses for labels and assignees rather than inventing another.
  • Actions, placed with the existing merge/close controls: Mark as draft / Mark as ready (whichever the current state implies), and Retarget, which presents a branch picker fed by loadBranches().

Two things the UI must say, because the server enforces them and a silent refusal is worse than a warning:

  1. Retargeting stales the reviews — an approval was of the diff against the old branch. Say so in the confirmation.
  2. A squash or rebase merge is refused while anything is stacked on this merge request, since it would rewrite the commits the stack builds on. The merge sheet offers Squash, Fast-forward and Rebase at MRView.swift:63-65; when !stacked.isEmpty, offer only Fast-forward.

A draft merge request does not merge. When mr.draft, the merge control should say so rather than presenting a button the server will refuse.

  • Step 1: List row — draft chip and stacked hint
  • Step 2: Detail — draft badge, Stack section, Reviewers section
  • Step 3: Detail — draft/ready and retarget actions, with the two warnings above
  • Step 4: Build and run the full suite. Expect no drop from 258 + Tasks 1-2's additions.
  • Step 5: Commit — git commit -m "Draft, stack and reviewers on the merge request screen"

Task 4: Flip four parity rows and open the merge request

  • Step 1: In the Merge requests table, set draft, ready, retarget, request a review and stacked merge requests to yes for iOS. Touch no other row — in particular leave choose body markup, which is MR 11.

Land it via a worktree off origin/main in krz/gitbay, merged --strategy ff. That repo requires signed commits, so squash and rebase merges are refused, and its working tree usually holds unrelated work — never switch its branch.

  • Step 2: Run the full suite; record the real number.
  • Step 3: Open the merge request.

Notes for whoever executes this

mr review request is three words. ["mr", "review", "request", ...]. The approve path is ["mr", "review", verdict, ...]. A two-word slip posts a review and returns 200, so nothing surfaces — which is exactly why two tests assert the full argv.

Absence is the common case for all four new fields. They are omitempty. Most merge requests carry none of them, so the decoding must default rather than require. Every test above that pins an absent field is pinning the ordinary case, not an edge case.

Draft is a flag, not a state. state stays "open". Do not add a .draft case to any state enum, and do not let isOpen change meaning — every state = 'open' rule on the server still means what it did.

Do not offer a merge strategy the server will refuse. Squash and rebase are refused while anything is stacked on the merge request.