fix(cli): close @path completion dropdown on Enter accept#1
Closed
Alex-ai-future wants to merge 17 commits into
Closed
fix(cli): close @path completion dropdown on Enter accept#1Alex-ai-future wants to merge 17 commits into
Alex-ai-future wants to merge 17 commits into
Conversation
…wenLM#4532) (QwenLM#4533) * feat(skills): add /skills enable/disable with manage dialog Adds workspace-scoped skill toggle parity with codex's /skills command. Disabled skills are hidden from both `<available_skills>` (model-facing) and the `/<skill-name>` slash command surface, taking effect live within the same session. - New `skills.disabled: string[]` setting (UNION-merged across scopes, requiresRestart: false). Live read via `Config.disabledSkillNamesProvider` attached at construction so the first `<available_skills>` build at cold-start (interactive, non-interactive, ACP) honors persisted entries. - `/skills manage` opens a checkbox dialog over skill names. Locked rows for higher-scope (systemDefaults / user / system) entries are rendered outside the MultiSelect to avoid the [x]-on-disabled visual conflict. Workspace writes exclude locked names so settings stay clean. - `/skills enable <name>` and `/skills disable <name>` non-interactive shortcuts. Trust gate refuses on untrusted workspaces (where workspace settings are dropped from the merge); ACP-mode guard refuses since `context.ui.reloadCommands` is a no-op there. - Filter applied inside `SkillCommandLoader` and `BundledSkillLoader` (kind: SKILL only) — never via `CommandService`'s global denylist, which would also hide same-named built-ins or MCP prompts. - `SkillTool.refreshSkills` excludes disabled skills from `availableSkills`, `pendingConditionalSkillNames`, and `fileBasedSkillNames` so a same-named MCP prompt resurfaces. `validateToolParams` and `SkillToolInvocation.execute` mirror the same `commandExists` → disabled-branch ordering so a disabled skill never shadows an MCP prompt during validation OR execution. - Refresh after change: strict `await reloadCommands(); await notifyConfigChanged();` (NOT `Promise.all`). `modelInvocableCommandsProvider` is re-registered inside the reload effect with a closure over the new CommandService instance, so refreshing SkillTool first would let it read a stale provider and leak the just-disabled skill back into `<available_skills>` as a command-form entry. 31 regression tests covering refresh order, same-name MCP prompt protection (validate + execute), execute-side guard, listing / completion / `<name>` filter, untrusted-workspace refusal, ACP-mode refusal, UNION-blocked enable warning, locked-row semantics, and the reserved-name (`enable` / `disable` / `manage`) tradeoff. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(skills): collapse /skills entry into single dialog flow - Bare `/skills` now opens the manage dialog directly in interactive mode (no more list-vs-manage split). Drops the `manage` subcommand entirely; `enable` / `disable` remain for non-interactive scripting and keyboard muscle memory. - Falls back to the original SKILLS_LIST emit in ACP/non-interactive mode where the dialog cannot render. - Polish: dialog header now shows total count + filter count ("3 / 12 skills · …"), search line is its own line under the title with a clear "Search:" label so users know typing filters live. Footer hint trimmed to navigation keys only since the action keys moved to the header. - Tests updated: bare `/skills` returns dialog action in interactive, listing in non-interactive; completion no longer prepends `manage`; added regression that explicitly asserts `manage` is gone from both subCommands and completion output. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(skills): drop enable/disable subcommands, dialog is the only entry Subcommands cluttered the auto-completion popup at `/skills` with enable/disable hints that overlap with what the dialog already does. Toggling now lives entirely in the dialog (open via bare `/skills` in interactive mode, or edit settings.json directly in non-interactive). - Removed `enableSubCommand` and `disableSubCommand`. - Removed dead helpers: `SUBCOMMAND_NAMES`, `ensureLiveRefreshAvailable`, `refreshAfterChange`, `emitTrustError`, `getWorkspaceDisabled`, plus the now-unused `SettingScope` import. - Completion only suggests skill names (for the legacy `/skills <name>` invocation shortcut, which stays — works in any mode and lets users trigger skills marked `disable-model-invocation`). - Tests trimmed from 21 to 9: removed all subcommand suites, replaced with an assertion that `subCommands` is empty and that completion never surfaces `manage` / `enable` / `disable`. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(skills): rebind dialog keys — Space toggle, Esc save+exit, Enter invoke Previous binding (Enter saves, Esc cancels) felt off — it forced users to hit Enter just to commit a single toggle, and Esc surprisingly discarded changes. New binding matches the user's mental model: - Space — toggle the highlighted skill on/off (unchanged) - Esc — save pending toggles and exit (auto-save semantic; the earlier "cancel without save" path is gone) - Enter — save pending toggles, close the dialog, and invoke the highlighted skill via `handleFinalSubmit('/<name>')`. The dialog now doubles as a launcher: pick a skill, hit Enter, the prompt goes out. Implementation: - Split the old `handleConfirm` into `persistChanges` (the write + reloadCommands + notifyConfigChanged sequence) and two thin wrappers: `handleSaveAndClose` for Esc and `handleInvoke` for Enter. - Track the highlighted row via MultiSelect's `onHighlight` so Enter knows which skill to launch. - Header hint updated to "Space toggle · Enter invoke · Esc save & exit". - DialogManager passes `uiActions.handleFinalSubmit` through as the new `submitPrompt` prop. Esc-with-active-search still just clears the query (refining search without exiting is intuitive); Esc on empty search saves & exits. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(skills): /skil<Enter> opens dialog, dialog Enter fills input Two paper cuts in the previous flow: 1. Typing `/skil` and pressing Enter on the highlighted `skills` suggestion auto-completed to `/skills ` (with trailing space) and forced a SECOND Enter to actually open the dialog. 2. Enter on a skill row inside the dialog auto-invoked the skill (sent the prompt for me) — too aggressive. Users wanted Enter to "pick" the skill into the input box and let them review/edit before sending. Fixes: - Add `submitOnAccept?: boolean` to `SlashCommand` and `Suggestion`. When set, the InputPrompt accept-suggestion branch submits `/<value>` immediately instead of just inserting + waiting for another Enter. `skillsCommand` opts in. Other commands are unaffected. - Dialog Enter calls `setInputBuffer('/<skill-name>')` instead of submitting. Pending toggles still save first; dialog still closes. The user reviews the filled buffer and presses Enter themselves to send (or edits, or cancels by deleting). - Plumbed `setInputBuffer` through UIActions, wired to the chat input buffer's `setText` in AppContainer. Mirrors the pattern already used by `/arena start --models X` (AppContainer:1763). - `/skills` is now truly single-purpose: bare action opens the dialog in interactive mode (or lists in non-interactive). Removed the legacy `/skills <name>` invocation path and the completion function it required — invoking a specific skill goes through `/<name>` (loaded by SkillCommandLoader) or by picking inside the dialog. - Header hint updated: "Space toggle · Enter pick (fill input) · Esc save & exit". - Tests trimmed to 7 — dropped completion-suite (no completion fn anymore) and the `/skills <disabled-name>` error path; added positive checks that `subCommands` is empty, `completion` is undefined, and `submitOnAccept` is true. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(skills): localize /skills description, fix wording to cover pick The previous description ("Manage skills (open enable/disable dialog).") had two problems shown by the user's screenshot: 1. It only mentioned manage / enable / disable — but the dialog also lets users browse, search, and pick a skill (Enter fills the input buffer with `/<name>`). "Manage" undersells what the panel does. 2. It bypassed the i18n dictionary (no entry for the new English string), so a Chinese-locale CLI showed the English fallback while the surrounding command list rendered in Chinese — visually jarring. Fix: - Reword to "Open the skills panel (browse, search, toggle, pick)." Reflects all four things the dialog supports. - Add translations for all 9 supported locales (en, zh, zh-TW, ja, fr, de, pt, ru, ca) — same pattern as the existing "List available skills." key. - Add the new key to MUST_TRANSLATE_KEYS so the i18n enforcement test fails if a future locale forgets it. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(skills): localize SkillsManagerDialog (header, body, toasts) The dialog had a pile of hardcoded English strings that bypassed the i18n dictionary — the saved-toast "Skills configuration saved." in particular showed up as English in a Chinese-locale CLI right next to translated commands. Wrap them all through `t()`: - Title: "Manage Skills" - Subtitle counts: "{{count}} skills · " / "{{matched}} / {{total}} skills · " - Key hints: "Space toggle · Enter pick (fill input) · Esc save & exit · workspace scope" - Search row: "Search:" / "type to filter…" - Body: "No skills are currently available." / "All available skills are locked at a higher scope (see below)." / "No skills match the search." - Locked section: "Locked by higher-scope settings (cannot toggle here):" + " {{name}} {{description}} [locked: {{scope}}]" - Footer: "↑/↓ navigate · backspace edits search" - Loading/error: "Loading skills…" / "Failed to load skills: {{error}}" / "Press esc to close." / "SkillManager not available." - Toasts: "Skills configuration saved." / "Skills configuration saved, but refresh failed: {{error}}." / "Workspace is untrusted; …" Level labels (`Project` / `User` / `Extension` / `Bundled`) moved from a module-level `LEVEL_LABEL` constant into a `levelLabel()` function called at render time. The previous constant captured `t()` at import time, so toggling `/language` after startup wouldn't flip the label. `Bundled` is a new translation key (Project/User/Extension already exist in all locales for hooks/MCP/skill-loader callsites). Scope identifiers (System / User / SystemDefaults) inside the "[locked: …]" annotation stay as untranslated technical labels — they refer to settings file scopes by name and matching them exactly helps users locate the offending entry. Only the surrounding "locked: " label is translated. Translations added for all 9 supported locales (en, zh, zh-TW, ja, fr, de, pt, ru, ca). The most prominent strings ("Manage Skills", "Skills configuration saved.", and the key-hints subtitle) are added to MUST_TRANSLATE_KEYS so the i18n parity test fails if a future locale forgets them. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(skills): coerce skill counts to string for t() interpolation CI tsc --build failed (TS2322) because `t(key, params)` types its params as `Record<string, string>` but I passed raw `number` values for `matchedCount` / `totalCount` in the dialog header subtitle. Local `tsc --noEmit` had skipped the file in my pre-existing-error noise so this slipped through. Wrap the three offending values in `String(…)` — interpolation result is identical, types are now correct. Failing jobs (run 26429498681): - Lint - Test (macos-latest, Node 22.x) - Test (ubuntu-latest, Node 22.x) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(skills): address PR QwenLM#4533 review + CI failures CI fixes: - Regenerate vscode-ide-companion settings.schema.json so the lint "settings schema is up-to-date" gate passes after adding the `skills.disabled` setting. - Mock `buildDisabledSkillNamesProvider` in acpAgent.test.ts and gemini.test.tsx so the new live-read provider import resolves; update one positional `loadCliConfig` call assertion. Review fixes (qwen3.7-max via /review on 3d36560): - buildDisabledSkillNamesProvider: filter non-string entries before .trim().toLowerCase() so a stray `"disabled": [42]` in settings cannot brick `validateToolParams` / `execute` with a TypeError. - SkillsManagerDialog: - `lower()` now trims + lowercases (parity with Config / skillsCommand). Add `normalizeNames()` and use it for all set construction so whitespace/case-only edits in settings.json are treated as equal. - Esc-during-loading guard: if `skills`/`selectedKeys` haven't loaded yet, just close — never write `skills.disabled = undefined`. - `activeValue` is now seeded from `filteredUnlocked[0]` on mount and re-derived when the previous highlight is filtered out, so Enter on first render and Enter after a filter both target a visible row. - persistChanges returns 'ok' | 'untrusted' | 'error'; `setValue` is wrapped in try/catch so disk failures surface as a user-visible ERROR toast and the dialog still closes. - Skip the disk-write + reloadCommands + notifyConfigChanged round trip when the normalized disabled list is unchanged. - handlePick refuses to fill `/<name>` for a row the user just toggled off (or a locked row reachable via stale activeValue) — avoids submitting a guaranteed-fail `/disabled-skill`. - skill.ts (core): the disabled-skill → command-delegation branch no longer fires `SkillLaunchEvent` or calls `onSkillLoaded`; returnDisplay becomes "Delegated to command:" so telemetry / `/context` skill-token attribution don't conflate command runs with real skill execution. - skillsCommand: drop the duplicate `getDisabledSet` and use `config.getDisabledSkillNames()` so all surfaces share one normalization path. - i18n: add the missing `'All available skills are disabled. ...'` key (used in non-interactive `/skills` fallback) and the new `'Failed to save skills configuration: {{error}}'` key to all 9 locale files. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(skills): address 2nd-round PR review + fix worktree test mock CI fix: - Add `buildDisabledSkillNamesProvider` mock to `acpAgent.worktree.test.ts` (same issue as the other two test files fixed in the previous commit — this one wasn't caught locally because it's macOS-only in CI). Review fixes (qwen3.7-max 2nd round at 05:37:09Z + 06:03:17Z): - [Critical] `buildDisabledSkillNamesProvider` + dialog `namesFromScope` now wrap the raw `skills.disabled` value with `Array.isArray()` before iterating. A hand-edited `"disabled": "all"` or `42` no longer crashes the CLI on cold-start or the dialog on open. - [Suggestion] Error messages in skill.ts:305/440 no longer reference the dropped `/skills manage` subcommand — updated to `/skills`. - [Suggestion] `submitOnAccept` in InputPrompt now gates on `key.name === 'return'` — Tab fills the completion without auto- submitting, matching standard shell convention. - [Suggestion] The disabled-branch `commandExecutor` call is now wrapped in try/catch, matching the non-disabled path's graceful degradation on MCP failures. - [Suggestion] `handlePick` now calls `persistChanges()` even when the `!isEnabled` branch fires — pending toggles to other skills are no longer silently discarded. - [Suggestion] Phantom 2nd `reloadCommands()` eliminated via a one-shot suppression flag on SkillManager (`suppressNextSlashReload` / `consumeSlashReloadSuppression`) — the dialog sets it before `notifyConfigChanged` so `slashCommandProcessor`'s listener skips the redundant rebuild. - [Suggestion] Removed orphaned `'List available skills.'` translation key from all 9 locale files (dead code after description change). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(skills): allow j/k in search filter, defer only when idle j/k were unconditionally deferred to MultiSelect for vim navigation, making it impossible to search for skills containing those letters (e.g. "json", "jwt", "kotlin"). Now j/k are only deferred when the search query is empty; when the user is actively searching, MultiSelect receives `isFocused={false}` which disables its vim key handlers so j/k reach the printable-character branch and appear in the filter. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(skills): use disableVimNav instead of isFocused for j/k search isFocused={!query} over-disabled MultiSelect: arrows, space, and Enter were all dead during search because useSelectionList's entire keypress handler has `isActive: isFocused`. The outer handler still deferred those keys, so they reached nothing. Replace with a targeted `disableVimNav` prop that only suppresses bare j/k in useSelectionList while keeping arrows, Enter, ctrl+p/n, and space fully functional: - useSelectionList: new `disableVimNav` option; skips SELECTION_UP for bare 'k' and SELECTION_DOWN for bare 'j' when set, all other keys pass through normally. - MultiSelect: threads the prop to useSelectionList. - SkillsManagerDialog: `disableVimNav={!!query}` replaces `isFocused={!query}`. Behavior: arrows/space/Enter always work (navigate, toggle, pick). j/k work as vim-nav when idle, as search chars when typing. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(skills): revert dep-array misread + add test coverage for review Review comment 3303143705 pointed at `buffer,` in the useMemo dependency array, not the object literal. `buffer` is a correct dep (the useMemo callback references `buffer.setText`). The object at line 3489 already has the proper `setInputBuffer: buffer.setText`. No source change needed — reply will clarify. Test coverage (review comments 3304330911/17/23/27): - useSelectionList.test.ts: `describe('disableVimNav')` — bare j/k suppression, ctrl+n pass-through, arrow-key navigation. - slashCommandProcessor.test.ts: reload skipped when `consumeSlashReloadSuppression()` returns true. - skill.test.ts (core): commandExecutor throws in disabled branch → graceful fallback to disabled-error message. - config.integration.test.ts: `buildDisabledSkillNamesProvider` unit tests covering normal arrays, non-array inputs, mixed-type arrays, whitespace trimming, and empty-after-trim filtering. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(skills): return 'refresh-failed' from persistChanges + fix stale comments - persistChanges now returns 'refresh-failed' (not 'ok') when the reloadCommands/notifyConfigChanged catch block fires. Callers already gate on `result === 'ok'`, so handleSaveAndClose no longer shows a double toast (WARNING + INFO) and handlePick no longer fills the input buffer when the command list may be stale. - Replace all `/skills manage` / `/skills enable` / `/skills disable` references in code comments with `/skills` (or "via the `/skills` dialog") across 8 locations: SkillsManagerDialog.tsx, AppContainer.tsx (×4), UIActionsContext.tsx, UIStateContext.tsx, skill-manager.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(skills): preserve orphaned disabled entries + fix buffer dep churn - SkillsManagerDialog persistChanges: workspace `skills.disabled` entries that don't match any currently-loaded skill (other branch, uninstalled extension, deleted skill dir) are now preserved across save. Previously opening /skills and pressing Esc silently dropped them, losing the user's prior disable setting if the skill later reappears. - AppContainer useMemo dep array: replace `buffer` (new ref on every keystroke) with `buffer.setText` (stable useCallback ref). Prevents the entire UIActions memo from recreating on every keystroke. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(skills): use allSkills (not unlockedSkills) for orphan detection The orphan-preservation loop was checking against `unlockedSkills`, but locked skills (higher-scope disabled) are also absent from that set. This caused locked-skill names to be treated as orphans and re-emitted into the workspace `skills.disabled` write — violating invariant #2 (locked names never re-emitted). Switch to `allSkills` so only entries that don't match ANY currently- loaded skill (truly orphaned: other branch, deleted dir, uninstalled extension) are preserved. Locked skills are in `allSkills` and correctly excluded from re-emission. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(skills): remove stale showYoloStyling reference in prefixWidth calculation The merge with main brought in commit da4dad5 which replaced showYoloStyling with approvalModePromptStyle, but left a dangling reference in the prefixWidth ternary. Both branches (`* ` and `> `) are 2 chars wide, so the guard was always a no-op — collapse to a single `: 2` to match main. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
…t in disabled skill path (QwenLM#4804) The disabled-skill command delegation (line 457) used the executor result directly as text content, but ModelInvocableCommandExecutorResult is `string | { error: string }`. Handle the error object case the same way the non-disabled path does (lines 494-506). Fixes build break introduced by QwenLM#4532 merging after the type was widened.
…nLM#4800) PR QwenLM#4456 added a duplicate --list-extensions handler at line 965 and a preconnect guard, but PR QwenLM#4673 (merged earlier) already implemented the same fix at line 470 using the existing handleListExtensions() function. The code from QwenLM#4456 is unreachable because the early handler exits via process.exit(0) before execution reaches line 965. Removes: - Dead inline handler block in gemini.tsx (~40 lines) - Unnecessary preconnect guard for list-extensions - 3 test cases that tested the unreachable code path (~242 lines)
* ci(triage): fix qwen triage workflow prompt * fix(ci): prevent cross-event concurrency cancellation in triage workflow Add event_name to concurrency group so that issue_comment events (even those that will be skipped by the job if-condition) don't cancel in-progress pull_request_target or issues triage runs. Also add process label exclusion (welcome-pr, maintainer, help wanted, good first issue) to the triage skill rules. Closes QwenLM#4785 * fix(ci): remove target input to let skill auto-detect issue/pr type The workflow_dispatch target input defaulted to 'issue', which could cause PR numbers to be triaged with the wrong workflow. Remove it and let the triage skill auto-detect from GitHub metadata instead. * refactor(ci): remove kind parameter, let skill auto-detect issue/pr type The triage skill already infers the target type from GitHub metadata. Passing kind explicitly adds complexity without value and was causing the workflow_dispatch path to mismatch when the default was wrong. Simplify prompt to just `/triage $NUMBER`. * ci(triage): align triage skill argument hint * docs(triage): minimize pr workflow rerun diff * fix(ci): scope triage concurrency to job * docs(triage): shorten comment body guidance * fix(triage): harden repo scoping and align skill tools with workflow - workflow: pass `--repo ${{ github.repository }}` so /triage cannot target a different repo even under prompt injection - workflow: expand `coreTools` to cover read_file/grep_search/glob/agent /enter_worktree/exit_worktree — the skill needs these to run end-to-end - SKILL.md: rename legacy `task` → `agent`, drop stale `read_many_files` from allowedTools so it matches the workflow's coreTools - SKILL.md: add a Local invocation branch to the Duplicate Guard so maintainers running `/triage` from a terminal hit a defined path Addresses review feedback on QwenLM#4787.
* feat(vscode): surface ACP background notifications * feat(vscode): route shell notifications through ACP * fix(vscode): harden ACP background notifications
* feat(cli): support /copy N to copy Nth-last AI message Lets users grab earlier AI replies without scrolling — `/copy 2` copies the second-to-last AI message, `/copy 3 code python` extracts the last Python code block from the third-to-last, etc. Useful when the agent's final action is something low-signal (TODO update, status line) and the substantive output is one or two turns back. The arg parser strips a leading positive-integer token and treats it as a 1-based message index (1 = last AI message); the remaining tokens are passed unchanged to the existing code/LaTeX sub-selectors. `/copy code python 2` keeps its prior meaning (2nd python block in last message) because its leading token isn't a digit. Closes QwenLM#4744 * feat(cli): add argumentHint for /copy so completion menu shows syntax * feat(cli): simplify /copy argumentHint to [N], add four-component regression test Claude Code's /copy only exposes N as an arg ("Copy Claude's last response to clipboard (or /copy N for the Nth-latest)"); block selection happens in a UI picker, not via command syntax. Aligning the hint with that — the existing code/latex/<lang>/<index> sub-selectors still work but they were never advertised in a hint before and double-numeric "[N] … [<index>]" was confusing. Also lock in /copy 3 code python 2 (message-index + code + lang + within-message block-index) as a regression test, since that combo was not previously asserted. * feat(cli): inline /copy N hint in description across all 9 locales The `[N]` argumentHint alone is opaque — users see "[N]" in the completion menu but the description "Copy the last result or code snippet to clipboard" never says what N does. Claude Code's /copy solves this by inlining the hint in the description itself: "Copy Claude's last response to clipboard (or /copy N for the Nth-latest)". Mirror that pattern. The i18n key is the English source string, so all 9 locale files (en/zh/zh-TW/de/fr/pt/ca/ru/ja) must update both the key and the localized value to avoid orphaning translations and falling back to English. Translated each one to keep parity. Also drop "or code snippet" — code/latex sub-selection is a secondary feature documented in docs/users/features/markdown-rendering.md, and Claude Code's reference UX doesn't mention it in the description. * fix(cli): /copy N — N-aware result wording, rename _args, polish de translation Three review findings from a self-review pass: 1. Result strings hardcoded "last AI output" / "Last output copied" even when the user explicitly addressed an earlier message via /copy N. A user running `/copy 3 code` previously got "No matching code block found in the last AI output." — but they didn't ask about the last, they asked about the 3rd-last. Source label now branches on N: N=1 / no-N keep the original "last AI output" / "Last output copied" wording (tests stable); N>1 reads "AI message N". Covers the three "found in" error strings, the "contains no text to copy" branch, and the full-message success label. New tests assert the AI-message-N wording in the no-text and selector-miss cases. 2. The action handler signature still used `_args` (underscore-prefix indicates an unused parameter), but the body now reads from it via `parseLeadingMessageIndex(_args)`. Rename to `args` so the convention matches actual usage and other commands in this directory. 3. de translation `N-letzte` floats grammatically (adjective without a head noun); native speakers understand it but it reads clipped. Add the missing article: `für die N-letzte`.
…LM#4596) * fix: recurse into submodules when crawling git files * fix: filter stale crawler index entries * fix(core): preserve quoted git paths in crawler
* fix(core): harden auto mode self-modification checks * fix(core): address auto mode review feedback * fix(core): preserve shell-style absolute paths * fix(core): avoid regex slash trimming in shell semantics * test(core): cover shell rule relevance ordering * fix(core): close auto mode fallback review gaps * fix(core): harden auto mode cwd review paths * perf(core): Cache auto mode write path candidates * fix(core): refine auto mode protected write review * fix(core): track pushd in shell semantics * fix(core): harden dynamic shell cwd permissions * fix(core): harden auto-mode shell write detection * fix(core): harden shell semantic bypasses * fix(core): route pending auto allows through classifier * fix(core): avoid regex shell syntax trimming * fix(core): fire pending auto denial hooks * test(cli): cover ACP protected Bash auto review * fix(core): guard pending permission denied hook failures * test(cli): cover ACP auto denial for protected Bash writes * fix(core): guard PermissionDenied hook failures * fix(core): re-resolve auto mode write paths * fix(core): catch disguised protected shell writes * fix(core): catch additional protected shell writes * fix(core): harden raw protected redirect parsing * fix(core): close protected shell write gaps * fix(core): keep auto fallback protected writes pending * fix(core): detect sort output protected writes * fix(core): detect protected target-directory writes * fix(core): detect raw protected shell writes * fix(core): harden auto mode shell write detection * fix(core): detect attached downloader output flags * feat(core): configure auto classifier controls * fix(core): catch attached protected write flags * fix(core): enforce minimum classifier timeout * fix(core): close auto mode shell bypasses * test(core): cover find execdir protected writes * fix(core): detect awk in-place edits * fix(core): preserve auto mode denial prefix * test(core): cover awk long-form inplace flag * fix(core): handle pending auto fallback flow * fix(core): keep protected pending tools manual on fallback
…wenLM#4647) * fix(clipboard): use platform-native tools for image paste on Linux Replace @teddyzhu/clipboard native module with wl-paste/xclip on Linux to fix image paste in WSL2+Wayland environments. The native module uses X11 protocol and cannot read clipboard images when the session uses Wayland (common in WSL2 with WSLg). This causes clipboardHasImage() to return false even when the clipboard contains an image. Changes: - Use wl-paste --list-types to detect images (Wayland) - Use xclip -selection clipboard -t TARGETS -o to detect images (X11) - Handle image/bmp format from Windows clipboard (WSL2 exposes BMP) - Convert BMP to PNG using Python PIL when available - Detect clipboard tool via WAYLAND_DISPLAY when XDG_SESSION_TYPE is unset - Keep @teddyzhu/clipboard as fallback for macOS/Windows Fixes QwenLM#3517 Fixes QwenLM#2885 * test: update clipboard tests for platform-native tools The tests were mocking @teddyzhu/clipboard but the implementation now uses platform-native tools (wl-paste/xclip) on Linux. Update mocks to test the spawn-based implementation. * fix: address critical review comments 1. Fix command injection in Python BMP-to-PNG conversion - Use sys.argv instead of string interpolation - Prevents path traversal via single-quote injection 2. Fix BMP fallback dead code - When PIL is not available, return BMP file path instead of deleting the only copy and returning false - Update saveClipboardImage to handle non-PNG return paths * fix: address review suggestions for resource leaks and robustness - #3: Add proper cleanup in saveFromCommand error paths (kill child, destroy stream) - #4: Add 5s timeout for all spawned processes to prevent TUI hangs - QwenLM#7: Check exit code in checkClipboardForImage (code === 0) - QwenLM#8: Move fs.mkdir inside try/catch in saveClipboardImage - QwenLM#10: Merge checkWlPasteForImage/checkXclipForImage into checkClipboardForImage * fix: address all remaining review comments Source code fixes: - QwenLM#25: Add timeout to getWlPasteImageTypes (PROCESS_TIMEOUT_MS) - QwenLM#26: Add timeout to python3 spawn in BMP-to-PNG conversion - QwenLM#27: Wrap child.kill() in try-catch in timeout handlers - QwenLM#28: Replace dynamic import('node:fs/promises') with static statSync - QwenLM#30: Export resetLinuxClipboardTool() for testability - Add try-catch around spawn in checkClipboardForImage - Use stdio: ['ignore', 'ignore', 'ignore'] for python3 spawn Test fixes: - QwenLM#24: Use vi.hoisted() for mock functions (avoids hoisting issue) - QwenLM#31: Stub process.platform = 'linux' in beforeEach - Add default export to node:child_process mock - Use EventEmitter-based mock child for async behavior - All 7 tests passing * perf: cache wl-paste --list-types result to avoid redundant calls Avoid spawning wl-paste twice on the paste hot path: 1. clipboardHasImage calls wl-paste --list-types (check) 2. saveClipboardImage calls getWlPasteImageTypes (get types) Now the result is cached after the first call and reused. Cache is reset via resetLinuxClipboardTool() for testing. * fix: address remaining review suggestions - #1: Add child.stdout error handler in saveFromCommand - #2: Add macOS/Windows test coverage for @teddyzhu/clipboard fallback - #3: Fix .replace('.png', '.bmp') to use regex /\.png$/ to prevent path corruption * fix: address critical cache invalidation and other review feedback - #1 Critical: Reset cachedWlPasteImageTypes at start of clipboardHasImage to prevent stale data between paste operations - #1 Critical: Check exit code in getWlPasteImageTypes close handler, do not cache failed results - #2: Replace statSync with async fs.stat to avoid blocking event loop - #3: Remove async from close handler, use promise chain instead - #4: Return false instead of bmpPath when PIL conversion fails, as downstream expects .png files - #5: Capture stderr from spawned processes for diagnostics * fix: address remaining code review issues - #1: Narrow detection to only report supported formats (png/bmp) - #2: Do not cache results on timeout or error - #3: Use line-level matching instead of includes('image/') - #4: Replace execSync with execFileSync to avoid shell injection - #5: Upgrade BMP→PNG failure log to warn level with install hint * fix: restore getClipboardModule import caching (regression fix) The original Qwen Code cached the @teddyzhu/clipboard module import via getClipboardModule() with cachedClipboardModule and clipboardLoadAttempted. Our refactoring removed this caching, causing the module to be re-imported on every clipboardHasImage/saveClipboardImage call. Restored the original caching mechanism for macOS/Windows fallback path. * test: add saveClipboardImage success path and cache behavior tests - Add test for successful PNG save path - Add test for cache invalidation between clipboardHasImage calls - All 11 tests passing * fix: revert execSync to fix WSL2 clipboard detection execFileSync('command', ['-v', 'wl-paste']) fails because 'command' is a shell built-in, not an executable. execSync runs through a shell so it can find 'command'. Reverted to execSync to restore clipboard tool detection on WSL2. Also fixed TypeScript errors in tests by using (child as any) for mock event emitter properties. * fix: address critical file leak and filter issues from review - #1: Clean up bmpPath in catch block when PIL conversion fails - #2: Narrow getWlPasteImageTypes filter to only image/png and image/bmp - #3: Clean up empty PNG file when size guard fails - #3b: Fix typo python3-pyl → python3-pil * test: add xclip, BMP, error path test coverage; fix weak assertion - Add xclip/X11 path tests (detection, no image, not found) - Add BMP-to-PNG conversion tests (PIL failure, prefer PNG over BMP) - Add saveFromCommand error path tests (timeout, spawn error, stdout error) - Replace tautological 'successful PNG save' assertion with proper null-on-error tests - Fix ESLint: add no-explicit-any suppressions, prefix unused setupWaylandEnv Note: xclip save success path requires createWriteStream mock that vitest cannot fully support with ...actual spread. Detection and error paths verified. 19 tests passing. * fix: remove unused _setupWaylandEnv function that breaks TS build Fixes TS6133 error caused by noUnusedLocals: true in tsconfig.json. The function was generated by test agent but never called. * fix: clean up tempFilePath on PIL conversion failure When python3 PIL conversion fails mid-write, tempFilePath (the target .png) may have been partially written. Add fs.unlink(tempFilePath) in the catch block to prevent partial file leakage. Suggested by wenshao in PR review. * fix: address review feedback on file leaks and test coverage - Add tempFilePath cleanup when python3 PIL conversion fails mid-write - Restore image/bmp detection with clarifying comment (WSL2 Wayland) - Fix stat mock syntax (remove debug console.log, simplify) - Fix originalPlatform scope (was undefined in afterEach) Co-authored-by: Shaojin Wen <[email protected]> 19 tests passing, tsc + eslint clean. * ci: retrigger tests * fix: address review feedback on test coverage and defensive guard - Replace tautological saveClipboardImage assertion with meaningful spawn-argument verification - Wrap clipboardHasImage Linux branch in try/catch guard (preserve 'never throw, return false' contract) - Fix node:fs/promises mock to use importOriginal for indirect deps - Add readFile/writeFile/appendFile/access/copyFile/rename/rm/rmdir to mock (required by indirect deps like chatCompressionService) - Remove node:fs root mock to avoid cross-test pollution 19 tests passing, tsc + eslint clean. * fix: address review feedback on test coverage and defensive guard - Replace tautological saveClipboardImage assertion with spawn-arg verification (prefer PNG over BMP test) - Wrap clipboardHasImage Linux branch in try/catch guard - Fix node:fs/promises mock to use importOriginal for indirect deps - Add missing fs/promises methods (readFile etc.) required by deps - Remove node:fs root mock entirely to avoid cross-test pollution - Document xclip/BMP save success path: blocked by vitest built-in module mock limitation 19 tests passing, tsc + eslint clean. * fix: secure clipboard temp filename with random UUID suffix Add random UUID to temp filename to prevent predictable path symlink attacks (Critical review feedback). The UUID makes the path unguessable, eliminating the symlink attack vector. 19 tests passing, tsc + eslint clean. * fix: add O_EXCL protection against symlink attacks in saveFromCommand Use fs.open with O_EXCL flag (O_WRONLY|O_CREAT|O_EXCL) to atomically create the file, refusing to follow symlinks. Combined with the random UUID filename from the previous commit, this fully addresses the symlink attack vector identified in review. Also update 'prefer PNG over BMP' test: with O_EXCL, the save path fails when mkdir is mocked (directory doesn't exist), so the test now verifies format detection only rather than the full save pipeline. 19 tests passing, tsc + eslint clean. * fix: capture python3 stderr for BMP conversion errors Use stdio 'pipe' for stderr instead of 'ignore' so users see useful diagnostic messages (e.g. ModuleNotFoundError: No module named PIL) when python3 BMP-to-PNG conversion fails. 19 tests passing, tsc + eslint clean.
Adds qwen3.7-plus (and qwen3.6-plus) to the multimodal modality patterns and to the DashScope vision-model prefixes. Closes QwenLM#4802.
* feat(cli): prevent system sleep while running * fix(cli): spread actual node:os exports in test mocks for sleepInhibitor barrel compat The sleepInhibitor module (added to core barrel export) imports 'platform' as a named export from node:os and instantiates a module-level singleton. Test mocks in systemInfo.test.ts and add.test.ts replaced the entire node:os module with incomplete overrides, causing 'defaultPlatform is not a function' during module evaluation. Fix: use importOriginal to spread actual node:os exports before applying test-specific overrides, matching the pattern already used by other os mocks in the codebase. * fix(core): use caffeinate -is on macOS to also prevent lid-close sleep caffeinate -i only prevents idle sleep. Adding -s also prevents system sleep (including lid-close on AC power), matching the Linux systemd-inhibit semantics which blocks all sleep transitions. This was the most common real-world failure scenario: a user closing the laptop lid during a long tool execution would still trigger sleep on macOS despite the inhibitor being active. Addresses PR QwenLM#4434 review feedback. * fix(core): address sleep inhibitor review feedback * fix(core): harden sleep inhibitor per review feedback - Register a process 'exit' handler so the caffeinate/systemd-inhibit/ PowerShell subprocess is killed when the parent exits, preventing an orphaned process from blocking system sleep indefinitely. - Pass a curated environment instead of an empty one: an empty env stripped PATH (command resolution) and DBUS_SESSION_BUS_ADDRESS/XDG_RUNTIME_DIR (required by systemd-inhibit over D-Bus on Linux) and SYSTEMROOT/WINDIR (required by PowerShell on Windows). - Guard the whole 'error' handler with `this.child === child` so a stale child's error cannot poison spawnFailedForCurrentRun and block respawn. * fix(core): refine sleep inhibitor per follow-up review - settingsSchema: mark preventSystemSleep requiresRestart (it's read once at startup via Config.preventSystemSleep, so a runtime toggle needs a restart). - Latch spawnFailedForCurrentRun on unsupported platforms so acquire() doesn't re-check and re-log on every call. - Sanitize the systemd-inhibit --why reason (strip control chars, cap length) since it is visible in process listings on shared systems. - Correct the caffeinate comment: -s only blocks system sleep on AC power; on battery lid-close sleep is not prevented (macOS limitation). - Tests: cover dispose() (kills child, resets state, idempotent), stop()'s kill-throws catch, the stale-child error guard, the unsupported-platform latch, reason sanitization, and the ACP Session acquire/execute/release wrap.
…4549) * feat(ci): add PR review workflow using bundled /review skill Trigger review via `@qwen-code /review` in PR comments, or automatically on PR open/reopen for OWNER/MEMBER/COLLABORATOR. Supports workflow_dispatch for manual dry-run testing. Key design choices: - Direct qwen CLI invocation (not qwen-code-action) for real-time logs - --ci flag skips build/test/autofix, bounds verification budget - --kill-after=10s ensures clean termination on timeout - Fork PRs are skipped (fail-closed) - Anti-injection prefix on user-provided review instructions * fix(ci): add --auth-type openai, address review findings - Add --auth-type openai for non-interactive CI mode - Use dynamic default_branch instead of hardcoded 'main' - Add fetch-depth: 0 for full git history - Fix greedy ## pattern to non-greedy # in instruction extraction - Improve fallback condition to cover cancelled runs - Align setup-node to v6.4.0 matching other workflows - Clarify CI mode docs: allow gh pr review/comment write calls - Clarify agent count for CI mode (skip Build & Test regardless of repo) * ci(review): switch runner from ubuntu-latest to ecs-qwen Use self-hosted ECS runner for lower API latency and faster cold start. * ci(review): use CI_BOT_PAT for review comments Restore bot PAT so review comments appear as qwen-code-ci-bot instead of github-actions[bot]. * ci(review): use pre-installed node/qwen on ecs-qwen runner Remove setup-node and npm install steps since ecs-qwen runner already has Node.js and qwen CLI pre-installed. Also use full runner label array [self-hosted, linux, x64, ecs-qwen] to match the gate-review workflow convention. * ci(review): use shallow clone (fetch-depth: 1) on ecs-qwen Full clone is unnecessary since qwen CLI fetches diffs via GitHub API. Shallow clone avoids the slow full-history fetch on self-hosted runner. * fix(ci): resolve yamllint quoted-strings violations - Quote runs-on array items with single quotes - Quote ref expression value - Replace dynamic timeout-minutes expression with fixed 60 (shell-level timeout already handles the configurable value) * ci(review): remove ci flag dependency * ci(review): harden PR review workflow * ci(review): address workflow review comments * ci(review): bound lightweight review runs * ci(review): prevent review comment retriggers * ci(review): reduce yolo tool surface * ci(review): keep review tool surface intact [skip ci] * fix(ci-review): handle multi-line trigger comments The `startsWith(body, '@qwen-code /review ')` check requires a trailing space after `/review`. When a user writes a multi-line comment like: @qwen-code /review Focus on security the character after `/review` is `\n`, not ` `, so the job was silently skipped. Use `format()` to construct a newline-containing string for an additional `startsWith` check on all three comment-based triggers. Resolves review feedback from wenshao.
…nLM#4618) SchemaValidator.validate() retries with coercion when initial validation fails. fixBooleanValues() rewrote every string "true"/"false" it found — regardless of the field's declared type — into a real boolean. When a tool call legitimately carries the text "true"/"false" in a string field (for example an old_string, new_string or content argument) and validation failed for an unrelated reason, that string was silently corrupted into a boolean, changing the operation the model requested. Make fixBooleanValues schema-aware and only coerce a value when the field's schema accepts boolean and does not also accept string. This mirrors the sibling fixStringifiedJsonValues()/getAcceptedTypes() helpers, which already consult the schema for exactly this reason, and preserves the intended leniency for genuine boolean fields (including nullable booleans and arrays of booleans). Co-authored-by: Pluviobyte <[email protected]> Co-authored-by: Cursor <[email protected]>
When accepting an @path suggestion with Enter, the dropdown did not close for folder paths because handleAutocomplete intentionally omits the trailing space (to allow continued Tab-completion deeper into the directory). Without the space, the @ completion pattern re-matches and re-shows the dropdown. Fix: call completion.resetCompletionState() after accepting on Enter. Tab is intentionally excluded so users can still continue navigating deeper into directories. Also adds two regression tests: - Enter after @path accept resets completion state - Tab after @path accept does NOT reset completion state Signed-off-by: Alex <[email protected]>
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Alex-ai-future
added a commit
that referenced
this pull request
Jul 1, 2026
- Reject inline prompt + scope flag combination with clear error (#1) - Add mutual exclusivity check for --project and --global (#5) - Verify setValue scope parameter in tests + add --global test (#2) - Extract scopeSuffix to shared variable, remove duplication (#3) - Remove dead i18n keys 'Select Model (this project)' / '(global)' (#4) - Fix scopeSuffix placement on model line not API key line (QwenLM#8) - Add fr.js / ja.js translations for scope keys (QwenLM#10) - Remove unused export ModelDialogPersistScope (#6) - Wrap non-interactive help text in t() with new flags (QwenLM#7) - Fix argumentHint grouping to show mode vs scope flags (QwenLM#11) Signed-off-by: Alex <[email protected]>
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
11 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.
Summary
When accepting an
@pathsuggestion with Enter, the dropdown did not close for folder paths. File paths worked fine because they append a trailing space after completion, which breaks the@completion pattern. Folder paths intentionally omit the trailing space (to allow continued Tab-completion deeper into the directory tree), so the@completion pattern re-matches and re-shows the dropdown.Fix
Call
completion.resetCompletionState()after accepting a suggestion with Enter. Tab is intentionally excluded so users can still continue navigating deeper into directories by pressing Tab.Tests
Added two regression tests:
should reset completion state on Enter after accepting @path suggestionshould autocomplete @path on Tab without submitting or resetting completionVerification
🤖 Generated with Qwen Code (18-shotted by Qwen-Coder)