Repository navigation
Conversation
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 14:33
3af19e5 to
2711bbe
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 11, 2026 14:34
7c162ba to
593c4fe
Compare
Contributor
|
Commit: Regression test reportSelected 12 changed tests (limit=20, 0 not selected) 12 tests: regression=8 unreleased=0 coverage=4 failing=0
TestAccept/cmd/aitools/install-host-metadata: branch ✅ | main (8b4f052) ❌ | latest (v1.26.0) ❌main (8b4f052): latest (v1.26.0): TestAccept/cmd/aitools/telemetry/apps-init-tip: branch ✅ | main (8b4f052) ❌ | latest (v1.26.0) ❌main (8b4f052): latest (v1.26.0): TestAccept/cmd/aitools/telemetry/hint: branch ✅ | main (8b4f052) ❌ | latest (v1.26.0) ❌main (8b4f052): latest (v1.26.0): TestAccept/cmd/aitools/telemetry/install: branch ✅ | main (8b4f052) ❌ | latest (v1.26.0) ❌main (8b4f052): latest (v1.26.0): TestAccept/cmd/aitools/telemetry/install-failure: branch ✅ | main (8b4f052) ❌ | latest (v1.26.0) ❌main (8b4f052): latest (v1.26.0): TestAccept/cmd/aitools/telemetry/update-uninstall: branch ✅ | main (8b4f052) ❌ | latest (v1.26.0) ❌main (8b4f052): latest (v1.26.0): TestAccept/cmd/aitools/telemetry/workspace-unreachable: branch ✅ | main (8b4f052) ❌ | latest (v1.26.0) ❌main (8b4f052): latest (v1.26.0): TestAccept/cmd/version: branch ✅ | main (8b4f052) ❌ | latest (v1.26.0) ❌main (8b4f052): latest (v1.26.0): |
Collaborator
✅ Integration test reportCommit: 2a658e6
Top 12 slowest tests (at least 2 minutes):
|
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 16:53
2711bbe to
95f78a5
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 11, 2026 16:53
593c4fe to
deca41f
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 16:54
95f78a5 to
668167b
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 11, 2026 16:55
deca41f to
e980d56
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 16:57
668167b to
eef5046
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 11, 2026 16:57
e980d56 to
e155c53
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 17:26
eef5046 to
d0c5cc6
Compare
simonfaltum
reviewed
Nov 11, 2026
simonfaltum
left a comment
simonfaltum
left a comment
Member
There was a problem hiding this comment.
🤖 Automated review from simonfaltum's review agent (pr-review skill) at e155c53. Each finding was verified against the code; simonfaltum may follow up.
Not approving yet: 1 blocking issue(s), tagged [blocking]; 2 open design questions.
This PR removes the interactive Proceed? confirm after the agent picker in databricks aitools install. The picker selection now counts as confirmation, the funnel cancel test moves to the picker, and a changelog fragment is added. The commit itself is correct: the picker already hides agents that would be skipped. However, the branch is built on an outdated revision of #7477. It conflicts with the current base, its diff undoes #7477's later fixes, and after a rebase the tests stop compiling until the leftover promptProceed override in TestInstallAgentCallerInPTYNeverPrompts is removed. There are also two open design questions: whether losing the clean decline is intended, and what to do with the plan summary and PLAN_CONFIRMED step that remain.
Questions
- With the confirm gone, an interactive user who sees the plan can no longer back out cleanly. Ctrl-C at the picker exits non-zero (USER_ABORTED), and an empty selection errors, where answering
nused to print "Cancelled." and exit 0. Detected agents start checked, so a single Enter now installs plugins, writes state and cleans up legacy skills.aitools uninstallandcompletion installstill askProceed?. Is losing the clean decline intended? Would you rather keep the confirm, drop it as here, or drop it and show the install scope in the picker title so the scope is still visible before anything runs? - Now that nothing follows the plan summary,
printPlanSummaryrepeats the picker right before the per-agent install lines, and its skip branch can no longer run.PLAN_CONFIRMEDnow fires unconditionally right afterAGENTS_SELECTED, so the new funnel can't show how many users decline at the plan, and the proto comment ("confirmed, by the user or implicitly") no longer describes a real step. Should the summary stay as it is, shrink to just the scope line, or go? And shouldPLAN_CONFIRMEDbe kept, dropped or renamed, with the proto comment and any dashboards updated?
Comments outside the changed lines
- [nice to have]
.github/PULL_REQUEST_TEMPLATE.md:.agents/skills/pr-checklist/SKILL.md:48says to follow.github/PULL_REQUEST_TEMPLATE.mdexactly, with## Changes,## Whyand## Testsin that order and no invented sections such as## Summary. This body uses## Summary,## Evidenceand## Merge Danger, and the agent-disclosure line is not italic as line 54 asks. Please restructure the body under the template headings (the evidence fits under## Tests, the merge-danger notes under## Why) and italicize the disclosure line. - [nit]
cmd/aitools/install.go:184: The comment above the--output jsonflag check still says the flags are required "so no scope prompt, agent picker, or confirm is shown". Install no longer has a confirm, so a reader will look for a prompt that doesn't exist. Drop "or confirm". - [nit]
cmd/aitools/funnel_test.go:277:TestInstallFunnelEmptyPickerSelectionIsCancelledsays it "finishes CANCELLED like a declined confirm", but install no longer has a declined confirm. The(there is no "Proceed?" confirm)note at lines 239-240 also describes removed code rather than the test, so after merge it reads as history. Consider rewording both, for example: the picker is the last prompt before the install. SinceTestInstallFunnelInteractiveCancelnow drives the picker with an error, this test could also become a third row in that table:{name: "nothing selected", err: errNoAgentsSelected, want: "FINISHED:CANCELLED:"}.
| } | ||
| } | ||
|
|
||
| func TestInstallAgentCallerInPTYNeverPrompts(t *testing.T) { |
Member
There was a problem hiding this comment.
[blocking] This branch is built on an older revision of #7477 (eef5046). aitools/no-prompts-for-agents has since been force-pushed (now d0c5cc6), and GitHub reports the PR as conflicting. Measured against the current base, the diff also undoes #7477's review fixes: it brings back the callerIsAgent seam and main_test.go's TestMain, drops startFunnel's interactive argument (agent callers in a PTY are reported Interactive=true again), removes isolateAgents(t) calls and the SDK source-URL comment, and restores stubLegacyCleanup. A rebase alone won't be enough: the new base's TestInstallAgentCallerInPTYNeverPrompts overrides promptProceed with t.Fatal("plan confirm shown to an agent caller"), and that override merges without a textual conflict, but this PR deletes promptProceed. I simulated the rebase and resolved the rename conflict, and go vet ./cmd/aitools/ then fails with install_test.go:873:17: undefined: promptProceed. Please rebase with git rebase --onto origin/aitools/no-prompts-for-agents eef5046e4, delete that override, and keep the base's isolateAgents(t) at the top of TestInstallInteractivePickerWithoutConfirm. CI is green only because it ran on the stale stack.
Contributor
Author
There was a problem hiding this comment.
Rebased onto the current aitools/no-prompts-for-agents (a791ac7). The diff against the base now only touches the confirm: the promptProceed override in TestInstallAgentCallerInPTYNeverPrompts is gone, isolateAgents(t) stays at the top of the picker test, and TestInstallFunnelInteractiveCancel follows the base's new funnel order (AGENTS_SELECTED fires after the picker, so a picker cancel ends at SCOPE_RESOLVED) and its "prompt failed" row passes an error already tagged by promptError, the way the real picker returns it. go vet ./cmd/aitools/ and go test ./cmd/aitools/... pass.
| } | ||
|
|
||
| func TestInstallInteractivePickerAndConfirm(t *testing.T) { | ||
| func TestInstallInteractivePickerWithoutConfirm(t *testing.T) { |
Member
There was a problem hiding this comment.
[nit] As the PR description notes, TestInstallInteractivePickerWithoutConfirm also passes with the old confirm in place: without a terminal the confirm reads stdin and defaults to Yes. Its name and the // No "Proceed?" confirm follows the picker comment at line 379 claim something it can't detect. Either name it after what it checks (the picked agent gets the plugin) or make it fail if a prompt runs after the picker.
Contributor
Author
There was a problem hiding this comment.
Renamed it to TestInstallInteractivePickerInstallsPickedAgent and dropped the // No "Proceed?" confirm follows the picker comment (a791ac7). Making it fail when a prompt runs after the picker would need a hook that this PR removes, so the name now says what it checks. On the parent it fails only without a controlling terminal (huh: could not open a new TTY); with one, the old confirm reads stdin and defaults to Yes.
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
2 times, most recently
from
November 11, 2026 19:30
2e61f20 to
6251610
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
2 times, most recently
from
November 11, 2026 19:42
a791ac7 to
a45b450
Compare
Contributor
Author
|
Answers to the two questions in the review (a45b450):
The two wording nits are also fixed: the |
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 20:07
6251610 to
bc7b149
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 11, 2026 20:07
a45b450 to
eb589c2
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 21:04
bc7b149 to
4a46f77
Compare
Contributor
Author
|
Follow-ups to the two questions (24a70e8):
PR body: I've kept the current format. |
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 22:27
4a46f77 to
ab35f36
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 11, 2026 22:27
24a70e8 to
bf2f7d3
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 22:28
ab35f36 to
a475e72
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 11, 2026 22:32
bf2f7d3 to
a358695
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 22:39
a475e72 to
a1aa2c3
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 11, 2026 22:39
a358695 to
627fc32
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 11, 2026 22:59
a1aa2c3 to
4f96c90
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 11, 2026 22:59
627fc32 to
7aa11b6
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 12, 2026 00:21
4f96c90 to
a4e1e52
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 12, 2026 00:21
7aa11b6 to
473c8b4
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 12, 2026 00:27
a4e1e52 to
42b3163
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 12, 2026 00:27
473c8b4 to
1353f9b
Compare
simonfaltum
approved these changes
Nov 12, 2026
simonfaltum
left a comment
simonfaltum
left a comment
Member
There was a problem hiding this comment.
🤖 Automated review from simonfaltum's review agent (pr-review skill) at 1353f9b. Each finding was verified against the code; simonfaltum may follow up.
No blocking issues found, approving. 5 non-blocking comment(s) below.
This PR drops the Proceed? confirm after the interactive agent picker in databricks aitools install. It also shows the install scope in the picker title, shrinks the post-picker summary to one scope line, and labels plugin agents "skills" under --skills-only. The change is small, CI and the aitools and telemetry tests pass at this head, and the earlier review threads are all addressed. What remains is non-blocking: a --skills-only preselect mismatch for agents whose CLI is missing, skip rows that still say "plugin", a changelog line that overstates the relabel, two small test-style nits, and the PR-template format from the earlier round.
Agreeing with existing threads
- [nice to have] PR body does not follow the repo PR template: #7478 (review)
| label := agentStateLabel(a.DisplayState(ctx)) | ||
| state := a.DisplayState(ctx) | ||
| label := agentStateLabel(state) | ||
| if item.delivery == deliverySkills && (state == agents.StateAvailable || state == agents.StateInstalledCLIMissing) { |
Member
There was a problem hiding this comment.
[nice to have] With --skills-only, a plugin agent whose config dir exists but whose CLI is not on PATH is now labeled skills but still starts unchecked, because IsPreselected (libs/aitools/agents/detect.go:82) only pre-checks plugin agents in StateAvailable. Before this change the row read plugin · CLI not found, which at least explained the empty box. Example: ~/.claude and ~/.cursor exist, claude is not on PATH, and the user runs databricks aitools install --skills-only (or the databricks skills install alias) in a terminal. The picker shows Claude Code skills unchecked next to Cursor skills checked, and Enter, which now starts the install, leaves Claude Code out. A non-interactive --skills-only run would include it, because selectAgents detects by config dir there. Suggest pre-checking skills-delivery rows by config dir in agentChoices, for example preselect: a.IsPreselected(ctx) || (skillsOnly && a.Detected(ctx)), so the picker default matches the non-interactive run.
Contributor
Author
There was a problem hiding this comment.
Done in da4d9e2. In agentChoices, preselect is now a.IsPreselected(ctx) || (skillsOnly && a.Detected(ctx)), so under --skills-only a plugin agent whose CLI is not on PATH starts checked, matching the non-interactive run. Plugin delivery is unchanged. TestAgentChoicesSkillsOnlyPreselectsByConfigDir covers it with ~/.claude present and no claude on PATH, and it fails without the change.
Comment on lines
433
to
434
| if item.delivery == deliverySkip { | ||
| return label + " · " + item.reason |
Member
There was a problem hiding this comment.
[nit] The --skills-only relabel only covers rows that get skills delivery, so skipped plugin agents still say plugin. With codex on PATH, databricks aitools install --skills-only --scope project prints Codex CLI plugin · does not support project-scoped skills (GitHub Copilot too). The user asked for skills only, the row names the plugin, and the reason talks about skills. Suggest passing skillsOnly into agentChoiceLabel and, when it is set, labeling every found agent with deliverySkills.String(). For plugin agents, StateAvailable || StateInstalledCLIMissing is the same as state != agents.StateNotFound. That also replaces the new "skills" literal, which .agents/rules/style-guide-go.md asks to avoid when the package already has the value. A ScopeProject case in TestAgentChoicesSkillsOnlyLabelsPluginAgentsSkills would pin it.
Contributor
Author
There was a problem hiding this comment.
Done in da4d9e2. agentChoiceLabel now takes skillsOnly and, when it is set, labels every found agent (state != agents.StateNotFound) with deliverySkills.String(). That covers skipped rows and replaces the "skills" literal. TestAgentChoicesSkillsOnlyLabelsSkippedPluginAgentSkills checks that Codex CLI under project scope reads skills · does not support project-scoped skills. The label test also has a ScopeProject case now.
| @@ -0,0 +1 @@ | |||
| * `databricks aitools install` no longer asks "Proceed?" after the agent picker; the picker selection confirms the plan. The picker title now shows the install scope, its hint says Enter installs, `--skills-only` runs label every agent "skills", and the plan summary after it is reduced to the scope line. ([#7478](https://github.com/databricks/cli/pull/7478)) | |||
Member
There was a problem hiding this comment.
[nit] The fragment says --skills-only runs "label every agent 'skills'", but agents that are not found still read not found, and under project scope Codex CLI and GitHub Copilot read plugin · does not support project-scoped skills. This line ships in the release notes, so suggest "label plugin agents 'skills'".
Contributor
Author
There was a problem hiding this comment.
Changed in da4d9e2. The fragment now reads --skills-only runs "label plugin agents 'skills' and pre-check every agent whose config dir exists", which also covers the preselect change.
| fakeBinsOnPath(t, "claude") | ||
| ctx := cmdio.MockDiscard(t.Context()) | ||
|
|
||
| labels := func(skillsOnly bool) map[string]string { |
Member
There was a problem hiding this comment.
[nit] labels := func(skillsOnly bool) map[string]string {...} is an inline closure that only names a repeated operation, which .agents/rules/style-guide-go.md:16 rules out. Suggest a small named helper, such as func choiceLabels(ctx context.Context, scope string, skillsOnly bool) map[string]string. That would also make a project-scope case easy to add.
Contributor
Author
There was a problem hiding this comment.
Done in da4d9e2. The closure is now a named helper, choiceLabels(ctx, scope, skillsOnly), and the test has a project-scope case. The new preselect test uses a named choicePreselected helper too, not a closure.
Comment on lines
+429
to
+445
| ctx, test := cmdio.SetupTest(t.Context(), cmdio.TestOptions{PromptSupported: true}) | ||
| defer test.Done() | ||
| go drainReader(test.Stdout) | ||
| go drainReader(test.Stderr) | ||
| stderr := make(chan []byte) | ||
| go func() { | ||
| b, _ := io.ReadAll(test.Stderr) | ||
| stderr <- b | ||
| }() | ||
|
|
||
| cmd := newTestInstallCmd() | ||
| cmd.SetContext(telemetry.WithNewLogger(ctx)) | ||
|
|
||
| require.NoError(t, cmd.RunE(cmd, nil)) | ||
| assert.True(t, pickerCalled) | ||
| assert.True(t, proceedCalled) | ||
| test.Done() | ||
| // The picker gets the resolved scope for its title, and the scope line | ||
| // follows the picker. | ||
| assert.Equal(t, installer.ScopeProject, pickerScope) | ||
| assert.Contains(t, string(<-stderr), "Install scope: project") |
Member
There was a problem hiding this comment.
[nit] This block repeats runInstallWithStderr (install_test.go:962) line for line: SetupTest, drain stdout, read stderr through a channel, telemetry.WithNewLogger, run, test.Done(). _, stderr := runInstallWithStderr(t, true) followed by the same assertions passes at this head and removes about 15 lines.
Contributor
Author
There was a problem hiding this comment.
Done in da4d9e2. The test now calls _, stderr := runInstallWithStderr(t, true) and keeps the same assertions.
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 12, 2026 08:47
42b3163 to
d6c9440
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
2 times, most recently
from
November 12, 2026 08:50
172757e to
da4d9e2
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 12, 2026 09:00
d6c9440 to
914a32c
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 12, 2026 09:00
da4d9e2 to
efae3db
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 12, 2026 09:06
914a32c to
7577e8d
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 12, 2026 09:06
efae3db to
6aa9729
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 12, 2026 09:11
7577e8d to
2f670b2
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 12, 2026 09:11
6aa9729 to
f5bf3ea
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 12, 2026 09:19
2f670b2 to
c2d7335
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 12, 2026 09:19
f5bf3ea to
ac0065e
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 12, 2026 10:01
c2d7335 to
8c2aca6
Compare
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 12, 2026 10:01
ac0065e to
2a658e6
Compare
anton-107
force-pushed
the
aitools/no-prompts-for-agents
branch
from
November 12, 2026 10:43
8c2aca6 to
7a33789
Compare
`databricks aitools install` (interactive, no agents on the command line) no longer asks `Proceed?` after the agent picker. Agents that would be skipped are not offered in the picker, so choosing agents already confirms the plan. The install scope moves into the picker title, so it is visible before anything runs, and the plan summary after the picker shrinks to the scope line. ```diff install (interactive picker path) - promptAgentSelection # "Select agents to set up", "enter to confirm" + promptAgentSelection # "Select agents to set up (<scope> scope)", "enter to install" buildPlan - printPlanSummary # "Plan (<scope> scope):" + one line per agent - promptProceed # "Proceed?" — No => "Cancelled.", exit 0 - if !proceed: funnel CANCELLED, return nil funnel PLAN_CONFIRMED + printInstallScope # "Install scope: <scope>" install plugins / skills ``` - `promptProceed` and `defaultPromptProceed` are removed. `promptAgentSelection` takes the scope for its title. - `printPlanSummary` becomes `printInstallScope`. Its per-agent lines repeated the picker, and its skip branch could no longer run. - The picker hint reads "space to toggle, enter to install", since Enter now starts the install. - Under `--skills-only` (and the `databricks skills install` alias), every found agent is labeled "skills" in the picker, skipped rows included, because that is what they get. The removed per-agent summary lines were the only place this showed before. Rows are also pre-checked by config dir in that mode, so a plugin agent whose CLI is not on PATH starts checked, matching the non-interactive run. - `PLAN_CONFIRMED` is kept and still follows `AGENTS_SELECTED` right away; its comment in `libs/telemetry/protos` now says the picker selection is the confirmation. - `TestInstallFunnelInteractiveCancel` now cancels at the picker, which is the last prompt before the install. - Changelog fragment: `.nextchanges/cli/aitools-install-drop-proceed-confirm.md`. Co-authored-by: Isaac <no-reply@databricks.com>
anton-107
force-pushed
the
aitools/drop-proceed-confirm
branch
from
November 12, 2026 11:02
2a658e6 to
51b22e3
Compare
This branch has not been deployed
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.
Stacked on #7477.
Stacked on: aitools: never prompt a coding agent caller in install
Summary
databricks aitools install(interactive, no agents on the command line) no longer asksProceed?after the agent picker. Agents that would be skipped are not offered in the picker, so choosing agents already confirms the plan. The install scope moves into the picker title, so it is visible before anything runs, and the plan summary after the picker shrinks to the scope line.promptProceedanddefaultPromptProceedare removed.promptAgentSelectiontakes the scope for its title.printPlanSummarybecomesprintInstallScope. Its per-agent lines repeated the picker, and its skip branch could no longer run.--skills-only(and thedatabricks skills installalias), every found agent is labeled "skills" in the picker, skipped rows included, because that is what they get. The removed per-agent summary lines were the only place this showed before. Rows are also pre-checked by config dir in that mode, so a plugin agent whose CLI is not on PATH starts checked, matching the non-interactive run.PLAN_CONFIRMEDis kept and still followsAGENTS_SELECTEDright away; its comment inlibs/telemetry/protosnow says the picker selection is the confirmation.TestInstallFunnelInteractiveCancelnow cancels at the picker, which is the last prompt before the install..nextchanges/cli/aitools-install-drop-proceed-confirm.md.Evidence
go test ./cmd/aitools/ -count=1 -v -run 'TestInstallFunnelInteractiveCancel|TestInstallFunnelEmptyPickerSelectionIsCancelled|TestInstallInteractivePickerInstallsPickedAgent|TestAgentPickerTitleNamesScope|TestPrintInstallScope|TestAgentChoicesSkillsOnly|TestInstallAgentCallerInPTYNeverPrompts'at this head: all pass.TestInstallInteractivePickerInstallsPickedAgentmocks project scope (not the global default) and checks that the picker gets it, thatInstall scope: projectfollows the picker on stderr, and that the picked agent gets the plugin.TestAgentChoicesSkillsOnlyLabelsPluginAgentsSkillschecks that Claude Code is labeled "plugin" normally and "skills" under--skills-only. It fails without the label change.TestAgentChoicesSkillsOnlyLabelsSkippedPluginAgentSkillschecks that a skipped Codex CLI row under--skills-only --scope projectreadsskills · does not support project-scoped skills, andTestAgentChoicesSkillsOnlyPreselectsByConfigDirchecks that Claude Code with no CLI on PATH starts checked under--skills-onlyonly. Both fail without the change.TestAgentPickerTitleNamesScopeandTestPrintInstallScopepin the picker title and the scope line.Proceed?in place, the picker test failed in a shell without a controlling terminal (huh: could not open a new TTY) and passed with one, because the confirm then read stdin and defaulted to Yes. So the test is named after what it checks.go test ./cmd/aitools/... ./libs/telemetry/...and./task test-exp-aitoolspass at this head (Linux). No acceptance test drives the interactive picker, so no acceptance output changes.Merge Danger
Door: two-way
Removes one prompt and shortens the text around the picker. Reverting the commit restores both.
Blast Radius: small
CANCELLED:USER_ABORTEDinstead of the old "Cancelled." with exit 0 andCANCELLEDwith no category.(<scope> scope), its hint saysenter to install,--skills-onlyrows sayskillsinstead ofpluginand are pre-checked by config dir, and thePlan (<scope> scope):block with one line per agent becomes a singleInstall scope: <scope>line.This pull request and its description were written by Isaac.