Skip to content

fix(chat): bound a waiting copy by the PC clock of every read and a queued row stamp - #2973

Merged
devswha merged 6 commits into
mainfrom
fix/chat-waiting-skew-2949
Nov 5, 2026
Merged

devswha merged 6 commits into
mainfrom
fix/chat-waiting-skew-2949

Conversation

@devswha

@devswha devswha commented Nov 5, 2026 •

Copy link
Copy Markdown
Owner

Change

Fixes #2949.

#2898 bounded a sent message's waiting copy with the PC's server_time from the chat's first read of
its turns. Three windows stayed open (and two more were found by reading the code): an idle chat
kept that first read and the drift allowance grew with it, a queued row fell back to the turns'
times, a bridge upgraded under an open chat was never asked again, a cached answer was timed by
the mount, and a wall clock step on this device could keep the copy beside its own turn until it
expired. In each case an identical turn another device or the terminal wrote just before the send
could clear its waiting copy (#2849's symptom), or the copy could stay too long.

The fix gives the browser a fresh PC time for every read and every queued try, and measures what
has passed since on this browser's monotonic clock:

  • GET /api/pane/conversation sends x-server-time on both 200 and 304 answers (the body's
    server_time stays). It is not part of the ETag, so an unchanged transcript still answers 304.
  • The remote-PC proxy (server/machine-api.ts) passes x-server-time through.
  • Over the relay, an older phone refuses a response header it does not list, so the PC sends
    x-server-time only to a phone that offered the new server-time-header mux feature in HELLO
    (shared/relay-mux.ts, server/relay/session.ts, shared/relay-channel.ts; docs/relay.md).
  • src/lib/api.ts records each answered read's PC time with performance.now() as the answer
    arrived (observeWaitingClock), whatever becomes of the body. A cached answer drawn on remount
    therefore keeps its real receipt time, and each read replaces the one before.
  • waitingBaseline turns that into pcFloor: the PC's clock was at least the answer's time plus
    the monotonic time since (less 0.1% drift) when the send began. A turn older than that is not the
    send's own. No wall clock time of this browser enters that bound, so a clock step does not move
    it. Without any PC time (an older bridge) the previous turns-only bound applies unchanged.
  • A queued row now carries sending_at (PendingMessage, optional): the PC's clock as the try went
    out, before anything is typed. It is dropped when the row is queued or held again and kept while
    uncertain. A row's waiting copy is matched against it directly, so a sending frame replayed
    after the Enter is still settled by its own turn.

Design reference: as in #2717 and #2898, the waiting copy follows Claude Code's queued-message look;
nothing visible changes here, only when the copy clears.

This PR changes an HTTP contract (a response header), a relay mux feature and the pending-message
frame, so it waits for a real review.

Validation

  • Failing first, checked by breaking the owner of each and watching the test go red:
    • src/lib/sentWaiting.test.ts: another device's identical turn before the send (rewritten for
      the new clock), an idle chat bounded by a later 304 from an upgraded bridge (chat: a waiting copy can still clear early for queued sends, idle chats and upgraded bridges #2949 items 2 and
      3), an empty chat. Removing the pcFloor bound fails all three. A wall clock step (item 5) and
      an hour of drift are covered as non-regressions.
    • src/lib/machineSession.test.ts: a queued row whose sending frame came after its Enter, with
      another device's identical turn 300 ms before the PC took it (item 1). Using the chat's clock
      instead of sending_at fails it.
    • src/lib/conversationFetch.test.ts: a 304's x-server-time refreshes the bound, timed as the
      answer arrived (items 2 and 4). Ignoring the 304 header fails it.
    • server/relay/session.contract.test.ts: x-server-time only to a phone that offered the
      feature. Sending it to every phone fails it.
    • server/pane-routes.test.ts (header on 200 and 304), server/machine-proxy.test.ts (passed
      through on a 304), server/pending-input.test.ts (sending_at stamped and dropped).
  • bun run typecheck passed; the affected unit and contract files passed (1,059 tests, 0 fail).
  • bun run check fast on e064856, merged with main (heavy wrapper on old-desktop): workflow syntax,
    generated types and themes, typecheck, build and unit tests all passed. Unit tests: 12,016 pass,
    0 fail, 37 skip.
  • Reviews: omo and Codex (Terra, xhigh). Findings and how each was resolved are in two PR comments;
    two pre-existing windows went to Chat waiting copy: an immediate send has no PC stamp, so a PC clock correction or a long-idle tab still misjudges its turn #2974. A sleeping device (whose monotonic clock stops) is counted
    by the wall clock, and a queued row its chat never read is bounded by its stamp.
  • Skipped locally: check full (integration and browser lanes, left to CI) and native macOS,
    Windows and phone runs. There is no visual change.

