diff --git a/CHANGELOG.md b/CHANGELOG.md index 23eebdd..065e834 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ For subset counts use different phrasing (e.g. "fourteen new #83 cases"). ## Unreleased +- **feat(terminal): the streaming handshake replay now announces its real START offset ahead of the first chunk, so a client whose connection dies mid-replay resumes from what it already painted instead of re-pulling the whole replay (issue #94)** — #87's streaming replay (`ttyHandshakeReplayBudget` = 8 MB, ~128 × 64 KB frames) widened the "replay painted, `sync` not yet arrived" death window from one ≤64 KB frame to the full budget: the client's `handshakeSynced` latch deliberately refused to advance the cursor frame-by-frame until the closing `sync`, because `RingBuffer.ReadSince` silently clamps a stale `since` to the oldest live byte and accumulating from the request could publish a cursor over bytes never delivered — so a drop mid-replay re-pulled everything from the old cursor, which under sustained network degradation does not converge. Fix (the issue's minimal form, landed one step cheaper): the catch-up loop now brackets the replay with TWO sync frames — `{"type":"sync","offset":S,"start":true}` ahead of the FIRST binary chunk, where S = the first chunk's `next` − len(body), so S and the chunk come from the SAME `ReadSince` call (the clamped oldest-live value whenever clamping happened, never the raw request; no new `Manager` surface), and the unchanged closing sync after the last chunk. The frame REUSES the `"sync"` type instead of adding a frame type plus a capability gate: any recipient provably parses `"sync"` already (the `caps=sync` token or the inferred explicit-`since` opt-in, issue #98 — and every catch-up request presents `since` by definition, so no catch-up client can be a pre-whitelist page), and the shipped client's existing handler — adopt the ABSOLUTE offset, lift the latch — is precisely the wanted semantics, so the client needed ZERO code changes and even a never-refreshed v0.5.1 page benefits from a new daemon. The `"start":true` marker exists for consumers that must know WHEN the replay ends (the test harness stops at the sync WITHOUT it and fails if a start frame ever arrives after a chunk); the UI deliberately ignores the field, treating both sync frames alike, and S + Σ(frame bytes) equals the closing offset unless the ring rolls a full cycle mid-replay and evicts bytes the loop had not sent yet — then a later chunk clamps to the new oldest live byte, the sum lands short, and the closing sync, the absolute authority, jumps the gap (those bytes are undeliverable anyway, the same rule the read→subscribe window already lives by). The beyond-head tail degrade (`since` past head — defensive) announces its tail's start the same way (S = head − len), so every replay addressed to a cursor-bearing client carries the frame; only the CURSOR-LESS first-connect tail (a single ≤64 KB frame whose client may not parse `sync` at all — that path's `syncCap` is a genuine gate, not an inference), `since=-2` (zero binary frames by design, the #86 contract untouched) and the caught-up-at-head consult exit still send the closing sync ALONE, and `completeHandshake` stays single-shot per connection. Pinned server-side by `TestHandleInstanceTTYWS_ReplayStartOffsetPublishesClampedStart` (a `since` below the oldest live byte publishes the CLAMPED start, never the raw request), `TestHandleInstanceTTYWS_MidReplayResumeContinuesFromRenderedBytes` (a drop after the start sync plus two chunks reconnects from start+painted and receives EXACTLY the un-rendered remainder), `TestHandleInstanceTTYWS_StartSyncOnlyForChunkedReplaysWithACursor` (the closing sync arrives ALONE only on the three paths with no cursor or no chunks: cursor-less tail / `-2` / caught-up), the extended 8 MB budget test (start 0 + delivered budget == closing offset; the re-request's start IS the published cursor) and the extended `TestHandleInstanceTTYWS_CursorAheadOfHeadFallsBackToTail` (the beyond-head degrade announces the tail's real start, S = head − len); client-side by the new issue-#94 `node --test` case inside the 68-case total below, which drives start sync → replay frames → a mid-replay drop → and pins the reconnect's `since` at the rendered cursor. Contract documented in `docs/API.md` (TTY WS) and `docs/ARCHITECTURE.md`. + - **test(ui): the `node --test` case-count guard now checks EVERY occurrence of the coverage-claim phrase instead of the first match (issue #95)** — `TestTerminalStatusChangelogCount` located the claim with `FindSubmatch`, which returns the first "Pinned by N `node --test` cases" in the file; since entries are prepended in reverse chronological order, the anchor was positional — any future entry using the phrase and landing above the #80 entry would silently hijack the guard (a stale number above plus drift in the real claim would still pass, and entries quoting subset counts carried no constraint at all). Of the issue's two proposals this takes the stricter one: every occurrence of the phrase must equal the derived total (`grep -c '^test('`), making the claim position-independent — the phrase now means "the file's total case count" wherever it appears, and subset counts keep their distinct phrasing (e.g. #83's "fourteen new #83 cases"). Verified by injection: a stale claim above the anchor fails, and a correct claim above plus a corrupted #80 claim fails — the exact silent-failure mode the issue named. Pinned by `TestTerminalStatusChangelogCount`. @@ -32,7 +34,7 @@ Release focused on dsh 0.2.x support (issue #84), terminal WebSocket reliability - **fix(terminal): a throttled browser tab lost all terminal output, silently and forever (issue #82)** — a backgrounded tab, a slow link or a laptop sleep left the terminal frozen on a stale screen while the shell kept producing output, and neither side could notice. Root cause: `pty.broadcast` fanned each chunk out with `select { case ch <- chunk: default: }` and the `default:` arm was **empty** — the chunk was dropped. `ws.Conn.writeFrame` blocks inside `rw.Flush()` on the hijacked TCP socket and `Upgrade` sets no write deadline, so as soon as the client stops reading, `conn.WriteBinary` blocks, the handler's select loop stops draining, the subscriber's 64-slot channel fills, and from that instant **every** chunk was discarded — unboundedly, with no counter, no signal to the client and no disconnect — so one missed clear-screen or cursor move corrupted everything rendered afterwards. Fix: an overflowing subscriber is now **removed, closed and drained** rather than kept and skipped, so the handler reaches `!ok` (bounded: the backlog never reaches the socket writer, though a parked receiver can still win a few buffered chunks) and answers with a `1013 subscriber overflow: slow consumer` close frame and returns; the browser's `ws.onclose` (which since #87 already logs abnormal close codes and reasons) prints the cause in the console with no client change, reconnects, and resumes **incrementally from its own cursor** on the #87 offset contract already shipped in #87. The channel capacity is deliberately unchanged — a longer queue only postpones the disconnect of a consumer that cannot keep up, and the disconnect is the correct outcome. The threshold is concrete rather than "a bit slow": `pumpLogs` reads the PTY in 1024-byte chunks and each subscriber queue holds 64 of them, so a consumer is disconnected after roughly **64 KB** of output it has not taken, and recovery costs the client the browser's hard-coded 5 s reconnect (`index.html`) plus an incremental resume from its own cursor. The teardown close write is also now **bounded**: `conn.SetWriteDeadline(5s)` immediately before it, on that failure path only, because this branch exists precisely because the socket stalled — an unbounded close would have moved the stall from `WriteBinary` to `WriteClose` and parked the handler forever, which would not have fixed #82 at all. Stated exactly what that bound covers, because an earlier wording here overstated it: the deadline bounds **only this teardown close-frame write**; a handler already blocked inside a live `conn.WriteBinary` never reaches this branch and is reclaimed only when that write returns — `Upgrade` sets no write deadline, and issue #83 — since merged, on the liveness side — added only a READ deadline and deliberately scoped a write deadline out; a write deadline for every connection remains unimplemented, by design, not by oversight. What actually keeps the failure path from flushing the backlog onto the stalled socket is upstream: Go delivers a closed channel's **buffered** values with `ok == true` before it ever reports `!ok`, so the overflow arm now **drains the closed subscriber's queue at the source** (`for range` over the collected subscribers, **after `subsMu` is released** — never inside it: that loop terminates only because the channel it ranges over is already closed, which is a convention this code keeps rather than something the compiler enforces, and `subsMu` is process-global, so draining under it and dropping or reordering the one `closeLocked()` line that precedes it would wedge every OTHER instance's broadcast at `subsMu.Lock()` with no panic, no log and no timeout — a daemon-wide silent freeze traded for the old loud local `panic: send on closed channel`; so `closeLocked` now reports whether it performed the close and only those subscribers are collected, which makes "we drain only channels this critical section closed" provable instead of assumed), making the handler's very next receive the resync signal instead of up to 64 queued chunks (~64 KB) of `WriteBinary`; discarding that backlog loses nothing **that is still in the ring buffer**, because `pumpLogs` writes every chunk to the ring buffer *before* broadcasting it (that ordering is now commented as load-bearing) and each drained chunk comes back on the reconnect's replay from the client's #87 cursor — the whole premise of #82 — with the one exception both `docs/API.md` and this entry now name: a chunk broadcast after `Manager.dropBuffer` closed and dropped the ring was never stored at all (`RingBuffer.Close` sets `closed=true, data=nil, used=0` and does not reset `head`, and the buffer is no longer reachable through the Manager), so it is unreplayable and a still-connected client would have received it on the wire pre-drain — the same post-swap loss ARCHITECTURE §4.1 already declares intentional. The overflow is also now **logged server-side** (`s.logger.Printf`, nil-guarded, matching the existing tty-resize precedent in `app.go`): this disconnect is user-visible and the client retries on a hard-coded 5 s timer with no backoff, so without the log line a repeatedly-overflowing consumer was a silent reconnect+replay loop whose only trace was a browser-console warning. That nil guard is defensive against a state production cannot reach — `app.New` rejects a nil logger — and it is no longer dead inside the suite: `ttyWSTestServer` injects a real logger into the `Server` it mounts, so the overflow test asserts the line fired with the instance id in it (until then every TTY WS test ran with `s.logger == nil`, and deleting the line left `internal/app` green, which made this sentence and `docs/API.md`'s "the server logs the overflow" unverified). That frame is best-effort by design and resync never depends on it arriving: the browser reconnects on any socket close, reasoned or not. A write deadline covering healthy connections (which would make the whole thing gentler) remains unimplemented — issue #83 shipped without one, deliberately scoping itself to read-deadline liveness. That close gave the channel a second closer, so `subs` now holds a `*subscriber` (its channel plus a `closed` flag) whose `closeLocked` does the check-and-close under `subsMu`; moving `close(ch)` inside the lock would **not** have been enough, because `cancel`'s `delete` is a no-op on a channel the overflow already removed and it would still double-close — `panic: close of closed channel`, in `pumpLogs`' goroutine, taking the daemon down. `SubscribeOutput`'s public signature is unchanged (the framework `outputSub` assertion and the dsh-web bridge keep working), and an id key now leaves `subs` as soon as its last subscriber overflows, preserving the "key exists iff it has a live subscriber" invariant `cancel` already kept — now pinned at the registry level, not just per id, by a `registeredInstanceCount` delta assertion in `TestBroadcast_OverflowDisconnectsSubscriber` (the per-id `liveSubscribers` check answers 0 for a stale empty set exactly as happily as for a removed key, so before that assertion the `delete(subs, id)` in the overflow arm — and this sentence's claim about it — was unpinned: removing the delete left the whole pty package green). Pinned by `TestBroadcast_OverflowDisconnectsSubscriber`, `TestBroadcast_WithinCapacitySubscriberIsNeverDisconnected`, `TestSubscribeOutput_CancelAfterOverflowDoesNotPanic` and `TestSubscribeOutput_CancelRacesOverflow` in `internal/instance/pty/driver_test.go` (clean under `-race -count=10`), and end to end by `TestHandleInstanceTTYWS_SubscriberOverflowClosesConnectionWith1013` in `internal/app/tty_ws_test.go`, which drives the same condition over a real hijacked WebSocket — the client goes quiet after the handshake while the producer outruns the subscriber's queue — and pins everything downstream of the close: the 1013 reason on the wire, nothing served afterwards, and the registry drained. That test's kind is a double of the driver, so the double now mirrors the drain too — delete, close, then drain after its own lock, with `closeLocked` reporting the close — and the test counts the live binary frames before the close against the exact arithmetic (`published - 16 - 1` reached the handler, so exactly that many may reach the wire): 16 more and the backlog was flushed onto the stalled socket, which is the behaviour the drain exists to remove. Without the mirror the e2e test would still have been pinning the *pre*-drain shape, and reverting the production drain would have left `internal/app` green. Stated exactly, as the test's own comment now says: it does **not** reproduce the TCP stall (a publish is a mutex plus a non-blocking send against the handler's 32 KB `write()` syscall, so the flood outruns the drain by orders of magnitude and the socket buffers never become the binding constraint), and its deferred-cancel check pins the **deregistration** only, because net/http recovers handler panics into stderr — the exactly-once close is pinned in the pty package, where a panic does fail the test. Deliberately **not** done here: a write deadline on the TTY socket, which would turn TCP backpressure into a bounded stall but changes behaviour for every connection including healthy ones, remains unimplemented — issue #83 (since merged) scoped itself to read-deadline liveness and deliberately left the write deadline out. Contract documented in `docs/ARCHITECTURE.md` §4.1, and in `docs/API.md` (TTY WS: the `1013 subscriber overflow: slow consumer` close code in the server→client outcomes and the handshake protocol's step 8). - **feat(dsh-web): adapt to the dsh 0.2.x RPC wire + `--no-open` (issue #84)** — the embedded integration now targets **dsh 0.2.x only**; 0.1.x support is dropped rather than dual-branched — a user still on dsh 0.1.x now hits a Spawn that fails at the hard version gate with a readable error, and the frontend says 0.1.x is unsupported instead of offering the "use it from the local 127.0.0.1 environment" fallback (dropped from the version-gated branches as dead advice, yet deliberately kept in the version-unknown `cap._error` branch, as the follow-up fix bullet below details). Two breaking upstream changes are implemented, both verified by hand against the installed `dsh 0.2.0-rc.2`: (1) **endpoint segments became `/`** — `POST /api/workspace/create`, `/api/session/create`, `/api/subagents/prompt` (the subagent namespace is **plural**), with the envelope's `method` still required to equal the URL-path endpoint (bare `/api` still 404s); the 0.1.x dotted names (`workspace.create`) 404 on 0.2. (2) **every verb's single object argument is nested at `payload.args.request`** — `{type:'client-request', rpcId, method:'workspace/create', payload:{args:{request:{path:…}}}}`; the envelope and the `{ok, value}` answer shape are unchanged, and the `value` shapes (`{workspace:{workspaceId,…}, created}` / `{sessionId, agentPreset?}`) are unchanged. Code: `bootstrap.go` gained a `requestPayload` wrapper and the two bootstrap calls use the slash endpoints; `proxy.go` matches `/api/session/create` for the session-id tee, `ownSessionWriteMethods` / `ownSessionIDsInBody` / `classifyRPCBody` switched to the slash names with a shared `decodeArgsRequest` unwrapping `payload.args.request` (no legacy fallback — the real gateway rejects a flat payload with "Remote payload must contain exactly one plain-object args field"). Gates: `minVersion 0.2.0`, `supportedMaxVersion 0.3.0` (exclusive), `minRemoteVersion 0.2.0`, `NpxPin 0.2.0-rc.2` (npm publishes it as both `latest` and `next`, and it is the exact build every finding was verified against); `probeAndGate`'s error now names the slash endpoints and `--no-open`. Frontend: the two hard-coded `0.1.5` remote-floor fallbacks and the "(0.1.x)" warning text in `kinds/dsh_web.js` (plus `index.html`) became `0.2.0` / `(0.2.x)`. **Issue #84**: the launcher appends **`--no-open`** — dsh 0.2.x opens the host's default browser on every boot, and since the SPA is embedded in an iframe each Start spawned a stray browser tab; the ready line still prints (`printUrl` stays true), so the ready-line parse is unaffected. It is appended **after `--port 0`**, grouped with the web-app options — launcher flags (`--patch`) must stay first (`allowUnknownOption` + `passThroughOptions`; `TestWebArgs` pins `--no-open` after both `--patch` and `--host`). Verified unchanged and deliberately untouched: the restrict overlay rows (`storage-json` / `directory-picker` / `client-hmr` + `insert: directory-picker-browse` all compose on 0.2.0 per `--dump-config`, and a live instance's `pluginInventory/list` shows no enabled non-active entry with no `dsh-client-ui-directory-picker-*` surface loaded), the auth relay (`GET /?token=` → 303 + `dsh-auth-*` cookie), `/api/remote.mux`, and the injected `static/dsh-bridge.js`. Test mocks (`testdata/dsh-mock.go`, `testdata/dsh-mock-modern.go`) now report `0.2.0-rc.2`, answer the slash endpoints, and reject a payload without `args.request`. Docs: ARCHITECTURE §9 / PRD §7 / API §5.14–5.15 / README (+zh-CN) reworded, and `docs/plans/dsh-native-ui/PLAN.md` gained a marked dsh-0.2.x amendment block (the 0.1.x history is kept as history). - **fix(dsh-web): the review-round fixes on top of the 0.2.x adaptation** — four follow-ups forced by two independent code reviews of that same change (the adaptation itself is documented above and not repeated here). **fix(dsh-web): the advisory version gate could not classify a prerelease dsh** — `isSupportedVersion` / `hardVersionOK` compared the RAW `dsh --version` string, so a `-rc.N` suffix made `splitVersion`'s `Atoi` fail and `versionLess` answer `false` for BOTH bounds. The npx launch mode is the only caller that passes the raw pin (`driver.go` assigns `version = NpxPin`), so every npx-launched instance reported `version_supported:false` and the frontend painted the permanent red "dsh version is too new or the restrict overlay is not effective" bar even though `overlay_verified` was true. Both gates now core-parse through `parseVersion` exactly like `isRemoteCapable`, so `0.2.0-rc.2` classifies as core `0.2.0` while `0.1.9-rc.1` is still refused by the hard gate. Pinned by the prerelease cases in `version_test.go`, `TestVersionGatesAgreeOnPrereleaseCore`, and `TestSpawnNpxModeReportsVersionSupported` (a real npx-mode `Spawn` asserting `VersionSupported == true`). **fix(dsh-web): the mock toolchain accepted any argv, so a rejected flag stayed invisible** — both `testdata/dsh-mock*.go` doubles ignored unknown options, so a dsh build that ever dropped `--no-open` (or one where `--patch` moved behind an app-level option and `allowUnknownOption` + `passThroughOptions` forwarded it to the web app, killing the boot) would leave every test green while every real Start failed. Both mocks now validate the recognised web flag set (`--patch`, `--host`, `--port`, `--no-open`, `--trusted-host`, plus the launcher-level `--dump-config`) and fail like commander does (`unknown option ''` on stderr, exit 1); `TestMockDshWebFlagContract` pins both mocks across positive (`webArgs` boots, `dumpArgs` dumps, `--trusted-host`'s value token is skipped) and negative (`--no-browser`, `--allow-origin`) cases. **test(dsh-web): all eight write-driving RPC verbs are now pinned, not three** — `ownSessionWriteMethods` carries the dsh 0.2 slash endpoints (`session/prompt`, `session/cancel`, `session/fork`, `session/rename`, `session/selectModel`, `session/attachment`, `session/updateQueue`, plus the plural-namespaced `subagents/prompt`), but only three were exercised, so a typo in any of the other five silently disabled own-session attribution for that verb — a session this daemon actually drives would be reported as foreign activity — with every test green. `TestProxySessionOwnMarking` is now table-driven over all eight with distinct ids, an exact-set comparison, and a bidirectional coverage guard that fails if the map and the table drift apart. **frontend/docs corrections carried out in the same review round** — the "use it from the local 127.0.0.1 environment" fallback was dropped from the two VERSION-GATED dsh capability warnings (`cap.missing` / `!cap.remote_capable` — dead advice once `minRemoteVersion == minVersion == 0.2.0`, since `hardVersionOK` refuses a sub-0.2.0 dsh in EVERY mode, loopback included) and is deliberately kept in the `cap._error` branch, where the capability fetch itself failed so the installed dsh version is UNKNOWN and a local Start may well succeed; the stale `/api/events.mux` / `/api/events.host` carrier paths became the real `/api/remote.mux` in `docs/ARCHITECTURE.md` §9, `docs/PRD.md` §7 and `docs/plans/dsh-native-ui/FEASIBILITY.md`; the false "dsh has no authentication surface" claim was corrected wherever it survived (dsh 0.2.x DOES carry browser-session auth — launch token → `dsh-auth-*` cookie — and the per-instance proxy is its sole credential holder); `docs/API.md` §5.14 now shows the reachable `version` (`0.2.0`, the parsed core) with `missing_dsh` split into its own failed-instance example; and `docs/plans/dsh-native-ui/pr5-remote-e2e.sh` now drives the real `/api/remote.mux` handshake and FAILS on a non-101 instead of recording it. -- **fix(terminal): a new instance could show `[Process Stopped]` over a live PTY (issue #80)** — a just-created terminal sometimes rendered the red `[Process Stopped]` banner, greyed-out styling, and no visible input or output, while blind typing still reached the process and produced correct results; switching tabs or pressing Refresh "fixed" it. Root cause: the frontend collapsed the server's **seven-state** status enum into a `status === 'running'` boolean. `Manager.Start` persists a new record as `starting` and flips it to `running` on the kind's ready signal in a separate goroutine (a store version conflict costs 3 × 10 ms of retries), so an instance created moments earlier is routinely observed as `starting` at selection time — and the activation path treated that transient state as terminal: it disconnected the transport, replayed the log with the banner, and never reconnected. Worse, nothing could recover from that: `selectInstance()` returns early when the tab is already selected, and the 2 s poll's `renderTerminalSessions()` only toggles visibility, so `activate()` runs exactly once per selection change. The instance then stayed transport-less forever: input silently degraded to the buffered HTTP fallback (which is why typing "worked") while output, published only to WS/SSE subscribers, never came back. Fix: both activation paths (`kinds/pty.js` and index.html's legacy no-renderer fallback) now classify the status into three buckets — **live** (`running`, `unhealthy`: the process is alive and takes I/O; `unhealthy` means a probe failed, not that the process died), **pending** (`starting`, `stopping`: transient, nothing concluded yet) and **terminal** (`stopped`, `failed`, `exited`) — and only the terminal bucket paints the banner, greys out the terminal, refuses input, or blocks the connect. Re-activation moved from selection-driven to poll-driven: `reconcileTerminalSessions()` runs on every `refresh()` with fresh state and **promotes** the active session as soon as its status turns live (`connectTTY` only, guarded against a CONNECTING socket so the poll cannot flap it; no `loadLog`, since the WS handshake replays the screen state itself — the tail on a first connect, and only the bytes after the client's `since` cursor once it holds one (issue #87, shipped with this branch)), and replays the log once on entry into the terminal bucket, tracked per session via `lastKnownStatus`, so a crashed or failed instance still gets its banner. An `unhealthy` instance also keeps its transport and its reconnect-on-close retry instead of being reported as stopped (note: `unhealthy` and `stopping` are reachable only through the enum — `runLifecycle` persists only `running` and the terminal statuses, and a kind's health-probe result is discarded, so those two branches cannot be exercised against a live backend today; tracked separately). The status-bar text for the renderer-owned web kinds (`reasonix`, `dsh-web`) follows the buckets too, so a freshly created instance reports `starting...` where it used to read `stopped` for the whole `starting` window — next to an iframe that `kinds/reasonix.js` keeps navigating. Both activation paths also refuse an id `state.instances` cannot resolve (the server accepts that socket and closes it with 1013, which surfaced as a dropped connection), `activate()` shares the promotion's in-flight guard so a tab switch mid-bring-up no longer opens a second socket, and a failed terminal replay is retried with an exponential backoff instead of once per 2s tick forever (a re-select bypasses it), and an explicit tab re-selection now takes over a queued reconnect instead of freezing for the rest of the 5s window — the poll-driven promotion still waits, because taking over from it every tick would flap the connection. The takeover cancels the queued retry itself rather than leaving it to `disconnectTTY`, which activation only reaches after its `loadLog` settles: a retry firing in that window opened a socket and replayed the tail, and the connect that followed opened a second and replayed it again. Pinned by 67 `node --test` cases (`testdata/terminal_status.test.mjs`, which slices the shipped `index.html` / `kinds/pty.js` and runs them against stubbed page state, wired into `go test` via `TestTerminalStatusHandling`, whose count `TestTerminalStatusChangelogCount` re-derives from the file rather than trusting this sentence) plus the source-contract `TestTerminalStatusBucketsReplaceRunningEquality`. Contract documented in `docs/ARCHITECTURE.md` §5.3 / §5.4 / §5.6 / §5.8 and `docs/API.md` — the Start endpoint's `201` body is `status: "starting"`, not `"running"`, and the doc said otherwise, which is what invited the bug. +- **fix(terminal): a new instance could show `[Process Stopped]` over a live PTY (issue #80)** — a just-created terminal sometimes rendered the red `[Process Stopped]` banner, greyed-out styling, and no visible input or output, while blind typing still reached the process and produced correct results; switching tabs or pressing Refresh "fixed" it. Root cause: the frontend collapsed the server's **seven-state** status enum into a `status === 'running'` boolean. `Manager.Start` persists a new record as `starting` and flips it to `running` on the kind's ready signal in a separate goroutine (a store version conflict costs 3 × 10 ms of retries), so an instance created moments earlier is routinely observed as `starting` at selection time — and the activation path treated that transient state as terminal: it disconnected the transport, replayed the log with the banner, and never reconnected. Worse, nothing could recover from that: `selectInstance()` returns early when the tab is already selected, and the 2 s poll's `renderTerminalSessions()` only toggles visibility, so `activate()` runs exactly once per selection change. The instance then stayed transport-less forever: input silently degraded to the buffered HTTP fallback (which is why typing "worked") while output, published only to WS/SSE subscribers, never came back. Fix: both activation paths (`kinds/pty.js` and index.html's legacy no-renderer fallback) now classify the status into three buckets — **live** (`running`, `unhealthy`: the process is alive and takes I/O; `unhealthy` means a probe failed, not that the process died), **pending** (`starting`, `stopping`: transient, nothing concluded yet) and **terminal** (`stopped`, `failed`, `exited`) — and only the terminal bucket paints the banner, greys out the terminal, refuses input, or blocks the connect. Re-activation moved from selection-driven to poll-driven: `reconcileTerminalSessions()` runs on every `refresh()` with fresh state and **promotes** the active session as soon as its status turns live (`connectTTY` only, guarded against a CONNECTING socket so the poll cannot flap it; no `loadLog`, since the WS handshake replays the screen state itself — the tail on a first connect, and only the bytes after the client's `since` cursor once it holds one (issue #87, shipped with this branch)), and replays the log once on entry into the terminal bucket, tracked per session via `lastKnownStatus`, so a crashed or failed instance still gets its banner. An `unhealthy` instance also keeps its transport and its reconnect-on-close retry instead of being reported as stopped (note: `unhealthy` and `stopping` are reachable only through the enum — `runLifecycle` persists only `running` and the terminal statuses, and a kind's health-probe result is discarded, so those two branches cannot be exercised against a live backend today; tracked separately). The status-bar text for the renderer-owned web kinds (`reasonix`, `dsh-web`) follows the buckets too, so a freshly created instance reports `starting...` where it used to read `stopped` for the whole `starting` window — next to an iframe that `kinds/reasonix.js` keeps navigating. Both activation paths also refuse an id `state.instances` cannot resolve (the server accepts that socket and closes it with 1013, which surfaced as a dropped connection), `activate()` shares the promotion's in-flight guard so a tab switch mid-bring-up no longer opens a second socket, and a failed terminal replay is retried with an exponential backoff instead of once per 2s tick forever (a re-select bypasses it), and an explicit tab re-selection now takes over a queued reconnect instead of freezing for the rest of the 5s window — the poll-driven promotion still waits, because taking over from it every tick would flap the connection. The takeover cancels the queued retry itself rather than leaving it to `disconnectTTY`, which activation only reaches after its `loadLog` settles: a retry firing in that window opened a socket and replayed the tail, and the connect that followed opened a second and replayed it again. Pinned by 68 `node --test` cases (`testdata/terminal_status.test.mjs`, which slices the shipped `index.html` / `kinds/pty.js` and runs them against stubbed page state, wired into `go test` via `TestTerminalStatusHandling`, whose count `TestTerminalStatusChangelogCount` re-derives from the file rather than trusting this sentence) plus the source-contract `TestTerminalStatusBucketsReplaceRunningEquality`. Contract documented in `docs/ARCHITECTURE.md` §5.3 / §5.4 / §5.6 / §5.8 and `docs/API.md` — the Start endpoint's `201` body is `status: "starting"`, not `"running"`, and the doc said otherwise, which is what invited the bug. - **fix(terminal): log replay served the oldest buffered bytes instead of the newest (issue #81)** — re-selecting an idle terminal instance replayed the *oldest* 64 KB of the ring buffer, so the UI showed stale session-start output; only a window resize (SIGWINCH → TUI repaint) recovered the current screen. The kind refactor had rewired `Manager.Tail` to `ReadLogs(handle, 0, n)` — `ReadSince(0, n)` clamps to the oldest live byte — orphaning the correct `RingBuffer.Tail`. `Kind.ReadLogs` now defines a negative `since` as tail semantics (newest bytes, cursor at the end offset); the pty driver serves it from `RingBuffer.Tail`, `Manager.Tail` requests it and clamps non-conforming cursors, the log endpoints route an omitted/negative `since` to the tail, `GET /api/instances/log` returns the end offset in `X-Log-Offset`, and the browser cursor became a real byte offset. Follow-ups: #82 (broadcast drop / resync), #83 (weak WS liveness check), #86 (follow-from-live-end streaming), #87 (WS handshake replay duplication). ## v0.5.0 (2026-09-11) diff --git a/docs/API.md b/docs/API.md index b00d645..a0bbbce 100644 --- a/docs/API.md +++ b/docs/API.md @@ -528,8 +528,10 @@ Bi-directional stream for terminal output/input with PTY support. OLDEST live byte (the #81 symptom); on this WS endpoint the catch-up loop reads forward from 0 and streams every chunk it reads straight to the socket, so the effect there is a replay of everything still live in - the ring — up to the 8 MB handshake budget — with `sync` at the end of - the last chunk actually written. The shipped UI never sends `0`; + the ring — up to the 8 MB handshake budget — with the closing `sync` at + the end of the last chunk actually written (the replay is bracketed by + the start/closing sync pair described below). The shipped UI never sends + `0`; it omits `since` entirely when its cursor is unknown (`CURSOR_UNKNOWN`) and sends `-2` when the screen is painted but the offset is not. - **`since>0`** → the server replays the bytes at or after that offset, in @@ -575,7 +577,8 @@ Each replay read is capped at 64KB. When the delta since `since` exceeds 64KB, the server loops reads until it is caught up and **streams each chunk to the socket as it reads it** instead of retaining one, so the whole delta is delivered and peak memory stays ~64KB (one chunk) regardless of the ring -cap. The `sync` offset is the end of the **last chunk actually written** to +cap. The closing `sync`'s offset is the end of the **last chunk actually +written** to this socket — never an offset over bytes the client did not receive **and can still receive**, because a client cursor only moves forward and would never ask for them again. The two deliberate exceptions both publish the ring's real @@ -606,6 +609,42 @@ still in the ring buffer is silently clamped to the oldest live byte; a tail (beyond head, defensive) — the client's own number is never echoed back as authoritative. +On that streaming catch-up path the replay is bracketed by TWO sync frames +(issue #94). Ahead of the FIRST binary chunk the server sends +`{"type":"sync","offset":,"start":true}`, where S is the replay's real +start offset: the first chunk's end offset minus its length, so S and the +first chunk come from the same read — the requested `since` when no clamp +happened, the clamped oldest live byte when it did, never the raw request +value. The closing sync after the last chunk is unchanged. A client that +adopts S on arrival may advance its cursor by every replay frame's wire +length from chunk 1 on, so a connection that dies mid-replay resumes from +the bytes it already rendered instead of re-pulling the whole (up to 8 MB) +replay from its old cursor — under sustained network degradation the pre-#94 +behaviour does not converge. The frame REUSES the `"sync"` type rather than +inventing a new one: every client that receives it already provably parses +`"sync"` (the caps token, or the inferred explicit-`since` opt-in above — +any catch-up request presents `since` by definition), its existing handler — +adopt the absolute offset, lift the per-connection latch — is exactly the +wanted semantics, and a new frame type would have to fight the stale-page +leak class (issue #98) with a new capability gate. The `"start":true` field +exists for consumers that must know WHEN the replay ends (test harnesses, +third-party clients); the shipped UI deliberately ignores it and treats both +sync frames alike. S + Σ(replay frame bytes) equals the closing offset +UNLESS the ring rolls a full cycle mid-replay and evicts past the loop's +cursor between two reads — a later chunk then clamps to the new oldest live +byte, the sum lands short, and the closing sync jumps the gap (truthfully: +those bytes are undeliverable anyway, the same rule the read→subscribe +window already lives by). The beyond-head tail degrade (`since` past head — +defensive, unreachable through normal flows) announces its tail's start the +same way, S = head − len(tail): the client presented a cursor, so EVERY +replay addressed to one carries a start frame. The closing sync arrives +ALONE only where there is no cursor to advance: the cursor-less +first-connect tail replay (a single ≤64KB frame whose death window is the +pre-#87 one — and a cursor-less client may not parse `sync` at all, which is +why THAT path's `syncCap` is a genuine gate, not an inference), the +`since=-2` handshake (zero binary frames by design) and the caught-up-at-head +exit (nothing to announce). + In `since=-2` mode the published offset comes from `Manager.EndOffset` — the head read with a zero-length body, because there is no replay to attach it to. Two windows surround that read and they behave differently, so do not @@ -632,16 +671,30 @@ conflate them: 1. Server sends `{"type":"ready"}` immediately after connection 2. Client should wait for this message before sending resize 3. Client sends `{"type":"resize","cols":80,"rows":24}` to start data flow -4. Server sends initial log (the tail, or only the bytes after `since`) as binary frames — and in `since=-2` mode sends NO binary frame at all -5. Server sends `{"type":"sync","offset":}` (text frame, **only to +4. Server sends `{"type":"sync","offset":,"start":true}` (text frame, + issue #94) announcing the replay's real start offset S — the requested + `since`, or the clamped oldest live byte when `since` predated it — + ahead of the FIRST binary frame. Sent on every replay addressed to a + cursor-bearing client (the streamed catch-up AND the beyond-head tail + degrade); skipped only by the cursor-less first-connect tail and by + empty replays. Same opt-in as the closing sync in step 6 — every client + that receives it provably parses `"sync"`, and a client that ignores + the `start` marker is merely back to the pre-#94 behaviour of waiting + for the closing sync +5. Server sends the initial log (the tail, or only the bytes after + `since`) as binary frames — and in `since=-2` mode sends NO binary + frame at all +6. Server sends `{"type":"sync","offset":}` (text frame, **only to clients that opted in via `caps=sync` or presented an explicit `since`** - — issue #98) — the end + — issue #98) — the CLOSING sync: the end offset of that replay (in `since=-2` mode, the live head it refused to replay); the client stores it and sends it back as `since` - on its next reconnect -6. Real-time output continues as binary frames -7. Client receives first data and triggers second resize (50ms delay) for TUI redraw -8. If the client stops draining the live stream, the server closes the + on its next reconnect. On the streaming catch-up path this is the + second sync frame — the start announcement of step 4 preceded the + replay; everywhere else it is the only one +7. Real-time output continues as binary frames +8. Client receives first data and triggers second resize (50ms delay) for TUI redraw +9. If the client stops draining the live stream, the server closes the connection with **`1013` / reason `subscriber overflow: slow consumer`** (issue #82) instead of silently dropping output — see Close codes below; the client reconnects with the `since` cursor it already holds, exactly @@ -662,7 +715,7 @@ conflate them: *Server → Client:* - Ready: `{"type":"ready"}` (text frame) - Output: binary frames (terminal output chunks) -- Sync: `{"type":"sync","offset":}` (text frame, **opt-in via `caps=sync` — or inferred from an explicit `since` parameter — since issue #98**, sent after the handshake replay — even when that replay was empty. The client latches it per connection: replay frames received BEFORE the sync never touch the cursor — the sync publishes the authoritative end of the whole replay in one step; binary frames received AFTER it advance the cursor by their wire byte count — every one of them is ring-buffer output, as the server closes the connection rather than writing diagnostics as binary) +- Sync: `{"type":"sync","offset":}` (text frame, **opt-in via `caps=sync` — or inferred from an explicit `since` parameter — since issue #98**), the cursor handoff of the handshake, sent even when the replay was empty. Since issue #94 a replay addressed to a cursor-bearing client is bracketed by TWO of these: `{"type":"sync","offset":,"start":true}` ahead of the FIRST replay chunk carries the replay's real (possibly clamped) start offset, and the closing sync after the last chunk carries its end offset — every other path (the cursor-less tail, `since=-2`, an empty replay) sends the closing sync alone. The client latches per connection: a replay frame advances the cursor by its wire byte count only once SOME sync has arrived — against a post-#94 server that is the start sync, so frames count from chunk 1 on (S and chunk 1 come from the same server-side read, so the base is truthful); against a pre-#94 server the latch lifts only at the closing sync, which publishes the end of the whole replay in one step. Either way the closing sync is the absolute authority — whenever the summed frames and it drift (a ring rollover mid-replay can evict unsent bytes, landing the sum short), it wins. Counting binary frames is sound at all because every one of them is ring-buffer output: the server closes the connection rather than writing diagnostics as binary. The `"start":true` marker exists for consumers that must know WHEN the replay ends; a client that ignores it and treats both syncs as absolute offsets to adopt implements the full contract) - Heartbeat: `{"type":"ping"}` (text frame, every 10 s, issue #83; **opt-in via `caps=ping` since issue #98**) — the application-level mirror of the RFC 6455 ping the server sends on the same tick. Browsers answer the protocol ping automatically from their network stack (which refreshes the server's 45 s read deadline); this text frame is the heartbeat browser JavaScript can observe, since `onmessage` never fires for control frames. Clients MUST whitelist `ping` (and `pong`) as control types and MUST NOT render them as terminal output — and the server only sends this frame to clients that declared `ping` in the handshake `caps` list, so a client without the whitelist never receives it. **Liveness (issue #83):** half-open TCP sockets (laptop sleep, NAT/proxy idle timeout) keep `readyState === OPEN` without ever firing `onclose`, so liveness rides on heartbeat traffic in both directions. Server: pings every 10 s; arms a 45 s read deadline before every read and reaps a peer that has sent nothing for that long (the browser's automatic Pong refreshes it). **Non-browser clients get no automatic Pong** — `internal/ws`'s own client returns `opPing` as an ordinary message and installs no responder — so any Go or embedded client of this endpoint MUST answer protocol pings with Pong and/or send `{"type":"ping"}` periodically, or it will be reaped at the 45 s deadline. Client: stamps the arrival of every frame, probes `{"type":"ping"}` every 5 s once READY, and reconnects after 30 s without heartbeat traffic (3 × the ping interval, under the server's 45 s backstop). The web UI's **primary detector is the 2 s poll** — `ensureTerminalLiveTransport()` notices the stale stamp on the active session at ~30 s and queues the reconnect (returning `false`: a queued reconnect is not yet a live transport); the 5 s watchdog is the fallback that also covers sessions the poll does not promote. Liveness is never inferred from the absence of program output — a prompt, `vim` or `top` emit zero bytes for hours and stay connected. A write deadline was deliberately **not** part of issue #83: normal-traffic writes on this socket carry none (the only bounded write here is the 5 s deadline on #82's overflow close-frame), so a handler blocked mid-write is reclaimed when that write fails or returns, not by the read deadline. @@ -715,10 +768,17 @@ Client Server |-- {"type":"resize", --->| Notify terminal size | "cols":80,"rows":24} | | | + |<-- {"type":"sync", ----| Start offset of the replay (issue #94): + | "offset":1024, | only on a replay addressed to a cursor- + | "start":true} | bearing client (streamed catch-up and the + | | beyond-head tail degrade); S is the + | | requested `since` or the clamped oldest + | | live byte, never the raw request |<-- binary output -------| Replay: tail, or bytes after `since` - |<-- {"type":"sync", ----| End offset of that replay — only when the - | "offset":4096} | client opted in via ?caps=sync or presented - | | ?since (issue #98) + |<-- {"type":"sync", ----| CLOSING sync: end offset of that replay — + | "offset":4096} | only when the client opted in via + | | ?caps=sync or presented ?since + | | (issue #98) | | |--- (50ms delay) -------| | | diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1826fc0..56a05dc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -113,7 +113,7 @@ Each running instance owns a bounded, **in-memory** ring buffer (`internal/frame **Cursor semantics (`since` / `next`).** `head` is a monotonic total-bytes-written counter for the instance. Clients pass it as `since` to read incrementally; the server returns the bytes plus an advanced cursor. When no new data is available, the cursor is returned unchanged (preserves the SSE 1 s poll-loop contract). When `since` points to data already evicted from the ring (oldest live byte > since), the read silently clamps to the oldest live byte. A negative or omitted `since` requests **tail semantics**: the newest `maxBytes` are returned with the cursor set to the current end offset (the bare `GET /api/instances/log` response and the MCP `instance_log_tail` tool drive this path; kinds without log capture return an empty body and the manager clamps the cursor to `0`). **`since=-2` is the one negative value that is NOT tail semantics** (issue #86): the *follow-from-the-live-end* sentinel, for a client whose screen already holds the tail but whose `X-Log-Offset` was stripped by a proxy. Both stream endpoints branch on it explicitly, ahead of their `since < 0` tail branch — `parseInt64Default` passes any negative value through untouched, so an unbranched `-2` would be indistinguishable from the omitted `since` that caused the duplication. It means **send nothing, publish the current end offset, then stream only new data**: the SSE stream emits one empty `log` frame carrying the head, the WS handshake emits no binary frame at all and puts the head in `sync`. The head comes from `Manager.EndOffset` — `Tail(id, 0)`, the same lookup, kind delegation and cursor clamps, but a zero-length read, because copying 64 KB to throw it away on every stripped-header connect is pure waste (and it reports `0` for a non-running instance exactly as `Tail` returns `("", 0, nil)`). The browser names the two unknown states: `CURSOR_UNKNOWN` (-1: nothing painted, so `since` is omitted and the tail arrives) and `CURSOR_FOLLOW_LIVE_END` (-2: painted but unoffset, so no replay) — and the -2 claim is gated on content actually reaching the screen, since `writeSanitizedTerminalOutput` returns the number of characters it wrote (`.length`, not bytes) and sanitizing can empty a non-empty tail, so an empty paint keeps -1 and the replay stays available instead of hiding output. `TestFollowLiveEndSentinelAgreesAcrossTheWire` pins the `-2` against `sinceFollowLiveEnd` because no compiler sits between `internal/app/app.go` and the page. `kinds/pty.js` owns its own **prefixed** `PTY_CURSOR_UNKNOWN` and deliberately does not declare the other: every script in the page is a classic script sharing ONE global lexical environment, so a second top-level `const CURSOR_UNKNOWN` would throw `SyntaxError` in the second script and take the entire inline application down with it. That hazard is guarded, not remembered — `testdata/terminal_status.test.mjs` collects the top-level declarations of every non-vendor classic script plus the inline block and fails on any name two of them declare. The sentinel also has a scope: the non-streaming `GET /api/instances/log` answers any negative `since` with the tail, which is the sentinel's opposite, so that endpoint returns **400** for `-2` rather than silently inverting it. What `-2` costs is a window, and it is the window BEFORE the head read, not after it: the client painted the tail up to some head `H1`, the sentinel publishes `H2 >= H1`, the client adopts `H2`, so `[H1, H2)` is in no replay, no painted body and no live frame while the cursor already sits past it — permanently unrecoverable for that client, milliseconds normally and seconds if the WS handshake times out first. -The WS handshake (`completeHandshake`, issue #87) drives **tail only for a cursor-less first connect**; a reconnect with `since >= 0` loops `Manager.ReadSince` until caught up, **streaming every 64KB chunk straight to the socket as it reads it** instead of retaining one, so the whole delta is delivered and peak memory stays ~64KB (one chunk) regardless of the ring cap, and the `sync` offset it publishes equals the end of the **last chunk actually written** to that socket — never an offset over bytes the client never received **and could still receive**, because a client cursor only moves forward and would never ask for them again; on a caught-up exit that offset is the final read's own `next` (the ring's head), which is the one deliberate exception and is truthful precisely because an exhausted or closed ring can no longer deliver what sits behind it — the alternative, publishing a zero, would force a full tail replay of bytes the client already has. One handshake replays at most 8 MB (`ttyHandshakeReplayBudget`), counted in bytes already written and checked before the next read; once it is spent the loop stops, leaving the undelivered remainder **ahead** of the published cursor so the next reconnect re-requests exactly those bytes — deferred, never skipped. Bytes produced between the last chunk written and the live subscription sit behind the published cursor and are re-requested on the next reconnect (self-healing contiguity, not absolute coverage). A zero-progress first read consults `Tail` for the real head: cursor exactly at head is the normal caught-up exit (empty replay, sync == cursor == head), while a cursor past head — unreachable through normal flows since Restart mints a new id — degrades to the first-connect tail rather than echoing a bogus cursor. +The WS handshake (`completeHandshake`, issue #87) drives **tail only for a cursor-less first connect**; a reconnect with `since >= 0` loops `Manager.ReadSince` until caught up, **streaming every 64KB chunk straight to the socket as it reads it** instead of retaining one, so the whole delta is delivered and peak memory stays ~64KB (one chunk) regardless of the ring cap, and the `sync` offset it publishes equals the end of the **last chunk actually written** to that socket — never an offset over bytes the client never received **and could still receive**, because a client cursor only moves forward and would never ask for them again; on a caught-up exit that offset is the final read's own `next` (the ring's head), which is the one deliberate exception and is truthful precisely because an exhausted or closed ring can no longer deliver what sits behind it — the alternative, publishing a zero, would force a full tail replay of bytes the client already has. The streamed replay is bracketed by TWO sync frames (issue #94): `{"type":"sync","offset":S,"start":true}` ahead of the FIRST chunk — S = first chunk's `next` − length, the clamped oldest-live value whenever the request's `since` predated it, never the raw request — lifts the client's per-connection cursor latch at chunk 1 so a mid-replay drop resumes from the bytes already painted instead of re-pulling the whole replay (the death window #87 widened from one ≤64KB frame to up to 8MB), and the closing sync after the last chunk re-pins the end absolutely — the beyond-head tail degrade (`since` past head) announces its tail's start (head − len) the same way, so EVERY replay addressed to a cursor-bearing client carries the frame; only the cursor-less first-connect tail goes without; reusing the `"sync"` type means every recipient provably parses it (the caps token or the inferred `since` opt-in, issue #98) and needs no client change, while `"start":true` lets consumers that must know when the replay ends tell the two apart. One handshake replays at most 8 MB (`ttyHandshakeReplayBudget`), counted in bytes already written and checked before the next read; once it is spent the loop stops, leaving the undelivered remainder **ahead** of the published cursor so the next reconnect re-requests exactly those bytes — deferred, never skipped. Bytes produced between the last chunk written and the live subscription sit behind the published cursor and are re-requested on the next reconnect (self-healing contiguity, not absolute coverage). A zero-progress first read consults `Tail` for the real head: cursor exactly at head is the normal caught-up exit (empty replay, sync == cursor == head), while a cursor past head — unreachable through normal flows since Restart mints a new id — degrades to the first-connect tail rather than echoing a bogus cursor. **Lifecycle.** - `Start` resolves the cap with budget enforcement, creates the buffer, adds it to `Manager.buffers[id]` (an `*atomic.Pointer[RingBuffer]`), and atomically increments `Manager.totalBufBytes`. diff --git a/internal/app/app.go b/internal/app/app.go index fe7a5e0..9544670 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -2525,6 +2525,21 @@ func (s *Server) handleInstanceTTYWSLiveness(w http.ResponseWriter, r *http.Requ // cursor parsed from the query string) rather than taking a parameter; // both call sites below pass that same variable. // + // On the streaming catch-up path the replay is bracketed by TWO sync + // frames (issue #94): one carrying the replay's real start offset S + // ahead of the FIRST binary chunk — S = first chunk's next - len, the + // clamped value whenever ReadSince clamped a stale cursor, never the + // raw request — and the closing one carrying the end offset of the last + // chunk written. The start frame lifts the client's handshakeSynced + // latch at chunk 1, so a mid-replay drop resumes from the bytes already + // painted instead of re-pulling the whole replay; the closing frame + // keeps the #87 invariant below and corrects any drift absolutely. The + // beyond-head tail degrade (cursor > head) announces its tail's start + // the same way — the client presented a cursor, so every replay chunk + // addressed to one is preceded by a start frame; only the CURSOR-LESS + // first-connect tail goes without (its single ≤64KB frame has the + // pre-#87 death window, and such a client may not parse sync at all). + // // Three cursors, three modes: a real `since >= 0` replays only [since, // head); the -1 sentinel (nothing painted) replays the tail; and the // sinceFollowLiveEnd sentinel (-2, painted but offset unknown) replays @@ -2627,6 +2642,45 @@ func (s *Server) handleInstanceTTYWSLiveness(w http.ResponseWriter, r *http.Requ cursor := since first := true var streamed int64 + // startPublished (issue #94) marks that the replay's real + // START offset has been announced to the client, so it can + // advance its cursor frame-by-frame from there instead of + // waiting for the closing sync frame. It is deliberately NOT + // `first`: the FIX-F consult below clears `first` before any + // chunk has been written, while the start offset must be + // published exactly once, ahead of the first CHUNK on the + // wire. + startPublished := false + // emitStartSync announces a replay's real START offset + // (issue #94) ahead of its first binary chunk, lifting the + // client's handshakeSynced latch at chunk 1 so a mid-replay + // drop resumes from the bytes already painted instead of + // re-pulling the whole (up to 8MB) replay. The frame REUSES + // the "sync" type rather than inventing a new one: the + // client's whitelist (parseTTYControlMessage) paints any + // unknown text frame into the terminal (the issue #98 leak + // class), and the existing handler — set cursor to the + // ABSOLUTE offset, lift the latch — is exactly the semantics + // wanted here. The "start":true marker lets a consumer that + // must know WHEN the replay ends (a test harness, a + // third-party client) tell this announcement apart from the + // closing echo; the in-repo client deliberately ignores it — + // every sync is an absolute offset to adopt. + // + // syncCap is PROVABLY true at both call sites — they sit in + // the catch-up branch, which requires since >= 0, i.e. an + // explicit `since` in the query, already the inferred opt-in + // half of syncCap (issue #98; review Minor-1). The guard + // stays so a future change to that inference can never leak + // the frame to a client that did not opt in — defensive + // depth, not an option any client has today. + emitStartSync := func(start int64) bool { + if !syncCap { + return true + } + payload := []byte(`{"type":"sync","offset":` + strconv.FormatInt(start, 10) + `,"start":true}`) + return conn.WriteText(payload) == nil + } for { if streamed >= ttyHandshakeReplayBudget { // Budget exhausted — the ONE budget guard, checked @@ -2694,8 +2748,19 @@ func (s *Server) handleInstanceTTYWSLiveness(w http.ResponseWriter, r *http.Requ // ever appeared it must degrade to the // first-connect tail, never to a cursor // the ring never held. Write the tail - // before claiming head over it. + // before claiming head over it. Unlike + // the CURSOR-LESS tail replay above, this + // tail rides the issue #94 contract: the + // client presented a cursor, so announce + // the tail's real start S = head - len + // ahead of the chunk and let its latch + // lift at a truthful absolute base — the + // closing sync then republishes the same + // head the frames summed to. if tail != "" { + if !emitStartSync(head - int64(len(tail))) { + return false + } if err := conn.WriteBinary([]byte(tail)); err != nil { return false } @@ -2737,6 +2802,32 @@ func (s *Server) handleInstanceTTYWSLiveness(w http.ResponseWriter, r *http.Requ // Write as we read: the chunk is on the wire now, so the // cursor published below is never ahead of it, and nothing // older than the next read is retained in memory. + if !startPublished { + startPublished = true + // S = next - len(body) is the true start of THIS + // chunk — the requested `since` when no clamp + // happened, the clamped oldest live byte when it + // did — because S and the first chunk come from the + // SAME ReadSince call, so no new Manager surface is + // needed and a stale `since` can never be echoed + // raw. The closing sync below still arrives with + // the end offset, so a client that only understands + // sync-at-the-end is merely back to the pre-#94 + // death window, and a misbehaving kind that returns + // a body without advancing the cursor (the + // defensive break below) is corrected by that same + // final absolute value. One corner on THAT + // defensive path (review Minor-2): next <= cursor + // there makes S = next - len(body) dip BELOW the + // client's own `since`, so a drop between this + // frame and the first chunk reconnects with a + // regressed cursor and re-renders [S, since) — + // bounded, self-healing, and still strictly better + // than the pre-#94 full re-pull it replaces. + if !emitStartSync(next - int64(len(body))) { + return false + } + } if err := conn.WriteBinary([]byte(body)); err != nil { return false } diff --git a/internal/app/tty_ws_test.go b/internal/app/tty_ws_test.go index aa179d1..1407967 100644 --- a/internal/app/tty_ws_test.go +++ b/internal/app/tty_ws_test.go @@ -447,16 +447,34 @@ func expectCloseFrame(t *testing.T, c *ws.Conn, code uint16, wantReason string) } // dialHandshakeFrames dials, completes the handshake with the first -// resize, and collects the replay up to the `sync` echo — but keeps the -// binary frames SEPARATE, so a test can pin how the replay was chunked +// resize, and collects the replay up to the CLOSING `sync` echo — but keeps +// the binary frames SEPARATE, so a test can pin how the replay was chunked // (each frame is one streamed ring-buffer read, never the whole delta // re-buffered) without an extra concatenation copy. func dialHandshakeFrames(t *testing.T, addr, path string) (c *ws.Conn, frames [][]byte, syncOffset int64) { + t.Helper() + c, frames, _, syncOffset = dialHandshakeStart(t, addr, path) + return c, frames, syncOffset +} + +// dialHandshakeStart is dialHandshakeFrames plus the replay's START offset +// from the issue #94 start sync frame (-1 when none arrived: the tail +// replay, the empty-replay consult exits and the sinceFollowLiveEnd path +// emit only the closing sync). On the streaming catch-up path the replay is +// bracketed by TWO sync frames — {"type":"sync","offset":S,"start":true} +// ahead of the first binary chunk and the plain closing echo after the last +// one — so the loop below stops only at a sync WITHOUT the start marker, +// and pins the wire invariant that the start announcement precedes every +// replay chunk. The "start" field itself is ignored by the in-repo client +// (every sync carries an absolute offset to adopt); it exists so consumers +// that must know WHEN the replay ends can tell the two frames apart. +func dialHandshakeStart(t *testing.T, addr, path string) (c *ws.Conn, frames [][]byte, startOffset, syncOffset int64) { t.Helper() conn := dialTTY(t, addr, path) sendResize(t, conn) + startOffset = -1 sawSync := false for !sawSync { op, p := readFrame(t, conn, 5*time.Second) @@ -473,12 +491,23 @@ func dialHandshakeFrames(t *testing.T, addr, path string) (c *ws.Conn, frames [] var ctl struct { Type string `json:"type"` Offset int64 `json:"offset"` + Start bool `json:"start"` } if err := json.Unmarshal(p, &ctl); err != nil { t.Fatalf("control frame %q is not JSON: %v", p, err) } switch ctl.Type { case "sync": + if ctl.Start { + if startOffset != -1 { + t.Fatalf("duplicate start sync frame: %q", p) + } + if len(frames) != 0 { + t.Fatalf("start sync arrived AFTER %d replay chunks — it must precede the first binary frame: %q", len(frames), p) + } + startOffset = ctl.Offset + continue + } syncOffset = ctl.Offset sawSync = true case "resize": @@ -494,7 +523,7 @@ func dialHandshakeFrames(t *testing.T, addr, path string) (c *ws.Conn, frames [] t.Fatalf("unexpected opcode %d during handshake", op) } } - return conn, frames, syncOffset + return conn, frames, startOffset, syncOffset } // dialHandshake dials the TTY endpoint, consumes the ready frame, sends @@ -723,10 +752,17 @@ func TestHandleInstanceTTYWS_ReplayBudgetTruncatesAndPublishesLastWrittenOffset( k.buf.Write(delta) // head = budget + 4 chunks head := k.buf.Offset() - c, frames, syncOffset := dialHandshakeFrames(t, addr, ttyWSPath(instID, "since=0&caps=sync")) + c, frames, startOffset, syncOffset := dialHandshakeStart(t, addr, ttyWSPath(instID, "since=0&caps=sync")) _ = c.WriteClose(ws.CloseMessage(1000, "bye")) _ = c.Close() + // The start sync (issue #94) opened the replay at the requested cursor — + // no clamp below the oldest live byte applies here — and the client's + // frame-by-frame advance from it must land exactly on the closing offset. + if startOffset != 0 { + t.Fatalf("start sync offset = %d, want 0 (the replay starts at the requested cursor)", startOffset) + } + var delivered int64 for i, f := range frames { if int64(len(f)) > 64*1024 { @@ -747,6 +783,9 @@ func TestHandleInstanceTTYWS_ReplayBudgetTruncatesAndPublishesLastWrittenOffset( if len(frames) != int(ttyHandshakeReplayBudget/(64*1024)) { t.Fatalf("replay arrived as %d frames, want %d full 64KB chunks", len(frames), ttyHandshakeReplayBudget/(64*1024)) } + if startOffset+delivered != syncOffset { + t.Fatalf("start %d + delivered %d != sync offset %d — advancing the cursor per frame from the start offset must land exactly on the closing offset", startOffset, delivered, syncOffset) + } // The published cursor is the end of the LAST chunk written, behind // head: the remainder stays re-requestable. if syncOffset != ttyHandshakeReplayBudget { @@ -758,12 +797,20 @@ func TestHandleInstanceTTYWS_ReplayBudgetTruncatesAndPublishesLastWrittenOffset( // Self-healing: a reconnect from the published cursor re-requests the // bytes the budget deferred — the last 4 chunks — and catches up to - // head, so nothing was lost, only deferred. - c2, replay2, sync2 := dialHandshake(t, addr, ttyWSPath(instID, "since="+strconv.FormatInt(syncOffset, 10)+"&caps=sync")) + // head, so nothing was lost, only deferred. Its start sync must open + // exactly AT the published cursor: resuming, not restarting. + c2, frames2, start2, sync2 := dialHandshakeStart(t, addr, ttyWSPath(instID, "since="+strconv.FormatInt(syncOffset, 10)+"&caps=sync")) _ = c2.WriteClose(ws.CloseMessage(1000, "bye")) _ = c2.Close() + if start2 != syncOffset { + t.Fatalf("start sync of the re-request = %d, want the published cursor %d — the replay must resume where the truncated one stopped", start2, syncOffset) + } + var replay2 []byte + for _, f := range frames2 { + replay2 = append(replay2, f...) + } want2 := delta[ttyHandshakeReplayBudget:] - if replay2 != string(want2) { + if !bytes.Equal(replay2, want2) { t.Fatalf("re-request after truncation = %d bytes, want the %d deferred bytes — the budget must defer, never skip", len(replay2), len(want2)) } if sync2 != head { @@ -771,6 +818,188 @@ func TestHandleInstanceTTYWS_ReplayBudgetTruncatesAndPublishesLastWrittenOffset( } } +// TestHandleInstanceTTYWS_ReplayStartOffsetPublishesClampedStart pins AC2 of +// issue #94: when the requested `since` predates the oldest live byte, +// ReadSince silently clamps to that byte — and the start sync must publish +// the CLAMPED value, never the raw request. Publishing the request would +// land the client's cursor PAST bytes it never received once it advances +// per frame — strictly worse than the pre-#94 re-pull this frame removes. +func TestHandleInstanceTTYWS_ReplayStartOffsetPublishesClampedStart(t *testing.T) { + t.Parallel() + // A 64-byte ring with 100 bytes written holds offsets [36, 100): a + // request from since=5 is clamped to 36 by the first ReadSince. + payload := make([]byte, 100) + for i := range payload { + payload[i] = byte('A' + i%26) + } + k := newTTYHandshakeKind(64) + addr, _, instID, _ := ttyWSTestServer(t, k) + k.buf.Write(payload) + + c, frames, startOffset, syncOffset := dialHandshakeStart(t, addr, ttyWSPath(instID, "since=5&caps=sync")) + _ = c.WriteClose(ws.CloseMessage(1000, "bye")) + _ = c.Close() + + if startOffset != 36 { + t.Fatalf("start sync offset = %d, want 36 (the clamped oldest live byte) — echoing the raw since=5 would strand the cursor past bytes the client never received", startOffset) + } + var replay []byte + for _, f := range frames { + replay = append(replay, f...) + } + if !bytes.Equal(replay, payload[36:]) { + t.Fatalf("replay = %d bytes starting at %v, want the live window payload[36:] (%d bytes)", len(replay), replay[:1], len(payload[36:])) + } + if syncOffset != 100 { + t.Fatalf("sync offset = %d, want 100 (the ring head)", syncOffset) + } + if startOffset+int64(len(replay)) != syncOffset { + t.Fatalf("start %d + replay %d != sync %d — the frame-by-frame advance from the clamped start must close exactly on the end offset", startOffset, len(replay), syncOffset) + } +} + +// TestHandleInstanceTTYWS_MidReplayResumeContinuesFromRenderedBytes pins AC1 +// of issue #94: a connection that dies MID-REPLAY — after the start sync and +// two chunks, before the closing sync — has already advanced its cursor to +// start+painted on the client, so the reconnect resumes with exactly the +// un-rendered remainder instead of re-pulling the whole replay from the old +// cursor (the pre-#94 death window, up to 8MB under a degrading link). +func TestHandleInstanceTTYWS_MidReplayResumeContinuesFromRenderedBytes(t *testing.T) { + t.Parallel() + // A 200KB delta in a ring that holds it comfortably: a multi-chunk + // streaming replay (64KB reads), long enough to die inside. + delta := make([]byte, 200*1024) + for i := range delta { + delta[i] = byte('a' + i%26) + } + k := newTTYHandshakeKind(int64(len(delta)) + 1024) + addr, _, instID, _ := ttyWSTestServer(t, k) + k.buf.Write(delta) + + // First connection: "dies" two chunks into the replay. Read frames + // manually — dialHandshakeStart would consume the closing sync this + // connection never lives to see. The client-side cursor at the drop is + // the start sync's S plus the wire bytes already painted. + c := dialTTY(t, addr, ttyWSPath(instID, "since=0&caps=sync")) + sendResize(t, c) + startOffset := int64(-1) + var painted int64 + for painted < 2*64*1024 { + op, p := readFrame(t, c, 5*time.Second) + switch op { + case wsOpBinary: + if startOffset == -1 { + t.Fatalf("binary replay chunk before the start sync — the client would advance its cursor from a base it never learned") + } + if !bytes.Equal(p, delta[painted:painted+int64(len(p))]) { + t.Fatalf("replay chunk at offset %d is not the delta's bytes there", painted) + } + painted += int64(len(p)) + case wsOpPing: + // Protocol heartbeat: not part of the replay. + case wsOpText: + var ctl struct { + Type string `json:"type"` + Offset int64 `json:"offset"` + Start bool `json:"start"` + } + if err := json.Unmarshal(p, &ctl); err != nil { + t.Fatalf("control frame %q is not JSON: %v", p, err) + } + switch ctl.Type { + case "sync": + if !ctl.Start { + t.Fatalf("closing sync arrived %d bytes into a 200KB replay — it must follow the LAST chunk", painted) + } + startOffset = ctl.Offset + case "resize", "ping", "pong": + // Size echo / app-level heartbeat: background traffic. + default: + t.Fatalf("unexpected control frame %q during the replay", p) + } + default: + t.Fatalf("unexpected opcode %d during the replay", op) + } + } + if startOffset != 0 { + t.Fatalf("start sync offset = %d, want 0 (the requested cursor, no clamp on a fresh ring)", startOffset) + } + // The drop: no closing sync was consumed, the cursor stands at the + // rendered byte count. _ = c.Close() via cleanup would also do, but the + // explicit close IS the mid-replay death this test is about. + _ = c.Close() + + // The reconnect sends the rendered cursor as `since`; the resume must + // deliver EXACTLY the un-rendered remainder — neither the two painted + // chunks again (the pre-#94 duplication) nor anything past head. + cursor := startOffset + painted + c2, frames2, start2, sync2 := dialHandshakeStart(t, addr, ttyWSPath(instID, "since="+strconv.FormatInt(cursor, 10)+"&caps=sync")) + _ = c2.WriteClose(ws.CloseMessage(1000, "bye")) + _ = c2.Close() + if start2 != cursor { + t.Fatalf("resume start sync = %d, want the rendered cursor %d — resuming must continue where the dropped replay died", start2, cursor) + } + var replay2 []byte + for _, f := range frames2 { + replay2 = append(replay2, f...) + } + if !bytes.Equal(replay2, delta[cursor:]) { + t.Fatalf("resume replay = %d bytes, want exactly the %d un-rendered bytes — no re-delivery of what was already painted", len(replay2), len(delta[cursor:])) + } + if sync2 != int64(len(delta)) { + t.Fatalf("resume sync offset = %d, want head %d", sync2, len(delta)) + } +} + +// TestHandleInstanceTTYWS_StartSyncOnlyForChunkedReplaysWithACursor pins the +// boundaries of the issue #94 frame. The contract is: EVERY replay CHUNK +// addressed to a cursor-bearing client is preceded by a start frame — the +// streamed catch-up (pinned by the resume/budget tests above) AND the +// single-frame beyond-head degrade (pinned by +// TestHandleInstanceTTYWS_CursorAheadOfHeadFallsBackToTail). What this test +// pins is the other side, the three paths that must send the closing sync +// ALONE: the CURSOR-LESS tail replay (no cursor to advance — and such a +// client may not parse `sync` at all, so that path's syncCap is a genuine +// gate), the sinceFollowLiveEnd path and the caught-up-at-head consult +// exit (both cursor-bearing, but zero chunks to announce). +func TestHandleInstanceTTYWS_StartSyncOnlyForChunkedReplaysWithACursor(t *testing.T) { + t.Parallel() + k := newTTYHandshakeKind(1024) + addr, _, instID, _ := ttyWSTestServer(t, k) + k.buf.WriteString("tail-body") // head = 9 + + // Tail path: no `since` at all — the one CURSOR-LESS replay. + c1, _, start1, sync1 := dialHandshakeStart(t, addr, ttyWSPath(instID, "caps=sync")) + _ = c1.Close() + if start1 != -1 { + t.Fatalf("tail replay announced a start offset %d — the client holds no cursor to advance (and may not parse sync at all), so the single ≤64KB frame keeps the pre-#87 death window", start1) + } + if sync1 != 9 { + t.Fatalf("tail replay sync = %d, want head 9", sync1) + } + + // Follow from the live end (-2): no replay at all, the closing sync IS + // the cursor handoff. + c2, _, start2, sync2 := dialHandshakeStart(t, addr, ttyWSPath(instID, "since=-2&caps=sync")) + _ = c2.Close() + if start2 != -1 { + t.Fatalf("follow-live-end announced a start offset %d — the -2 path emits zero binary frames and must stay untouched (issue #94 AC)", start2) + } + if sync2 != 9 { + t.Fatalf("follow-live-end sync = %d, want head 9", sync2) + } + + // Caught up exactly at head: the empty-replay consult exit. + c3, frames3, start3, sync3 := dialHandshakeStart(t, addr, ttyWSPath(instID, "since=9&caps=sync")) + _ = c3.Close() + if start3 != -1 { + t.Fatalf("caught-up reconnect announced a start offset %d — an empty replay has no first chunk to start at", start3) + } + if len(frames3) != 0 || sync3 != 9 { + t.Fatalf("caught-up reconnect frames=%d sync=%d, want an empty replay and head 9", len(frames3), sync3) + } +} + // TestHandleInstanceTTYWS_EmptyReadAfterConsultPublishesHeadNotZero pins // review finding B3, the FIX-F counterpart. The scripted queue drives the // same shape TestHandleInstanceTTYWS_ConsultSeesNewBytesDeliversThem uses @@ -1078,11 +1307,19 @@ func TestHandleInstanceTTYWS_CursorAheadOfHeadFallsBackToTail(t *testing.T) { addr, _, instID, _ := ttyWSTestServer(t, k) k.buf.WriteString("HELLO-RING") // head = 10 - c, replay, syncOffset := dialHandshake(t, addr, ttyWSPath(instID, "since=999&caps=sync")) + c, frames, startOffset, syncOffset := dialHandshakeStart(t, addr, ttyWSPath(instID, "since=999&caps=sync")) _ = c.WriteClose(ws.CloseMessage(1000, "bye")) _ = c.Close() - if replay != "HELLO-RING" { - t.Fatalf("replay = %q, want the tail HELLO-RING (cursor ahead of head must fall back to first-connect tail)", replay) + if len(frames) != 1 || string(frames[0]) != "HELLO-RING" { + t.Fatalf("replay = %d frames %q, want the single tail HELLO-RING (cursor ahead of head must fall back to first-connect tail)", len(frames), frames) + } + // The degrade announced the tail's real start like every replay + // addressed to a cursor-bearing client (issue #94 review): S = head 10 + // − len 10 = 0. A genuine 0, distinct from the harness's -1 "no start + // frame" sentinel — and exactly the value the client's latch needs to + // count the tail frame up to the closing sync. + if startOffset != 0 { + t.Fatalf("start sync offset = %d, want 0 (head 10 − tail 10) — the degrade announces the tail's real start like any other replay", startOffset) } if syncOffset != 10 { t.Fatalf("sync offset = %d, want 10 (the tail's end offset, not the client's bogus 999)", syncOffset) @@ -1287,13 +1524,16 @@ func TestHandleInstanceTTYWS_SubscriberOverflowClosesConnectionWith1013(t *testi // rather than treat the session as finished; // - its reason names the overflow, so the console explains the drop // instead of showing an unexplained disconnect; - // - no second `sync` precedes it. `sync` is written exactly once, in - // completeHandshake, before SubscribeOutput (app.go), so a second one - // here would advertise a cursor past bytes the client never received - // and its next reconnect would silently skip them. The resize echo the - // handler's own handshake queued is a TEXT frame on a different - // channel, so it may land anywhere in this sequence; it is not a - // cursor and is allowed through. + // - no `sync` precedes it. The handshake's sync frames — since issue + // #94 up to TWO of them, the start announcement ahead of the first + // replay chunk and the closing echo after the last — are ALL written + // inside completeHandshake, before SubscribeOutput (app.go), so one + // arriving HERE, on the live stream, would advertise a cursor past + // bytes the client never received and its next reconnect would + // silently skip them. The resize echo the handler's own handshake + // queued is a TEXT frame on a different channel, so it may land + // anywhere in this sequence; it is not a cursor and is allowed + // through. for { op, p := readFrame(t, c, 10*time.Second) if op == wsOpClose { diff --git a/internal/ui/static/index.html b/internal/ui/static/index.html index 291efc3..d10f806 100644 --- a/internal/ui/static/index.html +++ b/internal/ui/static/index.html @@ -4648,14 +4648,32 @@ // first of them and could land PAST bytes the client never // received — strictly worse than duplicating them. The sync // frame is the only frame carrying an authoritative ABSOLUTE - // offset, so it is the one that moves the cursor, publishing - // the end offset of the whole replay in a single step. Blast - // radius if the socket dies in the replay→sync gap: the cursor - // never advanced, so the reconnect re-requests from where the - // client really was and re-paints up to - // ttyHandshakeReplayBudget (8MB) now that the replay streams, - // instead of a single 64KB frame — bounded duplication that - // self-corrects on that reconnect. + // offset, so it is the one that moves the cursor. A post-#94 + // server sends it TWICE on the streaming catch-up path: a + // START sync ahead of the first replay chunk carrying the + // replay's real (clamped) start offset S, which lifts this + // latch at chunk 1 so the cursor advances frame-by-frame + // from S — a socket that dies mid-replay then resumes from + // the bytes already painted instead of re-pulling the whole + // replay — and the CLOSING sync after the last chunk + // carrying the end offset, which re-pins the cursor + // absolutely. Against a pre-#94 server only the closing + // sync arrives, and the blast radius of dying in the + // replay→sync gap is the old one: the cursor never advanced, + // so the reconnect re-requests from where the client really + // was and re-paints up to ttyHandshakeReplayBudget (8MB) — + // bounded duplication that self-corrects on that reconnect. + // Recorded trade-off of the start sync (issue #94 review): + // a drop landing between a frame that ends mid-UTF-8- + // sequence and the next one discards those 1-3 lead bytes + // (resetTTYOutputDecoder runs in ws.onclose) while the + // cursor has already stepped past them, so the character + // renders as U+FFFD where the pre-#94 full re-pull + // redelivered it. "Either way" names the two paths that + // have ALREADY stepped the cursor past those lead bytes + // — this replay path post-#94 and the live path, which + // has always had the same trade-off: cosmetic only, and + // the split character is unrecoverable on both. let handshakeSynced = false; let handshakeTimeout = null; let connectTimeout = setTimeout(() => { @@ -4833,6 +4851,23 @@ // Handshake offset echo (issue #87): the server tells // us the end offset of the replay it just sent, so a // later reconnect can resume incrementally with it. + // Since issue #94 the streaming catch-up path sends + // this frame TWICE — a START sync (the replay's real, + // possibly clamped, start offset) ahead of the first + // binary chunk, then the CLOSING sync (the end + // offset) after the last one. This handler needs no + // distinction: every sync carries an ABSOLUTE offset, + // so adopting it and lifting the latch is correct for + // both — the start sync makes the per-frame advance + // below truthful from chunk 1 on, and the closing + // sync re-pins the value the frames summed to. The + // sum can land SHORT only if the ring rolled a full + // cycle mid-replay and evicted bytes the loop had + // not sent yet (a later chunk then clamps above the + // running cursor): the closing sync jumps the gap, + // which is truthful — those bytes are undeliverable + // anyway, the same rule the read→subscribe window + // already lives by. if (Number.isFinite(controlMsg.offset) && controlMsg.offset >= 0) session.ttyOffset = controlMsg.offset; handshakeSynced = true; return; @@ -4861,9 +4896,15 @@ // broadcast, so wire bytes and buffered bytes are // the same bytes. BEFORE the latch the frames are // still replay bytes and must not touch the - // cursor: the sync frame that follows on the same - // TCP stream publishes the authoritative end - // offset of the entire replay in one step. + // cursor: the replay need not start where we asked + // (the stale-cursor clamp), so only an ABSOLUTE + // offset may move it. Post-#94 that is the START + // sync, which arrives ahead of the first replay + // chunk, so the latch is normally already lifted + // here and replay frames advance the cursor as + // they paint; against a pre-#94 server the latch + // lifts only at the closing sync, which publishes + // the end offset of the entire replay in one step. if (handshakeSynced) session.ttyOffset += u8.length; } } else if (session.term) { diff --git a/internal/ui/testdata/terminal_status.test.mjs b/internal/ui/testdata/terminal_status.test.mjs index 5418af0..9e93d92 100644 --- a/internal/ui/testdata/terminal_status.test.mjs +++ b/internal/ui/testdata/terminal_status.test.mjs @@ -1070,6 +1070,55 @@ test("issue #87: binary frames move the cursor only after the sync latch (FIX-B) assert.equal(h.api.parseTTYControlMessage('{"type":"bogus"}'), null); }); +test("issue #94: the start sync lifts the latch at chunk 1, so a mid-replay drop resumes", () => { + const h = ttyWsHarness({ ttyOffset: 13 }); + h.connect(); + const ws = h.sockets[0]; + assert.ok(ws.url.includes("&since=13"), "the reconnect carries the cursor"); + ws.readyState = 1; + ws.onopen(); + ws.onmessage({ data: '{"type":"ready"}' }); + + // A post-#94 server brackets a streaming replay with TWO sync frames: the + // start sync ahead of chunk 1 (the replay's real, possibly clamped, start + // offset — S and chunk 1 come from the same server-side read) and the + // closing sync after the last chunk. The "start" field is only a marker + // for consumers that must tell the two apart; the handler treats every + // sync as an absolute offset to adopt. + ws.onmessage({ data: '{"type":"sync","offset":13,"start":true}' }); + assert.equal(h.session.ttyOffset, 13, "the start sync adopts the replay's real start offset"); + + // Replay chunks after the start sync are safe to count frame-by-frame: + // the base is truthful (never the raw, maybe-clamped `since`), unlike the + // pre-sync frames pinned by the FIX-B case above. + ws.onmessage({ data: h.enc.encode("chunk-1-").buffer }); // 8 bytes + ws.onmessage({ data: h.enc.encode("chunk-2").buffer }); // 7 bytes + assert.equal(h.session.ttyOffset, 28, "replay frames advance the cursor once the start offset is known"); + assert.deepEqual(h.writes, ["chunk-1-", "chunk-2"], "the bytes still render in order"); + + // Die MID-REPLAY — before the closing sync. The cursor already stands at + // the rendered byte count, so the reconnect RESUMES from it instead of + // re-pulling the whole replay from 13 (the pre-#94 death window, up to + // ttyHandshakeReplayBudget under a degrading link). + ws.close(); + h.fireTimers(5000); + assert.equal(h.sockets.length, 2, "the reconnect opened a new socket"); + assert.ok(h.sockets[1].url.includes("&since=28"), + `a mid-replay drop must resume from the rendered cursor, got ${h.sockets[1].url}`); + + // The resume's own replay is bracketed the same way, and its closing sync + // re-pins the absolute end — the same value the frames summed to. + const ws2 = h.sockets[1]; + ws2.readyState = 1; + ws2.onopen(); + ws2.onmessage({ data: '{"type":"ready"}' }); + ws2.onmessage({ data: '{"type":"sync","offset":28,"start":true}' }); + ws2.onmessage({ data: h.enc.encode("rest").buffer }); // 4 bytes + assert.equal(h.session.ttyOffset, 32, "the resume advances from its own start offset"); + ws2.onmessage({ data: '{"type":"sync","offset":32}' }); + assert.equal(h.session.ttyOffset, 32, "the closing sync republishes the value the frames already summed to"); +}); + test("issue #87: the cursor survives the drop and the reconnect resumes from it", () => { const h = ttyWsHarness(); h.connect();