fix(space): rate-limit task-lifecycle recovery — banner-Cancel respawn + partial-resume recompute

[lsm]

Fixes two P2 rate-limit task-lifecycle edge cases deferred from #2271.

Finding A — banner Cancel left the task unrecoverable in-process. The cooldown banner's Cancel (cancel(false) + setIdle) left the task rate-limited but retryNow couldn't re-fire and the cross-restart sweep skipped the live idle session, so the visible Resume couldn't restart the consumed turn until a daemon restart. resumeRateLimitedSubSession now detects the parked state via a narrow bannerCancelled flag (set only by cancel(notifyResume=false) — narrower than the raw paused flag, which is also true mid-fireCooldownRetry while an auto-retry is actively starting) and re-spawns the execution (reset to pending + clear agentSessionId, stop + evict the idle session) so the workflow tick spawns a fresh replacement. The stale limitedSessionsByTask entry is cleared by the synchronous session.rate_limit_resume event fired inside handleInterrupt.

Finding B — merged restriction wasn't recomputed on partial session resume. The resume listener left the merged restriction untouched when other sessions remained, so the latest-deadline session resuming first left a stale later resetAt (delaying cross-restart recovery). limitedSessionsByTask is now Map<taskId, Map<sessionId, {resetAt,kind,reason}>> and recomputes (later resetAt + stronger kind + reason) from all/remaining entries on every pause and resume, keeping the persisted restriction consistent with the in-memory set.

Both changes are strictly safer than the prior behavior.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[greptile-apps]

Greptile Summary

Fixes two rate-limit task-lifecycle edge cases: (A) after the cooldown banner's Cancel, the task was unrecoverable in-process because retryNow couldn't re-fire and the cross-restart sweep skipped live idle sessions; (B) on partial session resume, the merged restriction wasn't recomputed, leaving a stale later resetAt and stronger kind in the DB.

• Finding A — banner-Cancel respawn: Adds a narrow bannerCancelled flag to RateLimitWatchdog. resumeRateLimitedSubSession now detects this state and calls the new respawnRateLimitedExecution, which resets the NodeExecution to pending and stops the orphaned idle session.
• Finding B — partial-resume recompute: Upgrades limitedSessionsByTask to carry full session entries and introduces recomputeTaskRestriction, called symmetrically on pause and resume.
• Tests added for multi-session partial-resume recompute, banner-cancel signal semantics, respawn-on-Resume flow, and the synchronous-event stale-entry cleanup invariant.

Confidence Score: 5/5

Safe to merge. Both lifecycle gaps are closed, the synchronous-event ordering invariant is locked in by a dedicated test, and changes are additive with no regression risk to the existing rate-limit path.

The banner-Cancel respawn relies on the session.rate_limit_resume event firing synchronously during stopSessionPreserveDb while subSessions still holds the session. The PR documents this thoroughly and a new test using a real InternalEventBus verifies the invariant. The recomputeTaskRestriction refactor applies the same merge semantics symmetrically on pause and resume, with all prior thread concerns addressed.

Files Needing Attention: No files require special attention. The most complex logic in task-agent-manager.ts is well-commented and thoroughly tested.

Important Files Changed

Filename	Overview
packages/daemon/src/lib/agent/rate-limit-watchdog.ts	Adds the narrow bannerCancelled boolean flag and exposes it via isRateLimitBannerCancelled(). Correctly scoped to distinguish banner-Cancel from a mid-fireCooldownRetry pause.
packages/daemon/src/lib/agent/agent-session.ts	Thin delegation: adds isRateLimitBannerCancelled() forwarding to the watchdog.
packages/daemon/src/lib/space/runtime/task-agent-manager.ts	Core of both fixes: upgrades limitedSessionsByTask, replaces markTaskRateLimited with synchronous recomputeTaskRestriction, adds respawnRateLimitedExecution, and changes resumeRateLimitedSubSession return type to retried
packages/daemon/src/lib/space/runtime/space-runtime.ts	Captures the new tri-state return and skips prepareSubSessionForWorkflowResume when outcome is respawned.
packages/daemon/tests/unit/1-core/agent/rate-limit-watchdog.test.ts	Adds two tests covering banner-only signal semantics and episode-boundary clearing.
packages/daemon/tests/unit/5-space/agent/task-agent-manager-cancel.test.ts	Full test suite for the respawn path including the synchronous-event stale-entry cleanup invariant using a real InternalEventBus.
packages/daemon/tests/unit/5-space/runtime/task-agent-rate-limit-listener.test.ts	Adds the multi-session partial-resume regression test verifying restriction shrinks to remaining sessions on partial resume.

_{Reviews (4): Last reviewed commit: "fix(space): rate-limit task-lifecycle re..." | Re-trigger Greptile}

[lsm]

🤖 Review by glm-5.1 (NeoKai)

Model: glm-5.1 | Client: NeoKai | Provider: Zhipu (GLM)

Recommendation: REQUEST_CHANGES — one P2 (respawn signal is too broad); the two core fixes themselves are correct.

Both core fixes are correct and I verified them end-to-end. 61/61 changed tests pass; lint + typecheck clean.

