Skip to content

merge(upstream): sync QwenLM/qwen-code upstream changes Apr 18-19 2026#1

Merged
TaimoorSiddiquiOfficial merged 7 commits into
mainfrom
merge/upstream-apr-19
Apr 19, 2026
Merged

merge(upstream): sync QwenLM/qwen-code upstream changes Apr 18-19 2026#1
TaimoorSiddiquiOfficial merged 7 commits into
mainfrom
merge/upstream-apr-19

Conversation

@TaimoorSiddiquiOfficial

Copy link
Copy Markdown
Owner

Upstream Sync — Apr 18-19, 2026

Syncs 6 upstream commits from QwenLM/qwen-code while preserving HopCode architecture.

Changes merged from upstream

Commit Description
4ee9ca9 feat(mcp): OSC 52 copy hotkey for OAuth authorization URL — press 'c' to copy URL during OAuth, works over SSH/tmux
afa7fc3 feat(cli): early input capture to prevent keystroke loss during startup
56e8227 perf(vscode): fix input lag in long conversations via React.memo
cd1be1c feat(vscode-ide-companion): agent execution tool display cards
a623655 fix(build): node --import tsx/esm instead of npx tsx — fixes bun compatibility
f340d95 ci(stale): enable 35+35 stale/close PR policy

Architecture preserved

  • All @hoptrendy/hopcode-core / @hoptrendy/webui import paths kept
  • HopCode branding, versioning (0.14.21), and repo URLs preserved
  • CI workflows unchanged — publish-npm.yml with VS Code Marketplace + Open VSX intact
  • Skills system and multi-provider auth model untouched

Conflict resolution

packages/vscode-ide-companion/.../toolcalls/index.tsx — kept @hoptrendy/webui, added ToolCallData to type imports (upstream addition needed for AgentToolCall support)

BZ-D and others added 7 commits April 18, 2026 20:22
…#3337) (QwenLM#3393)

When MCP OAuth authentication falls back to the "copy this URL into
your browser" path (e.g. remote/web terminal where the browser can't
auto-open), long URLs wrap across lines inside the bordered dialog and
the trailing │ border characters get selected alongside the URL,
forcing the user to manually strip them out before pasting.

Surface the URL on a dedicated event and let the user press 'c' to
push it to the local clipboard via an OSC 52 escape sequence. Works
through SSH and modern web terminals (iTerm2, Windows Terminal,
xterm.js-based emulators, tmux with set-clipboard, etc.) without a
subprocess, and falls back to a visible "copy the URL above manually"
hint when the terminal is not a TTY or OSC 52 is blocked.

Key points:
- OAuth provider emits OAUTH_AUTH_URL_EVENT carrying the full URL.
- AuthenticateStep listens, tracks it in state, and binds 'c' while
  authenticating (modifier/paste keys are filtered out).
- copyToClipboardViaOsc52 writes to stderr when it's a TTY,
  falls back to stdout, and wraps the sequence for tmux/GNU screen
  via DCS passthrough so multiplexed sessions still work.
- Honest feedback: distinct "copy request sent" / "cannot write to
  terminal" states with a short auto-revert so repeated presses reset
  the timer.

Fixes QwenLM#3337
…2590)

Preserve structured agent rawOutput through the VSCode session pipeline.

Render dedicated agent execution cards from shared webui components.
…LM#2550)

Extract message list into a React.memo component to prevent
re-rendering the entire chat history on every keystroke.

- Extract MessageList as a memoized component
- Wrap UserMessage, AssistantMessage, ThinkingMessage with React.memo
- Stabilize onFileClick callback with useCallback
- Remove console.log from render path
- Wrap handleToggleThinking with useCallback

Fixes QwenLM#2395