… a queued row's own stamp (#2949)

- GET /api/pane/conversation sends x-server-time on 200 and 304; the PC proxy passes it on, and the
  relay sends it only to a phone that offered the server-time-header mux feature.
- The browser records each answered read's PC time with its monotonic receipt time (lib/api.ts), so
  an idle chat, a cached remount and an upgraded bridge all use the latest read, and a wall clock
  step does not move the bound.
- A queued row carries sending_at (the PC's clock as it went out); its waiting copy is matched
  against that directly.
@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 8 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between bbad99c and e064856.

Files selected for processing: 20

  • changelog.d/waiting-copy-fresh-pc-clock.md
  • docs/relay.md
  • server/machine-api.ts
  • server/machine-proxy.test.ts
  • server/pane-routes.test.ts
  • server/pane-routes.ts
  • server/pending-input.test.ts
  • server/pending-input.ts
  • server/relay/session.contract.test.ts
  • server/relay/session.ts
  • shared/protocol.ts
  • shared/relay-channel.ts
  • shared/relay-mux.ts
  • src/components/ChatView.tsx
  • src/lib/api.ts
  • src/lib/conversationFetch.test.ts
  • src/lib/machineSession.test.ts
  • src/lib/machineSession.ts
  • src/lib/sentWaiting.test.ts
  • src/lib/sentWaiting.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@devswha

devswha commented Nov 5, 2026

Copy link
Copy Markdown
Owner Author

Review findings and how each was resolved

Reviews: omo (dori review) and Codex (Terra 6.1, xhigh, adversarial).

  • omo at 341f7bf: CLEAR.
  • Codex MEDIUM: a late response could bring back an obsolete clock. An older read whose body was parsed after a newer one replaced the newer read's PC time. Fixed in a8a7cc3: an answer replaces only those that arrived before it (by the browser's monotonic receipt time, taken as the headers arrive). New test in src/lib/sentWaiting.test.ts, which fails without the fix.
  • Codex MEDIUM: a PC clock set back between the last read and a send leaves the send's waiting copy beside its own turn until it expires (60 s); a forward correction lets an older identical turn clear it. Reproduced with a bun -e probe. Not looser than main before this PR. Follow-up: Chat waiting copy: an immediate send has no PC stamp, so a PC clock correction or a long-idle tab still misjudges its turn #2974.
  • Codex MEDIUM: the drift allowance after a long-idle tab. An hour without a read leaves 3.6 s in which an identical turn written just before the send can clear its waiting copy. Reproduced with a bun -e probe. Not looser than main before this PR, where the same allowance ran from the chat's first read. Follow-up: Chat waiting copy: an immediate send has no PC stamp, so a PC clock correction or a long-idle tab still misjudges its turn #2974. Both need a PC stamp on an immediate send, a contract change kept out of this PR.
  • Codex found no compatibility issue in the relay header negotiation or the PC proxy path.

@devswha

devswha commented Nov 5, 2026

Copy link
Copy Markdown
Owner Author

