chore(integration): sync main into daemon_mode_b_main (2026-05-25)

[doudouOUC]

Summary

Periodic main → integration sync, mirroring #4469's pattern (2026-05-24). Pulls 5 main commits that landed since #4469:

Commit	PR	Title
94da486e1	#4464	fix(weixin): send decryptable image payloads
8ef73599d	#4465	fix(weixin): allow Windows image paths inside workspace
ab26a5ab7	#4470	fix(cli): resolve stale closure race in text buffer submit handler
84f408017	#4468	feat(skills): add memory-leak-debug skill for heap snapshot diagnosis
4dc98484f	#4288	feat(cli): do not append trailing space for directory completions (#4092)

Why now

Unblocks #4490 (the reverse daemon_mode_b_main → main integration merge for v0.16-alpha), which is currently CONFLICTING precisely because of these 5 commits. After this PR merges, #4490 will auto-recompute mergeable status and the maintainer can flip it from DRAFT to ready-for-review.

Conflicts resolved (13 total)

11 manual UU (daemon-side preferred except text-buffer.ts)

File	Resolution
packages/acp-bridge/package.json	Kept HEAD's fuller description (F1 lift expanded the package surface; main has stale pre-F1 wording).
packages/cli/src/acp-integration/acpAgent.ts	Kept HEAD's WorkspaceMcpBudget import (F2 needs it).
packages/cli/src/acp-integration/acpAgent.worktree.test.ts (AA)	Kept HEAD's superset of mocks — main-side SessionStartSource + SessionEndReason already present in HEAD's version.
packages/cli/src/ui/commands/directoryCommand.{tsx,test.tsx}	Pure formatting (HEAD wrapped, main inline). Kept HEAD.
packages/cli/src/ui/commands/skillsCommand.ts	Pure formatting. Kept HEAD.
packages/cli/src/ui/hooks/useCommandCompletion.{tsx,test.ts}	Pure formatting. Kept HEAD.
packages/cli/src/ui/hooks/useSlashCompletion.test.ts	Pure formatting. Kept HEAD.
packages/core/src/config/config.test.ts	Kept HEAD's TrustGateError import (daemon-added).
packages/cli/src/ui/components/shared/text-buffer.ts (4 zones)	Took MAIN wholesale for #4470's stale-closure fix — stateRef.current.lines.join('\n') capture for the writeFileSync + text-comparison + dep array. Daemon side had useReducer as a dead import (verified via grep — file uses dispatch from a useCallback, not useReducer); main's useRef import is the live one needed for stateRef.

2 AA (main wholesale via git checkout --theirs)

• packages/core/src/permissions/dangerousRules.ts + .test.ts — Original #4151 Auto-mode added these on main; they came into daemon via the chore(integration): sync main into daemon_mode_b_main (2026-05-24) #4469 squash. Main then landed #4371 ("strip additional dangerous interpreter rules") as a follow-up that daemon side never saw. Take main's evolved version wholesale.

Verification

• packages/core tsc: 50 errors PRE-merge, 50 errors POST-merge — none introduced by this sync (pre-existing baseline; out of scope for an integration PR).
• packages/acp-bridge tsc: clean.
• Spot tests on every conflict-resolved file:
  • text-buffer.test.ts: 132/132 ✅
  • directoryCommand.test.tsx: 17/17 ✅
  • useCommandCompletion.test.ts: 24/24 ✅
  • useSlashCompletion.test.ts: 30/30 ✅
  • skillsCommand.test.ts: 1/1 ✅
  • Aggregate: 204 / 204 tests pass on the touched files.

Recommended merge strategy