Made-with: Cursor
…tartup (QwenLM#3319)

* feat(cli): add early input capture to prevent keystroke loss during startup (QwenLM#3224)

Start raw mode stdin listening immediately after setRawMode(true), buffer
user input during REPL initialization (200-500ms), then replay it once
KeypressProvider is mounted. Prevents keystrokes typed before the REPL
is ready from being silently dropped.

- Filter out terminal response sequences (DA, DA2, OSC, DCS, APC)
  while preserving real user input (arrow keys, function keys, etc.)
- 64KB buffer limit for safety
- Replay via setImmediate() to ensure subscribers are registered first
- Disable via QWEN_CODE_DISABLE_EARLY_CAPTURE=1
- Add benchmark-startup.sh / benchmark-startup-simple.sh for baseline
  startup time measurement

Co-authored-by: Qwen-Coder <[email protected]>

* fix(cli): fix bugs and optimize early input capture

- Fix getAndClearCapturedInput resetting captured flag, preventing potential re-arm
- Fix passthrough mode replay bypassing paste marker handling in KeypressContext
- Optimize buffer storage from O(n^2) concat to chunked collection
- Optimize filterTerminalResponses to use pre-allocated Buffer instead of number[]
- Add atomic stopAndGetCapturedInput API to prevent two-step usage errors
- Remove unrelated benchmark shell scripts
- Add test for stopAndGetCapturedInput

Co-authored-by: Qwen-Coder <[email protected]>

* fix(cli): fix listener leak, silent failures, and error handling in early input capture

- Register cleanup for stdin listener in gemini.tsx to prevent orphaned
  listener on any error path before UI mounts
- Add try-catch and cancellation guard to setImmediate replay in
  KeypressContext to handle component unmount and replay errors gracefully
- Stop capture immediately and warn when buffer limit is reached instead
  of silently dropping data with a debug-level log
- Capture stdin reference at registration time so removeListener always
  operates on the correct stream instance
- Add debug log when early capture is skipped due to non-TTY stdin

Co-authored-by: Qwen-Coder <[email protected]>

* fix(cli): fix early input capture being lost under React StrictMode

Move stopAndGetCapturedInput() from inside KeypressProvider's useEffect
to before render() in startInteractiveUI. When DEBUG=1, React StrictMode
deliberately runs effect→cleanup→effect, causing the first mount to drain
the buffer and schedule a replay that the cleanup immediately cancels. The
second mount found an empty buffer, silently discarding startup keystrokes.

By draining once before render() and passing the bytes as a stable prop,
StrictMode remounts always read the same data and can schedule replay on
the second (stable) mount.

Co-authored-by: Qwen-Coder <[email protected]>

* fix: handle split ESC prefixes in early input capture

Co-authored-by: Qwen-Coder <[email protected]>

* fix: conditionally flush pending startup capture bytes

Co-authored-by: Qwen-Coder <[email protected]>

* fix: drop incomplete escape sequences instead of replaying as user input

When capture stops with an incomplete ESC sequence in pendingTerminalResponse
(e.g. lone \x1b or \x1b[), classifyEscapeSequence returns 'incomplete'.
Previously shouldReplayPendingAtStop used !== 'terminal' which treated
incomplete sequences as user input. Changed to === 'user' so only
definitively-user input is replayed; ambiguous sequences are safely dropped.

Co-authored-by: Qwen-Coder <[email protected]>

---------

Co-authored-by: jinye.djy <[email protected]>
Co-authored-by: Qwen-Coder <[email protected]>
…nLM#3237)

* fix(build): invoke tsx directly via node --import instead of npx

npx resolution breaks when scripts/build.js is invoked under bun
(bun's npx wrapper intercepts and runs tsx inside bun's runtime, where
tsx's CJS entry fails to resolve). Using 'node --import tsx/esm' skips
the npx layer entirely and works under both npm and bun invocation.

* fix(build): use node --import tsx/esm for generate:settings-schema script

Matches the approach taken in scripts/build.js so running
`bun run generate:settings-schema` directly bypasses bun's npx wrapper
and avoids the `Cannot find module './cjs/index.cjs'` tsx CJS failure.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 <[email protected]>
…3375)

* ci(stale): enable 28+28 stale/close policy for pull requests

- Fix the repository guard so the workflow actually runs on
  QwenLM/qwen-code (it was previously gated to google-gemini/gemini-cli
  and never executed in this repo).
- Scope the behavior to pull requests for now; issue policy will be
  introduced separately once triage labels are in place.
- Mark a PR stale after 4 weeks without activity, then close it after
  another 4 weeks.
- Exempt pinned, security, status/blocked, status/on-hold, and
  status/ready-for-merge from auto-close.
- Remove the stale label automatically when activity resumes, and
  process the oldest PRs first on each run.

* ci(stale): loosen PR cadence from 28+28 to 35+35

Five weeks + five weeks gives contributors more slack around holidays
and busy periods, and reduces the first-run impact on the existing
backlog. The total window moves from 56 days to 70 days.

* ci(stale): move cron from 01:30 UTC to 00:30 UTC

Shift by one hour so results are ready before the Beijing work day
starts (08:30 local), while still avoiding the top of the hour (the
high-contention window for GitHub-hosted runners) and staying 30
minutes after release.yml at 00:00 UTC.

* ci(stale): drop redundant repo guard and document ops-per-run

- Remove the `github.repository == 'QwenLM/qwen-code'` job guard:
  scheduled runs are already disabled on forks by GitHub, and
  workflow_dispatch is manually-triggered so the guard adds no safety.
- Add a comment explaining the `operations-per-run: 100` rationale
  (rate-limit headroom given the ~150-PR backlog).
Merged 6 upstream commits while preserving HopCode architecture:

Features synced from upstream:
- feat(mcp): OSC 52 copy hotkey for OAuth authorization URL (QwenLM#3393)
  Press 'c' during OAuth to copy URL via terminal clipboard, works over SSH
- feat(cli): early input capture to prevent keystroke loss during startup (QwenLM#3319)
  Buffers keystrokes during REPL init, replays once UI is mounted
- perf(vscode): fix input lag in long conversations via React.memo (QwenLM#2550)
  MessageList/UserMessage/AssistantMessage wrapped with React.memo
- feat(vscode-ide-companion): agent execution tool display (QwenLM#2590)
  Render dedicated agent execution cards in webview
- fix(build): invoke tsx via node --import instead of npx (QwenLM#3237)
  Fixes bun compatibility for generate:settings-schema script
- ci(stale): enable 35+35 stale/close PR policy (QwenLM#3375)

Conflict resolution:
- packages/vscode-ide-companion/.../toolcalls/index.tsx: kept @hoptrendy/webui,
  added ToolCallData to imports (upstream added it)
- All @qwen-code/* import paths preserved as @hoptrendy/* (HopCode arch)
- HopCode branding, version, and CI workflows preserved

Co-authored-by: Copilot <[email protected]>
Copilot AI review requested due to automatic review settings April 19, 2026 02:05
@ecc-tools

ecc-tools Bot commented Apr 19, 2026

Copy link
Copy Markdown

Analyzing 200 commits...

@ecc-tools

ecc-tools Bot commented Apr 19, 2026

Copy link
Copy Markdown

Analysis Complete

Generated ECC bundle from 7 commits | Confidence: 65%

View Pull Request #2

Repository Profile
Attribute Value
Language TypeScript
Framework Not detected
Commit Convention conventional
Test Directory mixed
Changed Files (30)
Metric Value
Files changed 30
Additions 1740
Deletions 138

Top hotspots

Path Status +/-
packages/cli/src/utils/earlyInputCapture.ts added +380 / -0
packages/cli/src/utils/earlyInputCapture.test.ts added +349 / -0
packages/vscode-ide-companion/src/webview/App.tsx modified +119 / -87
packages/webui/src/components/toolcalls/AgentToolCall.tsx added +156 / -0
packages/vscode-ide-companion/src/webview/components/messages/toolcalls/index.test.tsx added +151 / -0

Top directories

Directory Files Total changes
packages/cli/src/utils 3 730
packages/vscode-ide-companion/src/webview 1 206
packages/vscode-ide-companion/src/webview/components/messages/toolcalls 2 167
packages/webui/src/components/toolcalls 2 161
packages/vscode-ide-companion/src/webview/hooks 2 126
Likely Future Issues (2)
Severity Signal Why it may show up
MEDIUM User-facing UI changes may ship without browser coverage 12 user-facing UI paths changed; 0 browser or e2e coverage files changed
MEDIUM Dependency or CI drift could surface after merge package.json changed; CI/workflow files changed; no lockfile changes detected
  • User-facing UI changes may ship without browser coverage: The PR changes components, pages, or other user-facing UI files without touching any obvious browser or end-to-end coverage.
  • Dependency or CI drift could surface after merge: Package or workflow changes landed without an accompanying lockfile update, which often turns into CI or release noise later.
Suggested Follow-up Work (2)
Type Suggested title Targets
PR test: add browser coverage for packages/cli/src/ui/components/mcp/steps/AuthenticateStep.tsx + packages/cli/src/ui/contexts/KeypressContext.tsx packages/cli/src/ui/components/mcp/steps/AuthenticateStep.tsx, packages/cli/src/ui/contexts/KeypressContext.tsx
PR chore: refresh lockfile and validate CI after dependency updates .github/workflows/stale.yml, package.json
  • test: add browser coverage for packages/cli/src/ui/components/mcp/steps/AuthenticateStep.tsx + packages/cli/src/ui/contexts/KeypressContext.tsx: Backfill browser coverage before another user-facing UI change lands on the touched surface.
  • chore: refresh lockfile and validate CI after dependency updates: Package or workflow changes without a lockfile refresh tend to turn into noisy follow-up fixes after merge.

Copy-ready bodies

test: add browser coverage for packages/cli/src/ui/components/mcp/steps/AuthenticateStep.tsx + packages/cli/src/ui/contexts/KeypressContext.tsx

## Summary
- Add browser or end-to-end coverage for the recently changed user-facing surface.

## Why
- Backfill browser coverage before another user-facing UI change lands on the touched surface.

## Touched paths
- `packages/cli/src/ui/components/mcp/steps/AuthenticateStep.tsx`
- `packages/cli/src/ui/contexts/KeypressContext.tsx`

## Validation
- Add or extend browser / e2e coverage for the changed component, page, or flow.
- Exercise the visible user journey that depends on the touched UI surface.

chore: refresh lockfile and validate CI after dependency updates

## Summary
- Refresh the lockfile and rerun CI after the dependency or workflow changes in this PR.

## Why
- Package or workflow changes without a lockfile refresh tend to turn into noisy follow-up fixes after merge.

## Touched paths
- `.github/workflows/stale.yml`
- `package.json`

## Validation
- Refresh the lockfile in the same package manager used by the repo.
- Run the repo typecheck / test / CI entrypoints that depend on the updated package graph.
Detected Workflows (3)
Workflow Description
feature-development-with-tests-and-shared-components Implements a new feature or capability, including backend logic, UI components, shared types, and corresponding tests. Frequently involves updating both implementation and test files, as well as shared type definitions and documentation/design plans.
infrastructure-or-configuration-update Updates build scripts, CI/CD workflows, or project configuration to improve build reliability, compatibility, or automation (e.g., updating how scripts are invoked, changing workflow policies).
merge-upstream-with-conflict-resolution Merges upstream changes into the current fork, applies conflict resolution, and ensures project-specific customizations (such as branding or architecture) are preserved.
Generated Instincts (27)
Domain Count
git 4
code-style 9
architecture 1
testing 7
workflow 6

After merging, import with:

/instinct-import .claude/homunculus/instincts/inherited/HopCode-instincts.yaml

Files

  • .claude/ecc-tools.json
  • .claude/skills/HopCode/SKILL.md
  • .agents/skills/HopCode/SKILL.md
  • .agents/skills/HopCode/agents/openai.yaml
  • .claude/identity.json
  • .codex/config.toml
  • .codex/AGENTS.md
  • .codex/agents/explorer.toml
  • .codex/agents/reviewer.toml
  • .codex/agents/docs-researcher.toml
  • .claude/homunculus/instincts/inherited/HopCode-instincts.yaml
  • .claude/commands/feature-development-with-tests-and-shared-components.md
  • .claude/commands/infrastructure-or-configuration-update.md
  • .claude/commands/merge-upstream-with-conflict-resolution.md

ECC Tools | Everything Claude Code

@TaimoorSiddiquiOfficial
TaimoorSiddiquiOfficial merged commit d17599e into main Apr 19, 2026
5 of 8 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b9d7806b8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

* user input being lost during startup.
*/

import { createDebugLogger } from '@qwen-code/qwen-code-core';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Import early capture logger from the shipped core package

earlyInputCapture.ts imports createDebugLogger from @qwen-code/qwen-code-core, but this repo’s CLI package depends on @hoptrendy/hopcode-core (see packages/cli/package.json), so a normal install cannot resolve the new specifier. Because gemini.tsx imports this module at startup, interactive CLI startup can fail with module-resolution errors before UI initialization.

Useful? React with 👍 / 👎.

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { ToolCallRouter } from './index.js';

vi.mock('@qwen-code/webui', async () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mock the same webui module that ToolCallRouter imports

This test mocks @qwen-code/webui, but ToolCallRouter now imports from @hoptrendy/webui (index.tsx), so the mock is never applied. The assertions in this file depend on mocked label output (for example agent:...), which makes the new test fail or become brittle because it exercises real components instead of the intended stubbed ones.

Useful? React with 👍 / 👎.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Syncs upstream changes from QwenLM/qwen-code (Apr 18–19, 2026) into this repo while keeping HopCode package/import architecture, adding: structured agent tool-call rendering, CLI startup input capture, OSC52 OAuth URL copy support, and a stale-PR workflow policy.

Changes:

  • Add structured rawOutput plumbing + dedicated AgentToolCall renderer for subagent execution progress/cards across webui + VS Code companion.
  • Add CLI “early input capture” to prevent keystroke loss during startup, with replay into the keypress layer.
  • Add OAuth authorization URL copy hotkey (OSC 52) and update stale workflow + build script.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/webui/src/index.ts Re-exports new agent tool-call component/utilities and types.
packages/webui/src/components/toolcalls/shared/types.ts Adds agent execution types and rawOutput to ToolCallData.
packages/webui/src/components/toolcalls/shared/index.ts Re-exports new agent execution types.
packages/webui/src/components/toolcalls/index.ts Exports AgentToolCall and type-guards from toolcalls entrypoint.
packages/webui/src/components/toolcalls/AgentToolCall.tsx New dedicated UI card for structured agent execution output.
packages/webui/src/components/messages/UserMessage.tsx Memoizes user message rendering to reduce rerenders.
packages/webui/src/components/messages/ThinkingMessage.tsx Memoizes thinking message rendering to reduce rerenders.
packages/webui/src/components/messages/Assistant/AssistantMessage.tsx Memoizes assistant message rendering to reduce rerenders.
packages/webui/src/components/ChatViewer/ChatViewer.tsx Routes tool calls to AgentToolCall when rawOutput indicates agent execution.
packages/vscode-ide-companion/src/webview/hooks/useToolCalls.ts Preserves/merges rawOutput across tool-call updates.
packages/vscode-ide-companion/src/webview/hooks/useToolCalls.test.tsx Adds test coverage for rawOutput preservation in the hook.
packages/vscode-ide-companion/src/webview/components/messages/toolcalls/index.tsx Routes to AgentToolCall by inspecting rawOutput (not just kind).
packages/vscode-ide-companion/src/webview/components/messages/toolcalls/index.test.tsx Adds renderer routing tests for agent executions.
packages/vscode-ide-companion/src/webview/App.tsx Memoizes the message list to reduce input lag in long conversations.
packages/vscode-ide-companion/src/types/chatTypes.ts Adds rawOutput to tool-call update types.
packages/vscode-ide-companion/src/services/qwenSessionUpdateHandler.ts Forwards rawOutput from session updates into callbacks.
packages/vscode-ide-companion/src/services/qwenSessionUpdateHandler.test.ts Adds test coverage ensuring rawOutput forwarding.
packages/core/src/mcp/oauth-provider.ts Emits an auth URL event for OAuth flows (for CLI copy hotkey).
packages/core/src/index.ts Re-exports OAuth event constants.
packages/cli/src/utils/events.ts Adds oauth-auth-url event to CLI event enum.
packages/cli/src/utils/earlyInputCapture.ts New early-stdin capture module to buffer keystrokes during startup.
packages/cli/src/utils/earlyInputCapture.test.ts Adds unit tests for early input capture and filtering logic.
packages/cli/src/ui/contexts/KeypressContext.tsx Accepts and replays captured startup input into keypress pipeline.
packages/cli/src/ui/components/mcp/steps/AuthenticateStep.tsx Adds OSC52 copy-on-c hotkey and subscribes to OAuth URL event.
packages/cli/src/i18n/locales/zh.js Adds translations for the copy-to-clipboard UX strings.
packages/cli/src/i18n/locales/en.js Adds English strings for the copy-to-clipboard UX.
packages/cli/src/gemini.tsx Starts early input capture and drains it before React render (StrictMode-safe).
package.json Uses node --import tsx/esm for bun compatibility in settings schema generation.
docs/plans/2026-03-22-agent-tool-display-design.md Adds an implementation plan doc for the agent tool display work.
.github/workflows/stale.yml Updates stale PR policy and disables stale issue handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

appEvents.removeListener(AppEvent.OauthDisplayMessage, displayListener);
appEvents.removeListener(AppEvent.OauthAuthUrl, authUrlListener);
};
}, []);

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OAuth event subscription effect uses t(...) but has an empty dependency array. With react-hooks/exhaustive-deps enabled in this repo, this will raise a lint warning and can also capture a stale t implementation if locale/context ever changes. Include t in the dependency list (or refactor so the listeners don't close over changing values) to satisfy the hooks rule.

Suggested change
}, []);
}, [t]);

Copilot uses AI. Check for mistakes.
Comment on lines +26 to +59
// Save original stdin
originalStdin = process.stdin;
originalIsTTY = process.stdin.isTTY ?? false;

// Create mock stdin
mockStdin = new PassThrough();
Object.defineProperty(process, 'stdin', {
value: mockStdin,
writable: true,
configurable: true,
});
Object.defineProperty(process.stdin, 'isTTY', {
value: true,
writable: true,
configurable: true,
});

delete process.env['QWEN_CODE_DISABLE_EARLY_CAPTURE'];
});

afterEach(() => {
resetCaptureState();

// Restore original stdin
Object.defineProperty(process, 'stdin', {
value: originalStdin,
writable: true,
configurable: true,
});
Object.defineProperty(process.stdin, 'isTTY', {
value: originalIsTTY,
writable: true,
configurable: true,
});

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test restores process.stdin.isTTY by writing a new boolean property descriptor. Other tests in this repo preserve and restore the original descriptor (e.g. packages/cli/src/gemini.test.tsx), which avoids errors if isTTY is non-writable/non-configurable in some Node/CI environments. Consider capturing Object.getOwnPropertyDescriptor(process.stdin, 'isTTY') and restoring that descriptor (or deleting the override when none existed) instead of forcing writable/configurable.

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +9
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.

**Goal:** Add a dedicated VSCode/web UI display for Agent tool executions so subagent progress, summaries, and failures render from structured `rawOutput` instead of falling back to the generic tool card.

**Architecture:** Preserve ACP `rawOutput` through the VSCode session/update pipeline into `ToolCallData`, then let the shared web UI router detect `task_execution` payloads and render a dedicated `AgentToolCall` component. Keep the change shared in `packages/webui` so VSCode and `ChatViewer` stay aligned.

**Tech Stack:** TypeScript, React, Vitest, shared `@qwen-code/webui` tool-call components.

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This plan doc contains tool-specific AI instructions ("For Claude... superpowers:executing-plans") and references @qwen-code/webui, but this repo’s public package import path is @hoptrendy/webui. To keep project documentation repo-agnostic and accurate, remove the AI-specific directive and update the package name/wording to match the HopCode packages used in the implementation.

Suggested change
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
**Goal:** Add a dedicated VSCode/web UI display for Agent tool executions so subagent progress, summaries, and failures render from structured `rawOutput` instead of falling back to the generic tool card.
**Architecture:** Preserve ACP `rawOutput` through the VSCode session/update pipeline into `ToolCallData`, then let the shared web UI router detect `task_execution` payloads and render a dedicated `AgentToolCall` component. Keep the change shared in `packages/webui` so VSCode and `ChatViewer` stay aligned.
**Tech Stack:** TypeScript, React, Vitest, shared `@qwen-code/webui` tool-call components.
**Goal:** Add a dedicated VSCode/web UI display for Agent tool executions so subagent progress, summaries, and failures render from structured `rawOutput` instead of falling back to the generic tool card.
**Architecture:** Preserve ACP `rawOutput` through the VSCode session/update pipeline into `ToolCallData`, then let the shared web UI router detect `task_execution` payloads and render a dedicated `AgentToolCall` component. Keep the change shared in `packages/webui` so VSCode and `ChatViewer` stay aligned.
**Tech Stack:** TypeScript, React, Vitest, shared `@hoptrendy/webui` tool-call components.

Copilot uses AI. Check for mistakes.
* user input being lost during startup.
*/

import { createDebugLogger } from '@qwen-code/qwen-code-core';

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

earlyInputCapture.ts imports createDebugLogger from @qwen-code/qwen-code-core, but this repo consistently uses @hoptrendy/hopcode-core (and there are no other references to @qwen-code/qwen-code-core). This will fail to build/resolve. Switch the import to @hoptrendy/hopcode-core (or the local core entrypoint used elsewhere in the CLI package).

Suggested change
import { createDebugLogger } from '@qwen-code/qwen-code-core';
import { createDebugLogger } from '@hoptrendy/hopcode-core';

Copilot uses AI. Check for mistakes.
}

const minutes = Math.floor(durationMs / 60_000);
const seconds = Math.round((durationMs % 60_000) / 1000);

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatDuration() can produce an invalid "Xm 60s" display because seconds is computed with Math.round(...) (e.g. 119999ms -> 1m 60s). Use Math.floor for seconds (or normalize when rounding hits 60) so the minutes/seconds output stays within 0–59 seconds.

Suggested change
const seconds = Math.round((durationMs % 60_000) / 1000);
const seconds = Math.floor((durationMs % 60_000) / 1000);

Copilot uses AI. Check for mistakes.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { ToolCallRouter } from './index.js';

vi.mock('@qwen-code/webui', async () => {

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test mocks @qwen-code/webui, but the router under test imports tool-call components from @hoptrendy/webui. As written, the mock won't apply and the test will exercise the real webui package instead (or fail if @qwen-code/webui doesn't exist). Update the vi.mock(...) target to @hoptrendy/webui to match the implementation import path.

Suggested change
vi.mock('@qwen-code/webui', async () => {
vi.mock('@hoptrendy/webui', async () => {

Copilot uses AI. Check for mistakes.
TaimoorSiddiquiOfficial pushed a commit that referenced this pull request Apr 24, 2026
The global ProxyAgent was overriding per-request dispatchers used by the
OpenAI SDK, causing 'API Error: Connection error. (cause: fetch failed)'.

- Remove setGlobalDispatcher(new ProxyAgent()) from channel/start.ts
- Remove unused undici import
- Bump version to 0.17.4 across all packages

Fixes #1
TaimoorSiddiquiOfficial pushed a commit that referenced this pull request Apr 24, 2026
When no proxy is set, return undefined instead of creating a default
undici Agent. This avoids potential compatibility issues with the
OpenAI SDK's default fetch behavior.

Fixes #1
TaimoorSiddiquiOfficial pushed a commit that referenced this pull request Apr 29, 2026
* feat(core): wire background shells into the task_stop tool

Phase B follow-up #1 from QwenLM#3634, unblocked by QwenLM#3471 (control plane) merging
in. The model can now cancel a managed background shell with the same
`task_stop` tool it uses for subagents — no more falling back to
`kill <pid>` via BashTool.

Lookup order: subagent registry first (existing behavior), then the
background shell registry as a fallback. Agent IDs follow
`<subagentName>-<suffix>` and shell IDs follow `bg_<8 hex chars>`, so the
two namespaces cannot collide in practice; the order is fixed for
determinism (a defensive test pins agent-wins-over-shell).

The shell cancel path resolves through the entry's own AbortController
(which `BackgroundShellRegistry.cancel` triggers); the child process
exit handler then settles the registry to `cancelled` and the on-disk
output file is preserved for inspection via `/tasks` or a direct `Read`.
This matches Phase B's "registry's own AbortController is the
cancellation source of truth" design without needing the in-flight
notification framework that subagents use.

Tests: 7 task-stop tests (was 4) — added cancel-shell happy path,
NOT_RUNNING for already-exited shell, and a defensive
agent-takes-precedence-on-id-collision case.

* fix(core): defer shell terminal transition until spawn handler settles

@doudouOUC noticed that the previous task_stop path called
`BackgroundShellRegistry.cancel(id, Date.now())`, which marked the entry
`cancelled` immediately. The spawn handler's settle path only records
real exit info via cancel/complete/fail when the entry is still
`running`, so the cancel-vs-exit race could permanently hide a real
completed/failed result and `/tasks` would show a terminal endTime
while the process was still draining.

Add a `requestCancel(id)` method to `BackgroundShellRegistry` that
triggers the entry's AbortController only; status stays `running` until
the settle path observes the abort and records the real terminal state.
The immediate-mark `cancel(id, endTime)` is reserved for `abortAll()` /
shutdown, where the CLI process is tearing down anyway and there is no
settle handler to wait for.

Tests updated:
- `task-stop.test.ts` cancel-shell happy path now asserts the entry
  stays `running` with `endTime` undefined post-stop, and the abort
  signal fires (the settle path's contract, not task_stop's, is the
  one that flips status).
- 3 new `requestCancel` tests in `backgroundShellRegistry.test.ts`:
  running → abort+still-running, terminal entry no-op, unknown id no-op.

---------

Co-authored-by: wenshao <[email protected]>
TaimoorSiddiquiOfficial pushed a commit that referenced this pull request May 5, 2026
…rovider (QwenLM#3788)

* fix(core): inject thinking blocks for DeepSeek anthropic-compatible provider

DeepSeek's anthropic-compatible endpoint
(https://api.deepseek.com/anthropic) rejects follow-up requests with
HTTP 400 ("The content[].thinking in the thinking mode must be passed
back to the API.") whenever a prior assistant turn carrying tool_use
omits a thinking block. The model can legitimately return a tool round
without thinking text, so qwen-code stored no thought parts and rebuilt
the next request with no thinking block, tripping the API's check.

Mirroring the existing OpenAI-side fix (QwenLM#3729, QwenLM#3747), the converter
now detects DeepSeek by base URL or model name and prepends an empty
{ type: 'thinking', thinking: '', signature: '' } block to assistant
turns missing one. Other anthropic-protocol providers are unaffected.

Verified against the live api.deepseek.com/anthropic endpoint:
- assistant with tool_use, no thinking → 400 (reproduces QwenLM#3786)
- assistant with tool_use, empty thinking injected → 200 OK

Refs QwenLM#3786

* fix(core): gate DeepSeek thinking-block injection on thinking mode

Address PR review feedback:

1. (Critical) Gate empty-thinking injection on the same per-request
   condition that emits the top-level `thinking` parameter. The previous
   implementation injected unconditionally on DeepSeek providers, but
   `buildThinkingConfig()` may omit `thinking` when reasoning=false or
   `thinkingConfig.includeThoughts=false` — which is exactly what
   suggestionGenerator / ArenaManager / forkedAgent do. Shipping
   thinking blocks without enabling thinking mode is a protocol
   violation that DeepSeek may reject. Move the option from converter
   constructor to a per-request `convertGeminiRequestToAnthropic`
   parameter so the generator can compute the gate correctly.

2. (CodeQL) Replace `baseUrl.includes('api.deepseek.com')` with
   `new URL(baseUrl).hostname` exact-match. The substring check would
   accept spoofed hosts like `api.deepseek.com.evil.com`.

3. Document the empty-signature workaround inline.

4. Rename the misleading "redacted_thinking" test case.

* fix(core): narrow DeepSeek thinking injection to tool_use turns + subdomain test

Address PR review round 2:

1. Narrow injection scope to assistant turns containing tool_use. Live
   verification against api.deepseek.com/anthropic showed plain-text
   assistant turns without thinking are accepted unchanged — only
   tool_use turns trigger the HTTP 400. Injecting on every assistant
   turn unnecessarily bloats replay history with synthetic blocks the
   API does not require. Existing thinking blocks on any turn are still
   preserved untouched.

2. Add test coverage for the subdomain hostname branch
   (us.api.deepseek.com → matches), addressing the gap noted in review.

3. Update existing negative-case tests (non-deepseek / spoofed /
   reasoning=false / includeThoughts=false) to use tool_use scenarios
   so they actually exercise the gating logic instead of trivially
   passing under the narrowed scope.

* docs(core): align DeepSeek thinking-injection comments with narrowed scope

Address PR review round 3 (copilot-pull-request-reviewer × 3): comments
in three locations still described the constraint as applying to "any
prior assistant turn", which was true before commit 8721b41 but no
longer matches the implementation. Update the doc comment on
isDeepSeekAnthropicProvider and the two test-suite header comments to
state the actual narrower contract: the API rejects only tool-use turns
that omit thinking blocks; plain-text assistant turns are accepted
unchanged.

Comment-only change; 58 tests still pass.

* fix(core): per-request DeepSeek detection + strip thinking when off

Address PR review round 4 (copilot-pull-request-reviewer × 2):

1. Stale provider-detection cache (HCia). The constructor cached
   isDeepSeekProvider once, but Config.setModel() mutates
   contentGeneratorConfig.model in place. After a runtime /model switch
   from a non-DeepSeek model to a DeepSeek one on the same auth config,
   buildRequest() would keep using the stale flag. Move the detection
   into buildRequest so each call sees the current model. The detector
   is cheap (URL parse + string compare).

2. Real thought parts leak through when thinking is disabled (HCib).
   The previous gate only blocked synthetic injection — but the
   converter still replayed any existing `thought: true` parts in
   request.contents as thinking blocks. Code paths that disable
   thinking against a session whose history was built with thinking on
   (suggestionGenerator / ArenaManager / forkedAgent) would still emit
   thinking blocks alongside an absent top-level `thinking` config —
   the same protocol mismatch the gate was meant to avoid.

   Add a `stripAssistantThinking` converter option, set in buildRequest
   to `isDeepSeek && !thinking`. The converter strips thinking and
   redacted_thinking blocks from assistant messages before message
   construction completes. Mirror behavior is already proven safe by
   live verification (DeepSeek currently tolerates either shape, but
   stripping makes the request body internally consistent and robust
   to future validation tightening).

3 new tests:
- converter strips thinking from assistant turns when option set
- generator strips real thought parts when reasoning=false
- generator reflects runtime model changes (no stale cache)

61 tests pass; lint + typecheck clean.

* fix(core): preserve thinking-only assistant turns instead of emitting empty content

Address PR review round 5 (copilot-pull-request-reviewer × 2 — code +
test):

stripThinkingFromAssistantMessages previously replaced message.content
with the filtered array unconditionally. For an assistant turn whose
only blocks are thinking/redacted_thinking (e.g. a round cut off by
max_tokens before any text or tool_use was emitted), this left
`content: []` — which Anthropic API rejects.

Dropping the message entirely was considered but would break the
required user/assistant alternation. Instead, fall back to leaving the
original blocks in place when stripping would empty the message.
DeepSeek empirically tolerates the residual `thinking-block +
no-thinking-config` shape (verified against api.deepseek.com/anthropic
in the V2/X scenarios), so leaving the message untouched is the safer
choice than emitting invalid structure.

Add regression test for the thinking-only turn shape.

62 tests pass; lint + typecheck clean.

* fix(core): validate thinking-block signature, rename option, gate output_config

Address PR review round 6 — five substantive items:

1. (Critical) Drop non-compliant thinking blocks lacking a `signature`
   field and replace them with a synthetic one. A `redacted_thinking`
   block round-tripped through Gemini Part format becomes
   `{ text: '', thought: true }` (no thoughtSignature) and converts
   back to `{ type: 'thinking', thinking: '' }` without `signature` —
   not spec-compliant. The previous `hasThinking` check accepted these
   as already-satisfying, leaving non-compliant blocks in the wire
   message. Tighten the check so they're filtered out and the
   synthetic injection runs. Live verification: DeepSeek currently
   tolerates both shapes (lenient), but normalizing is defensively
   correct against future tightening.

2. Rename converter option `ensureAssistantThinking` →
   `ensureThinkingOnToolUseTurns`. The new name reflects the actual
   contract (tool-use turns only, not every assistant turn).

3. Honor `thinkingConfig.includeThoughts: false` in `buildOutputConfig`.
   Previously a per-request opt-out dropped the top-level `thinking`
   parameter but still emitted `output_config.effort`, leaking a
   reasoning-shaped field into side queries that don't want it.

4. Add regression test for mixed text + tool_use assistant turns
   (common shape: model says something, then calls a tool).

5. Add explicit test for the signature-validation path: an existing
   compliant thinking block (with signature) is preserved untouched.

64 tests pass; lint + typecheck clean.

* fix(core): clean up non-compliant thinking blocks on plain-text turns + assert output_config gating

Address PR review round 7 (copilot-pull-request-reviewer × 2):

1. (Hp-x) Round-tripped redacted_thinking blocks were left malformed on
   assistant turns lacking tool_use. The previous structure only ran
   the cleanup pass when a tool_use block was present (early return on
   `!hasToolUse`), so plain-text turns kept the non-compliant
   `{ type: 'thinking', thinking: '' }` shape. Restructure into two
   sequential steps:
     a. Drop non-compliant thinking blocks (no `signature`) on every
        assistant turn — same fallback that avoids `content: []` if
        the message is thinking-only.
     b. Inject the synthetic empty thinking block on tool_use turns
        that still lack a compliant thinking block after step (a).

2. (Hp-7) The includeThoughts=false test asserted that the top-level
   `thinking` field is suppressed but didn't cover `output_config`,
   leaving regressions in the new `buildOutputConfig` gate uncaught.
   Tighten the assertion to also verify `output_config` is absent.

3. New converter test: cleanup runs on plain-text assistant turns too.

65 tests pass; lint + typecheck clean.

* test(core): add explicit redacted_thinking injection-path coverage

Address PR review round 8 (#30 — copilot reviewer). The converter
treats `redacted_thinking` as already satisfying the thinking-block
requirement (no synthetic injected), distinguished from a
signature-less `thinking` block which is non-compliant and gets
dropped/replaced. Existing tests covered the latter path; this adds
explicit coverage of the former.

processContent doesn't synthesize redacted_thinking from Gemini parts,
so the test reaches into the private helper directly. (#31 — subdomain
hostname coverage — already exists at line 602.)

66 tests pass; lint + typecheck clean.

* fix(core): per-request anthropic-beta + normalize thinking-only turns

Address PR review round 9 (copilot-pull-request-reviewer × 2):

1. (Hydz) thinking-only assistant turns (e.g. max_tokens cutoff or
   round-tripped redacted_thinking) hit the cleanup-empties fallback
   and kept the original non-compliant `{ type: 'thinking',
   thinking: '' }` block. The fallback now replaces the message with
   a synthetic empty thinking block (`signature: ''` included), which
   keeps the message non-empty AND spec-compliant.

2. (Hyd4) `anthropic-beta` was set once at construction from the global
   `reasoning` config, so requests with per-request
   `thinkingConfig.includeThoughts=false` still advertised
   interleaved-thinking / effort even though the body had dropped the
   matching fields. Move beta computation to a new
   `buildPerRequestHeaders` that derives the header from the actual
   `thinking` / `output_config` fields present in the request body, and
   pass it via `messages.create(..., { headers })`. The wire shape is
   now internally consistent.

Test updates:
- Drop the three constructor-time beta assertions; they no longer apply.
- Add four per-request header tests covering: both betas present,
  only interleaved-thinking, reasoning=false (no betas), and per-request
  includeThoughts=false (no betas).

67 tests pass; lint + typecheck clean.

* fix(core): preserve thinking text by normalizing in place + merge user beta flags

Address PR review round 10 (copilot-pull-request-reviewer × 2):

1. (H0oF) The previous cleanup filtered out every thinking block missing
   a `signature` field. But that shape is the normal output from
   OpenAI/Gemini/agent-runtime generators, which only set `thought:
   true` without a signature. Users switching providers mid-session
   would silently lose preserved thinking text on the first DeepSeek
   request. Change Step 1 to NORMALIZE in place: when a thinking block
   has no signature, set `signature: ''` rather than dropping the
   block. The original `thinking` text is preserved; DeepSeek
   empirically accepts empty signatures so the wire shape stays valid.

2. (H0oL) `buildPerRequestHeaders()` overwrote
   `customHeaders['anthropic-beta']` whenever the per-request override
   fired, regressing the customHeaders escape hatch for unrelated
   Anthropic beta features. Merge the user's flags into the computed
   list (deduped) so users can stack their own betas alongside
   interleaved-thinking / effort.

Test changes:
- Renamed and rewrote "drops non-compliant... plain-text" test to
  assert in-place normalization that preserves thinking text.
- Updated "replaces a non-compliant thinking block" comment + name to
  describe the normalization (the assertion was already correct because
  the test happened to use empty thinking text).
- The empty-content fallback in Step 1 is no longer reachable under
  the new logic, so the dedicated thinking-only-turn test now exercises
  only the strip path (where it remains relevant).
- Added 3 customHeaders[anthropic-beta] tests: merge with computed,
  passthrough when no thinking/effort, dedupe.

70 tests pass; lint + typecheck clean.

* docs(core): align thinking-injection comments with normalize semantics + add stream test

Address PR review round 11 (copilot-pull-request-reviewer × 3):

1. (H3Iw) Update the `ensureThinkingOnToolUseTurns` option docstring
   to describe in-place normalization (preserving thinking text by
   filling in `signature: ''`) instead of the old drop-and-replace
   semantics.

2. (H3I9) Same update on the `applyEmptyThinkingToToolUseTurns` helper
   JSDoc — clarify that signature-less thinking blocks are normalized
   in place (preserving original text), not dropped. Mention the
   common case of cross-provider history where non-Anthropic
   generators only set `thought: true`.

3. (H3I3) Add a streaming test asserting that
   `generateContentStream()` also attaches the per-request
   `anthropic-beta` header. The previous coverage only exercised
   `generateContent()`, leaving the streaming path's separate code
   path (line 144 in anthropicContentGenerator.ts) unverified.

71 tests pass; lint + typecheck clean.

* refactor(core): split DeepSeek thinking option in two + add header coexistence test

Address PR review round 12 (copilot-pull-request-reviewer × 2):

1. (H6ws) The single `ensureThinkingOnToolUseTurns` option was
   misleadingly narrow: the implementation also rewrote non-tool-use
   turns by normalizing malformed thinking blocks. Future callers
   could enable it expecting only the tool-use behavior. Split into
   two precisely-named options:
     - normalizeAssistantThinkingSignature: fill missing `signature`
       on every assistant `thinking` block (cross-provider history
       compat).
     - injectThinkingOnToolUseTurns: prepend synthetic empty thinking
       on tool_use turns missing one (issue QwenLM#3786 trigger).
   The generator wires both together for DeepSeek when thinking mode
   is on; either can be used independently if a future caller needs
   only one pass.

2. (H6w4) Add a test asserting that the per-request `headers` path
   coexists correctly with `customHeaders`: User-Agent and unrelated
   customHeaders entries stay in `defaultHeaders` while only the
   computed `anthropic-beta` rides on the per-request path. Defends
   against a future regression where header config might be routed
   through a code path that wipes the constructor defaults.

72 tests pass; lint + typecheck clean.

* fix(core): case-insensitive customHeaders[anthropic-beta] merge

Address yiliang114 review feedback (QwenLM#3788).

HTTP header names are case-insensitive by spec, and the Anthropic SDK
lower-cases them during merge. Previously buildPerRequestHeaders only
read the lower-case `anthropic-beta` key from customHeaders, so a
user-configured `Anthropic-Beta` or `ANTHROPIC-BETA` would be silently
overwritten by the per-request computed value.

Replace the direct dict lookup with collectCustomBetaFlags() which
walks all customHeaders entries and matches the key case-insensitively.
Multiple matching entries (unlikely but possible) are concatenated; the
existing dedupe pass handles any duplicates.

Add a regression test for both `Anthropic-Beta` and `ANTHROPIC-BETA`
key shapes.

73 tests pass; lint + typecheck clean.

* docs(core): align thinking-injection docs with normalize-in-place semantics + redacted_thinking strip test

Address PR review round 14 (copilot-pull-request-reviewer × 4):

1. (IAl4) PR description still described "dropped here so synthetic
   injection takes over" but the implementation now normalizes
   signature-less thinking blocks in place (preserving text). PR
   description rewritten to describe the two-pass model:
   normalize-in-place + injection-when-truly-missing.

2. (IAl7) `injectThinkingOnToolUseTurns` option docstring claimed
   signature-less blocks would be "seen as missing" so the synthetic
   replaces them. Updated to describe the actual flow: the
   normalization pass runs first, blocks become compliant in place,
   the injector then sees them as already-satisfying and prepends
   nothing. Helper JSDoc on `injectEmptyThinkingOnToolUseTurns` fixed
   the same way.

3. (IAl8) Strip-path coverage missed `redacted_thinking` blocks. Added
   regression test that verifies both thinking and redacted_thinking
   blocks are removed when `stripAssistantThinking` is set.

4. (IAl-) Renamed the converter test suite from "thinking-mode
   injection + normalization (DeepSeek thinking on)" to "DeepSeek
   thinking-mode normalization, injection, and stripping" so the
   title accurately covers all behavior the block exercises (including
   `stripAssistantThinking` cases later in the same describe).

74 tests pass; lint + typecheck clean.

* fix(core): exclude anthropic-beta variants from defaultHeaders to avoid wire duplication

Address PR review round 15 (copilot-pull-request-reviewer #1).

`buildHeaders()` previously spread the entire `customHeaders` map into
the SDK's `defaultHeaders`. After moving anthropic-beta computation to
the per-request path, a user-configured mixed-case `Anthropic-Beta`
key would survive in defaultHeaders verbatim, while the per-request
override added a lowercase `anthropic-beta`. The wire then carried two
physical headers for the same logical name — SDK behavior on duplicate
headers with different casings is undefined.

`buildPerRequestHeaders()` already merges those user flags
case-insensitively (commit 0d8b5de), so dropping the entry from
defaultHeaders is the right boundary: the per-request path owns the
header end-to-end. Other customHeaders entries continue to pass
through.

Add a regression test asserting no `Anthropic-Beta` (any casing) lands
in defaultHeaders while unrelated customHeaders are kept.

75 tests pass; lint + typecheck clean.
TaimoorSiddiquiOfficial pushed a commit that referenced this pull request May 5, 2026
…wenLM#3809)

* feat(core): hint to background long-running foreground bash commands

Phase D part (a) of Issue QwenLM#3634. When a foreground `shell` tool call
runs ≥ 60 seconds and completes (succeeds or errors), append an
advisory line to the LLM-facing tool result suggesting re-running with
`is_background: true` next time.

Why: today a foreground bash that takes minutes (build watcher, soak
test, slow npm install, polling loop) blocks the agent indefinitely.
The user is already paying for the wait; the agent's next turn could
have started running in parallel under `is_background: true`. Sleep
interception (QwenLM#3684) handled the egregious `sleep N` case at validate
time; this handles the legitimate-but-long case at result time.

Trade-offs:
- Threshold = 60s. Half the existing 120s foreground timeout. Long
  enough that normal `npm install` / `pytest` runs don't trigger;
  short enough that the hint surfaces before the timeout hard-kills.
- Advisory only — the command still runs to completion in the
  foreground for THIS invocation. The advice is for the agent's NEXT
  decision, not a corrective action on the current one.
- Fires on success AND error completions. The advice is the same
  ("background it next time") in both cases.
- Suppressed on aborted (timeout / user-cancel) — those paths already
  surface their own messaging and don't benefit from a "should have
  been background" reminder when the user / system already killed it.

Implementation:
- New constant `LONG_RUNNING_FOREGROUND_THRESHOLD_MS = 60000` in
  shell.ts, paired with the existing `DEFAULT_FOREGROUND_TIMEOUT_MS`.
- Helper `buildLongRunningForegroundHint(elapsedMs)` exported so
  future surfaces (UI, telemetry) can render the same text without
  duplicating the threshold logic.
- `Date.now()` bracketing around the spawn → `await resultPromise`
  block — mirrors what the background path already captures via
  `entry.startTime`.
- Append happens inside the existing non-aborted result builder;
  zero changes to the cancel / timeout arms.

Tests: 4 new cases — fires on long success, omits on short success,
fires on long error completion, omits on aborted. Uses vi fake timers
to drive wall-clock past the threshold without actually sleeping.

* fix(core): tighten long-run hint suppression + boundary tests + post-truncation insertion

Addresses 8 review threads on PR QwenLM#3809 — 6 from /review bots, 2 from
copilot — covering doc accuracy, code quality, behavioural gaps, and
test coverage.

**Behavioural fixes (real bugs)**:

- **Suppress on external signal kills** (`result.signal != null` with
  `aborted: false`). `shellExecutionService` only sets `aborted` when
  the AbortSignal we passed was triggered, so SIGTERM from container
  shutdown / k8s eviction / OOM killer / sibling process-group reap
  falls through to the non-aborted branch. The advisory shouldn't fire
  there — the process didn't run to its conclusion, so "next time,
  background it" doesn't fit. New test pins this with `signal: 15`
  (SIGTERM), `aborted: false`.

- **Append AFTER `truncateToolOutput`**. Previously the hint was
  appended inside the non-aborted result builder, which meant for
  long outputs it got wrapped in the "Truncated part of the output:"
  envelope — the LLM might read the advisory as part of the command's
  own output. New post-truncation insertion + test that pins ordering
  by mocking `truncateToolOutput` directly (real path needs
  `fs.writeFile` to actually succeed for the replacement branch to
  fire).

- **Hint wording mode-aware**. The dialog mention dropped the
  unconditional "(footer pill + Enter)" specifics, which would mislead
  non-TTY users (`-p` headless / ACP / SDK consumers — no dialog or
  pill exists there). Now qualified as "in interactive mode the
  Background tasks dialog also has...". `/tasks` and the on-disk
  output file are mentioned without qualifier (work in any mode).

**Code quality**:

- **Threshold programmatically coupled to timeout**:
  `LONG_RUNNING_FOREGROUND_THRESHOLD_MS = Math.floor(DEFAULT_FOREGROUND_TIMEOUT_MS / 2)`.
  If the timeout is tuned later, the threshold tracks automatically.

- **Docstring corrected**: removed the misleading "before it gets
  killed by the timeout" claim — the hint is on non-aborted path
  only, so timeout-killed commands never see it. The new docstring
  enumerates all suppression paths explicitly.

- **Removed stale line-number reference**: comment said "mirrors the
  background path's `entry.startTime` capture (line ~781)" which goes
  stale on file edits. Now refers conceptually.

**Test coverage gaps closed**:

- **Off-by-one boundary**: 59_999ms → no hint. Pairs with the existing
  60_000ms-exactly test (which fires) to pin the boundary tightly. A
  regression flipping `>=` to `>` would fail loudly.

- **Timeout path explicit**: previous "aborted" test exercised user-
  cancel only. With `vi.useFakeTimers({ toFake: ['Date'] })`,
  `AbortSignal.timeout()` doesn't fake (it depends on the real timer
  subsystem), so `combinedSignal.aborted` stayed false. New test
  follows the pre-existing `should handle timeout vs user cancellation
  correctly` pattern: stubs `AbortSignal.timeout` + `.any` to return
  an already-aborted combined signal, then verifies "Command timed out
  after Nms" appears AND no advisory.

* fix(core): per-invocation long-run threshold + debug-mode + test isolation

Six suggestions from /review's third pass on PR QwenLM#3809:

**Real semantic fix**:
- Long-run threshold now scales with the EFFECTIVE timeout, not the
  fixed default. A user who sets `timeout: 600_000` (10 min) gets the
  advisory at 5 min, not at 60s — respects the explicit timeout
  intent. Replaced the `LONG_RUNNING_FOREGROUND_THRESHOLD_MS` constant
  with a per-invocation `longRunThresholdFor(effectiveTimeout)` helper.

**Debug-mode visibility**:
- Debug mode previously snapshotted `returnDisplayMessage = llmContent`
  BEFORE the truncation + hint append, so debug-mode users saw the
  pre-hint content while the agent saw the advisory — agent suddenly
  suggesting `is_background: true` had no visible trigger in the TUI.
  Re-sync `returnDisplayMessage` after the hint append (debug-mode
  branch only) so the TUI mirrors what the agent sees.

**Type-safety footgun**:
- `if (typeof llmContent === 'string')` would silently drop the hint
  if `llmContent` ever becomes structured `Part[]`. Added an explicit
  `else` comment documenting the deliberate omission and the conditions
  under which to revisit (no string llmContent path exists today).

**Style**:
- Replaced the JSDoc `/** ... */` block on the (now-defunct) constant
  with a plain `//` comment block on the helper, matching the
  `DEFAULT_FOREGROUND_TIMEOUT_MS` / `OUTPUT_UPDATE_INTERVAL_MS` style.

**Test hygiene**:
- Wrapped both `vi.stubGlobal('AbortSignal', ...)` and
  `vi.spyOn(truncateToolOutput, ...)` in `try/finally` so failures
  during the test body don't leak the stub/spy into subsequent tests
  (would cause confusing cascading failures).
- Dropped the internal-roadmap "Phase D part (a)" reference from the
  test comment — future maintainers don't have the context.

**New test**:
- `threshold scales with the user-supplied timeout (not the default)`:
  sets `timeout: 600_000`, advances 100s, verifies no hint. Pins the
  per-invocation coupling so a regression to a fixed constant would
  fail loudly here.

* fix(core): tighten long-run hint suppression + boundary tests + post-truncation insertion (round 4)

Six suggestions from /review's pai/glm-5-fp8 pass on PR QwenLM#3809:

**Behavioural / UX**:
- **Hint now visible in non-debug TUI too.** Previously only debug
  mode mirrored the hint into `returnDisplay`; non-debug users saw
  the agent suggest `is_background: true` with no visible trigger.
  Now the hint is appended to `returnDisplayMessage` in both modes
  (full mirror in debug, terse-append in non-debug to preserve the
  output-or-status form).

**Test coverage**:
- **Debug-mode re-sync test added.** All other long-run hint tests
  run with `getDebugMode → false`; this one flips it to true and
  asserts the hint appears in `returnDisplay` too. Pins the re-sync
  so a regression that drops the debug branch would fail loudly.
- **Threshold-scaling positive test added.** The negative case
  (`timeout: 600_000`, advance 100s, no hint) was already pinned;
  paired now with the positive case (advance 305s, hint fires) so a
  regression to a fixed 60s threshold is caught at both ends.

**Style / consistency**:
- **`result.signal === null` (was `== null`).** Strict equality to
  match the rest of the file. The `signal` field is typed
  `number | null` so loose equality has identical semantics, but the
  inconsistency was noise.

**Doc clarity (timing semantics)**:
- **Comment explains why elapsedMs is computed BEFORE truncation.**
  Two reviewers disagreed on the timing — one read it as before
  truncation (correct, slightly under-reports), the other as after
  (incorrect read). The intent is to report the COMMAND's runtime,
  not the tool call's total time. Truncation is post-processing,
  not part of "agent blocking time", so excluding it is the right
  semantic. Inline comment now spells this out so future readers
  don't have to infer.

* fix(core): error-path hint surfacing + clock-resilient elapsed + threshold floor + observability

Round 5 of PR QwenLM#3809 review — 10 threads, mix of Critical and Suggestion:

**Critical fixes**:

1. **Hint survives the error path** (`#OWbA`). When result.error is
   set, coreToolScheduler builds the model-facing functionResponse
   from `error.message` ONLY (not llmContent — see
   convertToFunctionResponse + the toolResult.error branch in
   scheduler:1648-1724). My hint was being silently dropped on
   long-command-failed cases. Now the hint is appended to
   error.message too so the advisory survives whichever branch the
   scheduler takes.

2. **Hint wording de-ambiguated** (`#OU6o`). "prefer re-running with
   is_background: true" was ambiguous — model could read it as
   "re-run THIS command in the background", which on stateful
   commands (DB migrations, deploys, git push) would cause double
   side effects. Reworded to "Next time you run a SIMILAR
   long-running process..." with an explicit parenthetical that
   warns against re-running the just-completed command.

3. **Debug observability** (`#OU6s`). Added `debugLogger.debug` at
   the hint decision point with elapsedMs / threshold / aborted /
   signal — when a user reports "my 65s command didn't get the
   hint" the suppression branch is now visible in DEBUG output.

**Other behaviour fixes**:

4. **Threshold floor of 1000ms** (`#OU6r`). Pathological
   `timeout: 0` / `timeout: 1` would have given a 0-ms threshold,
   firing the hint on every invocation showing "ran for 0s".
   Floor at 1s makes that branch unreachable.

5. **`performance.now()` instead of `Date.now()`** (`#OU6v`). NTP
   corrections / VM clock drift between capture and read would
   silently make `elapsedMs` negative and skip the hint with no
   observable failure. Monotonic clock prevents that.

6. **Debug mode preserves truncation marker** (`#OU6w` / `#OWCq`).
   Previously `returnDisplayMessage = llmContent` after hint
   clobbered the "Output too long and was saved to: …" line
   appended during truncation. Switched to append-style re-sync in
   BOTH modes so prior content is preserved.

**Test coverage gaps closed**:

7. **Non-debug returnDisplay test** (`#OWCo`). Pinned that the
   user TUI gets the hint in the default (non-debug) mode too.

8. **Test rename** (`#OWCl`). The "debug-mode TUI mirror" test
   passed in non-debug too after the recent refactor; split into
   two tests, one per branch.

9. **Error-path hint test**. Added a test that pins `result.error?.message`
   contains both the original error text AND the hint, covering
   the scheduler-routing-via-error.message path that was silently
   broken before fix #1.

10. **Test: faketimers also fakes `performance`**. Since we
    switched to `performance.now()`, `vi.useFakeTimers({ toFake:
    ['Date'] })` no longer covered the elapsed measurement;
    extended to `['Date', 'performance']` so the threshold tests
    can drive the wall-clock with `advanceTimersByTimeAsync`.

#OU6t (else-comment for the type guard) was already addressed in
the prior round — the explicit else-with-comment is in place;
adding logging there would be noise.

* test(core): cover the MIN_LONG_RUN_THRESHOLD_MS floor branch

PR QwenLM#3809 review: the new `Math.max(MIN_LONG_RUN_THRESHOLD_MS, ...)`
floor in `longRunThresholdFor` was untested — only default-timeout
and large-custom-timeout cases existed. A regression that strips the
floor would let `timeout: 1` produce a 0ms threshold and fire a
"ran for 0s" advisory on every invocation; the test suite would not
catch it.

New test: build with `timeout: 1`, advance 500ms (below the 1000ms
floor), resolve with `aborted: false` to isolate the threshold logic
from the abort path. Asserts no hint appears. A regression that
removes the floor flips the assertion to fail.

* fix(core): structured delimiter on error.message hint + tighten timeout floor comment

Two of three threads from the latest /review pass on PR QwenLM#3809 (the
third — PR description / threshold scaling reconciliation — is fixed
in the PR description update, not in code):

- **`\n---\n` divider before hint in `error.message`** (`#Pt7C`).
  Downstream consumers of `error.message` (firePostToolUseFailureHook,
  telemetry grouping, SIEM alerting, hook-side error parsers) were
  receiving ~400 chars of advisory text mixed inline with the
  original error body — pattern-matching on error messages would
  absorb the advisory into the matched body. Added a `---` separator
  line so the boundary is unambiguous and split-able.

- **Threshold-floor comment narrowed to `timeout: 1`** (`#Pu9o`).
  The comment said the floor guards `timeout: 0` / `timeout: 1`, but
  `validateToolParamValues` rejects `timeout <= 0` at validate time,
  so `timeout: 0` can't reach `longRunThresholdFor`. Updated the
  comment to mention only the actually-allowed pathological case
  (`timeout: 1` and any value `< 2` rounds to 0).

Test updated to assert the `---` divider format with `toMatch`.

* fix(core): capture executionStartTime AFTER spawn so PTY import isn't counted

PR QwenLM#3809 review: copilot caught that `executionStartTime` was
captured BEFORE `await ShellExecutionService.execute(...)`, which
meant the elapsed measurement included `getPty()` dynamic-import
setup (~50-200ms on first call). The hint's "ran for Xs" reading was
slightly inflated, and the comment claiming "spawn → settle" wasn't
strictly accurate.

Moved the capture immediately after the execute() call returns its
{ result, pid } handle. The pid being set by that point confirms the
process has been spawned, so the subtraction is true post-spawn-to-
settle. Comment updated to reflect the actual semantics.

The displayed accuracy gain is small (50-200ms on a 60s+ threshold
is <1%), but the comment claim now matches what the code measures.
Tests unaffected — fakeTimers don't drive real dynamic imports, so
the threshold tests behave identically.

* fix(core): align long-run hint code/tests with ShellExecutionResult.error semantics

Four copilot threads on PR QwenLM#3809 — all rooted in the same
observation: `ShellExecutionResult.error` is reserved for
spawn/setup failures (per the field's doc comment in
shellExecutionService.ts), NOT for non-zero exit codes. My existing
code/tests conflated the two, making the error-path coverage less
realistic and the inline comments inaccurate.

**Test shape fixes**:

- `appends the hint when a long-running foreground command exits
  with error` → `exits non-zero`. Changed `error: new Error('exit
  1')` to `error: null` (the realistic shape for a non-zero exit
  without spawn failure). Added a comment explaining the field
  contract so future test authors don't repeat the conflation.

- `hint survives the error path (appended to error.message)`:
  reframed the mock from `spawn ENOENT` (which would resolve in
  <1s in practice, making the long-elapsed scenario unrealistic)
  to `PTY initialization failed after 75s` — a slow-spawn-failure
  shape that COULD plausibly take 75s. Test still pins the same
  CODE PATH; comment now acknowledges the edge-case nature
  ("rare but real: PTY init dragging, remote-fs exec syscalls,
  security scanners interposing").

**Comment corrections**:

- `returnDisplayMessage` build-order comment was misleading. It
  said "the hint is appended after both the truncation block and
  the returnDisplayMessage build" — but `returnDisplayMessage` is
  built BEFORE truncation. Replaced with a chronological enumeration
  (1. initial value, 2. truncation marker append, 3. hint append)
  that matches what the code actually does.

- Error-path preservation comment now acknowledges the narrow
  applicability (spawn failures only, exit codes don't reach this
  branch). Code is unchanged — the path is still real, just rare.

* test(core): pin empty-output success + background-no-hint paths

Two defensive tests for the long-running foreground hint:

- empty-output success at >=60s — exercises the
  returnDisplayMessage='' → hint append branch (write-only commands
  like `tar czf` / `cp -r` produce no stdout). Asserts the user-
  facing returnDisplay still surfaces the advisory even when the
  command produced nothing else to show.

- background never includes the hint — the foreground hint logic
  lives in executeForeground only, so today this can't fail; the
  test guards against a future refactor hoisting the advisory into
  a shared post-execute path that would tag every background launch
  with a nonsensical "ran for 0s, consider is_background: true"
  suggestion.
TaimoorSiddiquiOfficial pushed a commit that referenced this pull request May 9, 2026
…wenLM#3115)

* feat: add commit attribution with per-file AI contribution tracking via git notes

Track character-level AI vs human contributions per file and store
detailed attribution metadata as git notes (refs/notes/ai-attribution)
after each successful git commit. This enables open-source AI disclosure
and enterprise compliance audits without polluting commit messages.

* feat: enhance commit attribution with real AI/human ratios and generated file exclusion

- Replace line-based diff with a prefix/suffix character-level algorithm
  for precise contribution calculation (e.g. "Esc"→"esc" = 1 char, not whole line)
- Compute real AI vs human contribution percentages at commit time by analyzing
  git diff --stat output: humanChars = max(0, diffSize - trackedAiChars)
- Add generated file exclusion (lock files, dist/, .min.js, .d.ts, etc.)
  ported from an existing generatedFiles.ts
- Add file deletion tracking via recordDeletion()
- Update git notes payload format: {aiChars, humanChars, percent} per file
  with real percentages instead of hardcoded 100%

* feat: add surface tracking, prompt counting, session persistence, and PR attribution

Align with the full attribution feature set:
- Surface tracking: read QWEN_CODE_ENTRYPOINT env var (cli/ide/api/sdk),
  include surfaceBreakdown in git notes payload
- Prompt counting: incrementPromptCount() hooked into client.ts message
  loop, tracks promptCount/permissionPromptCount/escapeCount
- Session persistence: toSnapshot()/restoreFromSnapshot() for serializing
  attribution state; ChatRecordingService.recordAttributionSnapshot()
  writes to session JSONL; client.ts restores on session resume
- PR attribution: addAttributionToPR() in shell.ts detects `gh pr create`
  and appends "🤖 Generated with Qwen Code (N-shotted by Qwen-Coder)"
- Session baseline: saves content hash on first AI edit of each file
  for precise human/AI contribution detection
- generatePRAttribution() method for programmatic access

* fix: audit fixes — initial commit handling, cron prompt exclusion, failed commit counter preservation

- Handle initial commit (no HEAD~1) by detecting parent with rev-parse
  and falling back to --root for first commit in repo
- Exclude Cron-triggered messages from promptCount (not user-initiated)
- Add commitSucceeded parameter to clearAttributions() so failed/disabled
  commits don't reset the prompts-since-last-commit counter
- Add test for clearAttributions(false) behavior

* fix: cross-platform and correctness fixes from multi-round audit

- Normalize path.relative() to forward slashes for Windows compatibility
- Use diff-tree --root for initial commits (git diff --root is invalid)
- Replace String.replace() with indexOf+slice to avoid $& special patterns
- Fix clearAttributions(false→true) when co-author disabled but commit succeeded
- Use real newlines instead of literal \n in PR attribution text
- Add surface fallback in restoreFromSnapshot for version compatibility
- Fix single-quote regex to not assume bash supports \' escaping
- Case-insensitive directory matching in generated file detection
- Handle renamed file brace notation in parseDiffStat

* fix(attribution): also snapshot on ToolResult turns so resume keeps tool edits

Previously, recordAttributionSnapshot() only ran at the start of UserQuery
and Cron turns — before the tools for that turn had executed. A session
that wrote a file in turn 1 and committed in turn 2 (across process
boundaries via --resume) lost the tracked edit: the last persisted
snapshot was the turn-1-start snapshot (empty fileStates), so on resume
the attribution service restored empty state and no git notes were
attached to the commit.

Move the snapshot call out of the UserQuery/Cron conditional and run it
on every non-Retry turn. ToolResult turns are scheduled right after
tools execute, so their start-of-turn snapshot now captures any edits
those tools made. Retry turns are skipped since the state is unchanged
from the prior turn.

Added unit tests asserting the snapshot fires for ToolResult/UserQuery
turns and skips Retry turns.

Verified end-to-end in a scratch repo: write-file in turn 1 (no commit)
→ exit → --resume → commit in turn 2 → git notes now contain the
recorded file with correct aiChars and promptCount: 2.

* refactor(attribution): merge duplicate retry guard and update stale doc

Collapse the two back-to-back messageType !== Retry blocks in
sendMessageStream into one, and refresh chatRecordingService's
recordAttributionSnapshot doc comment to reflect that snapshots fire
on every non-retry turn (not just after user prompts).

* feat(attribution): split gitCoAuthor into independent commit and pr toggles

Matches the shape used upstream in Claude Code's `attribution.{commit,pr}`
so users can disable the PR body line without losing the commit-message
Co-authored-by trailer (or vice versa). The previous boolean forced both
to move together, which conflated two different surfaces.

- settingsSchema: gitCoAuthor becomes an object with nested commit/pr
  booleans, each `showInDialog: true` so both appear in /settings.
- Config constructor accepts legacy boolean (coerced to { commit: v, pr: v })
  so stored preferences from the pre-split schema carry over.
- shell.ts: attachCommitAttribution and addCoAuthorToGitCommit read .commit;
  addAttributionToPR reads .pr.

* feat(settings): add v3→v4 migration for gitCoAuthor shape change

Legacy gitCoAuthor was a single boolean and shipped ~4 months ago; the
previous commit split it into { commit, pr } sub-toggles. Without a
migration, users who had set gitCoAuthor: false would see the settings
dialog show the default (true) for both sub-toggles — misleading and
likely to flip their preference on the next save because getNestedValue
returns undefined when asked for .commit on a boolean.

- New v3-to-v4 migration expands boolean → { commit: v, pr: v },
  preserves already-object values, resets invalid values to {} with a
  warning.
- SETTINGS_VERSION bumped 3 → 4; existing integration assertions use the
  constant so the next bump is a single-line change.
- Regenerate vscode-ide-companion settings.schema.json to reflect the
  new nested shape.
- Docs: split the single gitCoAuthor row into .commit and .pr.

* test(migration): cover null/array/number and partial object for v3-to-v4

The migration already treats any non-boolean, non-object value as invalid
(reset to {} with warning), but the existing test only exercised the
string "yes" branch. Add parameterized cases for null, array, and number
so a future regression that accepts these in the valid bucket gets caught.
Also cover partial objects — the migration must not paternalistically
fill defaults; that responsibility lives in normalizeGitCoAuthor at the
Config boundary.

* fix(shell): address PR review for compound commits and PR body escaping

Two critical issues called out in review:

1. attachCommitAttribution treated the final shell exit code as proof
   that `git commit` itself failed. For compound commands like
   `git commit -m "x" && npm test`, the commit can succeed and a later
   step can fail; the previous code then cleared attribution without
   writing the git note. Now we snapshot HEAD before the command (via
   `git rev-parse HEAD` through child_process.execFile, kept independent
   of the mockable ShellExecutionService) and detect commit creation by
   HEAD movement, so attribution lands whenever a new commit was created
   regardless of later steps.

2. addAttributionToPR spliced the configured generator name into the
   user-approved `gh pr create --body "..."` argument verbatim. A name
   containing `"`, `$`, a backtick, or `'` could break the command or be
   evaluated as command substitution. Now we shell-escape the appended
   text per the surrounding quote style before splicing.

Tests cover the new escape paths for both double- and single-quoted
bodies, including a generator name designed to break interpolation
(`$(rm -rf /) "danger" \`eval\``) and one with an apostrophe.

* fix(attribution): address Copilot review on shell, schema, and totals

Six items called out on PR #3115 by Copilot:

- shell.ts: addAttributionToPR's bash quote escaping doesn't apply to
  cmd.exe / PowerShell, where `\$` and `'\''` aren't honored. Skip the
  PR body rewrite entirely on Windows — losing PR attribution there is
  preferable to corrupting the user-approved `gh pr create` command.

- attributionTrailer.ts + shell.ts call site: buildGitNotesCommand used
  bash-style single-quote escaping on the JSON note, which is broken on
  Windows. Switched to argv form (`{ command, args }`) and routed the
  invocation through child_process.execFile so shell quoting is bypassed
  entirely. Tests updated to assert the argv shape.

- commitAttribution.ts: when a tracked file's aiChars exceeded the diff
  --stat-derived diffSize (long-line edits where diffSize ≈ lines * 40),
  humanChars clamped to 0 but aiChars stayed inflated, leaving aiChars +
  humanChars > the committed change magnitude. Clamp aiChars to diffSize
  so the totals stay consistent.

- shell.ts parseDiffStat: only normalized rename brace notation
  (`{old => new}`). Cross-directory renames emit `old/path => new/path`
  without braces, leaving diffSizes keyed by the full string. Added a
  second normalization step.

- shell.ts: addAttributionToPR docstring claimed `(X% N-shotted)` but
  the implementation only emits `(N-shotted by Generator)`. Updated the
  docstring to match the actual behavior.

- settingsSchema.ts + generator: gitCoAuthor went from boolean to object
  in the V4 migration. The exported JSON Schema now wraps the field in
  `anyOf: [boolean, object]` (via a new `legacyTypes` hint on
  SettingDefinition) so users with a stored boolean don't see a spurious
  IDE warning before their next launch runs the migration.

* fix(attribution): parse binary diffs, source generator from model, sync schema $version

Three follow-up review items from Copilot:

- parseDiffStat now handles git's binary-diff format (`path | Bin A ->
  B bytes`) using the byte delta with a floor of 1. Without this,
  binary edits arrived at the attribution payload as diffSize=0 and
  were silently dropped. Also extracted the parser to a top-level
  exported function so the binary path is unit-testable; added five
  targeted cases (text/binary/rename normalisation/summary skip).

- attachCommitAttribution now passes `this.config.getModel()` into
  generateNotePayload instead of the user-configurable
  `gitCoAuthor.name`. The note's `generator` field reflects which
  model produced the changes — and CommitAttributionService's
  sanitizeModelName() actually has the codename to scrub now.

- generate-settings-schema.ts imports SETTINGS_VERSION instead of
  hardcoding `default: 3`, so a future bump propagates to the emitted
  JSON schema in one place. Regenerated settings.schema.json bumps
  $version's default from 3 to 4 to match the V4 migration.

* fix(attribution): repo-root baseDir, escape co-author trailer, switch to numstat

Three Critical items called out by wenshao:

- attachCommitAttribution was passing config.getTargetDir() as `baseDir`
  to generateNotePayload, but getCommittedFileInfo returns paths
  relative to `git rev-parse --show-toplevel`. When the working
  directory was a subdirectory of the repo, path.relative produced
  `../...` keys that never matched in the AI-attribution lookup,
  silently zeroing out attribution for every file outside getTargetDir.
  StagedFileInfo now carries an optional `repoRoot` (filled in by
  getCommittedFileInfo via `git rev-parse --show-toplevel`) and the
  caller prefers it over the target dir.

- addCoAuthorToGitCommit interpolated `gitCoAuthorSettings.name` and
  `.email` into the rewritten command without escaping. A name
  containing `$()`, backticks, or `"` could be evaluated as command
  substitution under double quotes, or break the user-approved
  `git commit -m "..."` quoting. Now escapes per the surrounding quote
  style with the same helpers addAttributionToPR uses, gates on
  non-Windows for the same shell-quoting reason, and fixes the regex
  to accept `-m"msg"` shorthand (no space) so users who type the
  bash-shorthand form aren't silently denied a trailer.

- parseDiffStat used `git diff --stat` output and approximated each
  line as ~40 chars by parsing a graphical text bar. Replaced with
  `git diff --numstat` which gives unambiguous integer
  additions+deletions per file; the heuristic remains but the parser
  is no longer fooled by the visual `++--` markers. Binary entries
  fall back to a fixed estimate so they still land in the map (rather
  than dropping out as diffSize=0).

Suggestions also addressed: stale duplicate JSDoc on
addCoAuthorToGitCommit removed, misleading `clearAttributions`
comments rewritten to describe what the boolean argument actually
does. Tests cover the new shorthand path, escape behavior, and
numstat parsing (text/binary/rename/malformed).

* fix(shell): shell-aware git-commit detection and apostrophe-escape handling

Two more Critical items called out by wenshao plus the matching Copilot
quote-handling notes:

- attachCommitAttribution and addCoAuthorToGitCommit now go through a
  shell-aware `looksLikeGitCommit` helper instead of a raw
  `\bgit\s+commit\b` regex. The helper splits the command on shell
  separators (`splitCommands`) and checks each segment, so `echo "git
  commit"` no longer triggers attribution clearing or trailer
  injection. The same helper bails on any segment that contains `cd`
  or `git -C <path>`, since either could redirect the commit into a
  different repo than our cwd — writing notes or capturing HEAD there
  would corrupt unrelated state.

- The post-command attribution call now runs regardless of whether the
  shell wrapper aborted. `git commit -m "x" && sleep 999` could move
  HEAD and then time out, leaving the new commit without its
  attribution note while the stale per-file attribution stayed around
  for a later unrelated commit. attachCommitAttribution still gates on
  HEAD movement, so it's a no-op when no commit was actually created.

- The `-m '...'` and `--body '...'` regexes used to match only the
  first quote segment, so a command like `git commit -m 'don'\''t'`
  (bash's standard apostrophe-escape form) would have the trailer
  spliced mid-message and break the command's quoting. The single-
  quote patterns now use a negative lookahead / inner alternation to
  either skip those messages entirely (commit path) or match the
  whole escape-aware body (PR path).

Tests cover the new behavior: quoted "git commit" is left alone, the
`cd && git commit` and `git -C` patterns get no trailer, and the
apostrophe-escape form passes through unchanged for both `-m` and
`--body`.

* fix(attribution): drop magic 100 fallback for empty deletions

Deleted files with no AI tracking now use diffSize directly. With
numstat as the input source, diffSize is an exact count, and an
empty-file deletion legitimately reports zero — a magic fallback would
only inflate totals.

* fix(shell): broaden git-commit detection, gate background, drop dead helpers

Five Copilot follow-ups:

- looksLikeGitCommit now strips leading env-var assignments
  (`GIT_COMMITTER_DATE=now git commit ...`) and a small allowlist of
  safe wrappers (`sudo`, `command`) before matching. The previous
  exact-prefix match silently skipped trailer injection on common
  real-world commit forms.

- A new looksLikeGhPrCreate (same shell-aware shape) replaces the raw
  `\bgh\s+pr\s+create\b` regex in addAttributionToPR, so quoted text
  like `echo "gh pr create --body \"x\""` no longer triggers a
  command-string rewrite.

- executeBackground refuses to run `git commit` and tells the user to
  re-run foreground. The BackgroundShellRegistry lifecycle has no
  hook for the post-command pre/post-HEAD comparison or git-notes
  write, so allowing the commit through would create the new commit
  without notes and leak stale per-file attribution into the next
  foreground commit.

- recordDeletion was unused outside its own test — removed (and the
  test). When AI-driven deletions need tracking we'll add it with an
  actual integration point rather than carrying dead API surface.

- generatePRAttribution was likewise unused; addAttributionToPR
  builds the trailer string inline. The two formats had already
  diverged. Removed the helper and its tests; reviving from git
  history is straightforward if a future caller needs it.

Tests: env-var and sudo prefixes now produce trailers; quoted
"gh pr create" leaves the command unchanged; existing 81 shell tests
still pass alongside the trimmed 25 commitAttribution tests.

* fix(shell): unified git-commit detection split by intent

Six items called out across CodeQL, Copilot, and wenshao:

- The earlier `looksLikeGitCommit`/`stripCommandPrefix` returned a
  single yes/no and rejected ANY `cd` in the chain. That fixed the
  wrong-repo case but also disabled attribution for `git commit -m
  "x" && cd ..` (commit already landed safely in our cwd; the cd
  came after). It also conflated three distinct decisions onto one
  predicate.

  New `gitCommitContext` returns both `hasCommit` and
  `attributableInCwd`, walking segments in order so that a `cd`
  AFTER the commit doesn't invalidate it. Callers now pick the right
  arm:
  - background-mode refusal uses `hasCommit` (refuses even
    `cd /elsewhere && git commit` since we can't attribute it
    afterward either way)
  - HEAD snapshot, addCoAuthorToGitCommit, and the
    attachCommitAttribution gate use `attributableInCwd`

- Tokenisation switches from a regex while-loop to `shell-quote`'s
  `parse`. Quoted env values like `FOO="a b" git commit` now skip
  correctly (the old `\S*\s+` form would cut after the opening
  quote). Eliminates the CodeQL polynomial-regex alert at the same
  time since the `\S*\s+` pattern is gone.

- attachCommitAttribution now snapshots prompt counters via
  `clearAttributions(true)` whenever a commit lands, even if no
  per-file attributions were tracked. Previously the early-return
  on `hasAttributions() === false` meant `promptCountAtLastCommit`
  never advanced, so a later `gh pr create` reported an inflated
  N-shotted count spanning multiple commits.

Tests: env-var and sudo prefixes still produce trailers; quoted
"git commit" / "gh pr create" leave commands unchanged; cd BEFORE
commit suppresses the rewrite while cd AFTER commit does not; `git
-C <path> commit` is treated as a commit (refused in background)
but not as attributable.

* fix(shell): position-independent git subcommand detection + bash-shell guard

Six review items, two of them critical:

- gitCommitContext was checking fixed-position tokens (`arg1`, `arg3`)
  and missed every git invocation that puts a global flag between
  `git` and the subcommand: `git -c user.email=x@y commit`,
  `git --no-pager commit`, `git -C /p -c k=v commit`, etc. In
  background mode these would slip past the refusal guard; in
  foreground they got no co-author trailer, no git note, and no
  prompt-counter snapshot. New `parseGitInvocation` walks past
  git's global flags (with their values) before reading the
  subcommand, and reports `changesCwd` for `-C` / `--git-dir` /
  `--work-tree`.

- The Windows guard on addCoAuthorToGitCommit and addAttributionToPR
  used `os.platform() === 'win32'`, which incorrectly skipped Windows
  + Git Bash (`getShellConfiguration().shell === 'bash'`). Switched
  both to gate on `getShellConfiguration().shell !== 'bash'` so Git
  Bash users keep the feature.

- attachCommitAttribution was re-parsing `gitCommitContext(command)`
  even though `execute()` already gates on `commitCtx.attributableInCwd`.
  Removed the redundant re-parse — drift between the two checks would
  silently diverge trailer injection from git-notes writes.

- tokeniseSegment (formerly tokeniseProgram) now logs via debugLogger
  on parse failure instead of swallowing silently. Easier to debug
  if shell-quote ever throws on something unusual.

- Added a comment on `cwdShifted` documenting that it's a one-way
  latch — `cd src && cd ..` will still skip attribution. The
  trade-off matches the wrong-repo guard's "better miss than corrupt
  unrelated repos" intent.

- Stale `--stat` reference in the aiChars-clamp comment updated to
  `--numstat` to match the actual git command in
  ShellToolInvocation.getCommittedFileInfo.

Tests: `git -c key=val commit` and `git --no-pager commit` now
produce a trailer; existing 82 shell tests still pass.

* fix(shell): refuse multi-commit attribution; misc review follow-ups

Five follow-ups from the latest review pass:

- attachCommitAttribution now refuses to write a single git note for
  shell commands that produce more than one commit (e.g.
  `git commit -m a && git commit -m b`). The singleton's per-file
  attribution map can't be partitioned across the individual commits,
  so attaching the combined note to HEAD would mis-attribute earlier
  commits' changes to the last one. Walks `preHead..HEAD` via
  `git rev-list --count`; on multi-commit detection it snapshots the
  prompt counters and bails with a debug warning instead of writing
  a misleading note.

- parseGitInvocation now recognises the attached `-C/path` form
  (e.g. `git -C/path commit -m x`). shell-quote tokenises that as a
  single `-C/path` token which previously fell to the generic flag
  branch with `changesCwd = false`, leaving an out-of-cwd commit
  classified as attributable.

- attachCommitAttribution dropped its unused `command` parameter
  (the caller already gates on `commitCtx.attributableInCwd`, so
  re-parsing was removed earlier; the parameter became dead).

- Added wiring guards in edit.test.ts and write-file.test.ts:
  AI-originated edits/writes hit `CommitAttributionService.recordEdit`,
  `modified_by_user: true` skips, and write-file's distinction
  between a true new file and an overwritten empty file (`null` vs
  `''` old content) is now pinned by `aiCreated` assertions.

* fix(attribution): partial-commit clear, symlink baseDir, gh/git flag handling

Two Critical items, two Copilot, and five wenshao Suggestions:

- attachCommitAttribution's `finally` block used to call
  `clearAttributions()` unconditionally, wiping per-file tracking
  for files the AI had edited but the user excluded from this
  commit. Added `clearAttributedFiles(committedAbsolutePaths)` to
  the service and the call site now passes only the paths that
  actually landed in this commit; entries for un-`add`ed files stay
  pending for a later commit.

- generateNotePayload now runs both `baseDir` and each tracked
  absolute path through `fs.realpathSync` before `path.relative`.
  On macOS in particular `/var` symlinks to `/private/var`, so the
  toplevel from `git rev-parse --show-toplevel` and the absolute
  path captured by edit/write-file tools could diverge — producing
  `../../actual/path` keys in the lookup that never matched and
  silently zeroed all per-file AI attribution.

- tokeniseSegment now consumes value-taking sudo flags (`-u`,
  `-g`, `-h`, `-D`, `-r`, `-t`, `-C`, plus the long forms). Without
  this, `sudo -u other git commit` left `other` standing in for
  the program name and skipped the trailer entirely.

- A duplicate JSDoc block above `countCommitsAfter` (a leftover
  from the earlier extraction of `getGitHead`) was removed; both
  helpers now have one accurate comment each.

- attachCommitAttribution's multi-commit guard now also runs when
  `preHead === null` (brand-new repo), via `git rev-list --count
  HEAD`. A compound `git init && git commit -m a && git commit -m b`
  no longer slips through and mis-attributes combined data to the
  last commit.

- addCoAuthorToGitCommit's `-m` matching switched to `matchAll` and
  takes the LAST match. `git commit -m "title" -m "body"` puts the
  trailer at the end of the body so `git interpret-trailers`
  recognises it; the previous first-match behaviour stuffed the
  trailer in the title where git treats it as plain message text.

- addAttributionToPR's `--body` regex accepts both space and
  `=` separators (`--body "..."` and `--body="..."`); the `=` form
  is common with gh.

- New `parseGhInvocation` walks past gh's global flags
  (`--repo`, `-R`, `--hostname`) so `gh --repo owner/repo pr
  create ...` is detected. The earlier fixed-position check at
  tokens[1]/tokens[2] missed any command with a global flag.

- getCommittedFileInfo now fans out the two `rev-parse` calls and
  the three diff calls with `Promise.all`. They're independent and
  serialising them was paying spawn latency 5× per commit.

Tests: sudo with `-u user`, multi `-m`, `gh --repo owner/repo`,
`--body="..."`, plus the existing 84 shell tests still pass.

* fix(attribution): canonicalize file paths centrally in CommitAttributionService

Two related Copilot follow-ups:

- recordEdit/getFileAttribution/clearAttributedFiles now run input
  paths through fs.realpathSync before storing/looking up, so a
  symlinked path (e.g. macOS /var ↔ /private/var) resolves to the
  same key regardless of which form the caller passes. Previously
  edit.ts/write-file.ts handed in non-realpath'd absolute paths
  while generateNotePayload tried to realpath only inside its
  lookup loop, leaving partial-clear and clear-on-finally paths
  unable to find entries when the forms diverged.

- restoreFromSnapshot also canonicalises on the way in so a
  session resumed from a pre-fix snapshot (where keys may not
  have been canonical) ends up with the same shape as newly
  recorded entries — otherwise a single file could end up with
  two parallel records.

- generateNotePayload's lookup loop dropped its per-entry realpath
  call (now redundant since keys are canonical at write time),
  keeping only the realpath of `baseDir` (which still comes from
  `git rev-parse --show-toplevel` and may be a symlink).

- Updated `clearAttributedFiles` doc to describe the new semantics:
  callers can pass either the resolved repo-relative path or an
  already-canonical absolute path, and either will match.

* fix(attribution): canonicalize-from-root cleanup; fix mixed-quote -m / gh -R=

Five review items, one Critical:

- attachCommitAttribution now canonicalises via the repo *root* (one
  realpath call) and resolves committed paths against that canonical
  root, rather than per-leaf realpath inside clearAttributedFiles.
  At cleanup time the leaf for a just-deleted file no longer exists,
  so per-leaf fs.realpathSync would fail and silently fall back to a
  non-canonical path that misses the stored canonical key — leaving
  stale attributions for deleted files.
  clearAttributedFiles drops its internal realpath and now documents
  the canonical-paths-required precondition explicitly.

- addCoAuthorToGitCommit picks the LAST `-m` regardless of quote
  style. Previously `doubleMatch ?? singleMatch` always preferred
  the last double-quoted match, so `git commit -m "Title" -m
  'Body'` injected the trailer into the title where git
  interpret-trailers would silently ignore it. Now compares match
  indices, and the escape helper follows the actually-selected
  match's quote style.

- parseGhInvocation handles `-R=value` (the equals form of the
  short `--repo` alias). `--repo=...` and `--hostname=...` were
  already covered; `-R=...` previously fell through to the generic
  flag branch and skipped the value.

- New tests for the symlink-aware canonicalisation: macOS-style
  `/var` ↔ `/private/var` mapping is mocked via vi.mock on
  node:fs, with cases for record-then-look-up under either form,
  generateNotePayload with a symlinked baseDir, partial clear via
  the canonical-root-derived path (deleted leaf), and snapshot
  restore canonicalisation.

- Doc-only: integration-test header comments updated from
  "V1 -> V2 -> V3" / "migration to V3" to reflect the actual V4
  end state (assertions already used the literal `4`).

* fix(shell): scope -m rewrite to commit segment, reject nested matches

Two Critical findings on addCoAuthorToGitCommit, plus a Copilot
maintainability nit:

- The `-m` regex used to scan the whole compound command, so
  `git commit -m "fix" && git tag -a v1 -m "release"` would target
  the LATER tag annotation (last -m wins) and splice the trailer
  there instead of the commit message. The rewrite now scopes to
  the actual `git commit` segment via a new
  findAttributableCommitSegment(): same shell-aware walk
  gitCommitContext does, but returning the segment's character
  range so the regex can be run on a slice and spliced back into
  the original command.

- Within the segment, a literal `-m '...'` *inside* a quoted body
  was treated as a real later -m. For
  `git commit -m "docs mention -m 'flag' for completeness"`, the
  inner single-quoted -m sits at a higher index than the real
  outer -m, and the previous index comparison would have it win —
  splicing the trailer mid-message and corrupting the quoting.
  The new code checks whether the candidate is nested inside the
  other quote-style's range (start/end containment) and prefers
  the outer match when so.

- Hoisted three constant Sets (sudo flag list, git global flags
  taking values, git global flags shifting cwd, gh global flags)
  out of the per-call scope to module constants. Functional
  no-op, but keeps the parsing helpers easier to read and avoids
  re-allocating the Sets on every command.

Two regression tests added for the cases above:
- inner `-m '...'` inside the outer message body is preserved
  literally and the trailer lands after the body
- `git tag -a v1 -m "release notes"` after a real
  `git commit -m "fix"` is left untouched, with the trailer
  appended to "fix" only

* fix(attribution): cd-leak, numstat partial failure, $() bailout, gh pr new alias

Five Critical/Suggestion items:

- `cd subdir && git commit` (or any non-attributable commit chain
  whose HEAD movement still happens in our cwd, e.g. cd into a
  subdirectory of the same repo) used to skip attribution AND fail
  to clear pending per-file entries. Those entries then leaked into
  the next foreground commit, inflating its AI percentage. New
  `else if (commitCtx.hasCommit)` branch in execute() compares pre-
  and post-HEAD; if HEAD moved we drop the per-file state. preHead
  is now snapshotted whenever ANY commit was attempted, not only
  attributable ones.

- getCommittedFileInfo's three diff calls run in `Promise.all`. If
  `--numstat` failed while `--name-only` succeeded, every file's
  diffSize would be 0 and generateNotePayload would clamp aiChars
  to 0 — emitting a structurally valid note with all-zero AI
  percentages. Detect the partial-failure shape (files non-empty,
  diffSizes empty) and return empty so no note is written.

- addCoAuthorToGitCommit and addAttributionToPR now bail when the
  captured `-m`/`--body` value contains `$(`. The tool description
  recommends `git commit -m "$(cat <<'EOF' ... EOF)"` for
  multi-line messages, but the regex's `(?:[^"\\]|\\.)*` body group
  stops at the first interior `"` from a nested shell token —
  splicing the trailer there breaks the command before it reaches
  the executor.

- looksLikeGhPrCreate now accepts `gh pr new` as well — it's a
  documented alias for `gh pr create` and was silently skipped.

- Removed `incrementPermissionPromptCount` / `incrementEscapeCount`
  and their getters: they had no production callers, so the backing
  fields just round-tripped through snapshots as 0. The four
  snapshot fields are now optional so pre-fix snapshots that carry
  non-zero values still load cleanly and just get ignored.

Three regression tests added: heredoc-style `-m "$(cat <<EOF...)"`
preserved literally, heredoc-style `--body` likewise, `gh pr new
--body "..."` rewritten with attribution.

* fix(attribution): --amend, --message/-b aliases, .d.ts over-exclusion

Four Copilot follow-ups, three of them user-visible coverage gaps:

- `git commit --amend` was diffing `HEAD~1..HEAD` for attribution,
  which spans the entire amended commit (parent → amended) rather
  than the actual amend delta. A message-only amend would emit a
  note attributing every file in the original commit to this
  amend. New `isAmendCommit` helper detects the flag and
  getCommittedFileInfo switches to `HEAD@{1}..HEAD` (the pre-amend
  HEAD vs the amended HEAD); if the reflog is GC'd we bail with a
  warni