Skip to content

fix(agent-manager): align Cmd+/ fallback with platform binding and one-shot echo - #12694

Merged
marius-kilocode merged 2 commits into
mainfrom
fix-cmd-slash-followup
Jul 30, 2026
Merged

fix(agent-manager): align Cmd+/ fallback with platform binding and one-shot echo#12694
marius-kilocode merged 2 commits into
mainfrom
fix-cmd-slash-followup

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

Follow-up to #12691 addressing two review findings on the webview-side Cmd/Ctrl+/ fallback in the Agent Manager.

Platform modifier gating. The declared workbench keybinding is ctrl+/ with mac: cmd+/, but the local fallback accepted e.metaKey || e.ctrlKey on every platform. On macOS that made Ctrl+/ toggle the terminal even though no binding exists for it, and on Windows/Linux the same was true for Meta+/. Worse, for a user with their own keybinding on the non-platform combo, VS Code's forwarding would run their command while the webview simultaneously toggled the terminal. The fallback now accepts only the modifier the workbench binding declares for the current platform (detected via the same navigator.userAgent check the Agent Manager already uses, injectable for tests).

One-shot echo dedup. The echo guard was a pure 500 ms timestamp window, so any showTerminal action arriving within the window after a local press was dropped, not just the echo of that press — this could swallow an unrelated invocation such as the command palette entry or a quick follow-up press routed through terminal.integrated.commandsToSkipShell. The guard is now a per-press counter: each local press queues exactly one echo, each consumed echo suppresses exactly one action, and unrelated invocations run normally. The timeout remains, but only as a safety valve that drops a never-arriving echo (e.g. when the user has rebound the key); an echo that outlasts it is indistinguishable from a real invocation and must run — no local design can close that gap, so the window alone could not either.

@marius-kilocode
marius-kilocode enabled auto-merge (squash) July 30, 2026 12:12
Comment thread packages/kilo-vscode/webview-ui/agent-manager/terminal/side.ts
Comment thread packages/kilo-vscode/webview-ui/agent-manager/terminal/side.ts
@kilo-code-bot

kilo-code-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Suggestion only — safe to merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/kilo-vscode/webview-ui/agent-manager/terminal/side.ts 88 mac?: boolean is a test-only override next to another copy of the `/Mac
Files Reviewed (2 files changed since last review)
  • packages/kilo-vscode/webview-ui/agent-manager/terminal/side.ts - 0 new issues
  • packages/kilo-vscode/tests/unit/agent-manager-terminal-side.test.ts - 0 issues

Resolved since the previous review: the stale-backlog case is fixed — press now clears pending when the previous press is older than ECHO_MS, so a dropped echo no longer leaves a permanent increment that could swallow a later unrelated showTerminal. The one-shot invariant holds for the rapid-press, echo-consumed, and spaced-press cases, and the added test fails against the old counter-only logic, so it exercises the real fix rather than restating it.

One residual is inherent rather than a defect: two presses less than ECHO_MS apart where the first echo is dropped still leave pending at 2, since a dropped echo and a slow echo are indistinguishable locally. The backlog self-heals on the next quiet period (echo() resets when stale) or the next spaced press. No new listeners, timers, or retained references, so no leak risk.

Fix these issues in Kilo Cloud

Previous Review Summary (commit 899d32b)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 899d32b)

Status: 2 Issues Found | Recommendation: Suggestions only — safe to merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 2
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/kilo-vscode/webview-ui/agent-manager/terminal/side.ts 185 A press whose echo never arrives (the dropped-forwarding case this fallback exists for) leaves pending incremented for the rest of the session, so an unrelated showTerminal within 500 ms of a later press can still be swallowed. Expiring the backlog at press time restores the one-shot invariant.
packages/kilo-vscode/webview-ui/agent-manager/terminal/side.ts 88 mac?: boolean is a test-only override next to a fifth copy of the `/Mac
Files Reviewed (3 files)
  • packages/kilo-vscode/webview-ui/agent-manager/terminal/side.ts - 2 issues
  • packages/kilo-vscode/tests/unit/agent-manager-terminal-side.test.ts - 0 issues
  • .changeset/agent-manager-terminal-shortcut-platform.md - 0 issues

The platform gating matches the declared keybinding in package.json (ctrl+/, mac: cmd+/), and the exclusive-modifier checks (metaKey && !ctrlKey / ctrlKey && !metaKey) correctly refuse combo presses. The changeset is present and user-facing. No new listeners, timers, or retained references are introduced, so no leak risk from this change.

Fix these issues in Kilo Cloud


Reviewed by claude-opus-5 · Input: 20 · Output: 5.1K · Cached: 433.6K

Review guidance: REVIEW.md from base branch main

@marius-kilocode
marius-kilocode merged commit af6bb00 into main Jul 30, 2026
24 checks passed
@marius-kilocode
marius-kilocode deleted the fix-cmd-slash-followup branch July 30, 2026 13:53
unixcrh pushed a commit to unixcrh/kilocode that referenced this pull request Aug 1, 2026
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
…e-shot echo (Kilo-Org#12694)

* fix(agent-manager): align Cmd+/ fallback with platform binding and one-shot echo

* fix(agent-manager): expire dropped terminal-shortcut echoes at press time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants