fix(sidebar): a finished pane reads ready once you view it #10
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/done-reads-idle-when-opened"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Change
The browser sends a
viewingframe when a pane is visible and focused; only then does a done pane read idle. Attaching alone does not count. herdr's own state is untouched. Closes #2.Validation
bun run check fastpasses. A contract test covers attach alone versus viewing, andviewed-panes.test.tsthe status rewrite.Finished pane not viewed: Done (green dot). After it is opened: before (left) still Done, after (right) Ready.
Video: a Done pane turns Ready once it is opened. Video
## Change `scripts/ui-regression.ts` raced the page in the "sent before the snapshot" step and failed about one run in five with `Timed out: the message sent before the snapshot is a live pending row` (CI runs 37631750944, 37675014452). The step closes the routed socket, holds the reconnect's `snapshot`, waits for `pendingReady(pendingPane)`, sends, then releases the snapshot. `pendingReady` read `inputStates`, which was keyed by pane only. The server sends its snapshot as soon as the socket opens, and that can be before the page has sent its `attach`. In that order the held-snapshot wait finished while `inputStates` still held the **closed** connection's last `input-ready`, so `pendingReady` resolved at once. The message then reached the bridge before the new attachment was ready. The server answered `input_not_ready`, and the composer kept the draft with "Not sent: The pane is not attached and ready". So no pending row appeared. An instrumented copy of the script (frame log, not committed) caught the failing order: ``` 85771 route: snapshot held 85771 pendingReady resolved {"ready":true,...} <- the old connection's state 85772 ws#2 -> attach 85807 route: snapshot released 85809 ws#2 -> submit (delivery: queue) 85820 ws#2 <- submit-result ok:false input_not_ready 86051 ws#2 <- input-ready <- the new connection is ready only here ``` In passing runs, `attach` goes out before the snapshot is held, and `pendingReady` waits for the new connection's `input-ready`. The product behaves as intended: a send before the pane's attachment is ready is refused and the draft stays (never queued while offline). So the fix is in the test. Each input state now records the Playwright WebSocket that reported it, and a pane counts as ready only on the page's current, open connection. After the close, the step also waits for the page to open its new connection. There are no new timeouts or sleeps. ## Validation On a herdr of the run's own (`bun run check run bun scripts/ui-regression.ts`): - Before (main at407c1a1): 2 of 14 normal runs failed with this exact assertion (1 of 6 plain, 1 of 8 with the frame log above). Widened race, 3 of 3 failed with the same assertion. The widening is a scratch copy, not committed: in the page, the reconnect's `open` handler (which sends the attach) is stalled 300 ms, and the route holds that connection's frames to the server, attach first and in order, for 1 s. That is the failing order above, made certain. - After (2868996): the same widened race, 3 of 3 passed; 10 of 10 normal runs passed. - `bun run check fast`: passed (1699 pass, 4 skip, 0 fail). It does not cover `scripts/`, which the runs above exercise. - CI on this PR: Fast checks and Integration and browser passed. No CHANGELOG entry: the change is test-only.## What Anonymous install and update counts, announced and on by default (opt-out): - **When:** one event when an install first runs, one each time it runs a new version, nothing in between. No event is sent before the app has shown its one-time notice (a line under the header with **What is sent**, **Turn off** and **Dismiss**), and the first one waits ten minutes after it, so **Turn off** on the notice stops it. The notice is recorded only once it was painted in a visible tab, and stays up until the server confirms the switch is off. - **What:** `event` (`install`/`update`), a random `install_id` made on the PC, `version`, `previous_version`, `os`, `arch`, `install_method` (`plugin`/`managed`/`source`). Nothing about terminals, agents, files or accounts. - **Off switch:** Settings → About → **Anonymous usage counts** shows the next event exactly as it would be sent and turns it off. `HERDR_WEB_TELEMETRY=0`, `DO_NOT_TRACK=1`, `CI` and the test environments block it whatever the switch says. - **Where it does nothing:** only the real entrypoint (`server/index.ts`) passes a `Telemetry`, so `createServer` in tests, remote-PC bridges and the demo answer `/api/telemetry` with 404 and the app shows neither the notice nor the switch. - **Receiver:** `telemetry/` is a Cloudflare Worker over one D1 table. It never reads the sender's address or any header but the content type, stores the day rather than the time, keeps each (install, event, version) once, and has Workers Logs off. Deploy steps and the queries to read the counts are in `telemetry/README.md`. A failed send is logged once and tried again at the next start; it never loops. Turning the switch off aborts a send in flight, the state file is re-read before each decision (two servers on one state directory agree), and a state directory that cannot be written leaves the app running with telemetry silent. ## Contract - `GET /api/telemetry` → `TelemetryStatus`; `POST /api/telemetry` `{ enabled?, notice_seen?: true }` with `x-herdr-update: 1` and same-origin (`shared/telemetry.ts`, catalogued in `shared/protocol.ts`). - `site/demo/transport.ts` answers 404 like a server without telemetry. - `telemetry/` is added to `tsconfig.json` and to the unit-test glob in `scripts/ci-tests.ts`. ## Before merging / releasing - The lead deploys the Worker right after the merge (the D1 database exists and its id is in `telemetry/wrangler.toml`; the account's workers.dev subdomain is `devswha`, so `TELEMETRY_URL` in `server/telemetry.ts` is right). Until then every send fails quietly: one log line per start, retried at the next start. ## Verification - `bun run test:unit`: 1790 pass, 0 fail (before rebasing onto v0.4.1); after the rebase `bun run typecheck` and the telemetry, Worker and i18n tests pass. - `bun run build` passes. - New tests: `server/telemetry.test.ts` (no send before the notice, install once, update from the last reported version, switch, environment blocks, retry after a refused send, route validation), `telemetry/worker.test.ts` (stored fields, no address, refusals), and a `telemetry API` block in `server/api.contract.test.ts` (404 without telemetry, notice and switch through HTTP, token gate). The contract block was not run locally; CI runs it. - Browser check against a real server and a stand-in receiver (Chromium): the notice shows on first open and the install event arrives once; **What is sent** opens Settings → About; the switch turns sending off on the server; after a reload the notice is not shown again; the notice wraps cleanly at 390px in Korean. ## Review pass (merge lane) - Merged `main` (v0.4.1, #598, #524); the changelog entry sits under `## [Unreleased]`. - Codex `gpt-6-astra` adversarial pass: six findings verified and fixed in c74606b and 2f8a0bc, each with a test that fails without the fix (`server/telemetry.test.ts`, `telemetry/worker.test.ts`); the rest are listed in a comment below. - `bun run check fast`: 1837 pass, 0 fail. `server/api.contract.test.ts`: 96 pass. `scripts/update-browser-qa.ts`: 4 PASS. - Browser check (Chromium, a real server and a stand-in receiver, 4 s grace): the notice shows, nothing is sent until the grace passed and then one install event arrives; a failed **Turn off** keeps the notice with "Could not turn it off. Try again.", the retry closes it, the switch is off on the server and nothing is sent after the grace; the failure line fits at 390px.## Change A chat message that starts with `/` and names a command Claude Code does not know seemed to vanish. It left the chat's box and never showed in the chat. Reproduced live on Claude Code 2.1.294, with the message sent exactly as the composer does (bracketed paste, then Enter): - Claude Code refuses it in the terminal (`● Unknown command: /xyzabc`, `● Args from unknown skill: hello there`) and clears its input. Nothing reaches the model. - The transcript records **no user turn** for it, only two entries: ```json {"type":"system","subtype":"informational","content":"Unknown command: /xyzabc","level":"warning",...} {"type":"system","subtype":"informational","content":"Args from unknown skill: hello there","level":"warning",...} ``` #583 shows a slash command's answer from `local_command` entries, and its test has `Unknown command: /gaol` inside `<local-command-stderr>`. This Claude Code version writes the refusal as `informational` instead, which the chat skipped. `parseClaudeTranscript` now shows `system`/`informational` entries as notices (`source: "informational"`), drawn the way #583's are: - Terminal codes are stripped with `stripTerminalControls`, and the text is capped like a command's answer. - Lines written within a second of each other join into one notice. The two lines above are 1 ms apart and read as one notice: `Unknown command: /xyzabc` with the dropped arguments under it. - An empty entry is skipped. The same subtype carries Claude's usage-limit notices (`Usage limit reached · continuing automatically at 4am · esc to cancel`, `Usage limit reset · continuing automatically`), which now show too. Those and the unknown-command pair are the only `informational` contents across this machine's transcripts from the last 30 days. ## Validation - New `server/conversation.test.ts` case: the two-line unknown command joined into one notice, the two usage-limit lines kept apart, terminal codes stripped, an empty entry skipped. It fails on `main` and passes here. - Live, against Claude Code 2.1.294 in an isolated herdr session: `GET /api/pane/conversation` returns the notice. In the chat it reads `Unknown command: /xyzabc` and opens to both lines. - `bun run test:unit` 1814 pass, `tsc --noEmit` clean. --------- Co-authored-by: Yoonwoo-Ha <274636136+Yoonwoo-Ha@users.noreply.github.com> Co-authored-by: Hako <devswha@gmail.com>## Change A chat message with an invisible character in it (a zero-width space, a joiner, a byte-order mark; pasted text often carries them) seemed lost. The chat cleared its box, but the message never reached the agent. Reproduced live on Claude Code 2.1.294, sent exactly as the composer does (bracketed paste, then Enter): ``` Removed 1 invisible character · review and press Enter to send ──────────────── ❯ helloworld test ──────────────── ``` Claude Code takes the characters out and, instead of sending, **keeps the message in its input** until it is reviewed. The chat has already cleared its box by then. That hint does not stay. With a turn already on screen its row goes to the status line (`✻ Cooked for 1s · done`): in 20 seconds of polling after an Enter it never showed, while `❯ timingtest` sat in the box the whole time. So the hint cannot be the only signal. **A card for the held message** (`parseClaudeHeld`, responder `claude-held`): - **When it shows.** Claude's input box holds a message, and either: - the hint is right over the box, or - the box holds what the chat last sent to this pane, less only invisible characters. - **Remembering the send.** For the second case `submitText` records the chat's last message per pane (`noteSubmitted`), only when it carried such characters, for 10 minutes. - **Matching.** `removedInvisible` compares the two without whitespace, since the box wraps a long message. Every character the box lacks must be invisible: format controls (zero-width, bidi, tags, BOM), combining marks, or Hangul fillers. - **The card.** It names how many characters were removed and shows the message as Claude holds it. - **Send** presses Enter. - **Discard** presses Ctrl+C, which clears Claude's input. - Like every card, an answer first checks that the screen still shows it. - **When it doesn't show.** Nothing else is a held message: the hint over an empty input (it lingers after a send), a box that differs in anything visible, or a quoted hint with output under the box. Typing a new draft removes the hint. ## Validation - New `server/prompt.test.ts` cases, built on the live screen: - the hint and the box give a card, with Send → `enter` and Discard → `ctrl+c`; - a held message of several lines, and the count of removed characters; - the box alone, plus what the chat sent, gives a card; a box that differs, or no send, gives none; - no card for an empty input, a quoted hint, or the words without a box; - `removedInvisible`, including U+FEFF (which `\s` would have taken for a space) and a Hangul filler. Two of these fail on `main`. - Live, Claude Code 2.1.294 in an isolated herdr session, through this server: - `POST /api/pane/prompt/answer` with **Discard** emptied Claude's input. - With **Send**, Claude answered the message. - In the browser: a message typed into the chat composer with a zero-width space produced the card. **Send** cleared the card and Claude replied. - `bun run test:unit` 1818 pass, `tsc --noEmit` clean. --------- Co-authored-by: Yoonwoo-Ha <274636136+Yoonwoo-Ha@users.noreply.github.com> Co-authored-by: Hako <devswha@gmail.com>Pull request closed