Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR adds Agent Client Protocol (ACP) support to RxCode, including registry-backed client installation/configuration, model discovery, and UI integration so ACP clients appear in the model picker and can power chat sessions.
Changes:
- Add ACP client management UI (installed clients + registry browser + editor sheet) and integrate ACP client icons into pickers.
- Introduce ACP runtime services: registry fetch + caching, binary installer, and an ACP JSON-RPC-over-stdio service with model probing/discovery events.
- Extend persistence/session plumbing to support ACP-originated sessions and persist ACP client specs + registry snapshots.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| RxCode/Views/SettingsView.swift | Adds a new “ACP Clients” settings tab; updates model picker section IDs/labels. |
| RxCode/Views/Settings/ACPClientSettingsTab.swift | New UI for installing, enabling, editing, and removing ACP clients + registry search. |
| RxCode/Views/MainView.swift | Updates model picker UI to handle per-section IDs/titles and show ACP client icons. |
| RxCode/Views/ACPIconView.swift | Adds remote icon loading + disk/memory caching for ACP client/agent icons. |
| RxCode/Services/PersistenceService.swift | Adds ACP session origin routing + persistence for ACP clients and registry snapshot URL. |
| RxCode/Services/ACPService.swift | New ACP transport implementation (process spawn, JSON-RPC framing, streaming events, model probing). |
| RxCode/Services/ACPRegistryService.swift | New service to fetch and snapshot the ACP registry with TTL caching. |
| RxCode/Services/ACPInstallerService.swift | New installer to download/extract ACP binary distributions into Application Support. |
| RxCode/App/RxCodeApp.swift | Adds ACP provider handling in menu bar status display. |
| RxCode/App/AppState.swift | Integrates ACP provider: client persistence, registry refresh, model label resolution, stream routing. |
| Packages/Sources/RxCodeCore/Models/StreamEvent.swift | Adds an event for “ACP models discovered” to sync picker state. |
| Packages/Sources/RxCodeCore/Models/SessionOrigin.swift | Adds .acpAgent session origin. |
| Packages/Sources/RxCodeCore/Models/AgentModel.swift | Adds AgentProvider.acp and default session origin mapping. |
| Packages/Sources/RxCodeCore/Models/ACPClient.swift | Adds ACP registry schema + installed client spec models. |
| Packages/Sources/RxCodeChatKit/StatusLineView.swift | Adds ACP provider label/version handling. |
| Packages/Sources/RxCodeChatKit/PlanCardView.swift | Keeps “plan ready” text visible alongside plan card. |
| Packages/Sources/RxCodeChatKit/MessageListView.swift | Stops suppressing plan-ready followups (but leaves some dead logic). |
Comments suppressed due to low confidence (1)
Packages/Sources/RxCodeChatKit/MessageListView.swift:338
_ = hasRecentPlanCardis now a no-op and leaves a dead variable (hasRecentPlanCard) in the function. Since plan-ready followups are intentionally no longer suppressed, consider removinghasRecentPlanCardand the related (now-unused) code paths entirely to avoid confusion and keep the logic easy to follow.
let hasExitPlan = assistantRun.contains { PlanCardView.containsExitPlanMode($0) }
var hasRecentPlanCard = false
for message in assistantRun {
if hasExitPlan && PlanCardView.isPurePlanFileWriteMessage(message) {
continue
}
// Plan-ready follow-up messages (e.g. "Plan is ready at /…/plans/foo.md")
// are intentionally kept visible alongside the plan card so the user
// sees the summary while approval is pending.
_ = hasRecentPlanCard
if PlanCardView.containsExitPlanMode(message) {
hasRecentPlanCard = true
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+71
to
+78
| actor ACPIconCache { | ||
| static let shared = ACPIconCache() | ||
| private var cache: [URL: NSImage] = [:] | ||
|
|
||
| func image(for url: URL) -> NSImage? { cache[url] } | ||
|
|
||
| func store(_ image: NSImage, for url: URL) { cache[url] = image } | ||
|
|
Comment on lines
+60
to
+67
| do { | ||
| let (data, _) = try await URLSession.shared.data(from: parsed) | ||
| guard let nsImage = ACPIconCache.makeImage(from: data, sourceURL: parsed) else { return } | ||
| await ACPIconCache.shared.store(nsImage, for: parsed) | ||
| image = nsImage | ||
| } catch { | ||
| // Fall back to placeholder; nothing to log noisily about. | ||
| } |
Comment on lines
+525
to
+528
| return (path, args, env) | ||
| case .custom(let command, let args, let env): | ||
| return (command, args, env) | ||
| } |
Comment on lines
+885
to
+894
| private func handleFsReadTextFile(streamId: UUID, id: JSONValue, params: JSONValue) async { | ||
| guard let path = params.objectValue?["path"]?.stringValue else { | ||
| sendError(streamId: streamId, id: id, code: -32602, message: "Missing path") | ||
| return | ||
| } | ||
| do { | ||
| let line = params.objectValue?["line"]?.intValue | ||
| let limit = params.objectValue?["limit"]?.intValue | ||
| let content = try Self.readTextFile(path: path, line: line, limit: limit) | ||
| sendResult(streamId: streamId, id: id, result: .object(["content": .string(content)])) |
Comment on lines
+900
to
+911
| private func handleFsWriteTextFile(streamId: UUID, id: JSONValue, params: JSONValue) async { | ||
| guard let path = params.objectValue?["path"]?.stringValue, | ||
| let content = params.objectValue?["content"]?.stringValue else { | ||
| sendError(streamId: streamId, id: id, code: -32602, message: "Missing path or content") | ||
| return | ||
| } | ||
| do { | ||
| try Self.writeTextFile(path: path, content: content) | ||
| sendResult(streamId: streamId, id: id, result: .object([:])) | ||
| } catch { | ||
| sendError(streamId: streamId, id: id, code: -32000, message: error.localizedDescription) | ||
| } |
Comment on lines
+46
to
+50
| static func installRoot() -> URL { | ||
| let fm = FileManager.default | ||
| let support = fm.urls(for: .applicationSupportDirectory, in: .userDomainMask).first! | ||
| return support.appendingPathComponent("RxCode/acp-binaries", isDirectory: true) | ||
| } |
Contributor
Author
|
🎉 This PR is included in version 1.3.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.