Keep the Planner while its workflows run, with Stop/Resume and a run card #230
If the last job finishes while the Planner's reply is being saved, `chat.busy` is still true, so cleanup gets skipped and never retried. I reproduced the helper and document staying loaded after the job was done.
Let's check again once the reply and any queued replies have finished. Only close the same helper, and only if it still has no work. Please apply this with the cleanup guard below, so a slow close cannot overwrite a newer helper's progress.
```suggestion let stopWatching = opened.session.watchRuns?.(next => { if (chat.retained?.session !== opened.session) return; publishRuns(context, next); if (next.active.length || next.paused.length) return; let releaseWhenIdle = async () => { if (chat.busy) await chat.running; if (chat.busy || chat.retained?.session !== opened.session) return; let current = opened.session.runs?.(); if (!current || current.active.length || current.paused.length) return; await chat.retained.release(); }; void releaseWhenIdle().catch(err => console.error("[chat] releasing completed workflow session failed:", err) ); }); ```
Applied in c29f101. When the last job ends while a reply, or a queued reply, is still running, cleanup is checked again once it finishes, and it releases only the same helper, only if it still has no active or paused runs. The deferred-cleanup test fails without it.
Assistant-verification: fail-before/pass-after passed: deferred-cleanup test
Assistant-workflow: goal (run 6091839b-867c-4965-aec9-b583f62d4b1f) Co-authored-by: Alex Lavaee <lavaman131@github.com>
Comments on earlier or unavailable lines
This guard goes with the cleanup change above. Closing an old helper can take long enough for a new one to start. In a regression test, the old cleanup changed the new helper's running card to ‘stopped’. Let's only clear the owner and cards if this is still the helper being closed.
```suggestion if (chat.agent === opened.session) { chat.agent = undefined; if (!chat.busy) chat.owner = undefined; publishRuns(context, undefined); } ```
Let's save the new card and its finish message before showing either to people. Right now the message is sent by `say()` above, then the card is sent here, and only afterwards do we try to save them. If saving fails, people can see a job finish and then lose that update after a reload.
Please handle these updates in order, save the card and message together, then send both. Moving only `state()` would still send the message too early. Please add a test where saving fails and check that neither update was announced. This needs a coordinated change, so a one-line suggestion would leave part of the bug in place.
This is the other half of the quick-job fix: a job we first hear about after it has ended should still get a finish message. The current check skips it because there is no earlier card. This suggestion allows that first message and still avoids repeating one for a job already recorded as finished.
```suggestion if (!ended || (previous && ENDED_RUN[previous])) continue; ```
A quick job can finish before the Planner's reply is done. We return here without keeping its card, so people never see that it finished or failed. I reproduced the missing card. Let's record its last state even when there is no helper left to keep running. Please apply this with the companion finish-message suggestion above.
```suggestion if (!runs || opened.binding.signal.aborted) return false; if (!runs.active.length && !runs.paused.length) { publishRuns(context, runs); return false; } ```
Fixed in c29f101. A run report is now built as a candidate outside shared chat state and saved first: the save stores the candidate card and transcript together, and only when it succeeds are the card and finish message added to the chat and announced together. While a save is pending, or after it fails, nothing else that broadcasts chat state or greets a joining client can show them. Reports are handled one at a time, so a report that arrives during a pending save is computed from the state after it settles.
The new test fails the save and checks that neither update was announced; others cover overlapping reports, a state broadcast during a pending save, and a failed save of a run's final report, which is replayed on release so the run still ends as finished rather than "was stopped".
Assistant-verification: fail-before/pass-after passed: save-failure, overlapping-report, state-leak and failed-final-save tests; bun test 1928 pass, 0 fail on the rebased branch
Assistant-workflow: goal (run 6091839b-867c-4965-aec9-b583f62d4b1f) Co-authored-by: Alex Lavaee <lavaman131@github.com>
Applied in c29f101, together with the finish-message change: a quick job that ended before the Planner's reply finished keeps its last card and gets its finish message. The quick-job test fails without it.
Assistant-verification: fail-before/pass-after passed: quick-job test
Assistant-workflow: goal (run 6091839b-867c-4965-aec9-b583f62d4b1f) Co-authored-by: Alex Lavaee <lavaman131@github.com>
Applied in c29f101: a run first reported as already ended gets one finish message, and a run already recorded as ended never gets another. The first-seen-ended test fails without it.
Assistant-verification: fail-before/pass-after passed: first-seen-ended test
Assistant-workflow: goal (run 6091839b-867c-4965-aec9-b583f62d4b1f) Co-authored-by: Alex Lavaee <lavaman131@github.com>
Fixed in c29f101, generalizing your guard into one rule: every run card belongs to the Planner session that reported it. A report replaces or updates only that session's cards, and releasing a session marks stopped exactly the unended cards it owns, with one "was stopped" message each, saved and announced together. A release never touches another session's cards, and it clears the owner and agent only when the released session is still the current agent and no turn is running. So a slow release of an old helper can no longer overwrite a newer helper's progress, and it still stops its own cards when a newer helper has taken over.
A table test runs four cases through the real invoke flow: no newer helper, a newer helper's run-less turn, a newer helper that reported its own runs, and a run-less turn after a failed save. Each asserts that the old helper's cards end stopped with one message and that the newer helper's cards and owner are untouched.
One known gap remains: if the release's own final save also fails, nothing is announced, and when a newer helper then reports a new run, the old helper's orphaned card is dropped without a stop message. It needs two failures in a row; I can follow up if you'd like it covered here.
Assistant-verification: fail-before/pass-after passed: the four-case release table test (the newer-helper-reported-runs case failed before) and the slow-release tests; bun test 1936 pass, 0 fail on #238 with Atomic 0.9.26-alpha.6 Assistant-workflow: goal (run 6091839b-867c-4965-aec9-b583f62d4b1f) Co-authored-by: Alex Lavaee <lavaman131@github.com>