feat(web-shell): unify scheduled task sessions — bind chat-created tasks + clock icon - #6453
Conversation
… keepalive The cron_create tool (core layer) writes durable tasks to disk without a sessionId because it has no access to the session bridge. The keepalive loop runs in the daemon process where the bridge IS available, so it retroactively binds unbound tasks to dedicated sessions — the same flow POST /scheduled-tasks uses for UI-created tasks. Each unbound task gets: spawnOrAttach(sessionScope:'thread'), named ⏰ prompt, sessionId written back to disk. This makes chat-created tasks show "查看对话" with a clock icon in the session list, matching the UI's "新建定时任务".
…e tasks The keepalive interval is 2-5 minutes, so a chat-created task could wait that long before being bound to a dedicated session — showing no "查看对话" link until the next tick. Adding a file watcher (same directory-watch + debounce pattern the scheduler uses) triggers an immediate tick when cron_create writes to disk, so the task is bound within ~500ms.
… keepalive Switch from creating a separate dedicated session to binding the task to the current chat session (so the first message is already in the transcript). The keepalive then renames that session to ⏰ prompt — the core layer can't rename sessions (no bridge access), but the daemon process can. A Set tracks renamed sessions to avoid repeated updateMetadata calls. Unbound tasks (legacy/CLI) still get new sessions via the existing bind path.
|
Thanks for the PR! Template looks good ✓ Problem: This is an observed UX inconsistency — chat-created scheduled tasks show "运行记录" instead of "查看对话" and lack the ⏰ clock icon in the session list. The author verified the fix on macOS with HTTP API evidence (task Direction: Aligned. Unifying the UX between chat-created and UI-created scheduled tasks is squarely within the daemon/web-shell feature area. The CHANGELOG shows prior investment in scheduled tasks and session management. No direction concerns. Size: Core path change is 1 production line in Approach: Good separation of concerns — One minor observation: the PR description mentions the core change sets Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的 UX 不一致——聊天创建的定时任务显示"运行记录"而非"查看对话",且会话列表缺少 ⏰ 时钟图标。作者在 macOS 上验证了修复(通过 HTTP API 确认 task 方向:对齐。统一聊天创建和 UI 创建的定时任务体验属于 daemon/web-shell 功能范畴。CHANGELOG 显示团队在定时任务和会话管理方面有持续投入。无方向性顾虑。 规模:核心路径仅 1 行生产代码( 方案:关注点分离良好—— 一个小观察:PR 描述说核心变更在 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading diff): To unify chat-created and UI-created scheduled task UX, I would: (1) keep Comparison with diff: The implementation is clean and well-structured. Key strengths:
No critical blockers or AGENTS.md violations found. Real-Scenario TestingUnit tests: 140/140 passed (20 keepalive + 120 cronScheduler). Daemon integration test: Started daemon with Before (installed build — N/A, feature is new)This is a new feature, not a bug fix. No "before" behavior to reproduce. After (this PR — daemon integration test)Key observations:
Linux note: The PR was tested on macOS by the author ( — Qwen Code · qwen3.7-max |
|
Reflection: This PR does exactly what it says — unifies the UX for chat-created and UI-created scheduled tasks. The approach is architecturally sound: binding stays a daemon concern, the core module gets a one-line pass-through, and the keepalive picks up unbound tasks via a file watcher with immediate debounce. My independent proposal matched the PR's approach closely, which gives me confidence the design is natural. The race condition handling goes beyond what I would have expected — five distinct failure modes are covered with tests, including the subtle case where a late-resolving The test results confirm it works: 140 unit tests pass, and the daemon integration test on Linux showed the keepalive detecting and binding an unbound task within seconds, with the ⏰ display name applied. The code is focused — every change serves the stated goal. No drive-by refactors, no scope creep. The i18n string changes are minimal (two keys in two locales). The Approving. ✅ 中文说明反思: 这个 PR 准确实现了其目标——统一聊天创建和 UI 创建的定时任务的 UX。架构上合理:绑定是 daemon 的职责,核心模块仅增加一行透传,keepalive 通过文件监听器以即时防抖拾取未绑定的任务。 我的独立方案与 PR 的方法高度一致,这让我对设计的合理性有信心。竞态条件处理超出了我的预期——五种不同的失败模式都有测试覆盖,包括 测试结果确认功能正常:140 个单元测试通过,Linux 上的 daemon 集成测试显示 keepalive 在几秒内检测并绑定了未绑定的任务,⏰ 显示名称也已应用。 代码聚焦——每个变更都服务于既定目标。没有顺手重构,没有范围蔓延。i18n 字符串变更最小(两种 locale 各两个 key)。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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. |
BZ-D
left a comment
There was a problem hiding this comment.
Found one blocking behavior regression around durable cron tasks outside qwen serve.
Reverts the auto-binding of durable tasks to the current session in createDurable(). Binding to a specific session means only that session can fire the task (#shouldFireDurable), but non-daemon paths (TUI, ACP, headless) have no keepalive to rehydrate the session after exit — making tool-created durable tasks go dormant. The daemon keepalive (bindAndNameSessions) already handles binding unbound tasks to dedicated sessions with ⏰ naming, so daemon-mode tasks get the same UX without the regression.
BZ-D
left a comment
There was a problem hiding this comment.
The original non-daemon durable-task regression is fixed. I found one remaining issue in the new daemon binding path.
wenshao
left a comment
There was a problem hiding this comment.
Review: Comment · 2 Critical · 5 Suggestion · 3 Nice to have
Summary: The PR's core logic (bind chat-created tasks, file watcher, interval timer) is sound in the happy path. Build passed; 119/119 cronScheduler tests passed. Two critical issues need addressing before merge: (1) spawnOrAttach has no timeout and could permanently freeze the keepalive, and (2) renaming the user's chat session to ⏰ <prompt> is a side-effect when multiple tasks are created from the same session.
What I liked:
- Clean separation: binding/naming logic is isolated in
bindAndNameSessions - The
renamedSet deduplication prevents redundantupdateSessionMetadatacalls - File watcher with debounce for immediate binding is a nice UX improvement
scheduledTaskSessionNamesanitization is thorough (ANSI, bidi, surrogate-safe)- The
cronScheduler.tschange is minimal and correct
See inline comments for details.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
scheduled-task-keepalive.ts:122 |
spawnOrAttach has no timeout — unlike loadSession which uses withTimeout(KEEPALIVE_REVIVE_TIMEOUT_MS), a hung spawn blocks the entire tick and all heartbeats |
Wrap in withTimeout(bridge.spawnOrAttach(...), 15_000, task.id) |
scheduled-task-keepalive.ts:125-146 |
Orphaned sessions on partial failure — if spawnOrAttach succeeds but updateCronTasks fails, the session is orphaned; POST route has rollbackSession() but this path doesn't |
Add best-effort rollback in catch, or track spawned-but-unbound sessionIds |
scheduled-task-keepalive.ts:132,156 |
Session naming uses task.prompt only — POST/PATCH routes use name ?? prompt; inconsistent if DurableCronTask.name is ever set |
Change to scheduledTaskSessionName(task.name ?? task.prompt) |
scheduled-task-keepalive.ts:324,329 |
File watcher setup failures are completely silent — no logging in catch block or error handler, unlike cronScheduler.ts:1088 which logs watcher errors |
Add log.debug('keepalive: file watcher setup failed', err) |
scheduled-task-keepalive.ts:200 |
renamed Set never pruned — reviveState has cleanup (L271-276) but renamed accumulates entries for deleted tasks forever |
Prune alongside reviveState using live task sessionIds |
scheduled-task-keepalive.ts:145-157 |
needsName retry without backoff — updateSessionMetadata failures retry every tick indefinitely, unlike reviveState exponential backoff |
After N consecutive failures, add sessionId to renamed to stop retrying |
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
Downgraded from Request changes to Comment: self-PR; CI failing: review-pr.
— GPT-5 via Qwen Code /review
When bindAndNameSessions spawns a dedicated session for an unbound task but the subsequent updateCronTasks write fails (or the task was deleted between read and write), the spawned session was left behind with no owning task — the next tick would see the task still unbound (or spawn more orphans). Add rollback: closeSession + removeSession on failure, matching the POST /scheduled-tasks rollback pattern. Also add positive test coverage for the new binding paths: - unbound task → spawn + name + write sessionId to disk - bound task without ⏰ prefix → named exactly once (renamed Set dedup) - task vanishes before write → spawned session is rolled back
|
@qwen-code /triage |
BZ-D
left a comment
There was a problem hiding this comment.
The two earlier issues I raised are fixed. I found one remaining keepalive lifecycle blocker in the new binding path.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…hardening BZ-D: spawnOrAttach in bindAndNameSessions had no timeout boundary — a hung spawn would keep running=true and stall all subsequent ticks, stopping heartbeats/revives for every scheduled-task session. Wrap with withTimeout (configurable via spawnTimeoutMs, default 30s) and attach a background handler to clean up late-resolved orphans. Also generalized withTimeout error messages to include the operation name, and made spawn timeout configurable for tests. Test improvements (GPT-5 review suggestions): - Assert spawnOrAttach payload (workspaceCwd + sessionScope: thread) - Verify SessionService.removeSession called during rollback - Regression test: createDurable stays unbound after enableDurable - Hung-spawn test: tick completes despite non-abortable spawn hang
wenshao
left a comment
There was a problem hiding this comment.
f3b31aec).
Inline (Critical): the spawnOrAttach withTimeout guard you replied was "Fixed in 237811c" is not in this branch — 237811c1 is not an ancestor of the current tip (parent is 17aae6bc). That fix and its hung-spawn test were lost in a branch reset; see the inline comment.
Non-blocking notes (not line-mappable):
-
The PR description is now stale. After
17aae6bcremoved the auto-bind,createDurable()writes the task unbound — nothing setsboundSessionIdat create time, so thejobToDurableTaskhunk is a no-op for thecron_createpath. The keepalive then binds the task to a freshly-spawned empty dedicated session (same as UI-created tasks), so 查看对话 opens an empty session, not the chat where the task was created. The end state is consistent with UI tasks (good), but description item 2 ("createDurablenow setsboundSessionIdto the current session ID" / "the first user message is already in the transcript") no longer matches the code and should be corrected so reviewers aren't misled. -
Several prior bot findings that GitHub now hides as "outdated" still apply to the current HEAD (they were posted on the dropped
237811c1, but the code is unchanged):renamedSet is append-only / added before the disk write and never pruned; the rename usestask.promptinstead oftask.name ?? task.promptand re-clobbers manual session names after a daemon restart; the directory watcher fires a full sweep on everylastFiredAtwrite, not just on create; and the bind.map()callback always returns a new array, defeatingupdateCronTasks's no-op write skip. Worth a pass before merge.
— claude-opus-4-8[1m] via Qwen Code /qreview
BZ-D
left a comment
There was a problem hiding this comment.
The previous keepalive wedge is mostly addressed, but the non-abortable spawn path still needs an in-flight guard before I can approve.
BZ-D
left a comment
There was a problem hiding this comment.
Still blocked on the duplicate in-flight spawn issue and the new test typecheck failure. Inline comments are on the affected lines.
505fe66 to
159289a
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
BZ-D
left a comment
There was a problem hiding this comment.
The previous duplicate in-flight spawn and typecheck failures are fixed. I found one remaining race in the disk write confirmation path.
ytahdn
left a comment
There was a problem hiding this comment.
LGTM. 增量变更逻辑严密:bindAndNameSessions 的超时/回滚/防重入处理完善,renamed Set 去重和 prune 正确,文件监听 persistent:false + unref 不阻塞进程退出,测试覆盖充分。
159289a to
3f94b39
Compare
BZ-D
left a comment
There was a problem hiding this comment.
LGTM. The previous blockers are addressed: createDurable stays unbound for non-daemon paths, keepalive binding now has timeout/rollback/late cleanup, duplicate in-flight spawn protection, and the disk write rechecks that the task is still enabled and unbound before attaching. Relevant tests and typecheck pass locally.
- i18n: sync English 'View history' → 'View conversation' to match Chinese '查看对话' - Prune renamed Set alongside reviveState when tasks are removed - fs.watch: clarify null filename handling for Linux (treat as match) - updateCronTasks: skip .map() when task not found (no-op optimization) - Add tests: disabled unbound exclusion, naming failure resilience
3f94b39 to
41a5a69
Compare
BZ-D
left a comment
There was a problem hiding this comment.
LGTM. Latest head keeps the prior keepalive fixes intact and updates the ScheduledTasksDialog tests for the new 'View conversation' label. Relevant tests and typecheck pass locally.
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
This PR makes scheduled tasks created via chat (
cron_createtool) consistent with tasks created via the Web Shell "新建定时任务" dialog, in three ways:CronScheduler.createDurablemethod now setsboundSessionIdto the current session ID when available.cron_create) cannot rename sessions (no bridge access), so the keepalive — which runs in the daemon process — retroactively renames bound sessions to⏰ <prompt>usingupdateSessionMetadata. ASettracks already-renamed sessions to avoid repeated calls.A file watcher on the tasks file triggers an immediate keepalive tick (~500ms debounce) when
cron_createwrites to disk, so the ⏰ name appears without waiting for the next keepalive interval (2–5 min).Why it's needed
Tasks created via chat showed "运行记录" (inline run list) instead of "查看对话" (view transcript), and the session list had no clock icon — inconsistent with UI-created tasks. Users expected both creation paths to produce the same UX.
Reviewer Test Plan
How to verify
npm run dev -- serve --token <token> --port <port>⏰prefix in its display nameEvidence (Before & After)
Before: Chat-created task shows "运行记录" with no clock icon in session list.
After: Chat-created task shows "查看对话", session list shows
⏰ 检查构建状态.Verified via HTTP API: task
fja9epavcreated via chat hadsessionId=b441146b-..., sessiondisplayName=⏰ 检查构建状态.Tested on
Environment
npm run dev -- serve --token testtoken --port 65500, local daemon on macOS.Risk & Scope
Linked Issues
N/A
中文说明
本 PR 做了什么
本 PR 让通过聊天(
cron_create工具)创建的定时任务与通过 Web Shell "新建定时任务"对话框创建的任务体验一致,具体包括三个方面:CronScheduler.createDurable方法现在在可用时将boundSessionId设为当前会话 ID。cron_create)无法重命名会话(无 bridge 访问权限),因此 keepalive 在 daemon 进程中 retroactively 将已绑定的会话重命名为⏰ <提示词>。使用Set跟踪已命名的会话以避免重复调用。任务文件上的文件监听器在
cron_create写入磁盘时触发立即 keepalive tick(~500ms 防抖),无需等待下一个 keepalive 间隔(2-5 分钟)。为什么需要
聊天创建的任务显示"运行记录"(内联运行列表)而非"查看对话"(查看对话记录),且会话列表没有时钟图标 — 与 UI 创建的任务不一致。用户期望两种创建路径产生相同的 UX。
风险与范围