feat(serve): add pollable daemon turn status - #9141
Conversation
Add GET /session/:id/turns/current and GET /session/:id/turns/:promptId so external callers can poll a turn's lifecycle state (queued / running / completed / cancelled / error) and result instead of holding the SSE stream for the whole turn lifetime. - Live state comes from the bridge's pending prompt queue; settled outcomes from persisted turn_result transcript records, so results survive daemon restarts and the daemon keeps no per-turn memory - Each prompt captures its own recording and settles exactly that one, so overlapping turns (DAEMON-003 deadline overlap) can never misattribute one turn's outcome to another promptId - Enforces the same client authorization as POST /session/:id/prompt Refs QwenLM#8680
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for the PR — this is the third iteration of the turn-status work, and by far the slimmest. Template: complete and bilingual ✓ Problem: real, not theoretical. This is the direct complement of #4585, which made Direction: aligned — read-only, additive, and squarely on the background-automation integration surface. One thing to be clear-eyed about: it is an always-on public contract (two new GET routes plus a Size: cross-package core infrastructure ( Approach: the scope finally feels right. Next to #8682 (≈7.8k lines) and #9080 (≈3k), this version drops the crash-journal and strict-persistence machinery and composes with what Risk: Moving on to code review. 🔍 中文说明感谢贡献——这是 turn-status 功能的第三个迭代,也是范围收敛得最好的一次。 模板:完整且双语 ✓ 问题:真实存在,不是理论假设。这是 #4585 的直接补全——#4585 把 方向:对齐——只读、纯增量,完全落在 background-automation 集成面上。需要清醒看待的一点:这是一个默认开启的公开契约(两个新 GET 路由 + 规模:跨包核心基础设施改动( 方案:范围终于合理了。相比 #8682(约 7.8k 行)和 #9080(约 3k 行),本版本砍掉了 crash-journal 和 strict-persistence 机制,转而与 风险: 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI read the full diff against Two hunks looked suspicious at first and I verified both against
Three behavior surfaces a maintainer should consciously sign off, all covered by the new tests:
Authorization is correct as far as static review can tell: both routes require the live owning runtime, parse the client id at the route, and re-resolve trust in the bridge exactly like The test suite is genuinely adversarial, not box-ticking: race paths (a terminal published while the persisted lookup is pending and while it rejects), rewind-clear races, the 64-entry cap, truncation flags, fork identity exclusion, and the exact tool-boundary semantics. sequenceDiagram
participant P1 as Daemon client
participant P2 as serve route
participant P3 as Bridge
participant P4 as ACP child
participant P5 as Transcript reader
P1->>P2: GET session turns by promptId
P2->>P3: getSessionTurnStatus - auth check
P3->>P3: check live FIFO queue
P3->>P4: control RPC turn_status
P4->>P5: best-effort flush then bounded backward scan
P5-->>P4: turn_result record or none
P4-->>P3: persisted result
P3->>P3: re-check live queue and terminal overlay
P3-->>P2: merged status or idle
P2-->>P1: 200 status or 404 prompt_not_found
The re-check after the awaited child read (including on the failure path) is what keeps a concurrent settle from regressing to stale data — that ordering is the heart of the design and it's implemented as described. Files changed (26 of 26 shown)
Test evidence — the PR's own CIThis is an unattended CI run, so per policy I did not build or execute any PR code; the evidence below is the PR's own CI read through the API at the reviewed commit. As of this pass the substantive suites are still running — no failures, no passes yet. The Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle the remaining behavioural claims: Real-scenario (tmux) testing: N/A for this pass — unattended CI never drives live product, and the change is daemon HTTP surface with no TUI component. A maintainer can trigger the isolated 中文说明代码审查:完整对照 两个起初可疑的改动点都已在 三处需要维护者有意识签核的行为面(均有新测试覆盖): 鉴权在静态审查范围内是正确的:两个路由都要求 live owning runtime,路由层解析 client id,bridge 层像 测试套件是有对抗性的:竞态路径(persisted 查询 pending 和 reject 时发布终态)、rewind 清空竞态、64 条上限、截断标志、fork 身份排除、精确的工具边界语义。 测试证据:本次为无人值守 CI 运行,按规则未构建或执行任何 PR 代码;以上证据来自 PR 自身 CI 在受审 commit 上的 API 读取。截至本次,主要套件仍在运行——尚无失败,也尚无通过。Finalize 任务会在 CI 落定后更新上方表格。未验证:需要真实运行的部分。 沙箱验证可以补齐剩余行为性结论: 真实场景(tmux)测试:本次不适用——无人值守 CI 不驱动真实产品,且本改动为 daemon HTTP 面,无 TUI 成分。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — the static review is genuinely clean (no blockers found; this is well-constructed work), but the core-scale escalation caps this at 3/5: a maintainer needs to sign off before any approval, and CI on the reviewed commit hasn't landed yet. Stepping back: my independent take before reading the diff was that this feature needs exactly three sources — live FIFO state only the bridge has, terminal outcomes bridging the transcript-visibility gap, and a bounded durable lookup for restarts — and that's precisely what this PR builds, each reusing an existing mechanism rather than adding a parallel one. The progression across the three iterations is the right direction: #8682 tried to make the result store durable under crashes (7.8k lines), #9080 trimmed it, and this version accepts an explicitly best-effort contract and documents its misses ( What gives me pause is not the code but the station it sits at: ~888 production lines across Two follow-ups for the thread, neither blocking: please close #9080 and #8682 if this supersedes them, and the 中文说明置信度:3/5——静态审查是真正干净的(未发现阻断问题,实现质量良好),但核心规模升级将其封顶在 3/5:需要维护者签核后才能批准,且受审 commit 的 CI 尚未落定。 退一步看:在读 diff 之前,我的独立判断是这个功能恰好需要三个数据源——只有 bridge 拥有的 live FIFO 状态、弥合 transcript 可见性间隙的终态结果、以及重启后有界的持久查询——而这正是本 PR 构建的,且每一部分都复用现有机制而非另起炉灶。三次迭代的演进方向正确:#8682 试图让结果存储在崩溃下持久(7.8k 行),#9080 收缩,本版本接受明确的 best-effort 契约并文档化其 miss 语义( 让我谨慎的不是代码,而是它所处的位置:约 888 行生产代码横跨 core services、acp-bridge 与 acp-integration 高风险路径,来自 fork,新增默认开启、无开关的公开接口。每一点单独看都有正当理由——合在一起正是两级门禁要求人类做合并决定的原因。审查没有发现需要修复的问题;只是这个收尾决定不属于我。 两个不阻断的跟进事项:如果本 PR 替代 #9080 和 #8682,请关闭它们;Stage 2 评论中的 — Qwen Code · qwen3.8-max Reviewed at |
|
⏸️ Deferring to @wenshao — clean static review (3/5, no blockers), but this one needs a human merge decision for three reasons:
Suggested lanes once CI settles: 需要维护者做合并决定的原因:核心路径规模升级(约 888 行生产代码,feat 类型不阻断但必须知会)、高风险路径 + fork 来源且 CI 未落定、新增默认开启的公开接口(best-effort 契约属产品决策)。 |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "session_turn_status" |
— Qwen Code · serve A/B
Closed as an accidental duplicate of #9080. All maintained work continues on #9080.