-
Notifications
You must be signed in to change notification settings - Fork 14.9k
feat(acp): add opt-in flag for question tool #13562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
rekram1-node
merged 5 commits into
anomalyco:dev
from
ImmuneFOMO:feat/acp-question-tool-opt-in
Feb 16, 2026
Merged
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
fc5bf9a
feat(acp): add opt-in flag for question tool
ImmuneFOMO e6217bc
Merge branch 'dev' into feat/acp-question-tool-opt-in
ImmuneFOMO fd3f58c
feat(acp): rename question flag to experimental
ImmuneFOMO d6d2698
Merge branch 'dev' into feat/acp-question-tool-opt-in
rekram1-node 9042851
tweak
rekram1-node File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isnt really ACP specific tho so I'd change the tool name.
Also can u make it experimental? I imagine we'd have better config for this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clarification on "make it experimental": should this be (1) explicit opt-in only via OPENCODE_EXPERIMENTAL_QUESTION_TOOL, or (2) also enabled when OPENCODE_EXPERIMENTAL=1? I implemented option 1 for now to keep ACP default unchanged, but I can switch to option 2 if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rekram1-node Thanks for the review. I updated everything you asked for. One thing I still do not fully understand, what did you mean by “This isnt really ACP specific”? In the current logic, this flag only affects ACP behavior, while app/cli/desktop keeps the question tool enabled as before. Since the flag only changes ACP behavior, an ACP in the name seemed logical to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flag should be about enabling the question tool, it doesnt have anything to do w/ acp at all.
The cli, desktop, app are all clients, but we also have an sdk. So there are a lot of places or other clients that may not have question tool enabled by default. So this would be about enabling question tool, it shouldnt be tied to acp.
And uh Ig keep the flag separate from that other experimental one...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ig thats kinda messy, so it doesnt have to be experimental but hopefully im making sense?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rekram1-node Just to confirm I understood correctly: you want a generic flag like OPENCODE_ENABLE_QUESTION_TOOL (not ACP-specific and not tied to OPENCODE_EXPERIMENTAL), and the registry logic should be: keep question enabled by default for app/cli/desktop, plus enable it for any other client when this flag is set. Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rekram1-node just reminding, in case you lost this message