1 commit

Author	SHA1	Message	Date
Shaojin Wen	51026d4d06	ci(serve): daemon A/B before/after preview on response-surface PRs (#6975) ... * ci(serve): daemon A/B before/after preview on response-surface PRs The daemon analog of the web-shell visual before/after bot. For PRs that touch the qwen serve response surface, build the CLI from BOTH the PR base (main) and head, drive a fixed set of endpoints against each daemon, and diff the JSON responses into a before/after field table posted on the PR. A PR with no response change -> "no change". - serve-ab-diff.mjs: structural JSON diff (set-aware arrays, volatile- field masking) -> before/after table; buildComment assembles all scenarios. Pure helpers unit-tested (12 tests). - serve-ab-drive.mjs: boots the built daemon, drives GET /health, /health?deep=1, /capabilities (no model -- dummy creds), captures JSON. - serve-ab.yml / serve-ab-publish.yml: capture (untrusted, no secrets) + privileged workflow_run publisher (validate+bind PR, dedup, TOCTOU), mirroring the web-shell split. Verified locally: real daemon boot + capture, the engine diffing real capabilities -> the before/after table, and no-change detection. The workflow wiring (2x build + drive in CI) is exercised on the next serve PR. * ci(serve): address before/after review — merge-base, base fail-safe, CI-wire tests Applies the transferable web-shell before/after review findings to the daemon A/B: - Diff against the MERGE-BASE, not the base-branch tip, so a PR branch behind main doesn't show others' already-landed daemon changes reversed as this PR's diff. - continue-on-error on the base checkout so a flaky base degrades to after-only instead of sinking the job (the base build/drive already was). - Wire serve-ab-diff.test.mjs into the github_ci_only test step — it uses only node builtins, so it runs there dependency-free (no npm ci needed). * feat(serve): add a create-session deep-health A/B scenario Add a `health-deep-with-session` scenario: the drive harness gains setup-request support, creates one session (POST /session), then probes GET /health?deep=1 — exercising the session lifecycle + cross-workspace session aggregation (#6961's exact case). Broaden the volatile mask to cover lastActivityAt/idleSinceMs/sessionId/clientId/workspaceCwd so the capture is deterministic, while the meaningful counts (sessions, activePrompts, pendingPermissions, connectedClients, channelAlive) stay diffable. Verified against the real daemon: two runs of the same build diff to "no change" (deterministic), and a sessions 1->2 change surfaces as a before/after table row. * ci(serve): address review — narrow trigger, degraded-base marker, test the entrypoint - Narrow the trigger from packages/core/src/** (~1100 mostly-unrelated files) to packages/cli/src/serve/** — the daemon's actual response surface (core has no serve/health subtree). (Suggestion) - diffCaptureDirs signals baselineMissing when head captures exist but the base build/drive produced none, so the comment says "diff skipped" rather than misreporting every field as added or "no change". (Suggestion) - Export + test diffCaptureDirs (the function the CI `comment` subcommand actually invokes) with temp-fixture dirs, covering the diff + degraded path. (Suggestion) - Wording: "PR base" not "main" (the workflow also runs for release/**). * ci(serve): address review round 2 — escape table values, guard setup/baseline/daemon - fmt escapes `|` and backtick so an arbitrary daemon value can't split a GFM table cell or close the code span. - diffCaptureDirs surfaces base-only (removed/failed-to-capture) scenarios instead of silently dropping them — otherwise a broken scenario lowers the "across N" count and masks the regression. - The drive checks each setup request's status and throws on non-2xx, so a failed POST /session can't let health-deep-with-session capture wrong state (0 sessions) and fake/mask a diff. - The drive awaits daemon exit after SIGTERM (SIGKILL after 5s) so a hung daemon can't linger. * ci(serve): clear-error JSON parse for both capture sides The after-side JSON.parse was unguarded while the before-side wasn't; a readJson helper now gives a 'invalid JSON capture at <path>' error for either side instead of a raw SyntaxError. * ci(serve): review round 3 — merge-base skip, surface malformed base, escape path, kill dead stdout - serve-ab.yml: retry the merge-base compare then SKIP the before build (after-only) instead of falling back to the base-branch tip; gate the base checkout/build on a resolved sha + successful checkout. - diffCaptureDirs: an existing-but-malformed base capture now surfaces (existsSync + readJson) instead of a bare catch treating it as {} (which reported every field as "added"); covered by a new test. - renderTable: escape backticks in the path field too (fmt already did for values). - Drop the dead stdout + its misleading "workflow reads it" comment (the publisher posts whenever body.md exists); log the count to stderr. * ci(serve): renderTable label 'PR base' not 'main' (release/** provenance) buildComment already said 'PR base', but the per-scenario table header + no-change line still said 'main'; align them (the workflow also runs for release/**, whose base is not main). --------- Co-authored-by: wenshao <wenshao@example.com>	2026-07-16 00:58:52 +00:00