fix(acp-server): include full bash command in permission requests - #3160
fix(acp-server): include full bash command in permission requests#3160jangjoe wants to merge 1 commit into
Conversation
ACP session/request_permission for a Bash tool call used to show only the first 50 characters of args.command (followed by an ellipsis when longer), because displayBlockToAcpContent did not recognise the `command` ToolInputDisplay kind and silently dropped display.command. Add a `kind: 'command'` branch that projects block.command into a text content entry on the wire so clients see the full command in the approval card. The 50-character action preview is unchanged. Resolve MoonshotAI#3106
🦋 Changeset detectedLatest commit: 2594b7b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2594b7b8e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (block.kind === 'command') { | ||
| return { type: 'content', content: { type: 'text', text: block.command } }; |
There was a problem hiding this comment.
When any Bash call reaches tool.call.started, this shared converter is also called by toolCallStartToSessionUpdate and toolCallStartedUpgradeToSessionUpdate in events-map.ts, so the ordinary tool card now receives the full command followed by the existing JSON-encoded arguments containing that same command; streamed calls can receive it again during the upgrade. This change is intended for session/request_permission, so add the command entry in buildPermissionToolCallUpdate rather than changing the converter used by normal tool-call notifications.
Useful? React with 👍 / 👎.
1 participant
Resolve #3106
ACP
session/request_permissionfor a Bash tool call used to show onlythe first 50 characters of
args.command(followed by an ellipsis whenlonger), because
displayBlockToAcpContentdid not recognise thecommandToolInputDisplaykind and silently droppeddisplay.command.Add a
kind: 'command'branch indisplayBlockToAcpContentthat projectsblock.commandinto a text content entry on the wire so clients see thefull command in the approval card. The 50-character
actionpreview thatthe engine packs into
ApprovalRequest.actionis unchanged.Test plan
packages/acp-server/test/convert.test.ts— newdescribe('displayBlockToAcpContent')block covering diff / file_io (both / one-sided) / plan_review (empty / with path) / command (short and longer-than-50) / generic-null.packages/acp-server/test/approval.test.ts—buildPermissionToolCallUpdatewith a long command asserts the full command lands incontent[0]and the 50-char preview stays in the trailingcontent.at(-1).packages/acp-server/test/e2e-turn.test.ts— end-to-end scripted LLM issues a Bash call; the capturedsession/request_permissionpayload'stoolCall.content[]contains the full command.