diff --git a/native/ios/PsycheApp/Sources/PsycheApp/Views/ActionSheetPresentation.swift b/native/ios/PsycheApp/Sources/PsycheApp/Views/ActionSheetPresentation.swift index 70edb891..f1262695 100644 --- a/native/ios/PsycheApp/Sources/PsycheApp/Views/ActionSheetPresentation.swift +++ b/native/ios/PsycheApp/Sources/PsycheApp/Views/ActionSheetPresentation.swift @@ -1,3 +1,4 @@ +import Foundation import PsycheCore enum ActionSheetSection: Equatable, Hashable { @@ -25,6 +26,16 @@ struct ActionSheetStatus: Equatable { let tone: ActionSheetStatusTone } +struct ActionSheetCancelControl: Equatable { + let label: String + let response: MobileActionResponse +} + +struct ActionSheetPullRequestDraft: Equatable { + let title: String + let body: String +} + enum ActionSheetPresentation { static func sectionOrder( hasScope: Bool, @@ -54,6 +65,23 @@ enum ActionSheetPresentation { option.danger == true ? .destructive : .normal } + static func visibleChoiceOptions(_ options: [MobileActionOption]) -> [MobileActionOption] { + guard let cancelOption = cancelOption(in: options) else { + return options + } + return options.filter { $0.id != cancelOption.id } + } + + static func cancelControl(for options: [MobileActionOption]) -> ActionSheetCancelControl { + if let cancelOption = cancelOption(in: options) { + return ActionSheetCancelControl( + label: cancelOption.label, + response: .choice(optionID: cancelOption.id) + ) + } + return ActionSheetCancelControl(label: "Cancel", response: .cancel) + } + static func inputLineRange(_ requestedMaximum: Int?) -> ClosedRange { 1...min(max(requestedMaximum ?? 6, 1), 12) } @@ -98,4 +126,54 @@ enum ActionSheetPresentation { static func primaryInputLabel(for action: PaneAction) -> String { action == .createPR ? "Create Pull Request" : "Continue" } + + static func controlIdentifier(for label: String) -> String { + let slug = label + .lowercased() + .map { character -> String in + character.isLetter || character.isNumber ? String(character) : "-" + } + .joined() + .split(separator: "-", omittingEmptySubsequences: true) + .joined(separator: "-") + return "remote-action-control-\(slug)" + } + + static func pullRequestDraft(from summary: String) -> ActionSheetPullRequestDraft { + let normalized = summary + .replacingOccurrences(of: "\r\n", with: "\n") + .trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalized.isEmpty else { + return ActionSheetPullRequestDraft(title: "", body: "") + } + guard let newline = normalized.firstIndex(of: "\n") else { + return ActionSheetPullRequestDraft(title: normalized, body: "") + } + + let title = String(normalized[.. String { + let trimmedTitle = title.trimmingCharacters(in: .whitespacesAndNewlines) + let trimmedBody = body + .replacingOccurrences(of: "\r\n", with: "\n") + .trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmedBody.isEmpty else { + return trimmedTitle + } + return "\(trimmedTitle)\n\n\(trimmedBody)" + } + + private static func cancelOption(in options: [MobileActionOption]) -> MobileActionOption? { + options.first { $0.id == "cancel" } + } } diff --git a/native/ios/PsycheApp/Sources/PsycheApp/Views/ActionSheetView.swift b/native/ios/PsycheApp/Sources/PsycheApp/Views/ActionSheetView.swift index 50ecf9f0..db5d494d 100644 --- a/native/ios/PsycheApp/Sources/PsycheApp/Views/ActionSheetView.swift +++ b/native/ios/PsycheApp/Sources/PsycheApp/Views/ActionSheetView.swift @@ -5,6 +5,8 @@ struct ActionSheetView: View { @ObservedObject var store: RemoteActionStore @State private var draft = "" + @State private var pullRequestTitle = "" + @State private var pullRequestBody = "" var body: some View { NavigationStack { @@ -15,7 +17,7 @@ struct ActionSheetView: View { ActionSheetPresentation.sectionOrder( hasScope: !presentation.scope.rows.isEmpty, hasConsequence: presentation.scope.consequence != nil, - hasRelatedFiles: !presentation.relatedFiles.isEmpty + hasRelatedFiles: standaloneRelatedFiles(for: presentation).isEmpty == false ), id: \.self ) { section in @@ -100,6 +102,7 @@ struct ActionSheetView: View { case .pullRequestReview(let review): pullRequestReviewSection( review, + paneID: presentation.paneID, message: presentation.message ) case .progress(let progress): @@ -145,6 +148,7 @@ struct ActionSheetView: View { private func pullRequestReviewSection( _ review: RemoteActionReview, + paneID: String, message: String ) -> some View { Section("Pull Request Review") { @@ -165,18 +169,23 @@ struct ActionSheetView: View { .foregroundStyle(PsycheTheme.amber) } - TextField("Summary", text: $draft, axis: .vertical) - .lineLimit(1...12) + TextField("Title", text: $pullRequestTitle) + .lineLimit(1) + .disabled(ActionSheetPresentation.editingDisabled(isSubmitting: store.isSubmitting)) + .accessibilityIdentifier("remote-action-pr-title") + + TextField("Summary", text: $pullRequestBody, axis: .vertical) + .lineLimit(3...12) .disabled(ActionSheetPresentation.editingDisabled(isSubmitting: store.isSubmitting)) + .accessibilityIdentifier("remote-action-pr-body") if !review.details.files.isEmpty { - LabeledContent("Review files") { - VStack(alignment: .trailing, spacing: 8) { - ForEach(review.details.files.indices, id: \.self) { index in - Label(review.details.files[index], systemImage: "doc") - .multilineTextAlignment(.trailing) - } - } + ForEach(review.details.files, id: \.self) { path in + relatedFileLink( + path, + paneID: paneID, + accessibilityIdentifier: "remote-action-pr-file-\(path)" + ) } } } @@ -232,8 +241,12 @@ struct ActionSheetView: View { @ViewBuilder private func relatedFilesSection(for presentation: RemoteActionPresentation) -> some View { Section("Related Files") { - ForEach(presentation.relatedFiles.indices, id: \.self) { index in - Label(presentation.relatedFiles[index], systemImage: "doc") + ForEach(standaloneRelatedFiles(for: presentation), id: \.self) { path in + relatedFileLink( + path, + paneID: presentation.paneID, + accessibilityIdentifier: "remote-action-file-\(path)" + ) } } } @@ -273,7 +286,7 @@ struct ActionSheetView: View { case .choice(let options): Section("Controls") { submittingIndicator - ForEach(options) { option in + ForEach(ActionSheetPresentation.visibleChoiceOptions(options)) { option in Button( role: ActionSheetPresentation.optionRole(option).buttonRole, action: { respond(.choice(optionID: option.id)) } @@ -284,9 +297,10 @@ struct ActionSheetView: View { .accessibilityElement(children: .ignore) .accessibilityLabel(option.label) .accessibilityValue(option.description ?? "") - .accessibilityIdentifier("remote-action-choice-\(option.id)") + .accessibilityIdentifier(ActionSheetPresentation.controlIdentifier(for: option.label)) } - responseButton("Cancel", response: .cancel) + let cancelControl = ActionSheetPresentation.cancelControl(for: options) + responseButton(cancelControl.label, response: cancelControl.response) } case .input: Section("Controls") { @@ -304,8 +318,8 @@ struct ActionSheetView: View { responseButton("Cancel", response: .cancel) responseButton( ActionSheetPresentation.primaryInputLabel(for: presentation.action), - response: .input(value: draft), - recoveryText: draft + response: .input(value: pullRequestSummary), + recoveryText: pullRequestSummary ) } case .progress: @@ -332,12 +346,14 @@ struct ActionSheetView: View { response: MobileActionResponse, recoveryText: String? = nil ) -> some View { - Button( - label, - role: role.buttonRole, - action: { respond(response, recoveryText: recoveryText) } - ) + Button(role: role.buttonRole, action: { respond(response, recoveryText: recoveryText) }) { + Text(label) + .frame(maxWidth: .infinity, alignment: .leading) + } .disabled(store.isSubmitting) + .accessibilityElement(children: .ignore) + .accessibilityLabel(label) + .accessibilityIdentifier(ActionSheetPresentation.controlIdentifier(for: label)) } private func dismissButton(_ label: String) -> some View { @@ -345,6 +361,9 @@ struct ActionSheetView: View { store.dismiss() } .disabled(store.isSubmitting) + .accessibilityElement(children: .ignore) + .accessibilityLabel(label) + .accessibilityIdentifier(ActionSheetPresentation.controlIdentifier(for: label)) } private func choiceLabel(for option: MobileActionOption) -> some View { @@ -384,17 +403,56 @@ struct ActionSheetView: View { private func resetDraft() { guard let presentation = store.presentation else { draft = "" + pullRequestTitle = "" + pullRequestBody = "" return } switch presentation.content { case .input(let input): draft = input.defaultValue + pullRequestTitle = "" + pullRequestBody = "" case .pullRequestReview(let review): - draft = review.defaultSummary + let reviewDraft = ActionSheetPresentation.pullRequestDraft(from: review.defaultSummary) + draft = "" + pullRequestTitle = reviewDraft.title + pullRequestBody = reviewDraft.body default: draft = "" + pullRequestTitle = "" + pullRequestBody = "" + } + } + + private func standaloneRelatedFiles(for presentation: RemoteActionPresentation) -> [String] { + switch presentation.content { + case .pullRequestReview(let review): + let reviewFiles = Set(review.details.files) + return presentation.relatedFiles.filter { !reviewFiles.contains($0) } + default: + return presentation.relatedFiles + } + } + + private func relatedFileLink( + _ path: String, + paneID: String, + accessibilityIdentifier: String + ) -> some View { + NavigationLink { + ActionSheetRelatedFileView(paneID: paneID, path: path) + } label: { + Label(path, systemImage: "doc") } + .accessibilityIdentifier(accessibilityIdentifier) + } + + private var pullRequestSummary: String { + ActionSheetPresentation.pullRequestSummary( + title: pullRequestTitle, + body: pullRequestBody + ) } private func color(for tone: ActionSheetStatusTone) -> Color { diff --git a/native/ios/PsycheApp/Sources/PsycheApp/Views/FileBrowserView.swift b/native/ios/PsycheApp/Sources/PsycheApp/Views/FileBrowserView.swift index eb5354ee..37dae9a0 100644 --- a/native/ios/PsycheApp/Sources/PsycheApp/Views/FileBrowserView.swift +++ b/native/ios/PsycheApp/Sources/PsycheApp/Views/FileBrowserView.swift @@ -239,6 +239,74 @@ private struct FileBrowserRow: View { } } +struct ActionSheetRelatedFileView: View { + @EnvironmentObject private var store: WorkspaceStore + + let paneID: String + let path: String + + @State private var file: BrowserFile? + @State private var isLoading = false + @State private var errorMessage: String? + + var body: some View { + Group { + if store.isStale { + ContentUnavailableView( + "Workspace out of date", + systemImage: "wifi.exclamationmark", + description: Text("Reconnect before inspecting this file.") + ) + } else if isLoading, file == nil, errorMessage == nil { + ProgressView("Loading file…") + } else if let file { + switch FileInspectionDestination.forFile(file) { + case .preview: + FilePreviewView(paneID: paneID, file: file) + case .diff: + DiffView(paneID: paneID, file: file) + } + } else if let errorMessage { + ContentUnavailableView( + "Couldn't load file", + systemImage: "exclamationmark.triangle", + description: Text(errorMessage) + ) + } else { + ProgressView("Loading file…") + } + } + .navigationTitle(path.components(separatedBy: "/").last ?? path) + .navigationBarTitleDisplayMode(.inline) + .task(id: store.isStale) { + guard !store.isStale else { + isLoading = false + return + } + await load() + } + .accessibilityIdentifier("action-sheet-related-file") + } + + private func load() async { + guard !isLoading else { return } + isLoading = true + errorMessage = nil + defer { isLoading = false } + + do { + let snapshot = try await store.listFiles(inPane: paneID) + guard let file = snapshot.files.first(where: { $0.path == path }) else { + errorMessage = "The host did not publish \(path) for this pane." + return + } + self.file = file + } catch { + errorMessage = error.localizedDescription + } + } +} + private struct FilePreviewView: View { @EnvironmentObject private var store: WorkspaceStore diff --git a/native/ios/PsycheApp/Tests/PsycheAppUITests/PsycheAppUITests.swift b/native/ios/PsycheApp/Tests/PsycheAppUITests/PsycheAppUITests.swift index 977ed617..5bb0899a 100644 --- a/native/ios/PsycheApp/Tests/PsycheAppUITests/PsycheAppUITests.swift +++ b/native/ios/PsycheApp/Tests/PsycheAppUITests/PsycheAppUITests.swift @@ -500,6 +500,123 @@ final class PsycheAppUITests: XCTestCase { ) } + func testMergeWorkflowHandlesSiblingAndFallbackConfirmationsBeforeSuccess() throws { + let app = launchApp() + openWebHomePane(in: app) + + openPaneActions(in: app) + app.buttons["Merge"].tap() + + let sheet = element("remote-action-sheet", in: app) + XCTAssertTrue(sheet.waitForExistence(timeout: 10)) + XCTAssertTrue(app.staticTexts["Sibling Agents Active"].waitForExistence(timeout: 10)) + XCTAssertTrue( + app.staticTexts.containing( + NSPredicate(format: "label CONTAINS[c] %@", "homepage preview") + ).firstMatch.waitForExistence(timeout: 10) + ) + + tapControl("Continue", in: app) + XCTAssertTrue(app.staticTexts["Parent Merge Target Unavailable"].waitForExistence(timeout: 10)) + + tapControl("Continue", in: app) + XCTAssertTrue(app.staticTexts["Merge Worktree"].waitForExistence(timeout: 10)) + + tapControl("Merge", in: app) + XCTAssertTrue( + app.staticTexts.containing( + NSPredicate(format: "label CONTAINS[c] %@", "Merged \"homepage polish\" into main.") + ).firstMatch.waitForExistence(timeout: 10) + ) + XCTAssertTrue(app.buttons["Done"].waitForExistence(timeout: 10)) + } + + func testMergeWorkflowUsesSingleExplicitCancelPathForUncommittedChoice() throws { + let app = launchApp() + openPane("ios-cockpit", in: app) + + openPaneActions(in: app) + app.buttons["Merge"].tap() + + XCTAssertTrue(app.staticTexts["Worktree Has Uncommitted Changes"].waitForExistence(timeout: 10)) + revealControl("AI commit (automatic)", in: app) + XCTAssertTrue(control("AI commit (automatic)", in: app).waitForExistence(timeout: 10)) + XCTAssertTrue(control("Manual commit message", in: app).waitForExistence(timeout: 10)) + XCTAssertTrue(control("Cancel merge", in: app).waitForExistence(timeout: 10)) + XCTAssertFalse(control("Cancel", in: app).exists) + + tapControl("Cancel merge", in: app) + + XCTAssertTrue(app.staticTexts["Merge cancelled"].waitForExistence(timeout: 10)) + XCTAssertTrue(app.buttons["Done"].waitForExistence(timeout: 10)) + XCTAssertTrue(control("Cancel merge", in: app).waitForNonExistence(timeout: 10)) + XCTAssertTrue(control("Manual commit message", in: app).waitForNonExistence(timeout: 10)) + } + + func testMergeWorkflowSurfacesHostFailureFromUncommittedChoice() throws { + let app = launchApp() + openPane("ios-cockpit", in: app) + + openPaneActions(in: app) + app.buttons["Merge"].tap() + + XCTAssertTrue(app.staticTexts["Worktree Has Uncommitted Changes"].waitForExistence(timeout: 10)) + tapControl("Manual commit message", in: app) + + XCTAssertTrue(app.staticTexts["Merge Failed"].waitForExistence(timeout: 10)) + XCTAssertTrue( + app.staticTexts.containing( + NSPredicate(format: "label CONTAINS[c] %@", "needs a manual commit") + ).firstMatch.waitForExistence(timeout: 10) + ) + } + + func testPullRequestReviewSupportsEditingAndRelatedFileNavigation() throws { + let app = launchApp() + openWebHomePane(in: app) + + openPaneActions(in: app) + app.buttons["Create Pull Request"].tap() + + XCTAssertTrue(app.staticTexts["Create Pull Request"].waitForExistence(timeout: 10)) + XCTAssertTrue( + app.staticTexts.containing( + NSPredicate(format: "label CONTAINS[c] %@", "create a GitHub pull request") + ).firstMatch.waitForExistence(timeout: 10) + ) + + tapControl("Create PR", in: app) + + let sheet = element("remote-action-sheet", in: app) + XCTAssertTrue(sheet.waitForExistence(timeout: 10)) + let changedFile = element("remote-action-pr-file-Sources/App.swift", in: app) + reveal(changedFile, in: sheet) + XCTAssertTrue(changedFile.waitForExistence(timeout: 10)) + changedFile.tap() + + XCTAssertTrue(element("action-sheet-related-file", in: app).waitForExistence(timeout: 10)) + app.navigationBars.buttons.element(boundBy: 0).tap() + + let titleField = textInput("remote-action-pr-title", in: app) + let bodyField = textInput("remote-action-pr-body", in: app) + reveal(titleField, in: sheet) + XCTAssertTrue(titleField.waitForExistence(timeout: 10)) + XCTAssertTrue(bodyField.waitForExistence(timeout: 10)) + + titleField.tap() + titleField.typeText(" safely") + bodyField.tap() + bodyField.typeText("\n- Confirmed from iOS") + XCTAssertTrue( + (titleField.value as? String ?? "").contains("safely"), + "Edited PR title should remain in the review form" + ) + XCTAssertTrue( + (bodyField.value as? String ?? "").contains("Confirmed from iOS"), + "Edited PR summary should remain in the review form" + ) + } + // MARK: - Split layout and focus /// Regular width has room for two terminals, and both must be live rather @@ -680,6 +797,43 @@ final class PsycheAppUITests: XCTestCase { split.tap() } + private func control(_ label: String, in app: XCUIApplication) -> XCUIElement { + element(controlIdentifier(for: label), in: app) + } + + private func revealControl(_ label: String, in app: XCUIApplication) { + reveal(control(label, in: app), in: element("remote-action-sheet", in: app)) + } + + private func tapControl(_ label: String, in app: XCUIApplication) { + let target = control(label, in: app) + reveal(target, in: element("remote-action-sheet", in: app)) + XCTAssertTrue(target.waitForExistence(timeout: 10), "Missing action control \(label)") + target.tap() + } + + private func reveal(_ target: XCUIElement, in container: XCUIElement, attempts: Int = 6) { + guard container.waitForExistence(timeout: 10) else { return } + for _ in 0.. String { + let slug = label + .lowercased() + .map { character in + character.isLetter || character.isNumber ? String(character) : "-" + } + .joined() + .split(separator: "-", omittingEmptySubsequences: true) + .joined(separator: "-") + return "remote-action-control-\(slug)" + } + /// Counts live terminals, which is how the two-session cap is observed /// from outside. private func renderedTerminalCount(in app: XCUIApplication) -> Int { @@ -714,7 +868,14 @@ final class PsycheAppUITests: XCTestCase { /// synchronous `.exists` check. SwiftUI exposes this identifier on a row /// descendant rather than on the cell itself. private func openWebHomePane(in app: XCUIApplication) { - let paneRow = app.cells.containing(.any, identifier: "now-pane-web-home").firstMatch + openPane("web-home", in: app) + } + + private func openPane(_ paneID: String, in app: XCUIApplication) { + let paneRow = app.cells.containing( + .any, + identifier: "now-pane-\(paneID)" + ).firstMatch XCTAssertTrue(paneRow.waitForExistence(timeout: 30)) paneRow.tap() } @@ -791,6 +952,14 @@ final class PsycheAppUITests: XCTestCase { return cell.exists ? cell : element(identifier, in: app) } + private func textInput(_ identifier: String, in app: XCUIApplication) -> XCUIElement { + let textView = app.textViews[identifier] + if textView.exists || textView.waitForExistence(timeout: 1) { + return textView + } + return app.textFields[identifier] + } + private func element(_ identifier: String, in app: XCUIApplication) -> XCUIElement { app.descendants(matching: .any).matching(identifier: identifier).firstMatch } diff --git a/native/ios/PsycheApp/UnitTests/ActionSheetPresentationTests.swift b/native/ios/PsycheApp/UnitTests/ActionSheetPresentationTests.swift index e00c7fc4..268e09ed 100644 --- a/native/ios/PsycheApp/UnitTests/ActionSheetPresentationTests.swift +++ b/native/ios/PsycheApp/UnitTests/ActionSheetPresentationTests.swift @@ -72,6 +72,38 @@ final class ActionSheetPresentationTests: XCTestCase { ) } + func testChoiceCancelControlUsesHostCancelOptionWhenAvailable() { + let options = [ + MobileActionOption(id: "ship", label: "Ship"), + MobileActionOption(id: "cancel", label: "Cancel merge"), + ] + + XCTAssertEqual( + ActionSheetPresentation.visibleChoiceOptions(options).map(\.id), + ["ship"] + ) + XCTAssertEqual( + ActionSheetPresentation.cancelControl(for: options), + ActionSheetCancelControl( + label: "Cancel merge", + response: .choice(optionID: "cancel") + ) + ) + } + + func testChoiceCancelControlFallsBackToGenericCancelWithoutHostOption() { + let options = [MobileActionOption(id: "ship", label: "Ship")] + + XCTAssertEqual( + ActionSheetPresentation.visibleChoiceOptions(options).map(\.id), + ["ship"] + ) + XCTAssertEqual( + ActionSheetPresentation.cancelControl(for: options), + ActionSheetCancelControl(label: "Cancel", response: .cancel) + ) + } + func testInputLineRangeUsesDefaultsAndClampsBounds() { XCTAssertEqual(ActionSheetPresentation.inputLineRange(nil), 1...6) XCTAssertEqual(ActionSheetPresentation.inputLineRange(0), 1...1) @@ -141,6 +173,32 @@ final class ActionSheetPresentationTests: XCTestCase { "Create Pull Request" ) XCTAssertEqual(ActionSheetPresentation.primaryInputLabel(for: .rename), "Continue") + XCTAssertEqual( + ActionSheetPresentation.controlIdentifier(for: "Continue"), + "remote-action-control-continue" + ) + XCTAssertEqual( + ActionSheetPresentation.controlIdentifier(for: "Create Pull Request"), + "remote-action-control-create-pull-request" + ) + } + + func testPullRequestDraftRoundTripsTitleAndBody() { + let draft = ActionSheetPresentation.pullRequestDraft( + from: "feat(ios): wire review flow\r\n\r\n\r\n## Summary\r\n- Ship it\r\n" + ) + + XCTAssertEqual( + draft, + ActionSheetPullRequestDraft( + title: "feat(ios): wire review flow", + body: "## Summary\n- Ship it" + ) + ) + XCTAssertEqual( + ActionSheetPresentation.pullRequestSummary(title: draft.title, body: draft.body), + "feat(ios): wire review flow\n\n## Summary\n- Ship it" + ) } @MainActor @@ -186,4 +244,81 @@ final class ActionSheetPresentationTests: XCTestCase { ]) XCTAssertEqual(options.map(\.danger), [nil, true, true]) } + + @MainActor + func testFixtureMergeFallbackChainSurfacesHostDrivenConfirmations() async { + let workspace = WorkspaceFixtures.workspace(named: WorkspaceFixtures.multiproject) + let requests = FixtureControlRequests(workspace: workspace) + let store = RemoteActionStore(controlRequests: requests) + + await store.start(action: .merge, onPane: "web-home", in: workspace) + XCTAssertEqual(store.presentation?.title, "Sibling Agents Active") + XCTAssertEqual( + store.presentation?.message, + "1 other agent (homepage preview) is using this worktree. Merging will close it. Proceed?" + ) + + await store.respond(.confirm) + XCTAssertEqual(store.presentation?.title, "Parent Merge Target Unavailable") + XCTAssertTrue( + store.presentation?.message.contains("Merge \"homepage polish\" directly into main instead?") == true + ) + + await store.respond(.confirm) + XCTAssertEqual(store.presentation?.title, "Merge Worktree") + XCTAssertEqual(store.presentation?.message, "Merge \"homepage polish\" into main?") + } + + @MainActor + func testFixturePullRequestStartsSpecializedReviewFlow() async { + let workspace = WorkspaceFixtures.workspace(named: WorkspaceFixtures.multiproject) + let requests = FixtureControlRequests(workspace: workspace) + let store = RemoteActionStore(controlRequests: requests) + + await store.start(action: .createPR, onPane: "web-home", in: workspace) + await store.respond(.confirm) + + guard case let .pullRequestReview(review)? = store.presentation?.content else { + return XCTFail("Expected pull request review presentation") + } + XCTAssertEqual(store.presentation?.title, "Create Pull Request") + XCTAssertEqual(review.details.files, ["Sources/App.swift", "Sources/Deleted.swift"]) + XCTAssertEqual( + ActionSheetPresentation.pullRequestDraft(from: review.defaultSummary), + ActionSheetPullRequestDraft( + title: "feat(website): ship homepage polish", + body: """ + ## Summary + - Publish the reviewed homepage polish changes. + + ## Changes + - Refresh the launch copy and supporting assets. + """ + ) + ) + } + + @MainActor + func testFixturePullRequestSubmissionPreservesEditedTitleAndBody() async { + let workspace = WorkspaceFixtures.workspace(named: WorkspaceFixtures.multiproject) + let requests = FixtureControlRequests(workspace: workspace) + let store = RemoteActionStore(controlRequests: requests) + + await store.start(action: .createPR, onPane: "web-home", in: workspace) + await store.respond(.confirm) + await store.respond( + .input( + value: ActionSheetPresentation.pullRequestSummary( + title: "feat(website): ship homepage polish safely", + body: "## Summary\n- Confirmed from iOS" + ) + ) + ) + + XCTAssertEqual(store.presentation?.title, "Create Pull Request") + XCTAssertEqual( + store.presentation?.message, + "Created PR \"feat(website): ship homepage polish safely\" with your edited summary: https://github.com/OpenCoven/psyche-build/pull/903" + ) + } } diff --git a/native/ios/PsycheCore/Sources/PsycheCore/Fixtures/FixtureControlRequests.swift b/native/ios/PsycheCore/Sources/PsycheCore/Fixtures/FixtureControlRequests.swift index 86ac9a31..2b3cff9e 100644 --- a/native/ios/PsycheCore/Sources/PsycheCore/Fixtures/FixtureControlRequests.swift +++ b/native/ios/PsycheCore/Sources/PsycheCore/Fixtures/FixtureControlRequests.swift @@ -200,41 +200,10 @@ public actor FixtureControlRequests: ControlRequesting { switch request.action { case .merge: - return .actionResult(MobileActionsResultResponse( - requestID: requestID, - sessionID: nil, - result: MobileActionResult( - type: "progress", - message: "Preparing merge status for \(context.paneTitle).", - title: "Merge", - progress: nil, - data: actionScope( - for: context, - consequence: "Checks merge status before the host offers the next merge step." - ), - dismissable: true - ) - )) + return startMergeAction(for: context, requestID: requestID) case .createPR: - let sessionID = nextActionSessionID() - pendingActions[sessionID] = .createPullRequestConfirm(paneID: request.paneID) - return .actionResult(MobileActionsResultResponse( - requestID: requestID, - sessionID: sessionID, - result: MobileActionResult( - type: "confirm", - message: "Push \(context.paneTitle) and create a pull request into \(context.targetBranch ?? "main")?", - title: "Create Pull Request", - confirmLabel: "Create PR", - cancelLabel: "Cancel", - data: actionScope( - for: context, - consequence: "Pushes the branch and creates a pull request on the paired host." - ), - relatedFiles: Self.pullRequestFiles - ) - )) + return startCreatePullRequestAction(for: context, requestID: requestID) case .rename: let sessionID = nextActionSessionID() @@ -302,6 +271,90 @@ public actor FixtureControlRequests: ControlRequesting { } } + private func startMergeAction( + for context: FixturePaneContext, + requestID: String + ) -> MobileControlResponse { + if context.paneID == "web-home" { + let sessionID = nextActionSessionID() + pendingActions[sessionID] = .mergeSiblingConfirmation(paneID: context.paneID) + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: sessionID, + result: MobileActionResult( + type: "confirm", + message: "1 other agent (homepage preview) is using this worktree. Merging will close it. Proceed?", + title: "Sibling Agents Active", + confirmLabel: "Continue", + cancelLabel: "Cancel", + data: actionScope( + for: context, + consequence: "Closes sibling panes before the host merges this branch into main." + ) + ) + )) + } + + let sessionID = nextActionSessionID() + pendingActions[sessionID] = .mergeUncommittedChoice(paneID: context.paneID) + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: sessionID, + result: MobileActionResult( + type: "choice", + message: "This worktree has uncommitted changes that must be committed before merging.", + title: "Worktree Has Uncommitted Changes", + options: [ + MobileActionOption( + id: "commit_automatic", + label: "AI commit (automatic)", + description: "Auto-generate and commit immediately", + isDefault: true + ), + MobileActionOption( + id: "commit_manual", + label: "Manual commit message", + description: "Write your own commit message" + ), + MobileActionOption( + id: "cancel", + label: "Cancel merge", + description: "Resolve manually later" + ), + ], + data: actionScope( + for: context, + consequence: "Requires the host to commit these changes before merging into main." + ), + relatedFiles: Self.mergeFiles + ) + )) + } + + private func startCreatePullRequestAction( + for context: FixturePaneContext, + requestID: String + ) -> MobileControlResponse { + let sessionID = nextActionSessionID() + pendingActions[sessionID] = .createPullRequestConfirm(paneID: context.paneID) + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: sessionID, + result: MobileActionResult( + type: "confirm", + message: "Push \(context.paneTitle) and create a GitHub pull request into \(context.targetBranch ?? "main")?", + title: "Create Pull Request", + confirmLabel: "Create PR", + cancelLabel: "Cancel", + data: actionScope( + for: context, + consequence: "Pushes the branch and creates a pull request on the paired host." + ), + relatedFiles: Self.pullRequestFiles + ) + )) + } + private func respondToAction( _ request: MobileActionRespondRequest, requestID: String @@ -315,6 +368,144 @@ public actor FixtureControlRequests: ControlRequesting { } switch (pending, request.response) { + case let (.mergeSiblingConfirmation(paneID), .confirm): + guard let context = paneContext(for: paneID) else { + return .error(MobileProtocolErrorResponse( + requestID: requestID, + code: "unknown_pane", + message: "Pane \(paneID) is not published by this fixture." + )) + } + let sessionID = nextActionSessionID() + pendingActions[sessionID] = .mergeFallbackConfirmation(paneID: paneID) + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: sessionID, + result: MobileActionResult( + type: "confirm", + message: "feat/home-preview is no longer available. Merge \"\(context.paneTitle)\" directly into \(context.targetBranch ?? "main") instead?", + title: "Parent Merge Target Unavailable", + confirmLabel: "Continue", + cancelLabel: "Cancel", + data: actionScope( + for: context, + consequence: "Confirms a fallback merge target before the host continues." + ) + ) + )) + + case (.mergeSiblingConfirmation, .cancel): + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: nil, + result: MobileActionResult( + type: "info", + message: "Merge cancelled", + title: "Sibling Agents Active" + ) + )) + + case let (.mergeFallbackConfirmation(paneID), .confirm): + guard let context = paneContext(for: paneID) else { + return .error(MobileProtocolErrorResponse( + requestID: requestID, + code: "unknown_pane", + message: "Pane \(paneID) is not published by this fixture." + )) + } + let sessionID = nextActionSessionID() + pendingActions[sessionID] = .mergeFinalConfirmation(paneID: paneID) + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: sessionID, + result: MobileActionResult( + type: "confirm", + message: "Merge \"\(context.paneTitle)\" into \(context.targetBranch ?? "main")?", + title: "Merge Worktree", + confirmLabel: "Merge", + cancelLabel: "Cancel", + data: actionScope( + for: context, + consequence: "The host performs the merge and reports the terminal result." + ) + ) + )) + + case (.mergeFallbackConfirmation, .cancel): + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: nil, + result: MobileActionResult( + type: "info", + message: "Merge cancelled", + title: "Parent Merge Target Unavailable" + ) + )) + + case let (.mergeFinalConfirmation(paneID), .confirm): + let paneTitle = paneContext(for: paneID)?.paneTitle ?? paneID + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: nil, + result: MobileActionResult( + type: "success", + message: "Merged \"\(paneTitle)\" into main.", + title: "Merge Worktree" + ) + )) + + case (.mergeFinalConfirmation, .cancel): + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: nil, + result: MobileActionResult( + type: "info", + message: "Merge cancelled", + title: "Merge Worktree" + ) + )) + + case let (.mergeUncommittedChoice(paneID), .choice(optionID)): + let paneTitle = paneContext(for: paneID)?.paneTitle ?? paneID + switch optionID { + case "commit_automatic": + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: nil, + result: MobileActionResult( + type: "error", + message: "Fixture host could not auto-commit the changes for \"\(paneTitle)\".", + title: "Merge Failed" + ) + )) + case "commit_manual": + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: nil, + result: MobileActionResult( + type: "error", + message: "Fixture host needs a manual commit before it can merge \"\(paneTitle)\".", + title: "Merge Failed" + ) + )) + case "cancel": + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: nil, + result: MobileActionResult( + type: "info", + message: "Merge cancelled", + title: "Worktree Has Uncommitted Changes" + ) + )) + default: + return .error(MobileProtocolErrorResponse( + requestID: requestID, + code: "invalid_action_response", + message: "Merge option \(optionID) is not supported by this fixture." + )) + } + case let (.rename(paneID), .input(value)): let title = value.trimmingCharacters(in: .whitespacesAndNewlines) apply { Self.retitlePane(paneID, to: title.isEmpty ? nil : title, in: $0) } @@ -341,17 +532,22 @@ public actor FixtureControlRequests: ControlRequesting { )) } let sessionID = nextActionSessionID() - pendingActions[sessionID] = .createPullRequestSummary(paneID: paneID) + pendingActions[sessionID] = .createPullRequestReview(paneID: paneID) return .actionResult(MobileActionsResultResponse( requestID: requestID, sessionID: sessionID, result: MobileActionResult( - type: "input", + type: "pr_review", message: "Review the pull request title and body before sending it.", title: "Create Pull Request", - placeholder: "Title, blank line, then body", - defaultValue: "Ship \(context.paneTitle)\n\nSummary of the fixture change.", - inputMaxVisibleLines: 6, + defaultValue: "feat(website): ship \(context.paneTitle)\n\n## Summary\n- Publish the reviewed homepage polish changes.\n\n## Changes\n- Refresh the launch copy and supporting assets.", + reviewData: MobileActionReviewData( + repoPath: context.worktreePath ?? "/fixture", + sourceBranch: context.sourceBranch ?? "feature", + targetBranch: context.targetBranch ?? "main", + files: Self.pullRequestFiles, + aiFailed: false + ), data: actionScope( for: context, consequence: "Creates a pull request on the paired host." @@ -363,12 +559,23 @@ public actor FixtureControlRequests: ControlRequesting { case (.createPullRequestConfirm, .cancel): return cancelledActionResult(requestID: requestID, title: "Create Pull Request") - case let (.createPullRequestSummary(paneID), .input(summary)): + case let (.createPullRequestReview(paneID), .input(summary)): + let parsed = Self.parsePullRequestSummary(summary) + guard !parsed.title.isEmpty else { + return .actionResult(MobileActionsResultResponse( + requestID: requestID, + sessionID: nil, + result: MobileActionResult( + type: "error", + message: "PR title cannot be empty", + title: "Create Pull Request" + ) + )) + } let paneTitle = paneContext(for: paneID)?.paneTitle ?? paneID - let trimmed = summary.trimmingCharacters(in: .whitespacesAndNewlines) - let message = trimmed.isEmpty - ? "Created a pull request for \(paneTitle)." - : "Created a pull request for \(paneTitle) with your edited summary." + let message = parsed.body.isEmpty + ? "Created PR \"\(parsed.title)\" for \(paneTitle): https://github.com/OpenCoven/psyche-build/pull/903" + : "Created PR \"\(parsed.title)\" with your edited summary: https://github.com/OpenCoven/psyche-build/pull/903" return .actionResult(MobileActionsResultResponse( requestID: requestID, sessionID: nil, @@ -379,7 +586,7 @@ public actor FixtureControlRequests: ControlRequesting { ) )) - case (.createPullRequestSummary, .cancel): + case (.createPullRequestReview, .cancel): return cancelledActionResult(requestID: requestID, title: "Create Pull Request") case let (.close(paneID), .choice(optionID)): @@ -492,10 +699,32 @@ public actor FixtureControlRequests: ControlRequesting { )) } + private static func parsePullRequestSummary(_ input: String) -> (title: String, body: String) { + let normalized = input + .replacingOccurrences(of: "\r\n", with: "\n") + .trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalized.isEmpty else { + return ("", "") + } + guard let newline = normalized.firstIndex(of: "\n") else { + return (normalized, "") + } + + let title = String(normalized[..