Skip to content

A follow-up staged later can jump the queue, and the presence read it depends on has no deadline #311

Description

@scgopi

Two follow-ups raised by the independent review of #309, neither blocking that merge, both worth fixing on their own.

1. A later follow-up jumps the whole queue

Two different readings decide "deliver now":

  • deliversLater consults the cached node.presence the poll last wrote.
  • The drain takes a live reading per item.

When they disagree — the poll saw idle, the session is mid-tool-call by the time the drain asks — three already-queued messages are retried while a fourth staged afterwards bypasses the queue and is typed in first.

Deterministic, and no overlapping drain is needed to produce it.

A queue that reorders under a predictable condition is only slightly better than one that reorders randomly: a loop cannot use arrival order to mean anything.

2. The presence read has no deadline

onReadPresencePTYProcessSession.waitCollectingOutput() has no timeout. ssh's ConnectTimeout=10 bounds the connect, not a hung remote command.

An await that never returns now holds isDrainingFollowUps for ever and freezes the queue permanently.

The reviewer specifically checked whether #309's new guard makes this worse than main, and it does not — main re-enters into the same hang and additionally re-delivers everything before the stuck item. So this is pre-existing, not a regression. But it is worth bounding, because:

"frozen" and "working" look identical from outside

which is the same property that let #304 and #306 go unnoticed for as long as they did. A drain that has been wedged for an hour reports exactly what a drain with nothing to do reports.

Suggested

  • Have both paths read presence the same way, or make the drain's decision authoritative and stop consulting the cached value in deliversLater.
  • Give the presence read a deadline, and treat a timed-out read as unknown rather than as a state — the same distinction A resumed Copilot session reads as absent, so a live loop shows FAILED #286 established when a failed zmx probe was being read as .absent.

Found by the review of #309; not introduced by it.

Related: #304, #306, #286

🤖 Generated with Claude Code

https://claude.ai/code/session_01BP43ags4cn8fq2ZZdv85J9

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions