📋 Review Summary

This PR introduces a draft design document for a VS Code IDE daemon adapter that enables the extension to connect to qwen serve through DaemonSessionClient. The documentation is well-structured, clearly delineates scope and boundaries, and follows the project's engineering principles for gradual migration. Overall assessment: solid draft design ready for discussion, with a few areas that could benefit from clarification before implementation begins.

🔍 General Feedback

• Strong architectural boundaries: The document clearly separates concerns between extension host and webview, correctly identifying that daemon tokens must never cross into webview JavaScript.
• Good merge safety posture: Explicitly documents default-off behavior, additive path, and reversibility—aligns well with Stage 1.5 wave PR requirements.
• Clear event mapping table: The daemon-to-IDE event mapping is well-defined and provides a solid contract for implementation.
• Honest about limitations: The "Explicit Non-Goals" and "Blockers Before Default Migration" sections set appropriate expectations.
• Positive pattern: The sibling connection path approach (not replacing AcpConnection) is the right incremental strategy.

🎯 Specific Feedback

🟡 High

• docs/developers/daemon-client-adapters/ide.md:22-28 - The settings structure mentions qwen-code.experimentalDaemon.* but doesn't specify where these settings are registered. Consider adding a note about whether these will be added to package.json contribution points or handled dynamically. This affects discoverability and type safety.

  • Suggested fix: Add a brief note under "Proposed Entry Point" clarifying the settings registration approach.

• docs/developers/daemon-client-adapters/ide.md:66-73 - The event mapping table lists model_switched mapping to "existing model-state callback where possible" with a hedge. This vagueness could block implementation. Either commit to a specific callback or mark as "not yet supported" to avoid silent degradation.

  • Suggested fix: Clarify which specific callback handles this, or move to a "partial support" section with explicit limitations.

🟢 Medium

• docs/developers/daemon-client-adapters/ide.md:38-48 - The "Minimal Flow" lists 8 steps but doesn't address error handling. What happens if /capabilities fails? What if session creation fails? Consider adding a step 9 or a separate "Error Handling" subsection.

  • Suggested enhancement: Add brief error handling expectations (e.g., "On any daemon connection failure, fall back to AcpConnection or show user-facing error").

• docs/developers/daemon-client-adapters/ide.md:51-59 - The relationship to AcpConnection mentions that unmatched events should "surface a clear unsupported-state warning." Consider specifying where this warning goes (UI toast? logs? status bar?) to guide implementation.

  • Suggested enhancement: Specify the warning channel (e.g., "log warning + status bar indicator" or "toast notification on first occurrence").

• docs/developers/daemon-client-adapters/ide.md:94-102 - The "Validation Plan" mentions smoke tests but doesn't specify whether these will be automated CI tests or manual verification. Given the "Tests-first" principle is checked, clarify the automation strategy.

  • Suggested enhancement: Add a note on which tests will be automated vs. manual smoke tests.

🔵 Low

• docs/developers/daemon-client-adapters/ide.md:1 - Consider adding a "Status" badge or note at the top indicating this is a DRAFT design document. This helps readers immediately understand this is not an implementation spec.

  • Suggested enhancement: Add > **Status**: Draft design — implementation pending at the top.

• docs/developers/daemon-client-adapters/ide.md:80-87 - The "Runtime Locality UX" section lists several "do not imply" items. Consider converting these into explicit user-facing documentation or tooltips that the IDE should show, rather than just implementation guidance.

  • Suggested enhancement: Add a note like "These limitations should be surfaced in IDE onboarding docs or connection status UI."

• docs/developers/daemon-client-adapters/ide.md:116 - The "Blockers Before Default Migration" list is comprehensive but could benefit from linking to tracking issues for each blocker. This would help readers follow progress.

  • Suggested enhancement: Add GitHub issue references next to each blocker where applicable (e.g., "- Typed daemon event schema (#XXXX)").

✅ Highlights

• Excellent security posture: The explicit statement "Daemon token never crosses into webview JavaScript" demonstrates clear security thinking about extension host boundaries.
• Well-scoped non-goals: The "Explicit Non-Goals" section is a model for incremental design—clearly prevents scope creep while acknowledging future work.
• Thoughtful UX consideration: The "Runtime Locality UX" section shows good awareness of user confusion points around remote vs. local resources—a common pain point in IDE integrations.
• Clear merge safety checklist: The explicit checklist at the end provides reviewers with a concrete verification path, which is especially valuable for docs-only PRs that precede implementation.

Superseded by #4199. The replacement keeps the IDE daemon adapter work on a feat/... branch, removes the [codex] PR title prefix, and adds a locally verifiable DaemonIdeConnection spike with unit coverage.