Squash (mirroring #4469's pattern). The 5 main PRs already each have their own commit + PR # link on main; daemon_mode_b_main only needs a single squash commit summarizing "pulled X commits".

🤖 Generated with Qwen Code

[github-actions]

📋 Review Summary

This PR syncs daemon_mode_b_main with 5 commits from main, introducing a new memory-leak-debug skill, URL credential redaction utility, and a git worktree-aware project root finder. The changes are well-documented, thoroughly tested, and follow established project conventions. Overall assessment: high quality, ready to merge.

🔍 General Feedback

• Strong documentation: The memory-leak-debug skill includes comprehensive step-by-step guidance with a worked example, making a complex debugging workflow accessible.
• Test coverage is excellent: All new functionality has corresponding tests with good edge case coverage (11 test cases for redaction, 5 for project root).
• Clean separation of concerns: Each file has a single, well-defined responsibility.
• Consistent code style: Matches existing project conventions (strict TypeScript, ESM imports, JSDoc comments).
• Thoughtful error handling: Both utility functions handle edge cases gracefully without throwing.

🎯 Specific Feedback

🟢 Medium

• packages/core/src/extension/redaction.ts:9 — The regex pattern [^/\s]+ in URL_CREDENTIALS_PATTERN is greedy and could potentially over-match in edge cases with multiple @ symbols in complex URLs. Consider adding a test case for URLs with fragments or query strings after credentials (e.g., https://user:pass@example.com/repo?query=value#fragment) to verify the redaction stops at the correct @. The current tests cover many cases but this specific scenario isn't explicit.

• packages/core/src/utils/projectRoot.ts:38 — The error handling block (lines 40-64) is quite verbose for a utility function. Consider extracting the logging logic into a helper function to improve readability:

  function logGitCheckError(gitPath: string, error: unknown) {
    // ... existing logging logic
  }

• packages/core/src/utils/projectRoot.ts:10 — The debug logger is instantiated but only used in error cases. Consider whether this warrants a dedicated logger instance, or if console.warn would suffice for the rare error cases. This is a minor consistency point — the project appears to use createDebugLogger elsewhere.

🔵 Low

• .qwen/skills/memory-leak-debug/SKILL.md:47 — The code block ending at line 109 has an unclosed backtick in the chrome-devtools get_node_retainers command (line 109 shows ` instead of ```). This appears to be a truncation in the diff, but verify the full file is complete.

• .qwen/skills/memory-leak-debug/SKILL.md:28-33 — The note about environment not persisting across tool calls is helpful, but consider adding a brief example of how to use the SESSION variable in subsequent commands (e.g., "Use this SESSION value in Step 2 commands like kill -USR2 $(bash .qwen/skills/memory-leak-debug/scripts/find-leaf-node.sh "$SESSION")").

• packages/core/src/utils/projectRoot.test.ts:70-77 — The symlink test case documents that symlinks are NOT followed, which is good. However, the comment mentions "this documents the current behavior, not a guarantee" — consider making this more definitive: either this is the intended behavior (in which case state it clearly) or it's a potential future enhancement (in which case consider a TODO comment).

• packages/core/src/extension/redaction.ts:12-15 — The JSDoc mentions "extension sources" but the function is exported from packages/core. Consider clarifying whether this is intended for extension-related code specifically, or for general URL redaction throughout the codebase.

✅ Highlights

• memory-leak-debug skill — Exceptional documentation quality. The step-by-step workflow with concrete examples, prerequisites, and cleanup instructions makes this immediately usable. The worked example linking to a real bug (React reconciler leak) provides valuable context.

• find-leaf-node.sh script — Simple, robust bash script with proper error handling (set -euo pipefail) and clear comments. The iterative approach to finding the innermost node process is elegant.

• projectRoot.ts — Excellent handling of git worktrees and submodules (.git as file vs directory). The JSDoc clearly explains the design decision and historical context.

• redaction.test.ts — Comprehensive test coverage including edge cases like percent-encoded credentials, embedded URLs in diagnostic messages, and malformed inputs. Test case naming is clear and descriptive.

• projectRoot.test.ts — The test for .git as a file (worktree layout) includes a comment explaining the bug that prompted the extraction — great institutional knowledge preservation.

• All new files — Proper copyright headers and SPDX license identifiers included consistently.

[doudouOUC]

@wenshao 这个 sync PR + #4490 是一个 chain，按 #4175 branching strategy 推进 v0.16-alpha 切版：

Action chain:

1. chore(integration): sync main into daemon_mode_b_main (2026-05-25) #4500 (this) — main → daemon_mode_b_main 5-commit sync。MERGEABLE，可直接 squash merge（mirror #4469 pattern）
2. chore(integration): merge daemon_mode_b_main into main — F1/F2/F3/F4-prereq + F5 alpha docs batch (#4175) #4490 — daemon_mode_b_main → main 反向 14-feature batch (F1/F2/F3/F4-prereq + F5 alpha docs)。当前 DRAFT + CONFLICTING，chore(integration): sync main into daemon_mode_b_main (2026-05-25) #4500 合掉后自动 unconflict，可翻 ready-for-review
3. 后续 PR 28 npm publish scaffolding for v0.16-alpha + PR 31 v0.16-alpha.0 cut 串行

这个 PR 安全性: 11 个 UU 手工解 + 2 个 AA 走 --theirs 拿 main 整文件；其中唯一非纯 formatting 的实质合并是 text-buffer.ts —— #4470 的 stale-closure 真 bug fix 完整搬过来（用 stateRef.current 替代 stale 闭包捕获的 text，detail 在 PR 描述里）。daemon 侧的 F2 改动（WorkspaceMcpBudget import 等）都保留了。

验证: core tsc 50 errors pre = 50 errors post（零新增 baseline）；acp-bridge tsc clean；5 个 spot test 共 204/204 通过。

Cron d55d6eb1 / ff07930c / 0dbb89df 等 PR-monitoring cron 都已退掉，没起新 cron 监控这俩。

[Copilot]

Pull request overview

This PR syncs in changes from main into the integration branch, bringing in new core utilities and a debugging skill intended to improve reliability (project root detection), security (URL credential redaction), and operational diagnostics (heap snapshot workflow).

Changes:

• Added a findProjectRoot() utility with Vitest coverage, intended to correctly detect git worktree/submodule roots where .git is a file.
• Added redactUrlCredentials() + tests to prevent credential leakage when rendering credentialed URLs.
• Added a memory-leak-debug skill with scripts and a worked example for heap snapshot-based leak diagnosis.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/core/src/utils/projectRoot.ts	New shared helper to locate the nearest git repo root by detecting .git as dir or file.
packages/core/src/utils/projectRoot.test.ts	Unit tests for findProjectRoot() covering directory, file, and symlink behavior.
packages/core/src/extension/redaction.ts	New redaction helper to strip userinfo credentials from URL-like strings.
packages/core/src/extension/redaction.test.ts	Unit tests for URL credential redaction across multiple edge cases.
.qwen/skills/memory-leak-debug/SKILL.md	New skill documenting a heap snapshot workflow using chrome-devtools CLI.
.qwen/skills/memory-leak-debug/scripts/find-leaf-node.sh	Helper script to find the leaf node PID in a tmux session process tree.
.qwen/skills/memory-leak-debug/examples/react-reconciler-performance-measure-leak.md	Worked example documenting diagnosis + remediation of a past leak.

---

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

[qwen-code-ci-bot]

Qwen Code Review (DEEP)

CI-safe profile adapted from bundled /review; focused tool-free passes: correctness/security, test coverage, maintainability/performance, and undirected audit.

Correctness / Security

• P2 packages/core/src/utils/projectRoot.ts:49-50 — The VITEST env-var check couples production module behavior to the test runner. VITEST is injected by the vitest framework, not by application config; if it leaks into a non-test context (e.g., a worker process spawned from a test, or an accidental export), non-ENOENT filesystem errors (EACCES, EPERM, EIO) are silently swallowed with no warning. Impact: real filesystem permission errors become invisible during affected runs. Suggested fix: remove the VITEST branch and rely solely on the ENOENT check for silence; let the debug logger's own level/filter handle test-time noise:

  if (!isENOENT) {
    // log warning
  }

• P3 packages/core/src/extension/redaction.ts:10 — URL_CREDENTIALS_PATTERN is anchored only by the scheme prefix ([a-z][a-z0-9+.-]*:\/\/). If redactUrlCredentials is ever called on free-form diagnostic text containing scheme:// fragments (stack traces, config dumps), it will redact any userinfo@-shaped substring, not only intentional credential-bearing URLs. The current callers (extension sources) are narrow, so this is low-risk today. Impact: over-redaction if the function's call surface widens without callers realizing the broad match. Suggested fix: add a JSDoc note on the match breadth, or tighten to known extension schemes if the caller set is closed.

No correctness or security issues found at P0/P1.

Needs Verification

• The diff shows only 7 newly-added files; the 11 manual conflict resolutions (including the text-buffer.ts stale-closure fix from #4470) and 2 AA --theirs takeovers are described in the PR body but not present in the supplied diff. A reviewer should confirm that packages/cli/src/ui/components/shared/text-buffer.ts actually uses useRef/stateRef.current (not the daemon-side useReducer dead import) and that packages/core/src/permissions/dangerousRules.ts matches main's post-fix(core): strip additional dangerous interpreter rules #4371 version.

---

Test Coverage

• P3 packages/core/src/utils/projectRoot.ts:49-73 - Error handling in catch block lacks test coverage. The code explicitly handles ENOENT vs non-ENOENT errors, checks for test environment (NODE_ENV === 'test' or VITEST), and conditionally logs warnings for non-ENOENT errors outside test environments. While these are defensive paths unlikely to execute in normal operation, they contain non-trivial logic (error classification, environment detection, conditional logging) that is not verified by tests. Consider adding a test that mocks fs.lstat to throw a non-ENOENT error (e.g., EACCES) to verify the logging behavior and environment detection work correctly.

No other concrete test coverage gaps found. The redaction.test.ts provides comprehensive coverage (11 tests) for the credential redaction function, including happy path, edge cases (encoded chars, special chars, malformed URLs), negative cases, and embedded credentials. The projectRoot.test.ts provides good coverage (5 tests) for the main functionality including the worktree bug fix.

Needs Verification

No coverage concerns requiring code outside the supplied context.

Validation Evidence

MISSING - The PR provides typecheck results (packages/core tsc: 50 errors pre = 50 errors post, no new errors; packages/acp-bridge tsc: clean) and spot test results on 5 conflict-resolved files (204 tests pass: text-buffer, directoryCommand, useCommandCompletion, useSlashCompletion, skillsCommand). However, the validation does not include test results for the two new test files added in this PR: redaction.test.ts and projectRoot.test.ts. The author should add test output showing that these new modules pass in the integration branch (e.g., cd packages/core && npx vitest run src/extension/redaction.test.ts src/utils/projectRoot.test.ts).

This is an automated, advisory, comment-only review — it never approves or requests changes.

---

Maintainability / Performance

No maintainability or performance issues found.

Notes on reviewed code:

• packages/core/src/extension/redaction.ts:10 — URL credentials regex is well-tested with comprehensive edge case coverage (encoded chars, embedded URLs, custom schemes). The (?:[^/\s]+@)+ pattern handles credentials containing unencoded @ characters, which the tests verify.

• packages/core/src/utils/projectRoot.ts:30-74 — Sequential fs.lstat calls walking up the directory tree is the standard approach for this use case. Directory depth is typically small (< 10 levels), and the function is not called frequently. Error handling properly distinguishes ENOENT from other errors with appropriate logging suppression in test environments.

• packages/core/src/utils/projectRoot.ts:55 — Test environment detection via process.env['NODE_ENV'] === 'test' || process.env['VITEST'] is a common pattern and well-commented.

Needs Verification

No concerns requiring additional code context.

Validation Evidence

PRESENT — The author provides comprehensive validation evidence:

1. TypeScript verification: packages/core tsc shows 50 errors PRE-merge = 50 errors POST-merge (zero new errors introduced by sync). packages/acp-bridge tsc clean.

2. Spot test results: 204/204 tests pass across all conflict-resolved files:

   • text-buffer.test.ts: 132/132
   • directoryCommand.test.tsx: 17/17
   • useCommandCompletion.test.ts: 24/24
   • useSlashCompletion.test.ts: 30/30
   • skillsCommand.test.ts: 1/1

3. Detailed conflict resolution documentation: Table showing all 11 manual UU conflicts with resolution rationale, plus 2 AA conflicts taken wholesale from main with explanation.

4. Specific verification of text-buffer.ts merge: Confirmed via grep that daemon-side useReducer was dead import (file uses dispatch from useCallback), validating decision to take main's useRef import for the stale-closure fix.

This is an automated, advisory, comment-only review — it never approves or requests changes.

---

Undirected Audit

• P2 packages/core/src/extension/redaction.ts:10 - The regex pattern [^/\s] correctly follows RFC 3986 (userinfo cannot contain unescaped /), but malformed URLs with / in credentials would only partially redact. Example: https://user:pass/word@host becomes https://***REDACTED***@/word@host, leaking the substring after /. While such URLs are technically invalid, copy-paste errors or buggy generators could produce them in logs. Impact: Partial credential exposure in telemetry/logs for malformed inputs. Suggested fix: Add an explicit test documenting this edge case, or broaden to [^@\s] to capture everything before the final @ (accepting that it may over-match on truly broken URLs).

• P3 packages/core/src/extension/redaction.ts:14 - The JSDoc states "The original source should still be preserved for installation and update operations" but doesn't indicate where this pattern is enforced or how callers should structure their code. Future maintainers auditing for security might apply this function too broadly and inadvertently break installation/update flows that need the raw credentials. Impact: Risk of breaking legitimate credential usage if redaction is applied indiscriminately. Suggested fix: Add a brief pointer like "See extensionManager.ts for the pattern of keeping unredacted source in a separate variable" or document the invariant more explicitly.

• P3 packages/core/src/utils/projectRoot.ts:42-54 - The function intentionally skips .git symlinks (documented in comments and tests), but provides no diagnostic when this occurs. Users with symlinked .git directories (uncommon but possible in some monorepo/worktree setups) receive null silently. The warning log only triggers for non-ENOENT filesystem errors, not for "found symlink, not following." Impact: Difficult to debug "why isn't my project root recognized?" issues in production. Suggested fix: Add a debug-level log when stats.isSymbolicLink() is true: logger.debug(\Found .git symlink at ${gitPath}, not following (see docs)`)`.

Needs Verification

• The PR verifies conflict resolution (204 tests on touched files) but does not explicitly confirm that the newly synced test files (redaction.test.ts, projectRoot.test.ts) pass in the daemon_mode_b_main environment. While these landed on main with passing tests, environment-specific issues (different tsconfig, dependencies, or mocks in daemon branch) could cause failures. Run: cd packages/core && npx vitest run src/extension/redaction.test.ts src/utils/projectRoot.test.ts.

• The redaction utility is presumably integrated into logging/telemetry paths, but the diff doesn't show call sites. Verify that all credential-bearing output paths (error logs, telemetry events, diagnostic messages) route through redactUrlCredentials() to prevent the exact leaks this utility is designed to catch.

Validation Evidence

PRESENT - The PR provides tsc verification (50 errors pre-merge = 50 errors post-merge, zero introduced), 204 passing tests on conflict-resolved files, and detailed conflict resolution rationale. However, explicit test runs for the newly synced files (redaction.test.ts, projectRoot.test.ts) in the daemon_mode_b_main context are not documented.

This is an automated, advisory, comment-only review — it never approves or requests changes.

---

Qwen Code /review · qwen3.7-max · tier DEEP