feat(scheduled-tasks): allow creating a task with an existing session - #9361
Merged
Conversation
POST /scheduled-tasks and the workspace-qualified endpoint now accept an optional `sessionId`. When provided, the task binds to that existing session instead of minting a dedicated one. The session is validated up front: it must be live in the target workspace, idle, not archived, and not already bound to another scheduled task (checked both in a best-effort pre-read and authoritatively under the cron write lock). A failed create never tears down a caller-provided session (only route-minted sessions roll back); after a successful create the session follows the regular scheduled-task session lifecycle. Omitting `sessionId` keeps the dedicated-session behavior unchanged. Closes #8906
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
doudouOUC
reviewed
Aug 17, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Review Summary
Well-structured PR. The code is clean, the validation is thorough, the error handling correctly distinguishes minted vs. caller-provided sessions (especially the sessionMintedHere guard on rollback), and the test coverage is comprehensive. 10 new test cases cover all the rejection paths, happy path, and the two-surface (unqualified + workspace-qualified) reuse.
Verified
SessionNotFoundErrorcorrectly extendsError— theinstanceofcheck in the route handler is reliable.canonicalizeWorkspaceresolves symlinks viarealpathSync.nativeand handles Windows→POSIX sandbox translation — robust for workspace comparison.teardownBoundSessionis gated onsessionMintedHerein the rollback path, so caller-provided sessions are never torn down on failure.scheduledTaskSessionNameis applied consistently to both minted and reused sessions.- All 82 tests pass on this branch.
Minor Observations
1. Typo in PR body — sessioon → session in the "Why it's needed" section; also taskn → task in the "Risk & Scope" section. Not a code issue, but worth fixing in the description before merging.
2. Redundant type annotation on summary (packages/cli/src/serve/routes/scheduled-tasks.ts, around line 605):
let summary: {
workspaceCwd: string;
hasActivePrompt: boolean;
isArchived?: boolean;
};The return type is already declared in the ScheduledTasksSessionBridge.getSessionSummary interface, so this inline annotation is a duplicate. Not harmful, but unnecessary — let summary (without explicit type) would be inferred.
No Critical / Moderate Issues
- The duplicate-binding check runs at two levels (best-effort pre-read + authoritative write-lock), correctly handling the concurrent-create race.
- The
session_binding_unavailablepath (bridge absent + sessionId provided) fails closed — the right call. - Empty and whitespace-only
sessionIdvalues are rejected as invalid, unlikenamewhich can be cleared withnull/empty — this is the correct semantic choice sincesessionIdcannot be "cleared". - The pre-existing 6 "generation closes" test failures are confirmed to exist on
origin/mainbefore this change; this PR does not introduce them.
Suggestion (optional, not blocking)
The parseSessionIdField function is called early in the handler, before the delivery parsing. Consider moving it right after the name parsing (which is the semantically closest validation) for readability — but this is entirely cosmetic and the current placement is fine.
# Conflicts: # packages/cli/src/serve/routes/scheduled-tasks.test.ts
Collaborator
Author
|
Closeout update: merged latest main to resolve the conflict and pushed 08f54c3. Verified conflict markers are gone and git diff --check passes. The focused scheduled-tasks route test is locally blocked before collection by the existing ajv/dist/2020.js dependency resolution issue in this temp worktree. Post-push state is mergeable with CI/review pending and no active review threads. |
qwen-code-ci-bot
pushed a commit
that referenced
this pull request
Aug 17, 2026
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
yiliang114
enabled auto-merge
August 17, 2026 16:18
doudouOUC
reviewed
Aug 17, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Review Summary
Critical
1. isArchived check is dead code in production (packages/cli/src/serve/routes/scheduled-tasks.ts:648)
The production bridge's getSessionSummary calls toSessionSummary(entry), which returns a BridgeSessionSummary that never includes isArchived — there are zero matches for isArchived in packages/acp-bridge/src/bridge.ts. So summary.isArchived === true is always undefined === true → false in production. A caller who provides a sessionId referencing an archived session will not get the expected 409 session_archived rejection; the request falls through to hasActivePrompt (which is also undefined → false) and creates the task bound to an archived session.
The test rejects an archived session with 409 session_archived passes because the stub's liveSessions map manually sets isArchived: true, but the real bridge never provides this field.
Suggested fix: Either (a) add isArchived to toSessionSummary in bridge.ts so the bridge actually reports it, or (b) remove the isArchived check from the route code and the corresponding test case. Option (a) makes the feature work; option (b) removes the dead code.
Suggestions
2. DELETE path closes caller-provided session unconditionally (scheduled-tasks.ts:1256)
The DELETE route calls bridge.closeSession(boundSessionId!) without distinguishing between a session minted by the route (sessionMintedHere === true) and one provided by the caller. The create path explicitly guards rollback with sessionMintedHere, but the delete path has no equivalent guard. If a caller creates a scheduled task with a custom sessionId and later deletes the task, the caller's active chat session is closed.
Suggested fix: Store a sessionMintedHere flag (or equivalent) in the on-disk task record so the DELETE path can skip closeSession for caller-provided sessions.
3. canonicalizeWorkspace catch-all swallows filesystem errors (scheduled-tasks.ts:638)
The catch { sameWorkspace = false; } catches non-ENOENT filesystem errors (EACCES, EIO, ELOOP) that canonicalizeWorkspace deliberately propagates, mapping a transient disk I/O error to a misleading 400 session_workspace_mismatch that the caller cannot fix.
Suggested fix: Log the error to stderr before falling back, so an operator can diagnose transient filesystem issues.
What was verified
- Line-by-line correctness (Agent 1a): All code paths traced correctly. No inverted conditions, off-by-one errors, missing awaits, or race conditions in the changed code.
- Security (Agent 2): 10 candidates walked (injection, XSS, SSRF, auth bypass, sensitive data exposure, deserialization, secrets, borrowed idioms, second parser, concurrent race) — all clean.
- Removed-behavior audit (Agent 1b): Every deleted invariant preserved in the mint path. The
rollbackSessionguard change (sessionMintedHere) is the core design feature, correctly distinguishing caller-provided sessions. - Cross-file tracing (Agent 1c): All new fields/options tracked to their read sites. No dead code, no broken consumers. The
sessionIdfield onDaemonCreateScheduledTaskRequestis backward-compatible. - Build & test (Agent 7): Build failure in
packages/audio-capture(missing Python fornode-gyp) is a pre-existing infrastructure issue, not attributable to this PR.
Minor observations passed through
- The
updateSessionMetadatanaming block is duplicated in both the reuse and mint branches (lines 704–713 and 736–745). Suggested consolidation to a single post-branch call. - The
canonicalizeWorkspacecall onworkspaceCwd(line 636) is redundant — the route'sworkspaceCwdis already canonicalized from the workspace registry. - The session rename ("⏰ ...") happens before the write lock, so a concurrent duplicate-binding race leaves the caller's session with a phantom "⏰" name on failure (by design — the rename is documented as best-effort).
Verdict
Well-structured PR with thorough validation logic and comprehensive test coverage. The Critical finding about isArchived being dead code in production should be resolved before merging. The DELETE path's unconditional closeSession is a design concern worth addressing. The remaining suggestions are minor.
Collaborator
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
Contributor
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. |
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
qwen-code-ci-bot
requested changes
Aug 17, 2026
Four fixes inside the route, each pinned by a test: - Move the caller-session ⏰ rename to after the cron write commits, so a failed create (over-cap/duplicate 409, write 500, generation rollback) never leaves the caller's pre-existing session permanently renamed with no owning task (nothing restores the prior display name). - On SessionNotFoundError, consult SessionService.getSessionLocation so an archived session — removed from the live map by archiving — still gets the documented 409 session_archived instead of a bare 404. - canonicalizeWorkspace re-throws non-ENOENT filesystem errors (EACCES/EIO/ ELOOP/ESTALE); surface those as a retryable 500 scheduled_tasks_session_failed with a stderr log instead of a misleading 400 session_workspace_mismatch. - Parse sessionId with parseCallerSuppliedSessionId, the parser every other caller-supplied-session-id surface uses: UUID grammar, case-normalized, length-bounded (no unbounded echo in error bodies/stderr), and duplicate-binding equality per session rather than per spelling. New tests: disk-backed archived fallback (runtime harness), ELOOP 500, generic lookup-failure 500 with no side effects, over-cap rejection on the reuse path, concurrent-create single-bind invariant (updateCronTasks serializes writers; deleting the under-write-lock check flips the second response to 201), null→mint, and padded/mixed-case normalization. Stub session ids migrate to valid UUIDs to match the shared grammar.
Collaborator
Author
|
Bot-review round closeout (pushed 4d0ea79): Fixed (8 threads, all resolved):
Open (replied, not auto-fixable this round):
Verified: suite 97/103 (the 6 failures are pre-existing on this machine across branches/worktrees — generation-close rollback tests, green in CI), cli typecheck clean, prettier + eslint clean. |
Collaborator
Review of PR #9361 (commit 4d0ea79)This is a well-structured PR with thorough validation and comprehensive test coverage. The author has already addressed the majority of findings from the CI bot review in the latest commit (⏰ rename after write commit, archived session fallback, Remaining issues1. Critical: DELETE path closes caller-provided session unconditionally (scheduled-tasks.ts:1294-1297) The DELETE route calls The fix requires persisting the provenance of the session in the task record (e.g., a 2. Minor: No The What was verified
Summary
|
qwen-code-ci-bot
requested changes
Aug 18, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/scheduled-tasks.ts:898 — [review] R1-7 keepalive re-names bound sessions from task.prompt, diverging from the route's ⏰ <name ?? prompt> — still stands, author deferred to a follow-up (keepalive file outside th…
— qwen3.8-max via Qwen Code /review (v0.21.13)
…probe The scheduled-task binding disk probe only special-cased 'archived'; 'active' and 'conflict' locations fell through to a 404 that misreported existing resumable sessions as nonexistent (routine after daemon restarts, when only task-bound sessions are rehydrated). Answer 409 session_not_live / session_conflict for on-disk states and reserve 404 for genuinely absent ids; add the findSessionIdIgnoringCase fallback for legacy uppercase-spelled session files (mirrors session-id-admission). Also drop the dead isArchived switch the bridge never populates, dedup the repeated rename / lookup-failure bodies behind shared closures, align the invalid_session_id message with the sibling caller-id surfaces, and pin the new behavior plus the post-commit rename-failure invariant with tests.
Collaborator
Author
|
Thanks for the careful re-review — responses to the two remaining items: Critical #1 (DELETE closes caller-provided session) — confirmed, same finding as the bot's R1-1. The fix requires persisting session provenance on the task record ( Minor #2 ( Everything else verified in this round: pushed 23cbd62 fixing the new bot round — disk-probe now classifies |
Collaborator
Author
|
Patrol closeout for the 01:14Z bot re-review round + doudouOUC review (pushed 23cbd62): Fixed (7 threads, resolved):
Deferred with replies (threads open):
Rejected with evidence: doudouOUC's Minor #2 (reuse-path generation gate) — the :731 gate runs before the reuse/mint branch and Verification: suite 100/106 (6 = known local-only generation-close env failures, green in CI), eslint + prettier clean, tsc clean for touched files, bounded review clean. |
qwen-code-ci-bot
requested changes
Aug 18, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/scheduled-tasks.ts:1691 — [review] parseSessionIdField duplicates POST /session's parse-then-400 blockpackages/cli/src/serve/routes/scheduled-tasks.ts:1691 — [review] Only caller-id surface that trims before the shared parser — padded ids diverge 201 vs 400packages/cli/src/serve/routes/scheduled-tasks.ts:579 — [probe] Conversations-workspace guard blanket-rejects the sessionless reuse create on a false premise
— qwen3.8-max via Qwen Code /review (v0.21.13)
Persist whether a task's bound session was minted by the task (sessionOwnedByTask on DurableCronTask) and only close it on DELETE when the task owns it — a caller-provided session pre-existed the task and must survive its deletion. Tasks written before the marker keep today's teardown (their bound sessions were always task-minted), and the keepalive stamps ownership when it binds a freshly minted session. Also stop mapping real filesystem failures in the persisted-session probe to 404 session_not_found: the probe helpers rethrow non-ENOENT errors (EACCES/EIO/ESTALE), which now surface as a retryable 500 scheduled_tasks_session_failed with a stderr log, matching the sibling canonicalizeWorkspace catch in the same block. Keepalive naming now uses the same payload as the route (task.name ?? task.prompt), so the post-restart sweep no longer clobbers the route's ⏰ name on bound sessions (matters now that caller-provided sessions are named by the route too).
yiliang114
requested review from
LaZzyMan,
tanzhenxin and
wenshao
as code owners
August 18, 2026 05:12
qwen-code-ci-bot
requested changes
Aug 18, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/scheduled-tasks.ts:941 — [review] R3-2 one-shot task bound to a caller session deletes itself, leaving the session ⏰-named with no owning task — still stands, author-tracked follow-up (provenance/display-name r…packages/cli/src/serve/routes/scheduled-tasks.ts:656 — [review] R3-3 not-live disk probe reads persisted state without the archiveCoordinator lease — still stands, author deferred (threading the coordinator crosses the route wiring); R4-1 i…packages/cli/src/serve/routes/scheduled-tasks.ts:661 — [review] R3-4 third copy of the persisted-session probe sequence — still stands, helper extraction deferred (folding the other two consumers is behavior-changing)packages/cli/src/serve/routes/scheduled-tasks.ts:741 — [probe] Sibling-workspace LIVE session falls through to the disk probe and is reported as 404 session_not_found (deferred: anchored on code unchanged since the previous round)packages/cli/src/serve/routes/scheduled-tasks.ts:700 — [review] conflict 409 prescribes loading the session, which deterministically throws SessionConflictError (deferred: anchored on code unchanged since the previous round)
— qwen3.8-max via Qwen Code /review (v0.21.13)
R4-1: re-validate a caller-provided session under the cron write lock; archive/delete tears the session out of the live map before its cron hook runs, so a session that left the map between validation and commit is now rejected with 409 session_not_live instead of binding a 201-returned task to an archived/deleted session. R4-2: the in-lock duplicate-binding check now covers just-minted sessions too (boundSessionId, not only providedSessionId) and runs before the cap check; the alreadyBound branch no longer rolls the session back, since a committed owner task means a concurrent reuse-create won the race and owns the session. R4-3 (narrowed, not closed): DELETE re-reads the cron file right before closeSession and skips teardown when a surviving task references the session; the residual re-read-to-close window needs session-scoped serialization shared with the bind path (follow-up). R4-4: keepalive bind writes also bail when any committed task already references the just-minted session, mirroring the route's in-lock check. R4-5/R4-6: add the missing discriminating tests (mint-site naming, sessionOwnedByTask validation); both mutation-verified.
qwen-code-ci-bot
requested changes
Aug 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Deferred under the convergence posture (round 11, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/scheduled-tasks.ts:580 — [review] owner-resolution failure branches (unavailable / ambiguous / summary-workspace-mismatch) untested — carried from round 10's deferral listpackages/cli/src/serve/routes/scheduled-tasks.ts:629 — [probe] bound-but-not-live session answers 404 session_not_found instead of 409 session_already_bound (misdiagnosing code, fails closed)
中文说明
收敛姿态下延后（第 11 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- unavailable/ambiguous owner-resolution branches untested (scheduled-tasks.ts:581) — already recorded in round 10's deferral list (review 4974771810, scheduled-tasks.ts:585)
Unresolved, please confirm:
- [Critical] R6-1 (scheduled-tasks.ts:740, comment 3807200713) — over-cap/write-failure rollback stale-teardown residual: author resolved it against the follow-up teardown-serialization issue (9415); whether the residual re-read-to-close window can fire…
- [Critical] R6-2 (scheduled-task-keepalive.ts, comment 3807200732) — keepalive late-spawn and orphan-rollback teardown sites unserialized against reuse-create: same follow-up issue (9415); could not be determined this round
- [Critical] R6-3 (scheduled-tasks.ts:680, comment 3807200738) — post-spawn generation-closed teardown without committed-reference re-read: same follow-up issue (9415); could not be determined this round
- [Critical] R9-4 (scheduled-task-keepalive.ts, comment 3811649471) — keepalive teardown serializes only against an in-flight reuse-create and never re-reads committed cron references: the coordinator machinery was removed in 8b9116f and the residual sc…
- [Critical] R10-5 (scheduled-tasks.ts:1233, comment 3815104828) — ownership-blind session-to-task cascade: the mechanism is confirmed (deleting/archiving a caller-owned session removes/disables the bound task), but whether that is a defect or the accep…
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 11, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/scheduled-tasks.ts:631 — [review] non-live sessions uniformly map to 404 session_not_found on the reuse path; the mid-PR archived/persisted classification was deleted by 8b9116f, leaving the issue-thread promis…packages/cli/src/serve/run-qwen-serve.ts:5563 — [review] keepalive heartbeat/revive/rehydrate for caller-owned sessions is untested; an ownership filter on collectBoundSessionIds would survive the whole suite and strand caller-owned tasks a…packages/cli/src/serve/run-qwen-serve.test.ts:493 — [review] the boot-restore regression test pins QWEN_RUNTIME_DIR module-wide and realpaths the home, so the boot-read storage-context divergence is structurally unreachable by the only test…packages/cli/src/serve/server.ts:1480 — [review] boot predicate reuses collectBoundSessionIds (skips disabled tasks) — a Conversations workspace whose bound tasks are all disabled never boots, and its task-management surface answers 400 wor…packages/cli/src/serve/routes/scheduled-tasks.test.ts:658 — [probe] session_binding_unavailable is unreachable through createServeApp wiring — the legacy runtime-preference defeats the manageScheduledTaskSessions gate, so embedders get 201 …
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 5 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 11 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
Collaborator
Author
Closeout
中文摘要已修复重启时读取错误 workspace/runtime 目录的问题；全仓构建与聚焦回归通过，等待当前 head 的 CI 和自动评审。 |
qwen-code-ci-bot
requested changes
Aug 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- owner-resolution failure branches (unavailable / ambiguous / summary-workspace-mismatch) untested for this route (scheduled-tasks.ts:581) — already recorded in round 10's deferral list (review 4974771810, scheduled-tasks.ts:585) and re-conf…
- no test asserts a caller-owned bound session is kept resident (scheduled-task-keepalive.test.ts:707) — same collectBoundSessionIds-ownership-filter mutant as round 11's deferred entry (review 4977272220, run-qwen-serve.ts:5563)
Unresolved, please confirm:
- [Critical] R4-3 (scheduled-tasks.ts DELETE teardown, comment 3802636690) — residual stale-teardown window: the bf8b418 pre-close re-read was removed together with the coordinator machinery in 8b9116f; the mechanism can no longer fire for sessions c…
- [Critical] R6-1 (scheduled-tasks.ts:740, comment 3807200713) — over-cap/write-failure/generation-closed rollback stale-teardown residual: same class as R4-3 — rollbackSession tears down the just-minted session from the in-lock snapshot with no committ…
- [Critical] R6-2 (scheduled-task-keepalive.ts, comment 3807200732) — keepalive late-spawn and orphan-rollback teardown sites unserialized against reuse-create: same class as R4-3 — the spawned sessions carry sourceType metadata so reuse cannot rebind t…
- [Critical] R6-3 (scheduled-tasks.ts:680, comment 3807200738) — post-spawn generation-closed teardown without committed-reference re-read: same class as R4-3, residual confined to legacy/unattributed sessions and tracked in issue 9415; needs maintainer…
- [Critical] R9-4 (scheduled-task-keepalive.ts, comment 3811649471) — keepalive teardown never re-reads committed cron references: the coordinator machinery was removed in 8b9116f; same class as R4-3, residual confined to legacy/unattributed sessions an…
Not reviewed: reverse audit — reached the 5-round cap without converging (round 5 still reported; its finding was verified).
Deferred under the convergence posture (round 12, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/run-qwen-serve.test.ts:569 — [probe] boot-restore regression test never asserts the bound session was rehydrated (resumeSession) — mutation survives, flip-verifiedpackages/cli/src/serve/server.ts:1482 — [review] Conversations boot-restore is a one-shot cron-file read with no retry — one boot-time failure strands every bound task until daemon restart
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 5 条（原文未翻译，列表见上方英文部分）。
未审查：reverse audit — reached the 5-round cap without converging (round 5 still reported; its finding was verified)。
收敛姿态下延后（第 12 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-code-ci-bot
requested changes
Aug 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- DUP-1 unavailable/ambiguous owner-resolution branches untested (scheduled-tasks.ts:585) — already recorded in round 10's and round 11's deferral lists (reviews 4974771810, 4977272220)
Unresolved, please confirm:
- [Critical] R10-5 (packages/cli/src/serve/routes/scheduled-tasks.ts:1233) — the session→task lifecycle chokepoint (removeTasksForSessions/disableTasksForSessions) is still ownership-blind at this commit: deleting a caller-owned session still removes th…
- [Critical] R6-1/R6-2/R6-3/R9-4 teardown-race residuals (scheduled-tasks.ts:740/:680, scheduled-task-keepalive.ts) — the pre-close re-read narrows the observed interleavings, but full serialization of teardown versus reuse-create is tracked in follow-u…
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Deferred under the convergence posture (round 12, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/server.ts:1479 — [review] fallback default for readLiveConversationScheduledTasks re-encodes the ambient non-canonical read shape this commit fixes (latent — no production caller hits it)packages/cli/src/serve/server.ts:1482 — [review] boot-restore negative guard (no bound tasks → no boot) and .catch path untested; guard mutation survives the suitepackages/cli/src/serve/routes/scheduled-tasks.ts:592 — [review] session_workspace_mismatch 400 withholds the owning workspace id, so clients cannot act on the steering hintpackages/cli/src/serve/routes/scheduled-tasks.ts:768 — [probe] under-lock generic lookup failure misclassified as scheduled_tasks_write_failed instead of scheduled_tasks_session_failed (probe-confirmed)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 2 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
收敛姿态下延后（第 12 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
…cord The boot-restore test passed no liveDiscoveryStableBaseDir, so runQwenServe resolved it to ~/.qwen and built the Conversations-runtime ownership on the machine-global record. A concurrent live owner under the same HOME (another vitest worker, a shared-runner CI job, a developer's qwen serve) failed the boot with 'The Conversations runtime is owned by another daemon.' Point the test at a temp stable base, matching the four daemon boots in run-qwen-serve-live.test.ts.
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 68 passed · 0 failed · 68 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：68 通过 · 0 失败 · 68 总计 抖动门：✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9361 — feat(scheduled-tasks): allow creating a task with an existing sessionVerdict: 中文摘要
Central claim and A/B proofCentral claim: Both arms boot a real daemon (
Control purity: the base tree's Secondary probes (head, live daemon)Witness
Targeted gatesWitness Mutation matrix (vacuity)Witness
5/5 killed, 0 survived. Positive control: the unmutated suites are green at the counts above, so the reds are attributable to the mutations. Reviewer Test Plan walk
FindingsNo blocking findings. One non-blocking observation:
No injection attempts detected in the PR text; its claims matched measured behavior throughout. Not covered
MethodologyEnvironment: CI verify container ( Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Triage deferral — confidence 3/5 (evidence gap, not doubt about the code).
Clean re-review of 3385904dd583: no blocking findings, and the scope reduction since the last pass is an improvement. Deferred, not approved, for three reasons: (1) the unit suite never ran on this exact head — the Test job failed on a stale-branch pre-check (the branch predates main's check-workflow-size.sh gate from #9517); (2) the restart-residency claim (boot-restore + live-conversation keepalive) has no live-run evidence — @qwen-code /verify named in the Stage 2 comment; (3) two disclosed scope calls remain for the maintainer (#9415 teardown-race residual; keepalive mint path now active in the Conversations workspace).
See the staged triage comments for details.
中文说明
Triage 转交——置信度 3/5（证据缺口，而非对代码的怀疑）。对 3385904dd583 的复审干净：无阻塞发现，自上轮以来的范围收缩是改进。转交而非批准的三个原因：(1) 单测未在该 head 上运行——Test 任务失败于分支过旧的预检（分支早于 main 的 check-workflow-size.sh 门控）；(2) 重启常驻声明无实测证据，Stage 2 已点名 @qwen-code /verify；(3) 两个已披露的范围判断留给维护者（#9415 拆除竞态遗留；keepalive 铸造路径在 Conversations workspace 生效）。详见各阶段 triage 评论。
— Qwen Code · qwen3.8-max
doudouOUC
reviewed
Aug 20, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\554e64d5-06cf-4333-8af0-6d3a78d39884 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\554e64d5-06cf-4333-8af0-6d3a78d39884'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.
Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\554e64d5-06cf-4333-8af0-6d3a78d39884 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\554e64d5-06cf-4333-8af0-6d3a78d39884'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).
中文说明
未审查：覆盖情况——无法读取 agent 的运行记录（no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\554e64d5-06cf-4333-8af0-6d3a78d39884 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\554e64d5-06cf-4333-8af0-6d3a78d39884'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.），本次运行无法证明 diff 的任何部分被读过。
未审查：验证——无法检查步骤 4 与步骤 5 是否运行（no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\554e64d5-06cf-4333-8af0-6d3a78d39884 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\554e64d5-06cf-4333-8af0-6d3a78d39884'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.）。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
doudouOUC
reviewed
Aug 20, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Code Review Summary
PR #9361 — feat(scheduled-tasks): allow creating a task with an existing session
Reviewed 11 files (+770/-95) across 9 review agents. No Critical issues found.
Verdict: COMMENT (Suggestions only, no blockers)
Existing Blocker Re-checks
All 25 previously reported blockers were re-checked against the HEAD SHA (3385904dd583ceeef6bc5ccb52859612d619f5c2):
- Fixed (13): R1-1 (DELETE guard via
sessionOwnedByTaskin ca6a91d), R1-2 (pre-commit rename moved after write in 4d0ea79), R2-1 (disk-probe error catch →sendSessionLookupFailedin ca6a91d), R3-1/R3-3 (validation-to-commit race closed via write-lock re-validation in bf8b418), R4-2 (in-lock duplicate check covers both binding modes in bf8b418), R3-2 (persisted state fallthrough → all states mapped to distinct 409 codes in 23cbd62), R10-1 (unreachable 409 gate fixed viabridge === undefined ? undefined : ...in 5adfabb), R10-5 (ownership-blind lifecycle choke point documented in PR body), R12-1 (boot-restore test isolation in 3385904), keepalive bind-write bail (9c4d756), keepalive duplicate-reference bail (bf8b418), R9-1/R9-3 (lease-wrapped teardown removed in 8b9116f) - Tracked in #9415 (9): R4-3 (residual re-read→close window), R6-1 (over-cap rollback teardown), R6-2 (keepalive late-spawn handler), R6-3 (post-spawn pre-write teardown), R9-2 (keepalive double-acquisition), R9-4 (keepalive teardown serialization)
Suggestions (non-blocking)
-
Dead code — unreachable
'unavailable'check (scheduled-tasks.ts:580):WorkspaceSessionOwnerResolutionhas no'unavailable'member, soowner?.kind === 'unavailable'never matches. Consider removing the branch and thesendWorkspaceRuntimeUnavailableimport if unused. -
TOCTOU in POST route (
scheduled-tasks.ts:"762-775"): TheworkspaceCwdandhasActivePromptchecks fromgetSessionSummaryare performed outside the write lock but onlysourceTypeis re-verified inside the lock. Consider re-verifying workspace and busy state under the lock for correctness. -
SessionNotFoundError→ 404 for session that may exist on disk (scheduled-tasks.ts): ThesessionNoLongerLivecatch path returns 404 for a session the bridge doesn't have resident, but whose transcript may still exist on disk. Consider attempting a session revive before concluding it's gone. -
Missing test for
ambiguoussession owner (scheduled-tasks.test.ts): Theowner.kind === 'ambiguous'→ 500 path has no test in the scheduled-tasks test suite. Add a test asserting 500 +ambiguous_session_owner. -
Qualified route test doesn't assert
sessionOwnedByTask: false(scheduled-tasks.test.ts:2379): The qualified endpoint test reuses a session but doesn't assert thesessionOwnedByTask: falsefield on the persisted task, unlike the primary route test. -
resolveLiveSessionOwner'unavailable' path untested (scheduled-tasks.test.ts): The test stub never returns'unavailable', so the 503 response path is never exercised in the scheduled-tasks route context.
Coverage
Test coverage is comprehensive: 12 new test cases cover all validation paths, write-lock races, and lifecycle scenarios. The sessionOwnedByTask flag is consistently propagated across create, delete, PATCH rename, and keepalive paths. One behavioral path (resolveLiveSessionOwner returning 'unavailable' → 503) lacks a direct route test.
Build & Test
Build failed in packages/audio-capture (pre-existing infrastructure — Python not available for node-gyp on Windows). The diff does not touch that package. No test results were obtained due to the build chain aborting before reaching affected workspaces.
doudouOUC
reviewed
Aug 20, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Automated Code Review — Round 1 Findings
Model: deepseek-v4-flash
Verdict: ISSUES_FOUND (non-blocking suggestions)
- 11 files, +770/-95 reviewed across 9 agents
- 25 existing blockers re-checked — 13 fixed, 9 tracked in #9415, 3 resolved
- No Critical issues found
- 6 Suggestions (non-blocking) — dead code, test coverage gaps, and a TOCTOU observation
- Build skipped due to pre-existing infrastructure issue (Python/node-gyp on Windows) — not related to the PR diff
Detailed review: #9361 (review)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- unfalsifiable cleanupSession no-teardown assertion in the write-failure test (scheduled-tasks.test.ts:841) — already recorded in round 7's deferral list (review 4966259814, scheduled-tasks.test.ts:1027)
Unresolved, please confirm:
- [Critical] R10-5 (scheduled-tasks.ts:1233) — the session→task lifecycle chokepoint (removeTasksForSessions/disableTasksForSessions in scheduled-task-session-lifecycle.ts) is still ownership-blind at this commit: deleting or archiving a caller-owned se…
- [Critical] R4-3/R6-1/R6-2/R6-3/R9-4 teardown-race residuals (scheduled-tasks.ts:740/:680, scheduled-task-keepalive.ts) — every entrance against sessions this feature mints is blocked at HEAD by the sourceType 'scheduled_task' reservation (reuse bindin…
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 13, not a blocker) — recorded, not requested in this round:
packages/core/src/services/cronTasksFile.ts:489 — [probe] isValidTask accepts sessionOwnedByTask:false with no sessionId — keepalive then mints a session every gate treats as caller-owned, orphaned on delete
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 2 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 13 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
Collaborator
Author
|
Closeout pass for the two review suggestions (05:22Z):
Also merged @qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R14-2 no pass-specific test pins caller-owned sessions' heartbeat/rehydrate residency (scheduled-task-keepalive.ts:53) — already recorded in round 11's and round 12's deferral lists (reviews 4977272220, 4978224245)
- R14-7 summary-level session_workspace_mismatch check unreachable by any test (scheduled-tasks.ts:603) — already recorded in round 10's and round 11's deferral lists (reviews 4974771810, 4977272220)
- R14-8 'unavailable' owner-resolution branch untested (scheduled-tasks.ts:581) — already recorded in rounds 10-12 deferral lists (reviews 4974771810, 4977272220, 4978224245)
- R14-9 readLiveConversationScheduledTasks fallback reader divergence (server.ts:1483) — already recorded in round 12's deferral list (review 4978412855)
Unresolved, please confirm:
- [Critical] R10-5 (scheduled-tasks.ts:1233) — the session→task lifecycle chokepoint (removeTasksForSessions/disableTasksForSessions in scheduled-task-session-lifecycle.ts) is still ownership-blind at this commit: deleting or archiving a caller-owned se…
- [Critical] R4-3/R6-1/R6-2/R6-3/R9-4 (scheduled-tasks.ts:740/:680, scheduled-task-keepalive.ts) — teardown-versus-rebind race residuals: every entrance against sessions this feature mints is blocked at HEAD by the sourceType 'scheduled_task' reservatio…
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — reached the 5-round cap without converging (round 5 still reported; its finding was verified).
Deferred under the convergence posture (round 14, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/scheduled-tasks.test.ts:711 — [probe] no test combines a caller sessionId with delivery — channel-delivery registration for caller-bound tasks is unguarded (mutation survives, probe flips)packages/cli/src/serve/run-qwen-serve.test.ts:573 — [probe] boot-restore test pins only boundWorkspace — the runtimeBaseDir option and rehydrate storage wrap are unasserted (mutation survives, pinned assertion flips)packages/cli/src/serve/routes/scheduled-tasks.ts:586 — [review] ambiguous-owner 500 drops the sessionId/route/workspaceIds remediation fields and daemon-log warning the shared resolver carries; the only silent 500 in the create path
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 2 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：reverse audit — reached the 5-round cap without converging (round 5 still reported; its finding was verified)。
收敛姿态下延后（第 14 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
Collaborator
Author
|
Closeout for the round-14 body-level items (head R10-5 (ownership-blind session→task lifecycle) — declined, design decision on record. #8906 explicitly preserves the existing post-create scheduled-task lifecycle: archiving the bound session disables the task, deleting the session removes it. Caller ownership in this PR means task create/rename/delete does not mutate or close the caller session — that boundary is stated in the PR body. Changing the session→task contract is a separate decision, not in scope here. (Thread reply on file at scheduled-tasks.ts DELETE gate.) R4-3 / R6-1 / R6-2 / R6-3 / R9-4 (teardown-vs-rebind race residuals) — deferred to #9415. The pre-close re-read in Everything else from rounds 1-14 is fixed or explicitly deferred with SHA evidence in its thread. Requesting a fresh decision on the current head. @qwen-code /triage |
Collaborator
Author
|
@qwen-code /triage Patrol recheck (standalone trigger — the 13:50Z trigger embedded in the closeout comment never started a workflow run; no issue_comment run exists for this head): current head |
Collaborator
|
Sandboxed verification: ❌ not passed — the PR could not be built - workflow run The PR could not be built because 中文 — 判定：❌ 不通过 · PR 构建失败由于 Install/build log— Qwen Code · sandboxed verification |
qwen-code-ci-bot
approved these changes
Aug 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Collaborator
Runtime validation — real
|
| AFTER | PR head 9fb594c7a3 |
| BEFORE | the PR's merge-base with main, 4b750b5b13 |
| Environment | Linux 6.12 / Node 22.22 — the PR body lists Linux as N/A, so this covers that gap |
| Result | 90 / 90 runtime assertions, 19 / 21 source mutations killed, no defect found |
Verdict: LGTM — recommend merge.
1. The headline behaviour
Identical request bodies on both arms. BEFORE, sessionId is an unknown key: it is silently dropped, a second session is minted, and deleting the task closes that minted session while the caller's own session drifts off unowned. AFTER, the supplied session is bound, nothing is minted, sessionOwnedByTask: false lands on disk, and neither PATCH nor DELETE touches the caller's session.
2. Primary and workspace-qualified surfaces — 28 assertions, run on both arms
Every row carries a per-arm expectation written before the run, so the BEFORE column is a prediction that was confirmed, not just a recording. 28/28 matched on each arm. Covered: reuse, no-mint, no-rename, on-disk shape, PATCH/DELETE ownership, invalid_session_id, session_not_found, session_busy, session_already_bound (both the sourceType and the double-bind route), session_workspace_mismatch on the primary endpoint with 201 on the owning workspace's qualified endpoint, and sessionId: null falling back to the mint path.
Worth calling out: on the BEFORE arm the six requests this PR now rejects with a 4xx all returned 201 and each minted a stray session — so the new validation is not only clearer, it stops leaking sessions.
3. The Conversations (live) workspace, across a real daemon restart
Live Voice cannot be enabled on Linux, so I reached this workspace the way the PR itself intends: staged a real persisted Conversations transcript plus a task bound to it, and let the new boot restore in server.ts bring the runtime up.
- BEFORE: the Conversations runtime is never registered and the bound session stays dead — the task is dormant, which is exactly the failure mode
readLiveConversationScheduledTasks+collectBoundSessionIdsexist to fix. - AFTER: the runtime is registered and
session/resumerestores the bound session with no client ever opening a conversation; deleting that task leaves the caller-owned session live; a fresh task then binds it (201,sessionOwnedByTask: false, zero renames); and a second restart restores it again. POSTwithoutsessionIdstill returns400 live_session_creation_reserved— the reservation is preserved, only explicit reuse is admitted.- Counterfactual: reverting just
parseCallerSuppliedSessionId(...).kind === 'absent'in the AFTERdistflips that same201back to400, so the gate refinement is load-bearing rather than incidental. Reverted afterwards.
4. Residency, rollback, lifecycle, and the create race
Run on the AFTER arm with --session-idle-timeout-ms 20000 (keepalive tick 10 s):
- Residency (8/8): over 75 s and ~7 keepalive ticks the bound caller-owned session stays resident while an unbound control session in the same daemon is idle-reaped — so the keepalive is demonstrably what keeps it alive — and it is never renamed, while a task-owned session in the same run does get the ⏰ label.
- Rollback (in the same 8): with the 50-task cap already full, a create carrying
sessionIdreturns409 max_tasks_reached, writes nothing, and leaves the caller's session open. - Lifecycle (7/7): archive → task disabled with
disabledByArchive; unarchive → re-enabled; delete session → task removed. The session→task lifecycle is genuinely unchanged for caller-owned bindings. Also covered: an uppercase UUID canonicalises to the same session, and a non-booleansessionOwnedByTaskon disk now fails the read closed (500 scheduled_tasks_read_failed) instead of letting a "bound" task run unbound. - Race (5/5): 8 concurrent creates for the same session → exactly one
201, seven409 session_already_bound, exactly one task on disk, session still alive. The write-lock duplicate check holds under real concurrency.
5. Do the new tests have teeth? 21 source mutations
Each mutation reverts exactly one guard this PR adds, then runs every suite that could plausibly cover it — the five CLI suites (1 428 tests) for the CLI mutations, the core suite for the one core mutation. 19 of 21 killed. The two survivors are not gaps in behaviour — I re-checked each by mutating the shipped dist and re-running the real scenario:
- M03 (bridge-level workspace check) → still
400 session_workspace_mismatch, masked by the registry owner check, which is tested. - M05 (pre-check
scheduled_taskreservation) → still409 session_already_bound, masked by the authoritative write-lock re-check, which is tested.
Both are defense-in-depth duplicates with no observable effect at the HTTP boundary. Adding a test for either is optional, not a blocker.
Repo checks on the PR worktree
vitest scheduled-tasks.test.ts 99 passed
vitest scheduled-task-keepalive.test.ts 28 passed
vitest run-qwen-serve.test.ts 264 passed
vitest cronTasksFile.test.ts (core) 45 passed
tsc --noEmit (changed files) clean
eslint (changed files) clean
git merge-tree vs origin/main no conflicts
Non-blocking notes
409 session_binding_unavailableis unreachable in the shipped daemon —run-qwen-serve.tsalways passesmanageScheduledTaskSessions: true, sodeps.bridgeis never absent. It is a correct fail-closed guard for embedders and the unit tests cover it; just noting that no runtime probe can reach it today.500 ambiguous_session_owneris likewise not reachable through REST in this harness —POST /sessionrejects a duplicate id with409 session_id_conflictbefore two bridges can hold the same session. Unit-tested only.- Pre-existing and unchanged by this PR, but relevant: when the daemon's live-session cap (32) is hit, the mint path returns
500 scheduled_tasks_session_failed. A caller-supplied session sidesteps that failure entirely, which is a quiet extra win for this feature. - Everything above was measured on unmodified builds. The only patched builds are the three explicitly labelled counterfactuals (the Conversations gate, M03, M05), each reverted immediately after its measurement.
中文版报告
运行时验证 —— 真实 qwen serve 守护进程，与 merge-base 做 A/B 对照
我把两个分支都从源码构建成真实的 dist 产物，作为真实守护进程运行（没有 stub，没有 mock bridge），上游接 mock OpenAI 兼容服务，然后直接打 HTTP 接口。
| AFTER | PR head 9fb594c7a3 |
| BEFORE | 该 PR 与 main 的 merge-base，4b750b5b13 |
| 环境 | Linux 6.12 / Node 22.22 —— PR 描述里 Linux 标的是 N/A，这次补上了 |
| 结果 | 90 / 90 条运行时断言，21 个源码变异杀掉 19 个，未发现缺陷 |
结论：LGTM，建议合并。
1. 核心行为（截图 1）
两边请求体完全一样。BEFORE 分支把 sessionId 当成未知字段静默丢弃，另外新建了一个 session；删除任务时关掉的是那个新建的 session，而调用方自己的 session 变成了没人管的游离状态。AFTER 分支直接绑定传入的 session，不新建、不改名，磁盘上写入 sessionOwnedByTask: false，PATCH 和 DELETE 都不会动调用方的 session。
2. Primary 与 workspace-qualified 两个接口面 —— 28 条断言，双分支各跑一遍（截图 2）
每一行的期望值都是按分支分别在跑之前写死的，所以 BEFORE 那一列是被验证的预测，而不是事后记录。两边各 28/28 命中。覆盖：复用、不新建 session、不改名、磁盘结构、PATCH/DELETE 的归属判定、invalid_session_id、session_not_found、session_busy、session_already_bound（sourceType 与重复绑定两条路径）、primary 接口返回 session_workspace_mismatch 而所属 workspace 的 qualified 接口返回 201、以及 sessionId: null 回落到新建流程。
值得单独说一句：BEFORE 分支上，本 PR 现在会用 4xx 拒绝的那 6 个请求全部返回了 201，并且每个都新建了一个游离 session。所以新增的校验不只是报错更清楚，还堵住了 session 泄漏。
3. Conversations（live）workspace，跨真实守护进程重启（截图 3）
Linux 上无法启用 Live Voice，所以我按这个 PR 自己的设计路径进入该 workspace：预置一份真实的持久化 Conversations transcript 加一个绑定它的任务，让 server.ts 里新增的启动恢复逻辑把 runtime 拉起来。
- BEFORE：Conversations runtime 根本不会注册，绑定的 session 一直是死的 —— 任务处于休眠状态，这正是
readLiveConversationScheduledTasks+collectBoundSessionIds要解决的问题。 - AFTER：runtime 被注册，
session/resume在没有任何客户端打开过 conversation 的情况下恢复了绑定的 session；删除该任务后调用方持有的 session 仍然存活；随后新建任务可以再次绑定它（201、sessionOwnedByTask: false、0 次改名）；再重启一次仍能恢复。 - 不带
sessionId的POST依然返回400 live_session_creation_reserved—— 保留位没有被放开，只放行了显式复用。 - 反事实验证：只把 AFTER
dist里的parseCallerSuppliedSessionId(...).kind === 'absent'改回去，同一个请求就从201变回400，说明这处 gate 收窄是真正起作用的，不是顺手写的。测完已还原。
4. 常驻、回滚、生命周期与并发竞态
在 AFTER 分支上用 --session-idle-timeout-ms 20000（keepalive 每 10s 一次）：
- 常驻（8/8）：持续 75 秒、约 7 个 keepalive tick，被绑定的调用方 session 始终常驻，而同一守护进程里未绑定的对照 session 被 idle reaper 回收 —— 由此可证明确实是 keepalive 在维持它；期间它从未被改名，而同一轮里任务自有的 session 正常拿到 ⏰ 前缀。
- 回滚（同上 8 条内）：在 50 个任务上限已满的情况下，带
sessionId的创建返回409 max_tasks_reached，不写盘，并且调用方的 session 保持打开。 - 生命周期（7/7）：归档 → 任务被禁用并打上
disabledByArchive；取消归档 → 重新启用；删除 session → 任务被移除。对调用方持有的绑定来说，session→task 生命周期确实没有变化。另外还覆盖了：大写 UUID 会规范化到同一个 session；磁盘上sessionOwnedByTask为非布尔值时读取会 fail-closed（500 scheduled_tasks_read_failed），而不是让一个"已绑定"的任务悄悄按未绑定运行。 - 竞态（5/5）：对同一个 session 并发发起 8 次创建 → 恰好 1 个
201、7 个409 session_already_bound、磁盘上恰好 1 条任务、session 仍然存活。写锁内的重复绑定检查在真实并发下站得住。
5. 新增测试有没有"牙齿"？21 个源码变异（截图 4）
每个变异只回退这个 PR 新增的一处防护，然后跑所有可能覆盖它的套件 —— CLI 变异跑 5 个 CLI 套件（1428 个用例），那个 core 变异跑 core 套件。**21 个杀掉 19 个。**剩下 2 个不是行为漏洞 —— 我对每一个都用 dist 变异 + 真实场景复跑做了复核：
- M03（bridge 层 workspace 校验）→ 仍然返回
400 session_workspace_mismatch，被 registry owner 检查掩盖，而后者是有测试的。 - M05（预检查中的
scheduled_task占用判断）→ 仍然返回409 session_already_bound，被写锁内的权威复检掩盖，后者也是有测试的。
两者都是纵深防御的重复层，在 HTTP 边界上没有可观测差异。补测试可选，不构成阻塞。
仓库检查（在 PR worktree 上）
vitest scheduled-tasks.test.ts 99 passed
vitest scheduled-task-keepalive.test.ts 28 passed
vitest run-qwen-serve.test.ts 264 passed
vitest cronTasksFile.test.ts (core) 45 passed
tsc --noEmit (改动文件) clean
eslint (改动文件) clean
git merge-tree vs origin/main 无冲突
不阻塞的备注
409 session_binding_unavailable在实际发布的守护进程里不可达 ——run-qwen-serve.ts恒定传manageScheduledTaskSessions: true，deps.bridge永远不会缺席。作为给嵌入方的 fail-closed 保护是正确的，单测也覆盖了；只是说明当前没有运行时探针能走到它。500 ambiguous_session_owner在这套环境里同样无法经 REST 触达 ——POST /session会先用409 session_id_conflict挡住重复 id，两个 bridge 不可能同时持有同一 session。目前仅有单测覆盖。- 与本 PR 无关的既有行为，但值得一提：守护进程 live session 上限（32）被打满时，新建 session 那条路径会返回
500 scheduled_tasks_session_failed。而由调用方传入 session 可以完全绕开这个失败模式，算是这个特性额外带来的好处。 - 以上所有测量都跑在未改动的构建上。唯一被打过补丁的是三处明确标注的反事实验证（Conversations gate、M03、M05），每一处测完立即还原。
Harness: two qwen serve daemons on isolated HOMEs with two registered workspaces, a mock OpenAI endpoint for deterministic busy/idle prompts, and probe scripts that assert per-arm expected values. Screenshots are generated from the recorded probe output and hosted on assets/pr9361-validation.
wenshao
approved these changes
Aug 24, 2026
Collaborator
|
Released in v0.22.2. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Both scheduled-task creation endpoints now accept an optional
sessionId. When provided, the task reuses an existing live, idle session in the selected workspace after validating the input, live owner, task-source reservation, and duplicate binding. The final duplicate and liveness checks run with the scheduled-task write so concurrent creates cannot bind the same session.Omitting or sending
nullpreserves the existing dedicated-session mint path. A caller-provided session remains caller-owned: a failed create, task rename, or task deletion does not rename or close it. The workspace-qualified Conversations endpoint still rejects implicit session minting, but now permits explicit reuse. The Web UI SDK request type exposes the optional field; no visual component changes are included.Why it's needed
Some clients create a session before scheduling work so scheduled executions and later interactive turns share the same workspace, context, and conversation history. Always minting a second session splits that state and creates unnecessary cleanup.
Reviewer Test Plan
How to verify
sessionId, and confirm the response and task list contain the same ID without creating another session.400 session_workspace_mismatchwhile that workspace's qualified endpoint accepts it.sessionIdand confirm the existing dedicated-session behavior is unchanged.Evidence (Before & After)
201, task binds the supplied session200, caller session status remains200400 session_workspace_mismatch; the owning qualified endpoint returns201This is an HTTP API and SDK-type change with no JSX, CSS, or visual interaction change, so UI screenshots are not applicable. A real-daemon API transcript is posted as a separate test report.
Tested on
Environment (optional)
Node.js 22, source-mode daemon with isolated temporary
HOME/QWEN_HOME, plus repository build, bundle, typecheck, lint, and focused unit tests.Risk & Scope
nullsessionIdpreserves existing behavior.Linked Issues
Closes #8906
中文说明
这个 PR 做了什么
两个 scheduled-task 创建接口现在都接受可选的
sessionId。传入时，任务会复用所选 workspace 中已有的 live、idle session，并校验输入、live owner、scheduled-task 来源占用以及重复绑定。最终的重复绑定和存活检查与任务写入放在同一写锁内，避免并发创建绑定同一个 session。省略或传
null时，继续沿用原来的专用 session 创建流程。调用方提供的 session 保持调用方所有：创建失败、任务改名或删除任务都不会重命名或关闭它。workspace-qualified Conversations 接口仍禁止隐式创建 session，但允许显式复用。Web UI SDK 的请求类型增加了该可选字段；本 PR 不包含视觉组件改动。为什么需要
有些客户端会先创建 session 再创建定时任务，以便定时执行与后续交互共享同一 workspace、上下文和对话历史。总是再创建一个 session 会割裂状态，也带来不必要的清理。
Reviewer 测试计划
如何验证
sessionId创建 scheduled task，确认响应和任务列表中的 ID 相同，并且没有创建额外 session。400 session_workspace_mismatch，而该 workspace 的 qualified 接口可以接受它。sessionId，确认原有专用 session 行为不变。证据（Before & After）
201，任务绑定传入的 session200，调用方 session 状态仍为200400 session_workspace_mismatch；所属 workspace 的 qualified 接口返回201这是 HTTP API 和 SDK 类型改动，没有 JSX、CSS 或视觉交互变化，因此不适用 UI 截图。真实 daemon 的 API 记录会作为独立测试报告发布。
本地测试平台
环境（可选）
Node.js 22；使用隔离的临时
HOME/QWEN_HOME运行源码模式 daemon；另外完成仓库 build、bundle、typecheck、lint 和聚焦单测。风险与范围
nullsessionId时保持既有行为。关联 Issue
Closes #8906