fix(gc): honour Keep Alive, widen grace to 30d, pin Zed missing-thread auto-repair - #2980
Merged
Conversation
The orphan reaper deleted the workspace of a spec task that was pinned with
Keep Alive and still actively in use, taking the agent's working directory and
its Claude transcript with it.
Reported as "after a restart the Zed session didn't come back -- blank agent
panel saying New Zed Agent Thread", and separately as "long running tasks stop
being able to use their own shell". Both are the same cause.
What happened to spt_01kz6r8evrdtpepqd59sjm0eev:
Aug 5 14:04:06 find-ai#23 merged
Aug 5 14:04:27 task correctly moves to done (terminal)
Aug 5 14:07 user still working in the session
~Aug 5 20:04 grace period (6h) elapses -> os.RemoveAll on
/data/workspaces/spec-tasks/spt_... WHILE the container runs
-> shell breaks (cwd deleted), .claude-state destroyed
Aug 10 11:40 restart -> empty workspace; zed_thread_id still points at a
session claude-agent-acp no longer has -> load_session fails
"Resource not found" -> silent fallback to an empty thread
Two independent defects:
1. Keep Alive was invisible to the reaper. liveSpecTaskIDsForReaper decided
liveness purely from terminal status and mtime; grep for KeepAlive across
api/pkg/hydra and api/pkg/external-agent returned nothing. The flag protected
the container from idle-shutdown but not the workspace, so an explicit "leave
this alone" was silently overridden. 22 tasks are currently done with
keep_alive=true, so this was not a one-off.
Keep Alive now outranks the status heuristic entirely, and is deliberately
not time-bounded: expiring an explicit "keep this" after N days reintroduces
the same surprise on a longer timer.
2. The grace period was 6h. That is a race guard, but reaping a task workspace
destroys conversation history, so the window really means "how long after a
task finishes might someone want to pick it back up". Raised to 30 days;
bringing a finished task back to life weeks later is normal, and losing its
history is not an acceptable price for reclaiming disk early.
Adds a regression test asserting Keep Alive survives terminal+archived+stale,
while an unpinned stale terminal task is still reapable so the GC keeps working.
Verified the test fails when the KeepAlive check is disabled.
NOT fixed here (follow-ups): the reaper is still blind to whether a container is
actually running, and a live work session still does not protect its own
spec-task workspace (spec-tasks/ is reaped against liveSpecTaskIDs only, so
liveSessionIDs offers no protection). Also, a genuine reap should clear
config.zed_thread_id and surface load_session "Resource not found" rather than
presenting an empty panel.
go build + go test ./api/pkg/external-agent/: pass.
Pins helixml/zed#79, which makes the dangling-thread case self-healing. Together with the reaper fix in this PR that is the full chain for the incident: Keep Alive is now honoured so the workspace is not deleted in the first place; and if a thread ever does go missing, Zed now reports it so Helix clears the stale zed_thread_id automatically instead of showing a blank agent panel.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
eae9b051e1 is the merge commit; verified content-identical to the tested tree (git diff aa2cbae046..eae9b051e1 is empty).
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
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.
Fixes the incident where a spec-task session came back with a permanently blank agent panel, and the related "long running tasks stop being able to use their own shell".
Paired Zed PR: helixml/zed#79
What happened
spt_01kz6r8evrdtpepqd59sjm0eev, pinned with Keep Alive ON:The done-transition was correct — the task's only PR really had merged. The bugs are downstream of it.
1. Keep Alive was invisible to the reaper
liveSpecTaskIDsForReaperdecided liveness purely from terminal status and mtime. GreppingKeepAliveacrossapi/pkg/hydraandapi/pkg/external-agentreturned nothing — the flag protected the container from idle-shutdown but not the workspace, so an explicit "leave this alone" was silently overridden.22 tasks are currently
donewithkeep_alive = true, so this was not a one-off.Keep Alive now outranks the status heuristic entirely, and is deliberately not time-bounded: expiring an explicit "keep this" after N days reintroduces the same surprise on a longer timer.
2. Grace period was 6 hours
Nominally a race guard, but reaping a task workspace destroys conversation history, so the window really means "how long after a task finishes might someone want to pick it back up". At 6h a PR merged at 14:04 lost its workspace the same evening.
Raised to 30 days. Bringing a finished task back weeks later is normal, and losing its history isn't an acceptable price for reclaiming disk early.
3. (Zed side) Auto-repair now actually fires
The recovery path here —
recoverMissingThread+isAuthoritativeMissingThreadError— was already built and correct. Zed just never sent thethread_load_errorthat triggers it, because it suppressed failures with norequest_id(which is exactly the reconnect case). Fixed in helixml/zed#79, so a dangling pointer now self-clears.Validation
go build+go test ./api/pkg/external-agent/— passKnown follow-ups (not in this PR)
spec-tasks/is reaped againstliveSpecTaskIDsonly, soliveSessionIDsoffers no protection.🤖 Generated with Claude Code