Second review round (omo at 6cddf78: BLOCK)

  • omo MEDIUM, reproduced: a device that sleeps. Many browsers stop performance.now() while the device sleeps, so after waking the bound stayed near its value before the sleep, and another device's identical turn written during the sleep could clear the send's waiting copy (fix(chat): a matching turn from another device can clear a newer send's waiting copy #2849's symptom; main held it). Fixed in ada735d: each answer also records the wall clock as it arrived, and once the wall clock went on more than 2 s (plus drift) further than the monotonic one, the wall clock's elapsed time counts. New test in src/lib/sentWaiting.test.ts, which fails without the fix.
  • omo MEDIUM (Codex), pre-existing: a queued row whose chat never read the pane dropped its sending_at stamp and fell back to the browser's wall clock. Fixed in ada735d: the stamp bounds the row with no shown turns. New test in src/lib/machineSession.test.ts, which fails without the fix.
  • omo MEDIUM (Codex), unreproduced, pre-existing: a sending frame that arrives after the row's own turn is already shown puts that turn in the baseline, so the copy stays until it expires. Same as main; left as is.
  • omo LOW, pre-existing: a PC clock set back between the last read and the send. Covered by Chat waiting copy: an immediate send has no PC stamp, so a PC clock correction or a long-idle tab still misjudges its turn #2974.

@devswha

devswha commented Nov 5, 2026

Copy link
Copy Markdown
Owner Author

Local merge gate (owner-approved): bun run check fast passed at this head (12016/0), omo CLEAR at this head, Codex Terra xhigh: 1 MEDIUM fixed, 2 pre-existing in #2974; threads resolved. Adds an x-server-time response header (relayed only to phones that offer server-time-header); CodeRabbit was rate-limited, so the omo + Codex reviews stand in. main moved two commits since, touching none of this PR's files. Merged without waiting for GitHub CI.

@devswha
devswha merged commit e82c9a5 into main Nov 5, 2026
18 of 21 checks passed
@devswha
devswha deleted the fix/chat-waiting-skew-2949 branch November 5, 2026 12:10
devswha added a commit that referenced this pull request Nov 6, 2026
…efore typing (#3165)

Fixes #2974.

## What changes
An immediate (composer) send now gets the same PC stamp a queued row
has. The bridge takes its clock right before it types the message and
returns it as `sending_at` on the `submit-result` (immediate sends, a
queued send typed at once with `typed_now`, and a held copy's resend
that typed). The browser bounds the send's waiting copy by that stamp
alone (`stampedBaseline` in `src/lib/sentWaiting.ts`), so neither
clock's moves since the chat's last read matter:

1. PC clock set back between the last read and the send: the send's own
turn is no longer taken as older than the send (it stayed faded for 60
s). A shown turn's time no longer bounds a stamped send either, since a
clock set back writes the new turn before it.
2. A send long after the last read (a hidden tab): the 0.1% drift
allowance no longer opens a window in which an identical turn written
just before the send clears its waiting copy.

An older bridge sends no stamp, and the read-based bound from #2973
stays for it. The queued-row path in `machineSession.ts` uses the same
helper (same behaviour, plus the shown-turn time no longer overrides the
row's stamp).

Protocol: `sending_at?: string` on `submit-result` (optional, additive;
older clients ignore it).

## Validation
- Failing first: the new tests in `src/lib/sentWaiting.test.ts`,
`src/lib/ws.test.ts` and `server/pending-input.contract.test.ts` fail on
origin/main (4 fails) and pass with the fix.
- `bun test src/lib/sentWaiting.test.ts src/lib/ws.test.ts
src/lib/machineSession.test.ts server/pending-input.contract.test.ts
server/pending-input.test.ts`: pass.
- `bun run check fast`: ok (unit 12666 pass, 0 fail).
- Not run: full integration/browser suites (CI runs them).
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.

chat: a waiting copy can still clear early for queued sends, idle chats and upgraded bridges

1 participant