Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 52 additions & 2 deletions RxCode/App/AppState+PullRequest.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import os
import RxCodeCore

/// Errors surfaced while opening a pull request from a briefing card.
Expand Down Expand Up @@ -51,8 +52,7 @@ extension AppState {
let briefing = threadStore.allBranchBriefingItems()
.first(where: { $0.projectId == project.id && $0.branch == branch })?
.briefing ?? ""
let raw = await generatePullRequestContent(briefing: briefing, branch: branch)
let (title, body) = Self.parsePullRequestContent(raw, branch: branch)
let (title, body) = await generateValidatedPullRequestContent(briefing: briefing, branch: branch)

// 3. Open the PR via autopilot.
let response: CreatePullRequestResponse
Expand Down Expand Up @@ -130,6 +130,56 @@ extension AppState {
}
}

/// Generate PR content and guarantee the title is a valid Conventional
/// Commit (its `<type>` is one of ``conventionalCommitTypes``). The model
/// occasionally returns a non-conforming title (e.g. `feature:` or a plain
/// sentence); when it does we re-prompt up to `maxAttempts` times before
/// falling back to a safe `chore:` title while keeping the generated body.
func generateValidatedPullRequestContent(
briefing: String,
branch: String,
maxAttempts: Int = 3
) async -> (title: String, body: String) {
var lastBody = ""
for attempt in 1...maxAttempts {
let raw = await generatePullRequestContent(briefing: briefing, branch: branch)
let (title, body) = Self.parsePullRequestContent(raw, branch: branch)
if Self.isConventionalCommitTitle(title) {
return (title, body)
}
lastBody = body
logger.warning("PR title is not a valid Conventional Commit (attempt \(attempt)/\(maxAttempts)); retrying: \(title, privacy: .public)")
}
logger.warning("PR title still invalid after \(maxAttempts) attempts; using fallback title")
return ("chore: update \(branch)", lastBody)
}

/// Conventional Commit `<type>` tokens accepted in commit and PR titles.
/// Single source of truth shared across title generation, normalization, and
/// validation.
static let conventionalCommitTypes: Set<String> = [
"feat", "fix", "docs", "style", "refactor", "perf",
"test", "build", "ci", "chore", "revert"
]

/// True when `title` matches `<type>(<optional-scope>)<!>: <description>` and
/// `<type>` is one of ``conventionalCommitTypes``. Used to gate generated PR
/// titles so a non-conforming title triggers a model retry.
static func isConventionalCommitTitle(_ title: String) -> Bool {
let trimmed = title.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { return false }
let pattern = #"^([A-Za-z]+)(\([^)\n]+\))?!?\s*:\s+\S.*$"#
guard let regex = try? NSRegularExpression(pattern: pattern),
let match = regex.firstMatch(
in: trimmed,
range: NSRange(trimmed.startIndex..<trimmed.endIndex, in: trimmed)
),
let typeRange = Range(match.range(at: 1), in: trimmed) else {
return false
}
return conventionalCommitTypes.contains(trimmed[typeRange].lowercased())
}

