feat(serve): preflight and env diagnostics routes (#4175 Wave 3 PR 13)#4251
Conversation
📋 Review SummaryThis PR adds two read-only HTTP routes ( 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
There was a problem hiding this comment.
Pull request overview
Adds serve diagnostics for daemon environment and readiness so remote clients can inspect runtime state before starting or using ACP-backed sessions.
Changes:
- Adds
/workspace/envand/workspace/preflightroutes with capability tags and SDK helpers/types. - Introduces closed
errorKindtaxonomy plus preflight/env status cell types. - Adds bridge, ACP-side diagnostics, tests, and protocol/user documentation.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/cli/src/serve/envSnapshot.ts |
Builds daemon env snapshots with proxy/env-var redaction rules. |
packages/cli/src/serve/envSnapshot.test.ts |
Covers env snapshot cells and redaction behavior. |
packages/cli/src/serve/status.ts |
Adds diagnostics types, error kinds, timeout error, and mapping helper. |
packages/cli/src/serve/status.test.ts |
Tests error-kind taxonomy and mapping behavior. |
packages/cli/src/serve/httpAcpBridge.ts |
Implements env/preflight bridge methods and daemon preflight checks. |
packages/cli/src/serve/httpAcpBridge.test.ts |
Tests idle env/preflight behavior and no ACP spawn. |
packages/cli/src/serve/server.ts |
Wires HTTP GET routes for env and preflight. |
packages/cli/src/serve/server.test.ts |
Tests route responses and bridge invocation. |
packages/cli/src/serve/capabilities.ts |
Advertises new feature tags. |
packages/cli/src/serve/index.ts |
Exports new diagnostics helpers/types. |
packages/cli/src/acp-integration/acpAgent.ts |
Adds ACP-side preflight cells. |
packages/cli/src/acp-integration/acpAgent.test.ts |
Tests ACP preflight extMethod behavior. |
packages/sdk-typescript/src/daemon/DaemonClient.ts |
Adds SDK methods for new routes. |
packages/sdk-typescript/src/daemon/types.ts |
Mirrors diagnostics and error-kind SDK types. |
packages/sdk-typescript/src/daemon/index.ts |
Re-exports new daemon diagnostics types/constants. |
packages/sdk-typescript/src/index.ts |
Re-exports public SDK diagnostics API. |
packages/sdk-typescript/test/unit/DaemonClient.test.ts |
Tests SDK route helpers. |
docs/users/qwen-serve.md |
Documents new user-facing diagnostics routes. |
docs/developers/qwen-serve-protocol.md |
Documents protocol shapes, capabilities, and error-kind semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
…ave 3 PR 13) Lay the type foundation for `/workspace/preflight` and `/workspace/env` (and the eventual MCP guardrails route) so cells emitted by all three share a closed `errorKind` taxonomy: - `SERVE_ERROR_KINDS` literal-list + `ServeErrorKind` union — the seven values from #4175 (`missing_binary`, `blocked_egress`, `auth_env_error`, `init_timeout`, `protocol_error`, `missing_file`, `parse_error`). - `BridgeTimeoutError` typed class — `withTimeout` now rejects with this rather than a plain `Error`, letting `mapDomainErrorToErrorKind` recognize init / heartbeat / extMethod timeouts via `instanceof` instead of regex-matching message strings. Message format is preserved bit-for-bit. - `mapDomainErrorToErrorKind` helper — one place to classify `BridgeTimeoutError`, `SkillError`, fs ENOENT/EACCES/EPERM, ModelConfigError subclasses (recognized by `name` field — they aren't on the public surface of `@qwen-code/qwen-code-core`), `SyntaxError`, plus message-regex fallbacks for legacy throw sites (`agent channel closed`, missing CLI entry path). - `ServeStatusCell.errorKind` tightened from open `string` to the closed `ServeErrorKind` union. Backward compatible — PR 12 never assigned the field. - SDK mirrors: `DAEMON_ERROR_KINDS` const + `DaemonErrorKind` type; `DaemonStatusCell.errorKind` tightened. Tests: 11 new unit tests in `status.test.ts` covering each mapping rule plus the BridgeTimeoutError shape. No route changes; no behavior changes for any existing path.
174a97d to
dfaea09
Compare
Fold-in 1 — round-1 review feedback (force-pushed
|
wenshao
left a comment
There was a problem hiding this comment.
Summary
Latest push (4 new commits since my initial pass) cleanly addresses everything Copilot flagged + the P1 I had on validateAuthMethod mutation:
envSnapshot.ts:81—safeProxyValuenow has a 3-stage parse (URL →http://-prepend → regex scrub) with<unparseable>final fallback, plus 4 new unit tests covering scheme-less, authority-only, unparseable, and lowercase variants. Docstring explicitly notes defense-in-depth againstredactProxyCredentials' SSH-authority preservation.httpAcpBridge.ts:3905—canUseRipgrep(true)with comment explaining why (mirrorsConfig.useBuiltinRipgrepdefault; otherwise misleads operators on workspaces where bundled rg is the only one).acpAgent.ts:894—buildAuthPreflightCellno longer callsvalidateAuthMethod(which mutatedprocess.env['GOOGLE_GENAI_USE_VERTEXAI']and reloaded settings). New pure path uses a hand-rolledAUTH_PREFLIGHT_ENV_KEYStable andBoolean(process.env[name])check. Status'unknown'for unmapped auth types (safer than falseauth_env_error).docs/users/qwen-serve.md:46-56— split the idle-state paragraph somcp/skills/providerskeep theirinitialized: falseclaim whileenv/preflightget their own paragraph statinginitialized: truealways.- Also added:
process.envsnapshot at function entry ofbuildEnvStatusFromProcess(atomic from JS' single-threaded model) so concurrent auth-flow / settings-reload can't tear the snapshot. Nice touch.
Verified on latest HEAD:
$ npx tsc --noEmit -p packages/cli → clean
$ npx vitest run --no-coverage src/serve/envSnapshot.test.ts src/serve/status.test.ts src/acp-integration/acpAgent.test.ts src/serve/httpAcpBridge.test.ts src/serve/server.test.ts
Tests ~420 passed
P3 — remaining follow-ups
These aren't blockers but worth threading into PR 14 / 14b:
acpAgent.ts:116—AUTH_PREFLIGHT_ENV_KEYSis a hand-rolled mirror of core'sAUTH_ENV_MAPPINGS; only the docstring guards against drift.httpAcpBridge.ts:3973—safeCheck's inner catch doesn't tagerrorKind: 'missing_binary'. The new outerPromise.allSettledcatch (line 3945) does, but it's unreachable forsafeCheckslots because the inner catch always resolves.envSnapshot.ts:125—runtime.valueisprocess.versions.nodeeven whendetectRuntime() === 'bun'. Cheap to fix now.
Recommendation
Approve. Rework is solid — happy to APPROVE formally once you confirm you want me to flip the review state.
wenshao
left a comment
There was a problem hiding this comment.
[Critical] integration-tests/cli/qwen-serve-routes.test.ts (~line 195): the capabilities assertion's expected feature list does not include workspace_env and workspace_preflight, but this PR registers both in capabilities.ts. The test uses .toEqual() for strict comparison — it will fail when the daemon advertises the new features. The unit test's EXPECTED_STAGE1_FEATURES constant in server.test.ts was correctly updated, but the integration test was missed.
// In integration-tests/cli/qwen-serve-routes.test.ts, add to the expected features array:
'workspace_env',
'workspace_preflight',
— DeepSeek/deepseek-v4-pro via Qwen Code /review
dfaea09 to
4638c33
Compare
Fold-in 2 — wenshao P3 review feedback (force-pushed
|
wenshao
left a comment
There was a problem hiding this comment.
Additionally, getWorkspacePreflightStatus() is only tested for the idle path (no ACP child). Two critical branches are untested: (1) live ACP path where daemon cells merge with ACP cells, and (2) ACP failure fallback where requestWorkspaceStatus throws and the catch block creates an envelope error + falls back to idle placeholders. Tests for both paths should be added to httpAcpBridge.test.ts.
wenshao
left a comment
There was a problem hiding this comment.
No new review findings. Downgraded from Approve to Comment: CI failing: Test (windows-latest, Node 22.x). — gpt-5.5 via Qwen Code /review
4638c33 to
ac7c0bb
Compare
Fold-in 3 — wenshao Critical/Suggestion review (force-pushed
|
wenshao
left a comment
There was a problem hiding this comment.
No review findings on this pass. Previous round's criticals (execSync → execFile migration, safeCheck errorKind, proxy redaction, validateAuthMethod mutation) are all resolved in the fold-in commits. CI has a Windows test failure worth investigating before merge.\n\nLow-confidence items for consideration (not blocking): (1) ACP_PREFLIGHT_KINDS docstring claims drift prevention but live builder doesn't consume it, (2) daemon binary probes lack result caching, (3) cell-level errors produce no server-side logs.\n\n
Pure helper that constructs the `/workspace/env` payload from `process.*`
state. No I/O, no ACP roundtrip, no globals beyond `process.env`. The route
itself lands in the next commit.
- `ServeEnvKind` discriminant: `runtime | platform | sandbox | proxy | env_var`
- `ServeEnvCell extends ServeStatusCell` with `name` + optional `present` /
`value`. Cells with `kind: 'env_var'` are presence-only — `value` is
ALWAYS omitted to keep secret env vars off the wire even by accident.
- `ServeWorkspaceEnvStatus` envelope: `{ v, workspaceCwd, initialized: true,
acpChannelLive, cells, errors? }`. `initialized` is structurally `true`
because env answers from the daemon process directly; `acpChannelLive`
reports whether a child is up but does not change the payload shape.
Whitelist policy:
- Auth/secret keys (presence-only): OPENAI/ANTHROPIC/GEMINI/GOOGLE/DASHSCOPE/
OPENROUTER `_API_KEY`, `QWEN_SERVER_TOKEN`.
- Non-secret keys (also presence-only for shape uniformity): base URLs, locale,
TZ, NODE_EXTRA_CA_CERTS, QWEN_CLI_ENTRY.
- Proxy vars (`HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY`/`ALL_PROXY` + lowercase
variants): credentials stripped via `redactProxyCredentials`, then
`URL().host` so the wire only carries `host:port`. NO_PROXY is a host list
rather than a URL so we pass the redacted form verbatim.
SDK mirrors: `DaemonEnvKind`, `DaemonEnvCell`, `DaemonWorkspaceEnvStatus`.
Tests: 9 unit tests covering the proxy-credential redaction, lowercase env
fallback, NO_PROXY pass-through, presence-only `env_var` invariant
(`'value' in cell === false`), whitelist enforcement, runtime tag detection,
and envelope shape.
Wire `buildEnvStatusFromProcess` from the previous commit through the
bridge, server, and SDK so remote clients can pre-flight the daemon's
runtime environment without spawning an ACP child.
- `workspace_env` capability tag (always advertised on a current daemon).
- `bridge.getWorkspaceEnvStatus()` answers entirely from `process.*` —
the route never consults ACP. `acpChannelLive` reflects whether a child
exists but does not change the payload, so an idle daemon and a busy
one return the same env shape.
- `app.get('/workspace/env', ...)` mirrors PR 12's one-liner pattern.
- SDK: `DaemonClient.workspaceEnv()` returning `DaemonWorkspaceEnvStatus`.
- Docs: bullet in `docs/users/qwen-serve.md` calling out the
presence-only redaction policy and the no-ACP-spawn guarantee.
Tests: server-level (env returned + `'value' in env_var === false`
assertion), bridge-level (idle and live both answer locally without
hitting ACP extMethod), SDK-level (recording-fetch round-trip on
`/workspace/env`). The `workspace_env` tag is added to the
`EXPECTED_STAGE1_FEATURES` capability list assertion.
… PR 13)
Wire the preflight route. Daemon-level cells are populated unconditionally
from `process.*` and `node:fs`; ACP-level cells fall back to `not_started`
placeholders when no child is alive so a poll never spawns one.
- `workspace_preflight` capability tag.
- `ServePreflightKind` discriminant (12 values: node_version, cli_entry,
workspace_dir, ripgrep, git, npm — daemon-level — plus auth, mcp_discovery,
skills, providers, tool_registry, egress — ACP-level).
- `ServePreflightCell extends ServeStatusCell` with `locality: 'daemon' | 'acp'`
+ free-form `detail`. `ServeWorkspacePreflightStatus` envelope.
- `createIdleAcpPreflightCells()` factory: emits the six ACP-level cells with
`status: 'not_started'` + a uniform `hint` so the bridge can stitch them in
alongside daemon cells without ever calling ACP.
- `bridge.getWorkspacePreflightStatus()`:
- Daemon cells via `buildDaemonPreflightCells` (Promise.all over Node-version,
CLI-entry resolution mirroring `defaultSpawnChannelFactory`, `fs.stat` on
`boundWorkspace` with ENOENT/EACCES/EPERM mapped to `missing_file`,
best-effort `canUseRipgrep` / `getGitVersion` / `getNpmVersion` warnings).
- ACP cells via `requestWorkspaceStatus` — idle factory returns the
`not_started` placeholders; live path delegates to ACP via the
`qwen/status/workspace/preflight` ext method (handler lands in next
commit). Bridge-side timeout / channel-close while consulting ACP folds
into envelope `errors[]` with `mapDomainErrorToErrorKind` classification;
daemon cells still render.
- `app.get('/workspace/preflight', ...)` route + JSDoc bullet.
- SDK: `DaemonPreflightKind` / `DaemonPreflightCell` / `DaemonWorkspacePreflightStatus`
mirrors; `DaemonClient.workspacePreflight()`.
Tests: server-level (route returns the bridge payload), bridge-level (idle
returns 6 daemon + 6 ACP `not_started` cells without spawning a channel),
SDK-level (`workspacePreflight()` round-trip). Capability test updated.
Populate the six ACP-level preflight cells inside the ACP child so
`/workspace/preflight` returns real values for live sessions.
- `extMethod(qwen/status/workspace/preflight, ...)` dispatches to a new
`buildAcpPreflightCells(config)` private method.
- Five cell builders, each returning a `ServePreflightCell` with
`locality: 'acp'`:
- `auth`: `validateAuthMethod(authType, config)` returning non-null
string → `auth_env_error`. Missing auth method → warning. Throws
classified via `mapDomainErrorToErrorKind` with `auth_env_error`
fallback.
- `mcp_discovery`: rolls up `getMCPDiscoveryState()` + per-server
`getMCPServerStatus(name)` counts. `connecting > 0` or in-progress
discovery → warning + `init_timeout`; `disconnected > 0` post-discovery
→ error + `protocol_error`.
- `skills`: `SkillManager.listSkills()`; SkillError throws are mapped
via the helper (`PARSE_ERROR` → `parse_error`, `FILE_ERROR` →
`missing_file`).
- `providers`: `getAllConfiguredModels()`; empty list with a configured
`authType` → warning + `auth_env_error`. ModelConfigError throws map
to `auth_env_error`.
- `tool_registry`: null registry → error + `protocol_error`. Otherwise
surfaces tool count.
- `egress`: stays `not_started`. PR 14 plugs in the real probe.
- `errorCell` private helper extended with optional `errorKind` parameter;
defaults to `mapDomainErrorToErrorKind(error)` so existing call sites
(`mcp` / `skills` / `providers` envelope errors) automatically gain
classification.
Tests: 2 new acpAgent tests — preflight returns the six expected ACP cells
with correct locality + statuses; preflight surfaces a `SkillError`
(`PARSE_ERROR`) on the `skills` cell as `errorKind: 'parse_error'`. The
core `vi.mock` block adds a SkillError class for `instanceof` matching
inside `mapDomainErrorToErrorKind`.
Document `/workspace/env` and `/workspace/preflight` end-to-end: - Common-cell shape: tighten `errorKind` from open `string` to the closed `DaemonErrorKind` enum (seven literals from #4175). Add an explicit redaction-policy paragraph covering env-var presence-only, proxy host:port reduction, and the whitelisted-secrets list. - Capability-tag list: add `workspace_env` and `workspace_preflight`. - New `### GET /workspace/env` section with sample payload, `DaemonEnvKind` / `DaemonEnvCell` types, and the redaction-policy paragraph spelling out which secret env vars are enumerated and how proxy URLs are reduced to `host:port`. - New `### GET /workspace/preflight` section with idle sample payload, `DaemonPreflightKind` / `DaemonPreflightCell` types, the seven-value `errorKind` semantics table, and the bridge-error fallback contract (mid-request ACP channel close → cells drop to `not_started` + envelope carries one `errors[]` entry). - Source-layout table: extend the `status.ts` row to mention the new `ServeErrorKind` / `BridgeTimeoutError` / `mapDomainErrorToErrorKind` surface; add a new `envSnapshot.ts` row.
ac7c0bb to
2d33514
Compare
Fold-in 4 — wenshao CHANGES_REQUESTED + Windows CI fix (force-pushed
|
wenshao
left a comment
There was a problem hiding this comment.
Verified on 2d33514 — 174-line delta since my prior "No review findings on this pass" comment (ac7c0bb) consists entirely of the three low-confidence items I'd flagged then, now turned into structural fixes.
Local validation
| Command | Result |
|---|---|
npx vitest run packages/cli/src/serve packages/cli/src/acp-integration packages/cli/src/utils/systemInfo.test.ts |
639/639 passed across 23 files |
npm run typecheck --workspace packages/cli |
exit 0 |
npx vitest run packages/sdk-typescript/test/unit/DaemonClient.test.ts |
75/75 passed |
npm run typecheck --workspace packages/sdk-typescript |
exit 0 |
Delta vs ac7c0bb (the prior "No review findings" sha)
status.ts—ACP_PREFLIGHT_KINDSis nowas const satisfies readonly ServePreflightKind[], with a new exportedAcpPreflightKind = (typeof ACP_PREFLIGHT_KINDS)[number].acpAgent.buildAcpPreflightCellsconsumes the constant via aRecord<AcpPreflightKind, () => ServePreflightCell | Promise<ServePreflightCell>>builders map and iterates withfor (const kind of ACP_PREFLIGHT_KINDS). Adding a new ACP kind to the constant without a matching builder is now a TS exhaustiveness error rather than a silent missing cell. Closes my earlier "the docstring claims drift prevention but the live builder doesn't consume it" note.envSnapshot.ts—readProxyVarexported with a JSDoc explaining why (Windowsprocess.envis case-insensitive, so the production caller passes a snapshot while the unit test injects a plain JS object with both keys distinct to actually exercise the??-vs-||semantics). Lets the unit test pin theHTTPS_PROXY=""override behavior explicitly.integration-tests/cli/qwen-serve-routes.test.ts—workspace_envandworkspace_preflightadded to the EXPECTED_STAGE1_FEATURES list, in lockstep with the registry. Same discipline the prior wave PRs codified.
Prior critical findings still resolved (174a97d round)
readProxyVaruses??not||(HTTPS_PROXY="" disables the proxy correctly).getGitVersion/getNpmVersionmigrated fromexecSynctoexecFilewith explicit timeout — daemon preflight no longer blocks the event loop on a hunggit/npm.safeCheckcatch path classifies failures throughmapDomainErrorToErrorKindso SDK consumers get structurederrorKindinstead of opaque "error".errorKindclosed-union narrowing — author kept it as a deliberate type-contract choice with rationale in the PR comment; the SDK is prerelease, so the blast radius is bounded and the type benefit (exhaustive switch / discriminated union narrowing at consumer call sites) is worth the breakage cost. Reasonable trade-off.
CI note
GitHub CI on this HEAD shows Test/CodeQL jobs as cancelled (Lint + Classify passed before the cancel). The cancellations look like force-push concurrency interrupts, not test failures. Local validation above covers the same surface; a manual re-run of the Qwen Code CI workflow on 2d33514 would give the merge UI the green it wants.
LGTM.
Local validation reportVerified on 2d33514 in a clean checkout. Unit / typecheck
Spot check on the delta since ac7c0bb (174 lines)
Prior critical findings still resolved (174a97d round)
GitHub CICurrently shows Test/CodeQL jobs as |
Resolves conflicts after main advanced through PR 13 (#4251 preflight + env diagnostics) and #4245 (capability registry test alignment). Two intentional integrations: 1. **`SERVE_ERROR_KINDS` taxonomy join.** PR 13 introduced a closed enumeration of structured error kinds (`SERVE_ERROR_KINDS` + sibling `DAEMON_ERROR_KINDS`) replacing PR 14's free-form `errorKind?: string`. Added `'budget_exhausted'` to both lists so the per-server / workspace cell semantics from PR 14 stay typed within the closed set. The `acpAgent.buildBudgetCells` local `errorKind` variable is retyped from `string | undefined` to `ServeErrorKind | undefined`. `status.test.ts` "exposes the seven roadmap-defined error kinds" pin updated to "eight" with `'budget_exhausted'` appended. 2. **`acpAgent.ts` import + errorCell collision.** PR 13 added `mapDomainErrorToErrorKind` + `errorCell(kind, error, errorKind?)` 3-arg signature; PR 14 added `buildBudgetCells` next to it. Both blocks kept verbatim with PR 13's wider `errorCell` signature taking precedence. Imports merged. No behavior change in any pre-existing PR 14 fix; the merge is purely structural alignment with main's PR 13 closed-taxonomy. 693/693 focused tests pass; typecheck + lint clean across 4 workspaces.
Summary
Adds two read-only HTTP routes to
qwen serveso remote TUI / channels / web / IDE clients can pre-flight the daemon's environment and readiness without spawning an ACP child:GET /workspace/env— daemon-process snapshot (runtime, platform, sandbox, proxy, presence-only secret env vars). Always answers fromprocess.*; never consults ACP.GET /workspace/preflight— readiness checks. Daemon-level cells (Node version, CLI entry, workspace dir, ripgrep, git, npm) always render; ACP-level cells (auth, MCP discovery, skills, providers, tool registry, egress) require a live ACP child and emitstatus: 'not_started'placeholders when idle.Closes the Wave 3 PR 13 deliverable from #4175.
Why
Stage 1 + PR 12 (#4241) gave us read-only status routes for MCP / skills / providers / session context. PR 13 fills the env + preflight gap so clients don't have to wait until a prompt fails to discover that auth isn't configured, ripgrep is missing, or the daemon is running an unsupported Node version. It's also the first PR to land the closed
errorKindtaxonomy from #4175 —missing_binary | blocked_egress | auth_env_error | init_timeout | protocol_error | missing_file | parse_error— that PR 14 (MCP guardrails) will reuse.What's in each commit
(Final SHAs after fold-in 4 + autosquash.)
c89173d75feat(serve): introduce ServeErrorKind and BridgeTimeoutErrorServeErrorKindenum,BridgeTimeoutErrortyped class (replaces plainErrorinwithTimeout),mapDomainErrorToErrorKindhelper (with TODO for follow-upBridgeChannelClosedError/MissingCliEntryErrorretyping in PR 22 bridge extraction),ServeStatusCell.errorKindtightened from openstringtoServeErrorKind. SDK mirrors (DAEMON_ERROR_KINDS,DaemonErrorKind).a21092483feat(serve): add buildEnvStatusFromProcess helperServeEnvKind/Cell/Statustypes + SDK mirrors. Atomicprocess.envsnapshot at function entry. Proxy redaction with 3-stage parse fallback (new URL→http://-prepend → regex scrub) and<unparseable>final fallback so the catch path NEVER returns the raw redacted input.readProxyVarexported with??semantics forHTTPS_PROXY=""Docker/K8s explicit-disable convention. Runtime cell selectsprocess.versions['bun']under Bun. No route wiring yet.8d03e5797feat(serve): add GET /workspace/env route54413870bfeat(serve): add /workspace/preflight daemon-cells pathServePreflightKind/ServePreflightCell/ServeWorkspacePreflightStatus, exportedACP_PREFLIGHT_KINDS+AcpPreflightKind),buildDaemonPreflightCells(Promise.allSettledover fs.stat + 3 child-process probes — never sinks the route on a sync throw), bridgerequestWorkspaceStatusintegration, server route, SDK helper. Cross-cutting fix insystemInfo.ts:getGitVersion/getNpmVersionmigrated from blockingexecSyncto asyncexecFile+ 5s timeout — daemon event loop no longer blocks on hung git/npm. Integration test (integration-tests/cli/qwen-serve-routes.test.ts) updated with the two new capability tags.7f8c69707feat(serve): wire ACP-side preflight cellsbuildAcpPreflightCellsdriven fromACP_PREFLIGHT_KINDSvia aRecord<AcpPreflightKind, () => …>builder map (TS exhaustiveness — adding a new ACP kind without a matching builder is a compile error). PureAUTH_PREFLIGHT_ENV_KEYScheck (novalidateAuthMethodside effects). MCP discovery rolls upconnected/connecting/disconnected/unknown. Skills builder defensively wrapsgetSkillManager(). Providers severity =errorwhenauthTypeset + 0 models. Drift-detector test (authPreflight.test.ts) walksAuthTypeenum → CI red on missing entries. Egress staysnot_starteduntil PR 14b.2d33514afdocs(serve): preflight and env protocol sectionqwen-serve-protocol.mdcovering capability tags, idle/populated payloads, redaction policy,errorKindsemantics with refinedinit_timeoutboundary (transitional MCPconnecting > 0is NOT a timeout).Strict invariants enforced
kind: 'env_var'cells never carry avaluefield — hard-asserted viaexpect('value' in cell).toBe(false)in two test layers.redactProxyCredentialsthenURL.hostso the wire only carrieshost:port.errorKindis a closed 7-value union; unrecognized errors geterrorKind: undefinedrather than a fabricated category./workspace/envand/workspace/preflightnever spawn an ACP child — bridge tests asserthandles.length === 0after idle calls.Engineering principles checklist
errorKindtightening is harmless — PR 12 never assigned the field)caps.featuresper PR 12 convention)qwen serveStage 1 routes / SDK behavior preserved (full vitest sweep green)Test plan (final — at merge)
npm run typecheck --workspace packages/cli— cleannpm run typecheck --workspace packages/sdk-typescript— cleannpx vitest run packages/cli/src/serve packages/cli/src/acp-integration packages/cli/src/utils/systemInfo.test.ts packages/sdk-typescript/test/unit— 639 cli + 75 SDK passednpx eslint packages/cli/src/serve packages/cli/src/acp-integration packages/sdk-typescript— cleanvalueleakage absent; proxy redaction covers authority-style + NO_PROXY + unparseable shapes; idle preflight assertshandles.length === 0Fold-ins after initial submission
dfaea094bPromise.allSettled, proxy redaction leak, atomic env snapshot,canUseRipgrep(true), MCPServerStatus exhaustive switch, providers severity, skills defensive wrap, docinitialized:falsesplit).4638c330fAUTH_ENV_MAPPINGSdrift detector,safeCheckinner catch errorKind, Bun runtime version) + 3 hygiene (mapDomainErrorToErrorKindhoist,ACP_PREFLIGHT_KINDSextraction,init_timeoutsemantics doc).ac7c0bb23readProxyVar??,execSync→execFile+ timeout, transitional MCP errorKind drop). 1 declined:errorKindclosed-union narrowing — wenshao waived in APPROVED review as "reasonable trade-off".2d33514afOut of scope (deferred)
blocked_egresserrorKind) — placeholder cell staysnot_started. PR 14 plugs in real TCP/TLS probing alongside MCP guardrails so we don't outbound-connect on every preflight GET.requestWorkspaceStatusstill doesas unknown as T. Pre-existing pattern from PR 12; structured decoder is a separate cleanup.🤖 Generated with Qwen Code