Skip to content

Commit 5cb946b

Browse files
yiliang114wenshao
andauthored
feat(scheduled-tasks): allow creating a task with an existing session (#9361)
* feat(scheduled-tasks): allow creating a task with an existing session 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 * fix(serve): harden scheduled-task session binding per bot review round 1 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. * fix(serve): classify persisted-but-not-live sessions in task binding 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. * fix(serve): gate scheduled-task delete teardown on session ownership 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). * fix(serve): close session-binding races in scheduled-task create/reuse 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. * fix(serve): keepalive must not tear down a session a committed task owns The round-14 review caught a regression in the duplicate-reference bail: it routed the "a committed task already references the just-minted session" case into the orphan rollback. In production wiring cleanupSession is deleteDaemonSessionIfOrphan, whose requireZeroAttaches passes for a just-minted session, and whose persisted removal cascades removeTasksForSessions — so the rollback killed the race-winning task's live session AND deleted its committed task from the cron file. The two no-write bail reasons are now distinguishable: the committed-reference check runs first and, when it fires, keepalive logs and continues without cleanup — the session is left to its owner (mirroring the route's symmetric alreadyBound branch, which performs no rollback for exactly this reason) and this task stays unbound on disk for the next tick to retry with a fresh session. The original bail (task no longer bindable) still rolls the orphan back, unchanged. Also pin three load-bearing behaviors that had no coverage: the duplicate-check-before-cap-check ordering at the cap boundary (session_already_bound, never max_tasks_reached with rollback), the DELETE pre-close re-read failure fallback (still closes the owned session), and the under-lock re-validation generic-error branch (500, never coerced to session_not_live). All three mutation-verified. * fix(serve): serialize scheduled-task session teardown with reuse-create binding (#9415) * fix(serve): extend scheduled-task session teardown serialization to rollback and keepalive sites (#9415 R6) * fix(scheduled-tasks): narrow existing session reuse * fix(scheduled-tasks): restore conversation-bound tasks * fix(scheduled-tasks): restore conversation runtime tasks * fix(scheduled-tasks): honor the session-management gate on the primary surface * test(scheduled-tasks): isolate the Conversations runtime ownership record 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. * test(scheduled-tasks): cover the ambiguous session-owner rejection path --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
1 parent 2855149 commit 5cb946b

11 files changed

Lines changed: 794 additions & 95 deletions

File tree

packages/cli/src/serve/core-runtime.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export {
1919
hashDaemonWorkspace,
2020
initializeDaemonMetrics,
2121
initializeTelemetry,
22+
readCronTasks,
2223
recordDaemonCancel,
2324
recordDaemonChannelLifecycle,
2425
recordDaemonPipeMessage,

0 commit comments

Comments
 (0)