Skip to content

Merge permission endpoints#194

Merged
gsabran merged 1 commit intomainfrom
merge-permission-endpoints
Oct 26, 2025
Merged

Merge permission endpoints#194
gsabran merged 1 commit intomainfrom
merge-permission-endpoints

Conversation

@gsabran
Copy link
Copy Markdown
Collaborator

@gsabran gsabran commented Oct 26, 2025

sendMessage/toolUse/permission -> sendMessage/toolUse/permission/acp

This simplifies the code, and resolves an issue where calls from Codex would not be routed correctly

@gsabran gsabran requested a review from Copilot October 26, 2025 03:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR consolidates permission-related endpoints by merging sendMessage/toolUse/permission into sendMessage/toolUse/permission/acp. The change simplifies routing logic and fixes an issue where calls from Codex were not being routed correctly.

Key Changes:

  • Removed the old /sendMessage/toolUse/permission endpoint and migrated functionality to /sendMessage/toolUse/permission/acp
  • Extracted pendingToolApprovalRequests into a shared module to enable reuse across implementations
  • Updated all client code and tests to use the new unified endpoint

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
local-server/src/server/server.ts Renamed import from plural registerACPClientEndpoints to singular registerACPClientEndpoint
local-server/src/server/endpoints/sendMessage/sendMessage.ts Removed registerClaudeCodeEndpoint import and registration call
local-server/src/server/endpoints/sendMessage/pendingToolApprovalRequests.ts New shared module exporting pendingToolApprovalRequests map
local-server/src/server/endpoints/sendMessage/claudeCode/sendMessageToClaudeCode.ts Removed local pendingToolApprovalRequests and endpoint registration, now imports from shared module
local-server/src/server/endpoints/sendMessage/claudeCode/tests/sendMessageToClaudeCode.test.ts Updated tests to import registerEndpoint from ACPClient and reference new endpoint path
local-server/src/server/endpoints/sendMessage/acp/clients/ACPClient.ts Replaced local pendingToolApprovalRequests with import from shared module
local-server/build.size Reflects reduced bundle size from code consolidation
local-server/build.sha256 Updated build hash
app/modules/services/LLMService/Tests/RequestStreamingHelperTests.swift Updated test to expect new endpoint path
app/modules/services/LLMService/Sources/RequestStreamingHelper.swift Removed conditional routing logic, now always uses /acp endpoint

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gsabran gsabran enabled auto-merge (squash) October 26, 2025 03:22
@gsabran gsabran merged commit 736a2eb into main Oct 26, 2025
12 checks passed
@gsabran gsabran deleted the merge-permission-endpoints branch October 26, 2025 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants