Thanks for the PR!

Template: the PR body doesn't follow the required template. Missing sections: "What this PR does", "Why it's needed", "Reviewer Test Plan" (How to verify, Evidence Before/After, Tested on table), "Risk & Scope", "Linked Issues", and the Chinese translation <details>. The current "Summary" + checkbox "Test plan" format doesn't give reviewers enough to verify the change. Please reformat.

On direction: clearly aligned — this is issue #4514 T3.9, explicitly tracked as a Tier 3 gap. The hooks PR (#4822) set the exact same precedent and shipped cleanly. Extending the diagnostic surface to extensions is a natural next step.

On approach: the scope feels right — follows the same layered pattern as the hooks PR (status types → HTTP route → workspace service → bridge → SDK → command). The /extensions command gaining non_interactive/acp modes with a list subcommand is a sensible addition. One observation: roughly 40% of the diff is prettier reformatting existing code (spreading ternaries in serializeHookConfig, IDLE_HOOK_EVENTS, etc.) that's unrelated to the feature. Not blocking, but splitting those into a separate commit would make the actual feature changes easier to review.

Once the template is filled in, happy to proceed to code review. 🔍

— Qwen Code · qwen3.7-max

📋 Review Summary

This PR adds a read-only diagnostic surface for installed extensions via a new HTTP endpoint (GET /workspace/extensions) and CLI slash command (/extensions list). The implementation follows established patterns from similar status endpoints (MCP, skills, hooks) and includes appropriate test coverage. Overall, the code is well-structured and maintains consistency with existing diagnostic surfaces.

🔍 General Feedback

• The implementation correctly follows the established pattern for workspace status endpoints, mirroring the structure of workspaceHooks, workspaceMcp, etc.
• Good separation of concerns: types in status.ts, HTTP route in server.ts, bridge integration in bridge.ts, and CLI command in extensionsCommand.ts.
• The SDK client helper (DaemonClient.workspaceExtensions()) provides a clean API for external consumers.
• Test coverage includes both the HTTP endpoint and the fake bridge stub, which is consistent with other status endpoint tests.
• The PR appropriately handles sensitive data by redacting URLs via redactUrlCredentials() and excluding resolvedSettings from serialization.

🎯 Specific Feedback

🟡 High

• File: packages/cli/src/ui/commands/extensionsCommand.ts:133-141 - The capability counting logic in listTextAction differs from the HTTP endpoint's counting in buildWorkspaceExtensionsStatus. The CLI command counts MCP servers, skills, and commands only, while the HTTP endpoint counts all 8 capability types (including agents, hooks, context files, channels, settings). This inconsistency could confuse users who see different capability summaries in different contexts. Consider aligning the CLI output to show all capabilities or extracting the counting logic to a shared utility.

🟢 Medium

• File: packages/acp-bridge/src/status.ts:882-892 - The createIdleWorkspaceExtensionsStatus function sets initialized: false but doesn't include an errors array. While optional, other idle status creators like createIdleWorkspaceMcpStatus include all optional fields for consistency. Consider adding errors: [] for pattern consistency.

• File: packages/cli/src/acp-integration/acpAgent.ts:2250-2252 - The hook counting logic uses a reduce pattern that's more verbose than necessary:

  hookCount: ext.hooks
    ? Object.values(ext.hooks).reduce(
        (sum, defs) => sum + (defs?.length ?? 0),
        0,
      )
    : 0,

  This could be simplified to:

  hookCount: ext.hooks
    ? Object.values(ext.hooks).reduce((sum, defs) => sum + (defs?.length ?? 0), 0)
    : 0,

  (minor formatting) or extracted to a helper function for clarity.

• File: packages/cli/src/serve/server.ts:1058-1066 - The endpoint handler doesn't include any request validation or rate limiting comments. While other status endpoints follow the same pattern, it's worth considering whether this endpoint needs any protection against abuse since it queries extension state on every call.

🔵 Low

• File: packages/acp-bridge/src/bridge.ts:3042-3047 - Consider adding a JSDoc comment to getWorkspaceExtensionsStatus method to match the style of other methods like getSessionHooksStatus which has /** Read session-scoped hook status for a live session. */.

• File: packages/acp-bridge/src/status.ts:846-855 - The ServeExtensionCapabilities interface could benefit from inline comments explaining what each count represents, especially for less obvious ones like channelCount or hasSettings.