/// Split generated PR text into a Conventional-Commit title and a markdown
/// body. Tolerant of code fences, heading markers, surrounding quotes, and a
/// stray `Title:` prefix. Falls back to a safe title when generation failed.
Expand Down
6 changes: 1 addition & 5 deletions RxCode/App/AppState+SessionLifecycle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -487,16 +487,12 @@ extension AppState {

// Drop leading lines that look like markdown headings or empty lines
// until we reach a Conventional Commits subject (or any plain text).
let conventionalPrefixes = [
"feat", "fix", "docs", "style", "refactor", "perf",
"test", "build", "ci", "chore", "revert"
]
var lines = text.components(separatedBy: "\n")
while let first = lines.first {
let trimmed = first.trimmingCharacters(in: .whitespaces)
let isHeading = trimmed.hasPrefix("#")
let isEmpty = trimmed.isEmpty
let startsWithType = conventionalPrefixes.contains { type in
let startsWithType = Self.conventionalCommitTypes.contains { type in
trimmed.lowercased().hasPrefix(type + ":") ||
trimmed.lowercased().hasPrefix(type + "(")
}
Expand Down
18 changes: 3 additions & 15 deletions RxCode/Services/FoundationModelSummarizationService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,21 +112,9 @@ actor FoundationModelSummarizationService {
threadSummaries: [(title: String, summary: String)]
) async -> String? {
guard !threadSummaries.isEmpty else { return nil }
let joined = threadSummaries.map { item -> String in
let title = item.title.trimmingCharacters(in: .whitespacesAndNewlines)
let summary = String(item.summary.prefix(1500)).trimmingCharacters(in: .whitespacesAndNewlines)
return "### \(title.isEmpty ? "Untitled thread" : title)\n\(summary)"
}.joined(separator: "\n\n")

let prompt = """
Write a concise overall briefing for one git branch by synthesizing the per-thread summaries below into a single coherent overview.
Cover the main themes, completed work, important decisions, files or areas touched, and unresolved follow-ups across the whole branch.
Do not list threads individually — produce a unified summary. Use 4-8 short bullet points. Reply with only the briefing.

Thread summaries (newest first):

\(joined)
"""
// Share the exact prompt used by the other providers so the categorized
// briefing format stays consistent regardless of backend.
let prompt = OpenAISummarizationService.branchBriefingPrompt(threadSummaries: threadSummaries)
let raw = await respond(
instructions: "You maintain concise local project summaries.",
prompt: prompt
Expand Down
12 changes: 9 additions & 3 deletions RxCode/Services/OpenAISummarizationService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,15 @@ actor OpenAISummarizationService {
}.joined(separator: "\n\n")

return """
Write a concise overall briefing for one git branch by synthesizing the per-thread summaries below into a single coherent overview.
Cover the main themes, completed work, important decisions, files or areas touched, and unresolved follow-ups across the whole branch.
Do not list threads individually — produce a unified summary. Use 4-8 short bullet points. Reply with only the briefing.
Write a concise briefing for one git branch by synthesizing the per-thread summaries below into a single coherent overview, grouped into clearly labelled categories so it is easy to scan.

Format rules (MUST follow exactly):
- Use GitHub-flavored markdown.
- Group related work under `###` category headings. Use ONLY these headings, in this order, and INCLUDE A HEADING ONLY when there is real work for it: Features, Fixes, Improvements, Docs, Refactors, Decisions, Follow-ups.
- Under each heading, write `- ` bullet points (1-5 per category), each a short, factual, past-tense statement. Mention the key files or areas touched inline within the relevant bullet.
- Synthesize across threads — do NOT list threads individually, and do NOT repeat the same point under more than one heading.
- Put unresolved or pending work under `### Follow-ups`.
- Reply with only the briefing markdown. No preamble, no closing remarks, no surrounding code fences.

Thread summaries (newest first):

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,21 @@ class MobileAppState @Inject constructor(
}

private suspend fun handleUnpair(fromHex: String) {
val desktop = _state.value.pairedDesktops.firstOrNull { it.pubkeyHex == fromHex } ?: return
// The unpair arrived over the relay this client is currently connected to,
// so it targets the pairing for that specific relay. Matching by pubkey
// alone would remove an entry for the same Mac on a *different* relay
// (see `PairedDesktop.matchForUnpair`).
val desktop = PairedDesktop.matchForUnpair(
desktops = _state.value.pairedDesktops,
fromHex = fromHex,
currentRelay = _state.value.relayUrl,
) ?: return
store.remove(desktop.id)
client.removePeer(fromHex)
// Only forget the crypto peer if no other pairing uses the same pubkey
// (e.g. the same Mac reached through another relay).
if (_state.value.pairedDesktops.none { it.pubkeyHex == fromHex && it.id != desktop.id }) {
client.removePeer(fromHex)
}
}

private fun handleSnapshot(fromHex: String, snap: Payload.Snapshot) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,34 @@ data class PairedDesktop(
val relayUrl: String? = null,
) {
val id: String
get() {
val normRelay = (relayUrl ?: "")
.trim()
.lowercase()
.trim('/')
return "$pubkeyHex::$normRelay"
get() = compositeId(pubkeyHex, relayUrl)

companion object {
/** Normalize a relay URL for comparison (trim whitespace/slashes, lowercase). */
fun normalizeRelay(relayUrl: String?): String =
(relayUrl ?: "").trim().lowercase().trim('/')

/** Build the composite id (pubkey + normalized relay) without an instance. */
fun compositeId(pubkeyHex: String, relayUrl: String?): String =
"$pubkeyHex::${normalizeRelay(relayUrl)}"

/**
* Selects the pairing an inbound unpair targets. The unpair arrives over
* the relay this client is currently connected to, so it identifies the
* entry for that specific relay — matching by pubkey alone would remove an
* entry for the same Mac on a *different* relay. Falls back to the sole
* pairing for a Mac when there is only one (covers legacy entries that
* predate stored relay URLs), and returns null when the choice is
* ambiguous so we never remove the wrong relay's entry.
*/
fun matchForUnpair(
desktops: List<PairedDesktop>,
fromHex: String,
currentRelay: String?,
): PairedDesktop? {
val samePubkey = desktops.filter { it.pubkeyHex == fromHex }
val targetId = compositeId(fromHex, currentRelay)
return samePubkey.firstOrNull { it.id == targetId } ?: samePubkey.singleOrNull()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,9 @@ private fun PairingCameraScreen(
var scanLocked by remember { mutableStateOf(false) }
var scannerKey by remember { mutableStateOf(0) }
var scanError by remember { mutableStateOf<String?>(null) }
// Manual scanning: the camera previews continuously, but barcode analysis
// only runs after the user taps "Scan" (not automatically on every frame).
var scanArmed by remember { mutableStateOf(false) }

LaunchedEffect(camPermission.status.isGranted) {
pairingLog("camera permission granted=${camPermission.status.isGranted}")
Expand Down Expand Up @@ -430,17 +433,20 @@ private fun PairingCameraScreen(
QRCameraPreview(
modifier = Modifier.fillMaxSize(),
scanKey = scannerKey,
enabled = !scanLocked && pairing !is PairingStatus.InProgress,
enabled = scanArmed && !scanLocked && pairing !is PairingStatus.InProgress,
onToken = { token ->
pairingLog("camera QR accepted: ${token.logSummary()}")
scanLocked = true
scanArmed = false
scanError = null
onToken(token)
},
onInvalidQr = {
if (!scanLocked) {
Log.w(TAG, "camera QR rejected: not an RxCode pairing token")
scanError = "That QR code is not an RxCode pairing code."
// Stop scanning and let the user reposition and tap again.
scanArmed = false
scanError = "That QR code is not an RxCode pairing code. Tap Scan to try again."
}
},
)
Expand Down Expand Up @@ -477,11 +483,18 @@ private fun PairingCameraScreen(
PairingCameraStatus(
pairing = pairing,
scanError = scanError,
scanArmed = scanArmed,
onScan = {
pairingLog("user armed camera QR scan")
scanError = null
scanArmed = true
},
onRetry = {
pairingLog("retrying camera QR scan")
onRetry()
scanError = null
scanLocked = false
scanArmed = false
scannerKey += 1
},
modifier = Modifier
Expand Down Expand Up @@ -512,6 +525,8 @@ private fun CameraShade() {
private fun PairingCameraStatus(
pairing: PairingStatus,
scanError: String?,
scanArmed: Boolean,
onScan: () -> Unit,
onRetry: () -> Unit,
modifier: Modifier = Modifier,
) {
Expand All @@ -524,8 +539,22 @@ private fun PairingCameraStatus(
Column(Modifier.padding(18.dp), verticalArrangement = Arrangement.spacedBy(10.dp)) {
when (pairing) {
PairingStatus.Idle -> {
Text("Align the QR code inside the frame.", style = MaterialTheme.typography.titleMedium)
scanError?.let { Text(it, color = MaterialTheme.colorScheme.error) }
if (scanArmed) {
Row(verticalAlignment = Alignment.CenterVertically) {
Icon(Icons.Outlined.QrCodeScanner, contentDescription = null, tint = MaterialTheme.colorScheme.primary)
Spacer(Modifier.width(10.dp))
Text("Scanning…", style = MaterialTheme.typography.titleMedium)
}
LinearProgressIndicator(modifier = Modifier.fillMaxWidth())
} else {
Text("Position the QR code inside the frame, then tap Scan.", style = MaterialTheme.typography.titleMedium)
scanError?.let { Text(it, color = MaterialTheme.colorScheme.error) }
Button(onClick = onScan, modifier = Modifier.fillMaxWidth()) {
Icon(Icons.Outlined.QrCodeScanner, contentDescription = null)
Spacer(Modifier.width(8.dp))
Text("Scan")
}
}
}
PairingStatus.InProgress -> {
Row(verticalAlignment = Alignment.CenterVertically) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package app.rxlab.rxcode.store

import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Test

/**
* Regression tests for relay-aware unpair matching. The bug: when the same Mac
* was paired through two relays, an inbound unpair matched by pubkey alone and
* removed the *other* relay's entry. [PairedDesktop.matchForUnpair] must resolve
* the entry for the relay the unpair actually arrived on.
*/
class PairedDesktopUnpairTest {

private fun desktop(pubkey: String, relay: String?) =
PairedDesktop(pubkeyHex = pubkey, displayName = "Mac", pairedAtEpochMs = 0L, relayUrl = relay)

@Test
fun picksEntryForArrivingRelay() {
val a = desktop("PUB", "wss://relay1.example.com/ws")
val b = desktop("PUB", "wss://relay2.example.com/ws")

val match = PairedDesktop.matchForUnpair(listOf(a, b), "PUB", "wss://relay2.example.com/ws")

assertEquals(b.id, match?.id)
}

@Test
fun normalizesRelayBeforeMatching() {
val a = desktop("PUB", "wss://relay1.example.com/ws")
val b = desktop("PUB", "wss://relay2.example.com/ws")

val match = PairedDesktop.matchForUnpair(listOf(a, b), "PUB", "WSS://Relay2.Example.com/ws/")

assertEquals(b.id, match?.id)
}

@Test
fun singleEntryFallbackForLegacyPairing() {
val legacy = desktop("PUB", null)

val match = PairedDesktop.matchForUnpair(listOf(legacy), "PUB", "wss://relay1.example.com/ws")

assertEquals(legacy.id, match?.id)
}

@Test
fun ambiguousRelayDoesNotGuess() {
val a = desktop("PUB", "wss://relay1.example.com/ws")
val b = desktop("PUB", "wss://relay2.example.com/ws")

val match = PairedDesktop.matchForUnpair(listOf(a, b), "PUB", "wss://relay3.example.com/ws")

assertNull(match)
}

@Test
fun ignoresOtherMacs() {
val mine = desktop("PUB", "wss://relay1.example.com/ws")
val other = desktop("OTHER", "wss://relay1.example.com/ws")

val match = PairedDesktop.matchForUnpair(listOf(mine, other), "PUB", "wss://relay1.example.com/ws")

assertEquals(mine.id, match?.id)
}

@Test
fun noMatchForUnknownPubkey() {
val a = desktop("PUB", "wss://relay1.example.com/ws")

val match = PairedDesktop.matchForUnpair(listOf(a), "NOPE", "wss://relay1.example.com/ws")

assertNull(match)
}
}
10 changes: 9 additions & 1 deletion RxCodeMobile/State/MobileAppState+Inbound.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,15 @@ extension MobileAppState {
failPairing(String(localized: "Your Mac declined the pairing request."))
}
case .unpair:
guard let desktop = pairedDesktops.first(where: { $0.pubkeyHex == inbound.fromHex }) else { return }
// The unpair arrived over the relay this client is currently connected
// to, so it targets the pairing for that specific relay. Matching by
// pubkey alone would remove an entry for the same Mac on a *different*
// relay (see `PairedDesktop.matchForUnpair`).
guard let desktop = PairedDesktop.matchForUnpair(
in: pairedDesktops,
fromHex: inbound.fromHex,
currentRelay: relayURL.absoluteString
) else { return }
Task { await self.removePairedDesktopAfterRemoteUnpair(desktop) }
case .snapshot(let snap):
guard acceptsActiveDesktopPayload(from: inbound.fromHex, type: "snapshot") else { return }
Expand Down
Loading
Loading