fix: stop ACP reconnect storm and exclusive-session attach races - #3
Conversation
7293a59 to
174605a
Compare
3xian
left a comment
3xian
left a comment
There was a problem hiding this comment.
Not merging this while it is still a draft, and the author already flags it as a starting point. CI is green and several of the ACP pieces look directionally right (grok agent --no-leader stdio matches grok-build; killing the stdio child on give-up; treating leftover pid as ours when status is error). The frontend cutover over-corrects and I would not land it as-is.
Blockers
-
pickSelectedIdnever dropsprev. Roster pages are 30 cards (useSessionIndex). Keeping selection when the open task is off the current page is the right idea.if (prev) return prevalso keeps a deleted / missing id forever, so the open pane can stuck-load a gone task. Keepprevwhen it is absent from this page, not when it is absent from disk. -
Disk-only open pane stops updating.
refreshDetailis removed fromsessions-changed,x.ai/fs_notify/x.ai/git_head_changed, andrefreshFromDisk. That is correct while ACP owns the tail (startingincluded). It is not correct for an unattached / disk-only task:updates.jsonlwrites will no longer refresh the open timeline until the user reselects. Gate the skip onisAttachedManagedStatus, do not delete the disk path. -
Send during
startingre-enters attach.ensureAttachednow requiresisLiveManagedStatus, which excludesstarting/stopping. Reconnect already setspid = None. The next send callsattach_agent, thensession_open_elsewhere_error(session_id, None)can treat the reconnect candidate (or a not-yet-reaped child) inactive_sessions.jsonas a foreign Grok pid. Old code reused the in-memory Starting agent. Either keep that handle through reconnect, or wait for Ready; do not attach again withignore_pid = None.
Also fix before ready-for-review
- Exclusive-session matching via
/already open in another Grok process/iis brittle. Use a typed error / stable code from the Tauri command. - Silent send no-op on exclusive-session is easy to misread as a dropped prompt. If the banner stays suppressed, the composer still needs a visible refusal.
reconnect_burst/ last-activate window is untested. The pid-zero unit test does not cover the storm.
Please mark ready only after (1)–(3). Happy to re-review then.
getSessionDetail returns the first history page. Reloading the selected task on every updates.jsonl write, roster refresh, or WebKitGTK focus painted as a session restart. Load the open pane once on select; keep the current task across roster pages; ignore window focus on Wayland.
Refuse attach if another pid already has the session. Do not paint an error banner; Open in Grok Build is the cue. Send no-ops.
Giving up after a reconnect burst left the stdio agent in active_sessions.json. PinkCode then treated that pid as Grok Build and stalled until Stop. Kill the child on give-up/fail, wait for it to leave the session lock, and show Disconnected instead of Open in Grok Build.
174605a to
579a10d
Compare
Roster pages are 30 cards. Keep the open pane when it is missing from this page but still on disk; drop it when get_session_card says gone.
Skip getSessionDetail while ACP owns the tail (starting included). Unattached tasks still silent-refresh so disk writes update the timeline.
ensureAttached keeps the in-memory Starting agent. Send during reconnect does not call attach_agent with ignore_pid = None, which treated the reconnect child as a foreign Grok pid.
|
Addressed the review on this stack (head e644b44). Blockers
Also
Nix packaging and permission-mode |
attach_agent rejects with { code: session_open_elsewhere, pid, message }.
Refused send keeps the draft. The fused turn strip shows red Not sent
with a muted reason (Already open in Grok Build / Still connecting), a
static mark, and no global banner.
Extract last-activate burst counting so three deaths within 5s give up, and a stale activate resets. The pid-zero wait_until_dead test is not this path.
e644b44 to
60f4baa
Compare
|
Folded the last three UX commits into the typed-error/composer commit. Head is now 60f4baa. |
2 participants
Disclaimer
This stack is AI-assisted / vibecoded against a live bug on my machine (PinkCode talking to Grok over ACP). I am not a PinkCode maintainer and I am not claiming full authorship of the approach. Treat it as a starting point: review hard, rewrite freely, reject anything that does not match how you want this to work.
What this fixes
Opening a session while ACP is attaching remounted the open pane from disk (first history page) and treated the child as not-live. That:
session/loadon each stdio start (timeline jump)Attach still refuses when another Grok process owns the session. Send does not paint an error banner for that case; the existing Open in Grok Build chrome is the cue.
Giving up after a reconnect burst now kills the stdio child so a leftover pid is not labeled Open in Grok Build.
Commits
fix: stop open-pane remount from disk and window focusfix(acp): stop Live/Starting flicker on reconnectfix: keep timeline buffers while ACP is startingfix: refuse attach when another Grok process owns the sessionfix: no-op send when another Grok process owns the sessionfix(acp): kill grok child when reconnect gives upNix packaging / pin changes are not in this PR.
Repro
Reconnect storm (~9s, mp4) - card stays STARTING, timeline remounts from disk.