• File: packages/cli/src/ui/commands/extensionsCommand.ts:125 - The markdown output format uses **Installed Extensions (${extensions.length} total, ${active.length} active)** which is good. Consider adding a note about the /extensions list command being available in non-interactive mode when the user is in interactive mode, or vice versa.

• File: packages/sdk-typescript/src/daemon/types.ts:1477-1526 - The type definitions are at the end of the file without any grouping comment separator like the other interfaces have. Consider adding a comment block separator for consistency with the rest of the file.

✅ Highlights

• Security handling: Excellent attention to sensitive data - using redactUrlCredentials() for source URLs and explicitly excluding resolvedSettings from serialization (as noted in the PR description).
• Test coverage: The test at line 1815-1858 in server.test.ts thoroughly validates the endpoint response shape, status code, and bridge call count.
• Type safety: The ServeExtensionInstallType and ServeExtensionOriginSource types are properly constrained unions, preventing invalid values from being serialized.
• Error handling: The buildWorkspaceExtensionsStatus method wraps the entire operation in a try-catch and returns a structured error response with initialized: false and an error cell, which is the correct pattern.
• Optional chaining: Proper use of optional chaining (ext.installMetadata?.source) throughout handles extensions without metadata gracefully.

Review round — wenshao CHANGES_REQUESTED

Addressed wenshao's latest review (2026-06-07)

Fixed:

• _args → args in listAction — removed misleading underscore prefix since the parameter is used (b39b6a0)

Deferred (framework-level):

• getCommandSubcommandNames not filtering subcommands by mode — this is the same framework-level gap identified in Codex review round 2. Runtime guards already prevent execution of interactive-only subcommands in ACP/non_interactive mode (fb8b8de). The metadata advertising issue affects all commands with mixed-mode subcommands, not just /extensions. Fixing it properly requires changes to commandMetadata.ts and the command registration framework, which is out of scope for this PR.

Local Verification Report

PR: #4832 feat/serve-extensions-status → daemon_mode_b_main
Scope: 14 files, +525/−51 (acp-bridge, CLI serve, SDK daemon)

Test Results

New Typecheck Errors (2)

Both in packages/cli/src/ui/commands/extensionsCommand.ts:

extensionsCommand.ts(31,16): error TS7030: Not all code paths return a value.
extensionsCommand.ts(169,16): error TS7030: Not all code paths return a value.

exploreAction (line 31) and installAction (line 169) return an object on some code paths (e.g., early-exit error message) but void on others. TypeScript flags the inconsistency. Consider either always returning a result object or typing the return as ... | void.

CLI Test Failure Analysis (all pre-existing)

All 12 CLI failures are pre-existing on daemon_mode_b_main:

1. Capability registry stale assertions (3 tests) — expected feature lists don't include features added by recent PRs.
2. runQwenServe afterEach hook timeouts (9 tests) — afterEach at server.test.ts:5284 times out at 10000ms.

Build Notes

• Core and acp-bridge rebuilt before test/typecheck (cross-package dist dependency).

Verdict

✅ PASS with nit — No new test failures. 2 new TS7030 typecheck warnings in extensionsCommand.ts (inconsistent return paths) — non-blocking but worth fixing before merge.

Verified locally by wenshao

Addressed wenshao's local verification (2026-06-07)

Fixed:

• TS7030 exploreAction and installAction inconsistent return paths — added explicit return undefined at function end to satisfy noImplicitReturns (6211ef9)

Thanks for the thorough local verification!

Addressed wenshao's round 3 review (2026-06-07)

Local Verification Report (Re-run after 2 new commits)

PR: #4832 feat/serve-extensions-status → daemon_mode_b_main
Scope: 14 files, +525/−51 (acp-bridge, CLI serve, SDK daemon)
Commits: 8 (was 6 at prior review — 2 new fix commits)

Test Results

Previous Issue Resolved

The 2 TS7030 errors in extensionsCommand.ts ("not all code paths return a value") flagged in the prior review have been fixed in the new commits. CLI typecheck now shows 43 errors, exactly matching the daemon_mode_b_main baseline.

CLI Test Failure Analysis (all pre-existing)

All 12 CLI failures are pre-existing on daemon_mode_b_main:

1. Capability registry stale assertions (3 tests) — expected feature lists don't include features added by recent PRs.
2. runQwenServe afterEach hook timeouts (9 tests) — afterEach at server.test.ts:5284 times out at 10000ms.

Verdict

✅ PASS — No new test failures, no new typecheck errors, no lint issues. Previous TS7030 nits resolved.

Verified locally by wenshao

@qwen-code-ci-bot