fix(server): restore feedback from closed immediate polls - #262
fix(server): restore feedback from closed immediate polls#262Julian-Dasilva wants to merge 2 commits into
Conversation
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
|
Fix folded into #261; closing this overlapping follow-up PR. |
Confidence Score: 3/5The PR should not merge until armed-poll disconnects preserve consumed feedback and restoration can no longer overwrite newer queued metadata. The immediate disconnect path is improved, but an equivalent disconnect during an armed poll still loses feedback and strands working presence, while a concurrent submission can have its newer snapshot and artifact failures replaced during restoration. Files Needing Attention: src/server.js, src/session-store.js Reviews (1): Last reviewed commit: "fix(server): restore feedback from close..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const result = await store.takeFeedback(key); | ||
| if (result.status === "feedback") markFeedbackDelivered(key, activePolls, deliveredFeedback, events); | ||
| finishFeedbackDelivery(key, result); |
There was a problem hiding this comment.
When an attached long-poll client disconnects while store.takeFeedback(key) is in flight, respond() still marks the consumed batch delivered and writes it to the destroyed socket without restoring it, causing feedback loss and leaving presence stuck on working.
| if (restoring) { | ||
| session.artifact_failures = Array.isArray(payload.artifact_failures) | ||
| ? JSON.parse(JSON.stringify(payload.artifact_failures)) | ||
| : []; | ||
| } | ||
| session.pending_prompts = session.prompts.length; | ||
| session.dom_snapshot = String(payload.domSnapshot || payload.dom_snapshot || ""); |
There was a problem hiding this comment.
If a new prompt submission completes after the disconnected poll's take but before restoration reacquires the store lock, restore mode appends the old prompts but replaces the newly queued artifact failures and DOM snapshot with older values, causing the next poll to receive stale or incomplete feedback metadata.
1 participant
Summary
Follow-up to #261. Restore feedback consumed by a client that disconnects during the immediate poll take, so the next poll can receive it and presence does not remain stuck on working.
What changed
src/server.js: check request closure before immediate delivery; requeue closed feedback throughqueuePromptsand log if the restored batch is incomplete.src/session-store.js: add an internal restore mode that preserves normalized prompts, layout-warning prompts, attachments, artifact failures, existing chat, and an already-ended session without creating duplicate chat records.test/server.test.js: add a production-composition regression that pausestakeFeedback, destroys the request, and verifies waiting presence plus exact next-poll recovery.Verification
npm test→ 931 passed, 5 skipped, 0 failed.npm run lint→ passed.npm run typecheck→ passed.npm run checkcompleted build and lint, then reported unrelated formatting failures in untracked.planning/telemetry/hook-metrics.jsonand existingtest/fixtures/layout-audit/real-editorial.html.Not covered / follow-ups
Test plan
node --test --test-name-pattern "a disconnect during immediate feedback take requeues" test/server.test.jsnpm testnpm run lintnpm run typecheck