Skip to content

fix(server): preserve sends and truthful presence across poll transitions - #261

Closed
Julian-Dasilva wants to merge 2 commits into
kunchenguid:mainfrom
Julian-Dasilva:fm/lavish-upstream-229
Closed

fix(server): preserve sends and truthful presence across poll transitions#261
Julian-Dasilva wants to merge 2 commits into
kunchenguid:mainfrom
Julian-Dasilva:fm/lavish-upstream-229

Conversation

@Julian-Dasilva

Copy link
Copy Markdown
Contributor

Fixes #229

  • Preserve Send submissions when no poll is attached by queuing them for the next poll.
  • Keep presence truthful on every poll exit path, including final feedback on ended sessions.

Keep feedback actions durable while the agent is working, close poll cleanup races, and clear delivered state for final feedback on ended sessions.

Fixes kunchenguid#229
@Julian-Dasilva
Julian-Dasilva force-pushed the fm/lavish-upstream-229 branch from d879c4f to f3510c2 Compare August 20, 2026 01:59
@Julian-Dasilva

Copy link
Copy Markdown
Contributor Author

@greptile-apps Please re-review this PR. The branch is now based on current upstream/main and contains one issue-229 commit with seven changed files.

@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

The PR is not yet safe to merge because an immediate poll can still lose feedback and strand working presence when its connection closes after the new guard but before response completion.

Julian-Dasilva stated that the immediate poll path was fixed in 9ea0104, but the handler still marks the destructively taken batch delivered and detaches its close listener before res.json completes, leaving a concrete disconnect window with the previously reported loss.

Files Needing Attention: src/server.js

Reviews (2): Last reviewed commit: "fix(server): restore feedback from close..." | Re-trigger Greptile

Comment thread src/server.js
Comment thread src/server.js
Comment on lines +449 to 450
detachRequestClose();
res.json(immediate);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Response completion escapes restoration

When the polling client disconnects after the closure check but before the immediate JSON response finishes, this branch marks the destructively taken batch as delivered and detaches the close listener before res.json completes, causing the feedback to be lost and presence to remain working without an agent receiving it.

@kunchenguid

kunchenguid commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Holding this. It is the right shape for #229 — Send stays queueable while the agent is working, and every poll exit path has to undo the presence it set — but I cannot auto-merge it.

Required CI (Require no-mistakes + build-and-test) has not run on this fork head (no Actions check suite, only Greptile Review fail). The upstream PR body also lacks the ## Pipeline / Updates from git push no-mistakes signature that the required check looks for. There is an HTML attestation comment, but that is not what CI greps, and its head_sha does not match this head. Please re-raise so a real no-mistakes body lands on this PR; I will not add the marker.

Greptile's leftover close-vs-res.json window is a tight TOCTOU after the restore guard. I am not treating that as a blocker, and I am not pinging the review bot. Ready-for-pr on #229 is not a merge vote — green required CI still has to land on this head.

@Julian-Dasilva

Copy link
Copy Markdown
Contributor Author

Content unchanged; re-raised through the required no-mistakes pipeline in #263.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Immediate-return poll leaves presence "working" with no poll attached, so the chrome silently drops the next Send

2 participants