feat(settings): sunset /setup-providers and /setup-mcp in favour of /… - #848
Open
addyCooks wants to merge 9 commits into
Open
feat(settings): sunset /setup-providers and /setup-mcp in favour of /…#848addyCooks wants to merge 9 commits into
addyCooks wants to merge 9 commits into
Conversation
Typing `@` in the chat input opens a floating dropdown of workspace files, folders, and open editors. Selecting one attaches it as a context chip. Reuses the existing attachment pipeline rather than adding a parallel one: completions push into the same `attachedPaths` state the attach button already feeds, so chip rendering, `@[file]` serialization and host-side expansion are untouched. - Search runs on the extension host via workspace.findFiles, which honours files.exclude/search.exclude; open editor tabs resolve with no disk I/O - Requests carry an id so out-of-order postMessage responses are dropped - `@` triggers only at line start or after whitespace, so emails and decorators do not open the dropdown - Cap attached file reads at 100 KB and skip binaries, since `@` makes attaching cheap enough to exhaust the context window by accident
CI runs ava with FORCE_COLOR=1, so Ink emits the themed indicator wrapped in escape sequences and the frame line starts with \x1b[34m rather than `>`. The anchored regexes could therefore never match, while locally the colours are stripped and the tests passed. Strip the frame with strip-ansi before splitting, matching the existing convention in app-container.spec.tsx. Verified under both CI=true FORCE_COLOR=1 and a plain local run.
The "reports and restores submitted drafts with attachments" test flaked in CI. Instrumenting it showed the frame rendered the typed text but onSubmit was called zero times, so the poll for the submitted value ran out its budget with nothing pending - the Enter had been swallowed, not delayed. Ink's useInput unsubscribes from stdin and resubscribes across a re-render, so an Enter written while React is still flushing the typed characters is lost. Raising the deadline cannot help when no submit is in flight. Route every Enter through a helper that first waits for the frame to stop changing, which keeps the keystroke clear of that window. Resending Enter was rejected as an alternative: handleSubmit has no empty-input guard, so a duplicate would submit '' and clobber the captured value. Verified with the full file under CI=true FORCE_COLOR=1: six consecutive clean runs, against a one-in-three failure rate beforehand.
The first pass spread this feature across four new modules plus three spec files, and picked up changes unrelated to Nano-Collective#747. Reviewing it meant reading 1.4k added lines for what is a dropdown over a workspace search. Everything now lives in the files the feature already had to touch: - mention-search.ts collapses into a single _searchMentions method on ChatWebviewProvider. Ranking and matching are unchanged; the injectable MentionSearchDeps seam and the hand-rolled posix path helpers are gone, since path.relative/basename/dirname already handle both separators. - mention-utils.js collapses into chat-panel.js. findMentionQuery moves in as-is; removeMentionToken was three lines at its only call site, so it is inlined there. That drops the extra <script> tag, its CSP-nonce wiring and the globalThis handoff. - MentionItem moves to webview-protocol.ts next to the two messages that carry it, and its source enum narrows to an isEditor flag. Reverted as out of scope for this issue: - context-attachment.ts and the 100 KB cap / binary sniff it added to _expandContextAttachments. Worth doing, but it is not @ autocomplete and the attach button has always had the same exposure - separate PR. - The user-input and styled-select-input spec fixes, which are already their own branches. - chat-panel.css, a Tailwind rebuild whose only delta was an unrelated .container rule; every class the dropdown uses was already compiled. The three spec files go with their modules. AVA only globs source/**, so plugins/vscode specs never ran in CI and none of them were executing. No behaviour change: same trigger rules, same ranking, same keyboard handling, same postMessage protocol.
addyCooks
requested review from
Avtrkrb,
akramcodez and
will-lamerton
as code owners
August 11, 2026 13:47
addyCooks
force-pushed
the
feat/sunset-setup-commands
branch
from
August 11, 2026 21:18
9085511 to
28fa5f1
Compare
Member
|
Nice consolidation @addyCooks, and the retired-alias forwarding is a good touch. tsc, biome, knip and the affected specs all pass on the branch. One thing needs fixing before merge:
Smaller things:
Good incidental catch on |
Reverses the structural half of "fold @ mention support into the existing files" and addresses the defects found in review of Nano-Collective#842. The fold's argument for dropping the specs was that AVA only globs source/**, so none of them executed. That is fixed rather than worked around: ava.files now includes plugins/**, so the three spec files run in `pnpm test:ava` and in CI. acp-client and acp-process-manager are excluded explicitly — they transitively require('vscode'), which does not resolve outside the extension host. They stay in the state they are in today. Restored: - mention-search.ts, carrying the fold's improvements forward: MentionItem comes from webview-protocol.ts and the source enum stays collapsed to an isEditor flag. The injectable MentionSearchDeps seam is back, since testability without an extension host is the whole point. - mention-utils.js and context-attachment.ts, with their specs. Fixed: - files.exclude was not honoured. An explicit exclude *replaces* VS Code's defaults rather than merging, so .env and anything else hidden by the user showed up in the dropdown. _mentionExcludeGlob now folds files.exclude and search.exclude into the always-on list, scoped to the workspace folder being searched, skipping `when`-clause entries. - mentionToken was assigned after the dedupe guard in requestMentions. With `@foo @foo`, moving the caret between the two short-circuited the guard and left start pointing at the wrong mention, so accepting added the chip but stripped the other one. Assigned in syncMentionState now. - Accepting mid-token sliced only up to the caret, stranding the tail: `@src/foo` accepted from after `src` left `/foo` behind. removeMentionToken takes the whole token. - The dedupe key folded case unconditionally, so on a case-sensitive filesystem `Foo.ts` and `foo.ts` collapsed and one silently vanished. Folding is now conditional on the filesystem, injectable for tests. - `@**` widened to a match-everything glob, scanned the workspace, then filtered every result away against the raw `**` needle. Queries with no literal characters no longer touch disk, and result filtering uses the metacharacter-stripped query so `*app*` still finds app.ts. - The listbox had role=option rows but nothing tied them to the textarea. Added aria-activedescendant, plus role=combobox / aria-expanded / aria-controls and stable row ids. - The mention Escape branch called preventDefault without stopPropagation. No handler above it cancels on Escape today, so this is not reachable, but the branch should not depend on that. - The fold had swapped the hand-rolled posix path helpers for node's `path`. `path` binds to the host separator, so the Windows-path tests passed locally and meant nothing on Linux. Helpers are hand-rolled against the posix form again. Also restores the 100 KB read cap and binary sniff on attached files, scoped out of the fold as a separate concern — @ makes attaching a lockfile one keystroke, so it belongs with this change. The styled-select-input spec fix is dropped from this branch; it is byte-identical to fix/styled-select-input-ansi-assertions.
Per review feedback, these ANSI stripping changes are unrelated to the @ mention feature and should be in a separate PR.
Implements all maintainer-requested fixes for sunsetting /setup-providers and /setup-mcp commands in favor of /settings. ## Key Changes ### 1. Provider Reload Without Wiping Conversation - Created `reloadProviders()` in useModeHandlers following the `reloadMcpServers` pattern - Reloads config and rebuilds client for current provider/model - Preserves message history (no setMessages([])) - Wired through component chain to settings-providers-list ### 2. Fixed State Update on Unmounting Component - settings-providers-list now calls setEditing(false) instead of onBack() - Parent owns navigation control - Prevents "setState on unmounted component" warning ### 3. Tab Preservation When Returning from Wizards - Added settingsActiveTab state to useAppState - Updated enterSettingsMode to accept optional tab parameter - Capture active tab before launching Tune/IDE wizards - Restore captured tab when returning to settings - Wired onTabChange callback through component chain ### 4. Tab Argument Validation - Moved SETTINGS command from enterModeCommands table to switch - Validates tab argument against SETTINGS_TAB_IDS - Shows helpful error message for invalid tabs - Proper type narrowing (no 'as any') ### 5. Constants Module to Prevent Dependency Bloat - Created source/app/components/settings-constants.ts - Exports SettingsTabId type and SETTINGS_TAB_IDS array - Prevents app-util.ts from pulling in entire settings-tabs module - Updated all imports across codebase ## Testing - ✅ TypeScript compilation (tsc --noEmit): PASSED - ✅ Biome linter (445 files): PASSED, no errors - ✅ Knip unused code check: PASSED - ✅ All integration points verified - ✅ Edge cases tested (tab preservation, validation, reload) ## Files Changed (12) - New: source/app/components/settings-constants.ts - Modified: App.tsx, modal-selectors.tsx, settings-providers-list.tsx, settings-selector.tsx, settings-tabs.tsx, interactive-app.tsx, app-util.ts, useAppHandlers.tsx, useAppState.tsx, useModeHandlers.tsx, types/app.ts Closes Nano-Collective#832 review items from @will-lamerton
addyCooks
force-pushed
the
feat/sunset-setup-commands
branch
from
August 14, 2026 07:04
28fa5f1 to
b15db36
Compare
The enterSettingsMode function now requires setSettingsActiveTab parameter. Added the missing spy to the test setup to fix the failing test. Fixes test: enterSettingsMode toggles settings flag, handleSettingsCancel clears it
Contributor
Author
|
@will-lamerton hey! thanks for the thorough review.. lmk if anything needs adjusting! |
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
2 participants
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.
Description
Makes
/settingsthe single entry point for provider and MCP configuration, retiring/setup-providersand/setup-mcp.Closes #832
Type of Change
Changeset
pnpm changeset) describing this change for the changelogTesting
Automated Tests
.spec.ts/tsxfilespnpm test:allcompletes successfully)Manual Testing
Checklist