Finding A (banner-Cancel respawn) — correct. After respawnRateLimitedExecution resets the NodeExecution to pending + agentSessionId: null, the workflow tick selects it (space-runtime.ts:8041-8043 filters on status === 'pending' only) and spawns a fresh session via spawnWorkflowNodeAgentForExecution. recoverWorkflowBackedTask registers the executor (ensureExecutorRegistered) and the tick loop is always running, so the replacement IS spawned. Clearing startedAt/completedAt is actually more correct than the recoverRateLimitedTasks path it mirrors (that one leaves a stale startedAt that can trip the timeout detector around space-runtime.ts:7699).

Finding B (partial-resume recompute) — correct. Merge rules (max resetAt, strongest kind), the no-op skip, the terminal-status guard, and the empty → restoreTaskFromRateLimit path all check out, and the new regression test covers the documented max-resumes-first case.

---

P2 — the respawn signal also matches the in-flight auto-retry window

The respawn branch (task-agent-manager.ts:2339) fires when retryNowAfterRateLimit() is false and isRateLimitPauseActive() (= watchdog paused) is true. The watchdog doc correctly excludes an armed cooldown (there retryNow() returns true). But there's a second non-banner state where both hold: after the cooldown timer fires (cooldownTimer = null, rate-limit-watchdog.ts:427) while fireCooldownRetry is awaiting the retry callback (rate-limit-watchdog.ts:465). In that window paused is still true (notifyResume hasn't fired yet) and retryNow() returns false (no timer, not startup-exhausted) — so a manual Resume respawns a session whose auto-retry is actively starting, instead of letting it finish.

It's recoverable: cancel() bumps the generation, so the in-flight fireCooldownRetry self-aborts at its generation guard (rate-limit-watchdog.ts:472) and the turn re-runs via the fresh spawn. But it's unintended (only banner-Cancel should respawn), discards the in-flight retry, mints a throwaway session, and on a repeatedly-rate-limited task with an impatient user can repeat each cycle.

Suggested fix: gate the respawn on a signal set only by the banner path — e.g. a bannerCancelled flag set in cancel(notifyResume = false) and cleared on the next scheduleRetry / notifyPause / reset — rather than on paused. (Alternatively, also require that no episode is active / no retryCallback is in flight.)

P3 — change-skip guard doesn't compare the reason string

recomputeTaskRestriction's no-op guard (task-agent-manager.ts:790-797) compares status, resetAt, and type but not restrictions.limit (the reason). If two limited sessions share the same resetAt but differ in reason and one resumes (or a same-resetAt session pauses with a new reason), only the reason changes → the write is skipped → a stale reason is persisted. Observability/diagnostics only; no recovery impact. One-line fix: add task.restrictions?.limit === reason.

---

Note on the Greptile "stale limitedSessionsByTask entry" comment

That concern is unfounded. respawnRateLimitedExecution → stopSessionPreserveDb → handleInterrupt → watchdog.cancel(notifyResume = true) publishes session.rate_limit_resume, and InternalEventBus.publish runs the (synchronous) resume listener before its first await — i.e. synchronously inside the stop, before the eviction (agentSessionIndex.delete / subSessions cleanup). The listener's findParentTaskIdForSubSession still resolves the task (the session is still in subSessions) and deletes the entry, so the map is cleaned. (The respawn unit test doesn't exercise this because its fake session doesn't wire the bus — a follow-up test asserting the entry is gone after respawn would lock this in, but it's not a defect.)

[lsm]

🤖 Review by glm-5.1 (NeoKai)

Model: glm-5.1 | Client: NeoKai | Provider: Zhipu (GLM)

Recommendation: APPROVE

Round-2 verification, fresh after the rebase to fdaae2650. Both prior findings are resolved and correctly scoped; 64/64 rate-limit/respawn tests pass, typecheck + lint clean, PR is OPEN + MERGEABLE with all 4 review threads resolved (none outdated). No outstanding findings.

P2 resolved — banner-only respawn gate. The respawn now keys off a new bannerCancelled flag instead of the raw paused flag. I confirmed bannerCancelled is set in exactly one place — the else branch of cancel() (notifyResume=false) in rate-limit-watchdog.ts — and that path is reached only from the cooldown banner's Cancel button (RateLimitCooldownBanner → session.cancelRateLimitRetry RPC → cancelRateLimitRetry() → cancel(false)); there are no other cancel(false) callers. It is cleared on every transition that should retire it (new episode in scheduleRetry, reset, notifyPause, notifyResume). During the in-flight fireCooldownRetry window the flag stays false, so a manual Resume there is a clean no-op rather than a wasteful respawn. New tests lock both halves in (banner-only-signal, new-episode-clears, and "does NOT respawn while an auto-retry is in flight").

P3 resolved — reason in skip-guard. restrictions.limit is now compared in the no-op guard, so a re-pause that only flips the reason persists.

Rebase note. recomputeTaskRestriction now uses isRateOrUsageLimited(task.status) from #2317; that helper is behavior-identical to the old inline rate_limited || usage_limited check, just consolidated.

Entry-cleanup (the Greptile concern). respawnRateLimitedExecution now documents why no explicit limitedSessionsByTask cleanup is needed (the synchronous session.rate_limit_resume fired inside handleInterrupt runs the resume listener before eviction and deletes the entry), and a new test asserts the map is empty after respawn — so a replacement session's later pause/resume resolves cleanly.