Discord: accept inline /acp action args via autocomplete#32800
Discord: accept inline /acp action args via autocomplete#32800zwright8 wants to merge 4 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
bot
added
channel: discord
Greptile SummaryThis PR switches the Key changes:
The logic is correct and the fix is well-scoped. One minor architectural concern: the generic Confidence Score: 4/5
Last reviewed commit: 6e7e168 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const isAcpActionArg = command.key === "acp" && arg.name === "action"; | ||
| const shouldAutocomplete = | ||
| resolvedChoices.length > 0 && | ||
| (typeof arg.choices === "function" || resolvedChoices.length > 25); | ||
| isAcpActionArg || | ||
| (resolvedChoices.length > 0 && | ||
| (typeof arg.choices === "function" || resolvedChoices.length > 25)); |
There was a problem hiding this comment.
Command-specific logic in a generic builder
buildDiscordCommandOptions is a generic helper, but isAcpActionArg introduces a hardcoded "acp" key check for a single command. If other commands need the same "prefer autocomplete over static choices" behaviour in the future, this will grow into an increasing list of special cases.
A more extensible approach would be to add an optional flag to CommandArgDefinition (e.g. preferAutocomplete?: boolean) and have the /acp command set it in the registry. That keeps the generic builder free of command-specific knowledge and makes the opt-in explicit and discoverable.
This is a style/architecture suggestion rather than a bug, so the current code is functionally correct.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/monitor/native-command.ts
Line: 118-122
Comment:
**Command-specific logic in a generic builder**
`buildDiscordCommandOptions` is a generic helper, but `isAcpActionArg` introduces a hardcoded `"acp"` key check for a single command. If other commands need the same "prefer autocomplete over static choices" behaviour in the future, this will grow into an increasing list of special cases.
A more extensible approach would be to add an optional flag to `CommandArgDefinition` (e.g. `preferAutocomplete?: boolean`) and have the `/acp` command set it in the registry. That keeps the generic builder free of command-specific knowledge and makes the opt-in explicit and discoverable.
This is a style/architecture suggestion rather than a bug, so the current code is functionally correct.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Addressed in the latest head commit ( I removed the command-specific
This keeps the builder generic and makes autocomplete preference explicit/discoverable per argument. |
|
Superseded by #33136, which consolidates the /acp inline action autocomplete fix plus the bound-thread bot system message guard. Closing in favor of that PR. |
2 participants
Summary
/acpactionoption wiring from static choices to autocomplete/acp close)/acpuses autocomplete and non-ACP action args still use static choicesWhy
With static choices on
/acp action, Discord can returnaction=nullwhen users type inline without selecting from the dropdown, which triggers the fallback button menu. Using autocomplete preserves suggestions but allows direct inline action input.Closes #32753.
Testing
pnpm exec oxfmt --check src/discord/monitor/native-command.ts src/discord/monitor/native-command.options.test.tspnpm exec vitest run src/discord/monitor/native-command.options.test.ts