Repository navigation
test(chat): count the escape scanner's work instead of timing it - #590
Merged
Merged
Conversation
The growth test divided two sub-millisecond timings (floor 0.05 ms), so it failed "Fast checks" on main by scheduler noise. It now counts the characters the parse reads through charCodeAt, indexOf, startsWith and slice, charges a regex run the square of its input, and asserts that doubling the answer at most 2.5x the count. No clock is read.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe malformed-answer parsing test replaces repeated elapsed-time measurements with instrumented work counts for unterminated escapes and printed closing tags. It restores patched methods in a ChangesMalformed-answer parsing test
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change replaces a flaky timing-based test with a deterministic work-counting test and does not affect runtime behavior. It looks safe to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
devswha
commented
Oct 8, 2026
devswha
left a comment
devswha
left a comment
Owner
Author
There was a problem hiding this comment.
Independent review of 76ff6d6 (lead session; the lane that wrote it is fix-timing).
Verdict: clean, merge when both required checks are green.
What I ran, in a scratch copy of 76ff6d6 (git archive, main's node_modules), Bun 1.4.2:
| Check | Result |
|---|---|
bun test ./server/conversation.test.ts -t "strips escapes", 30 runs |
30/30 pass |
the same test with server/conversation.ts from 5979118 (the pre-#588 scanner) |
fails: Expected: < 2.5, Received: 7.999 |
| the whole file on the PR head | 52 pass, 0 fail |
Read: the diff touches only the test. No clock is read any more, so the result does not depend on the runner's load; the count is a function of the input alone. The prototypes are wrapped only around one synchronous call and restored in finally.
Not run by me: the closing-tag mutation (3.999) and the 200-run loop from the description.
One thing to know, not a blocker: the test is tied to the primitives the scanner uses today (charCodeAt, indexOf, startsWith, slice, a regex per character). A rewrite that reads the answer another way would be counted as less work, or as none (NaN, which fails). The comment says so.
Background: main went red on the timing version at 57d7663 (run 37730991278) and passed on a rerun, which is what a ratio of two sub-millisecond timings does.
7 tasks done
aNNdii
added a commit
to aNNdii/herdr-web-ui
that referenced
this pull request
Oct 8, 2026
Upstream's settings redesign (devswha#562) replaces next's own (one page per section, with search and a phone list); next's file viewer settings become a File viewer page in upstream's dialog.
aNNdii
added a commit
to aNNdii/herdr-web-ui
that referenced
this pull request
Oct 8, 2026
chore(sync): merge upstream main (v0.4.0, devswha#229–devswha#590) into next
devswha
added a commit
that referenced
this pull request
Oct 9, 2026
…lane (#627) ## Change A failed browser lane kept herdr's server log, but no page screenshots and no reliable per-script timings. This adds wall-second timing and exit-code lines for all 17 browser scripts plus the shared demo build, and an EXIT summary of every script run so far. The first failure still stops the lane; the trap preserves its exit code and removes the temporary demo build. Order, checks, and retries are unchanged. With CI or CHECK_DIR set, existing screenshots go under `${CHECK_DIR:-.ci}/browser-evidence`. The failure artifact includes those PNGs and the existing herdr logs, with hidden files explicitly enabled for the narrowly scoped `.ci` paths. Evidence is deliberately restricted to sticky-modifier and file-viewer checkpoints (eight viewport PNGs on a complete passing run). The UI suite has evidence-only assertions and viewport changes, and the key-bar demo adds viewport changes and font waits, so their evidence branches remain off in the lane. No live user session is captured: these scripts use isolated test panes and synthetic transcripts. Outside CI/CHECK_DIR, explicit local UI_EVIDENCE_DIR behavior is preserved. Remaining #552 work: - Capture every open page at the exact failure point; the scripts have no common browser launcher/failure handler. - Playwright traces of failing contexts. - Timestamps on the web UI test servers' own lines; there is no common test-server output wrapper across these scripts. - Recording browser-received frames. - Multiple-run measurement for #548 and decisions on #553; this PR does not split the lane. - Investigating the seven historical fork runs with no jobs/logs. No CHANGELOG entry: CI/test-only changes are not user-visible, following #548, #581, and #590. ## Validation - Four cheap `scripts/ci-browser.test.ts` tests exercise the real shell lane against stub commands: passing and failing summaries, exact exit 7, first-failure stop, temporary-directory cleanup, evidence filtering, and CI/CHECK_DIR/local behavior. The first three failed on the unchanged lane before implementation, then all four passed. - Captured a failing stub run: both passing steps and the failing step print numeric seconds, the EXIT summary includes all three, and the process exits 7. - `bash -n scripts/ci-browser.sh` and `git diff --check` passed. - `dori heavy tri-client3 -- bun run check fast` passed: actionlint, generated types, typecheck, build, and 1,932 unit tests passed, 0 failed (six existing skips). - The PR's CI browser lane will provide the real-surface timing proof and comparison against recent main runs. Refs #552
jiunshinn
added a commit
to jiunshinn/herdr-web-ui
that referenced
this pull request
Oct 9, 2026
…swha#625, devswha#627, devswha#632, devswha#646, devswha#647, devswha#648, devswha#658) (#5) * fix(terminal): recheck the sender's right to type right before herdr is written to (devswha#624) Fixes devswha#545. Refs devswha#589 (both points in the issue body; the window in its follow-up comment is left as is, see below). ## Problem - **devswha#545**: a queued terminal chord or text passed its attachment, claim and authorization checks inside `serialize(...)`, then waited for `herdrRpc`'s Unix-socket connect. If the sender detached, switched to observe or had its device revoked during that wait, the request was still written when the connection resolved. - **devswha#589**: the `secret` handler re-checked the connection, attachment, claim and readiness after its live-screen read, but not (1) whether the pane's pty had been replaced under the same attachment (a live handoff), or (2) whether the sender had switched interact → observe → interact while the read was pending. ## Change - `herdrRpc(method, params, socketPath, timeoutMs, guard?)`: an optional guard asked once more right before `sock.write`. False sends nothing and rejects with `cancelled`. `paneSendText` / `paneSendKeys` pass it through. Callers without a guard are unchanged. - `keys` handler: the checks it already makes become the guard; a cancelled send is answered `input_failed` (as the existing cancel paths are, per `server/AGENTS.md`). - `input` handler (mirrored and queued branches): the same, through the existing `.catch(inputFailed)`. - `SocketData.roles` counts mode changes. The secret handler captures it and the attachment's pty when it accepts the secret, and refuses after the screen read with `read_only` (role changed) or `input_not_ready` (pty replaced). On a mirrored pane the secret's text also carries the guard. **Left as is (devswha#589 comment):** on a mirrored pane, once the secret's text is in, its Enter follows without another check. Withholding the Enter would leave the secret on the prompt line for the next Enter anyone presses, and no single key clears a line in every shell herdr mirrors (Ctrl+U for a POSIX tty, Escape for PowerShell/cmd). This matches the devswha#588 review's call. The issue stays open for that decision. **Cost to a legitimate user:** none found. The guard only refuses what the existing checks already refuse, evaluated a moment later. The app never sends `role` on its own (`src/AGENTS.md`), so the role count changes only for a client that switches roles itself. ## Validation - `server/rpc-guard.test.ts` (new, unit, a stand-in herdr on a Unix socket): a guard that turns false while the connect is pending writes nothing and rejects `cancelled`. Passes with the fix; fails with the guard check replaced by `if (false)` (1 fail). - `server/submit.contract.test.ts`: new case, a secret whose screen read is held while the sender switches to observe and back, answers `read_only` and types nothing. With the fix it passes, together with the two devswha#588 rejoin cases (3 pass). With the role check removed it fails (the secret was entered). Run through `bun run check run` on its own herdr. - `bun run typecheck` clean. - Not covered by a test: the pty-replaced branch (it needs a live handoff mid-read). It is one identity comparison next to the existing attachment check. No protocol change. `server/` changes, so a remote PC gets this with the next `REMOTE_BUNDLE_VERSION` bump. * fix(machines): a remote PC tells its connection server of a new agent at once (devswha#625) Fixes devswha#555. ## Problem On a remote PC, a pane whose agent has just been named could stay listed as a shell for up to 5 s. The connection server reads a remote PC's snapshot every 5 s and on each `pane-status` / `pane-exited` / `session-changed` frame from its bridge (`server/machines.ts`, `observe`). An agent herdr names **in a status event** therefore reaches it with that event's frame. An agent the bridge's collector first sees **in a snapshot it reconciles from** (a pane created a moment ago that got its agent after the roster's own read, or an agent that starts without a status change) has no frame behind it, so nothing asks for the roster until the next 5 s read. devswha#537 fixed the local PC with `MachineManager.localAgents`; a bridge runs with `machines: false`, so that call did nothing there. ## Change - `AgentNews` (`server/machines.ts`): the "is this news" test `localAgents` had (an agent named anew or another one; a pane first heard of as a shell is not), with `forget` / `keepOnly`. `MachineManager` uses it unchanged in behaviour. - `bridgeAgentNews(tell)`: a bridge's record. A status event's agent is recorded with no frame of its own, because its `pane-status` frame already makes the connection server read the roster. An agent first seen in a reconciled snapshot calls `tell`, which `createServer` wires to `broadcastAll({ type: "session-changed" })`. It never prunes on a reconcile's payload, which leaves out panes heard of while its snapshot was read. Panes that end are forgotten through `onPaneEnded`. No wire change: `session-changed` already exists and every connection server reads the roster on it. **Remote bundle:** the bridge's behaviour changes, so remote PCs get this only after `REMOTE_BUNDLE_VERSION` is raised at the next release. ## Validation - `server/machines-refresh.test.ts`, four new cases for `bridgeAgentNews`: news first seen in a reconcile tells once; a status event's agent tells nothing; a pane left out of a reconcile is not forgotten; an ended pane's id is news again. With the fix: 19 pass. With the `tell` removed: 3 fail. With the first version of this PR (tell on status, prune on reconcile): 2 fail. - The devswha#537 `localAgents` cases still pass after the refactor. - `bun run typecheck` clean. - Not run end to end: a live herdr reconcile that names an agent with no status event. I found no deterministic way to make herdr do that in a contract test. The first version's contract case went through a status event, a path the connection server already covered, so it was removed (see the Codex comment). * ci(browser): per-script timings and kept screenshots for the browser lane (devswha#627) ## Change A failed browser lane kept herdr's server log, but no page screenshots and no reliable per-script timings. This adds wall-second timing and exit-code lines for all 17 browser scripts plus the shared demo build, and an EXIT summary of every script run so far. The first failure still stops the lane; the trap preserves its exit code and removes the temporary demo build. Order, checks, and retries are unchanged. With CI or CHECK_DIR set, existing screenshots go under `${CHECK_DIR:-.ci}/browser-evidence`. The failure artifact includes those PNGs and the existing herdr logs, with hidden files explicitly enabled for the narrowly scoped `.ci` paths. Evidence is deliberately restricted to sticky-modifier and file-viewer checkpoints (eight viewport PNGs on a complete passing run). The UI suite has evidence-only assertions and viewport changes, and the key-bar demo adds viewport changes and font waits, so their evidence branches remain off in the lane. No live user session is captured: these scripts use isolated test panes and synthetic transcripts. Outside CI/CHECK_DIR, explicit local UI_EVIDENCE_DIR behavior is preserved. Remaining devswha#552 work: - Capture every open page at the exact failure point; the scripts have no common browser launcher/failure handler. - Playwright traces of failing contexts. - Timestamps on the web UI test servers' own lines; there is no common test-server output wrapper across these scripts. - Recording browser-received frames. - Multiple-run measurement for devswha#548 and decisions on devswha#553; this PR does not split the lane. - Investigating the seven historical fork runs with no jobs/logs. No CHANGELOG entry: CI/test-only changes are not user-visible, following devswha#548, devswha#581, and devswha#590. ## Validation - Four cheap `scripts/ci-browser.test.ts` tests exercise the real shell lane against stub commands: passing and failing summaries, exact exit 7, first-failure stop, temporary-directory cleanup, evidence filtering, and CI/CHECK_DIR/local behavior. The first three failed on the unchanged lane before implementation, then all four passed. - Captured a failing stub run: both passing steps and the failing step print numeric seconds, the EXIT summary includes all three, and the process exits 7. - `bash -n scripts/ci-browser.sh` and `git diff --check` passed. - `dori heavy tri-client3 -- bun run check fast` passed: actionlint, generated types, typecheck, build, and 1,932 unit tests passed, 0 failed (six existing skips). - The PR's CI browser lane will provide the real-surface timing proof and comparison against recent main runs. Refs devswha#552 * fix(alerts): one chime per alert across open tabs (devswha#632) ## What was wrong With the alert sound on and the app open in two tabs, every tab chimed for the same alert: each tab played its own audio and nothing told one tab what the other played (devswha#439 left the cross-tab half out because a Web Lock alone could drop a question no tab told). ## The rule (`src/lib/alertTurns.ts`) BroadcastChannel only, so it works on plain-HTTP LAN addresses where Web Locks do not exist. All times are this tab's own receive times, never a sender's. - A tab claims an alert (machine + pane + kind) only when it would chime it itself: alerts and sound on, not the pane in front of a visible tab, and its audio context running (`canPlayAlertSound`). A tab that would not chime never claims, so another tab chimes alone. - After a 150 ms window the lowest tab id among live claims plays and posts `chimed`; `chimed` cancels the others and settles that alert's claims. Only a `chimed` heard up to 400 ms before a tab's own alert counts as telling it, so the same pane can ask again soon after, and a tab ignores a late duplicate `chimed` for an alert it played itself. A `chimed` is sent only once the audio actually starts, also for a question queued behind another chime (`playAlertSound` now resolves when its notes begin). - No alert is dropped on a guess: a tab that deferred a question or a finish chimes it itself if no `chimed` arrives within 600 ms per lower claimant (so a third tab waits for the second), and at once when the winner withdraws (its audio could not play, or the tab is closing: `pagehide` withdraws its claims; `pageshow` reopens the channel). - No BroadcastChannel: chime at once, as before. A chime before the first tap or key is still skipped, never queued. Costs: an alert now waits about 150 ms for the window (up to a timer tick longer in a background tab). If timers in both tabs run more than 1.5 s late, both may chime: a double chime, never a lost question. ## Reproduction on b82584d The new two-tab case in `scripts/alert-sound-regression.ts` (which can now also run on its own) against the unchanged app: ```text $ dori heavy … bun run check run bun scripts/alert-sound-regression.ts PASS no alert sound before the page was tapped … (4 more PASS) two-tab shared alert: 2 chimes (4 notes) AssertionError: exactly one chime across two tabs for the same alert 4 !== 2 ``` The updated `src/lib/alertSound.test.ts` against the unchanged `alertSound.ts`: 10 pass, 4 fail (a queued question is confirmed only once it starts; an interrupted one is not). ## After - `HERDR_TEST_MODE=unit bun test ./src/lib/alertTurns.test.ts ./src/lib/alertSound.test.ts`: 28 pass, 0 fail. `bun run typecheck` clean. `bun run check fast` passed locally (1950 tests, 0 fail). - Browser, through `bun run check run bun scripts/alert-sound-regression.ts`: all 7 steps pass, including `two-tab shared alert: 1 chimes (2 notes)`, "two open tabs chime once for a shared alert" and "the other tab tells a question in front of the first tab". It passed twice: once in this worktree, and once with only the `src/` change applied to a clean checkout of b82584d. ## Test setup change The script's primer now waits (bounded) for each `working` event before reporting again. In one worktree under heavy load, two runs timed out at the first existing step, waiting for a `blocked` event that never reached the page. The same fix with the old primer passed in a clean checkout, so the fix did not cause it. The cause was not confirmed; the pacing is a robustness change only. Review rounds: see the Codex comments below (fixes bbb9f9f, 76450d2, 18ee46a and 1efe181; Lows listed there). ## Finishes get the safety net too (1efe181) A tab that deferred a finish used to drop it at the claim deadline, so a winning tab whose audio did not start, or that froze, left the finish unheard in every tab, where before this PR every tab chimed. A finish now waits like a question: the winner's withdraw redecides at once and the 600 ms rescue plays it if no `chimed` arrives. The winner confirms a finish as soon as it plays (`playAlertSound("done")` answers synchronously), so the rescue does not double it. The unit test "does not give a finish a question safety net", which pinned the old behavior, is replaced by "chimes a finish in another tab when the winning tab cannot play it", "chimes a finish once when the winning tab played it" and "rescues a finish when the winning tab never answers". Fixes devswha#449 ## A slow winner, and the pane's next question (after the CI failure on 1efe181) Run 37886117774 chimed one question twice in "two open tabs chime once for a shared alert". Two ways lead there, both fixed here: - The winner's `chimed` can come late: its audio waits behind a chime its tab still plays (a finish, a Settings preview), or the runner is slow. The other tab's 600 ms rescue then chimed the alert again. The winner now posts `playing` before its audio starts; a tab deferring that alert then waits for its `chimed` or `withdraw` (a withdraw still redecides at once), up to 5 s, after which it rescues. A winner that freezes mid-play therefore costs a late chime, never a lost one. - A tab that chimed the pane's previous question less than 1.5 s ago ignored every `chimed` for that pane and kind as a late duplicate, also the genuine one for the next question it was deferring (Codex's M2), and its rescue chimed again. A `chimed` now settles an alert this tab is waiting on; it is ignored as a duplicate only when nothing is pending. Tests: unit tests "takes another tab's chime of the pane's next question although it chimed the first one itself", "waits for a winner that is playing, however long its audio waits, and chimes once" (question and finish), "still rescues an alert whose winner said it was playing and then went silent" and "redecides at once when a winner that said it was playing withdraws". `scripts/alert-sound-regression.ts` gains "a winner that confirms late is not chimed over by the other tab", which holds every `chimed` back by a second in both tabs. * docs(readme): re-shoot the hero and the six feature clips on v0.4.0 (devswha#647) Re-shoots the README's hero video and all six feature clips on **v0.4.0**. That release reworked the UI: the two-list sidebar (Workspaces and Agents), the single status column, the composer with its + button, the greeting on an empty chat, and the key bar's Enter key. The old clips still showed v0.3.49 and the sidebar's Needs you list, which v0.4.0 removed. Docs only: WebPs, the four READMEs, `site/index.html`, and the hero's upload link in `scripts/build-site.ts`. No app code, no CHANGELOG, no version change. ## What changed | | Upload | Length | Change from the current README | |---|---|---|---| | Hero | [herdr-web-ui.mp4](https://github.com/user-attachments/assets/d854dbb6-64bd-4eba-81c7-fbd3f525726b) | 26.5 s | New story (see below) | | Switch to the live terminal | [upload](https://github.com/user-attachments/assets/32ca9aa4-960f-4629-9fb5-17c12ba35c80) | 11.5 s | The phone uses the key bar's new Enter | | Your herdr layout, in the browser | [upload](https://github.com/user-attachments/assets/22d10639-9aa6-4953-a5f3-a1b743f4053b) | 12.5 s | Status column replaces the READY/RUN/DONE badges; two-line rows | | Know when an agent needs you | [upload](https://github.com/user-attachments/assets/b228a2b8-6db5-4546-a15b-972056000cab) | 11.3 s | The Needs you list is gone; the desk shows the red ? and the card in the chat | | Send a screenshot from your phone | [upload](https://github.com/user-attachments/assets/a193b426-259c-47d9-bd73-10acc2594265) | 13.8 s | + replaces the paperclip; starts on the greeting | | Open what the agent made | [upload](https://github.com/user-attachments/assets/bc419d3e-ebda-4fb5-acc5-e498609559fe) | 10.0 s | Same beats on v0.4.0 | | Branch off a second agent | [upload](https://github.com/user-attachments/assets/150a4c9d-7667-4f63-ba56-8f91e6a25878) | 11.7 s | Reframed: the phone beside the browser's sidebar, ending on the nested row | **The hero, in order:** 1. herdr's own TUI, with Claude Code at work in checkout-api. 2. A dissolve into the browser's Terminal view of the same pane. Claude Code's screen stays still: both grids are 98x35 in the same cells. 3. The turn finishes in the browser, and a click on Chat shows the same session as a chat. 4. The camera pulls back to the phone, which shows the same chat. 5. On the phone: ⋯ → New worktree → Claude Code → Create. The browser's sidebar nests the new checkout. 6. It ends on the phone's greeting, "What should Claude do in worktree-…?". Both apps use their default themes: herdr's catppuccin and the web's amber. **Text changes:** - The hero's caption, `site/index.html`'s aria-label and show-note, and the site poster time (`at: "11.8"`, where both screens show the same chat). - All six alts in all four languages. Each one matches what its loop shows; attach's alt names the step that only the full video shows. - Two captions: the terminal one ("Enter beside it") and the attach one ("The + button"). - The footnote ("live recording … no cuts") still holds and is unchanged. **Checks:** - Each upload was fetched back: 200, a ranged GET gives 206, and its SHA-256 matches the local render. - `bun run build:site` passes. The hero downloaded at full size (3,964,804 B) and the poster was cut at 11.8 s. ## How it was made (disclosure) - **Real:** - herdr-web-ui **v0.4.0** (built `dist/`) on herdr 0.9.0, in the separate `herdr-web-ui-demo` session. - Real Claude Code 2.1.294 turns on a real account: 4 in the hero, 0–3 per clip take (worktree needs none). - Real git worktrees made by herdr, and real clicks and taps. - Recorded at the same moment on one clock (herdr's TUI, a 1280×800 desktop page and a 390×844 phone page) and played back 1:1, with no speed change and no time cuts. - The latest release is now v0.4.2; these videos were shot on v0.4.0. - **Staged:** - `checkout-api` is a small demo repo. web-dashboard (Codex · working), infra and release are props: their agent states are set through herdr's API. - Warm-up questions and tasks were sent through herdr's API off camera, not typed. Warm-ups also scroll Claude Code's header (plan line) off the screen. - Claude Code ran with acceptEdits, the publishing tools denied and spinner tips off. Bash and AskUserQuestion were denied where a clip doesn't need them (alerts keeps the question; attach's two later Bash prompts were approved off camera, after its cut ends). - The attach screenshot was rendered from the repo's own receipt page. - The phone is an emulated page on the same machine. - The host name reads "workstation", and paths read `~`. - **Composited:** - The wallpaper, the window and phone frames, and the camera moves. - The arrow and the tap disc, drawn at the recorded pointer or touch. - The hero's dissolve, a crossfade of two recordings of the same moment. For the film, herdr's terminal is xterm.js in a 1210×775 page and the browser's terminal font is 16 px (default 13), so the grids match. - **Privacy:** every frame of every render and its source takes passed the OCR check against the account, machine and network names. The flagged hits were OCR misreads of motion-blurred UI text, checked frame by frame. ## Known, left as the app does it - After Send, "No conversation yet" shows for about 0.2 s. - Switching to Chat shows "Loading conversation…" for 4–7 frames. - A new worktree's pane goes through its shell and terminal before its chat. In the hero this happens small, during a camera move. - Chat bubbles show a clock time. These could be app issues, but this PR does not touch the app. ## For the README session picking this up - Text that is translated by hand and worth a native read: the ko/ja/zh-CN alts and captions in README.ko.md, README.ja.md and README.zh-CN.md. - Source material lives in the film worktree, `_film/renders/herdr-web-ui-<clip>.mp4`. The film tooling (`scripts/film/…`) is uncommitted there and is not part of this PR. - Re-cutting a loop: `bun scripts/film/webp.ts scripts/film/cut-<clip>.ts --q 85 --out docs/media/readme/<clip>.webp`, run in that worktree. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Chinese page (57f0d64) `site/zh/index.html` gets the same update as `site/index.html`: the six clips link to their new uploads with the alts and captions from `README.zh-CN.md`, and the hero's aria-label and note tell the new story. Before this, the Chinese page would have kept the old upload links, the Needs you and paperclip alts, and the health.ts hero text. `bun scripts/build-site.ts` passes; `/zh/` renders the new clips with no 4xx or page errors. --------- Co-authored-by: e2e <e2e@example.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * fix(chat): read a Claude pane's session on a Mac whose locale puts the day first (devswha#646) ## Change On a Mac whose language puts the day before the month (English (UK), for one), every Claude pane's chat showed "Conversation unavailable — show terminal output" when herdr's Claude integration is not installed. Without the hook, the server finds the session through Claude's native PID record (`~/.claude/sessions/<pid>.json`) and accepts it only when its `procStart` equals `ps -o lstart` for that PID. Claude Code records the start in the C locale's order, but the server ran `ps` in its own locale: | | | |---|---| | `procStart` Claude 2.1.285 wrote | `Fri Oct 9 04:43:00 2026` | | `ps -o lstart` under `LANG=en_GB.UTF-8` | `Fri 9 Oct 04:43:00 2026` | So the record looked like a reused PID's, the pane had no session (`no_session_id`) and the chat fell back to the scrollback. `darwinProcessStart` now runs `ps` with `LC_ALL=C`, as `gjc-runtime.ts` already does since devswha#484. Linux (`/proc` ticks) and Windows (FILETIME) compare numbers and are unaffected. The test fixture built `procStart` with the same locale-dependent `ps` call as the code, so it passed in every locale. It now writes the record as Claude does, and a new macOS test runs the check under `en_GB.UTF-8`. ## Validation - `HERDR_TEST_MODE=unit bun test ./server/claude-store.test.ts` on macOS with `LANG=en_GB.UTF-8`: before the fix, "reads an exact live PID's session" and the new "reads it on a Mac whose locale puts the day first" fail (`Received: null`); after it, 35 pass. CI runs no unit tests on macOS, so these two skip there. - Against a live Claude Code 2.1.285 pane with no herdr hook: `claudeProcessSession` now returns the session and `claudeTranscriptFile` finds its `.jsonl`. - `bun run check fast`: generated types, typecheck and build pass. `test:unit` has 1926 pass and 3 fail, all in `scripts/check.test.ts` > `isolate`. They fail the same way on unchanged `main` on this Mac, because the temp directory makes the herdr socket path too long (120 > 100). * fix(plugin): discover Bun when launched with a minimal PATH (devswha#648) ## Change Closes devswha#637 - Unix build/start/action/phone 명령과 preflight가 같은 Bun 탐색을 사용합니다. - 기존 PATH 우선순위와 인자 경계를 보존하며 Windows 명령과 Bun 최소 버전 검사는 유지합니다. ## Validation 경로·인자·manifest 테스트 11개, 타입·빌드·생성 타입 검사 및 실제 Phone entrypoint 계약 검사 통과. 실제 `env -i HOME="$HOME" PATH=/usr/bin:/bin sh scripts/with-bun.sh --version`은 `1.4.2`를 반환했습니다. 전체 단위 실행은 1938 통과·33 생략·1 실패였습니다. 실패는 변경하지 않은 `server/collector.test.ts`의 recovery 타이밍 테스트이며, 분리 실행한 해당 파일 27개는 통과했습니다. 같은 collector의 다른 브랜치 전체 실행도 통과했으며 이 PR에서 해당 테스트를 바꾸지 않았습니다. Windows 실제 실행은 검증하지 않았습니다. 모든 검증은 별도 작업 공간과 소유한 테스트/demo 환경에서 수행했습니다. 사용자의 실행 중 앱·terminal은 변경하지 않았습니다. --------- Co-authored-by: studiood <253281802+od-studio-webagency@users.noreply.github.com> Co-authored-by: Hako <devswha@gmail.com> * fix(terminal): a tab you are not using leaves the pane's size alone (devswha#658) ## Problem A herdr-web-ui tab left open on a desktop took a shared pane's size even while nobody was using it. The terminal lens re-asserts its grid on `visibilitychange`, on `role-ack` (every reconnect), from its ResizeObserver, and through the attach a reconnect replays. A window that only turned visible behind another app counted, for example when the screen woke. So did one that reconnected in the background. Seen on a MacBook running herdr's own TUI in Ghostty, with the pane split to about 105 columns: 1. The phone opened the pane in the terminal lens (54×62). 2. The user came back to herdr in Ghostty. 3. A Chrome tab on the same pane, sitting behind Ghostty, turned visible and resized the pane to its own 202×54. 4. Herdr's TUI then drew the 202-column pane inside its 105-column split, cut off at the edge. The sidecar's argv (`pty-host.mjs 202 54 …`) showed the size came from that tab. Herdr 0.9.3 keeps an attach's size after it leaves, so the pane stayed that way. ## Change `PaneTerminal` drives the shared grid only while the tab is in use, meaning `document.visibilityState === "visible"` and `document.hasFocus()`: - `visibilitychange` refits only when the tab is visible with the focus. The window's `focus` still refits, so going back to a tab takes the pane back as before. - `role-ack` refits only in use. Out of use, the refit waits for the user. - The ResizeObserver still fits xterm, but sends `resize` only in use, so a window the system moves or resizes in the background does not take the pane. - Out of use (window `blur`, or a hidden or unfocused `visibilitychange`), the pane is marked `keepSize`. A reconnect in the background then attaches with `keep_size` and adopts the pane's size, as the chat lens already does. The next refit clears it. - A pane attach out of use (a reload behind another app, or the next pane after the one shown closed in herdr) goes with `keep_size` too. The lens effect, which runs on every load, and a chosen font's load resize the pty only in use, so they don't take the pane right after that attach. Out of use they fit only xterm. There are no server or protocol changes. Not addressed here: after a phone uses a pane, herdr's TUI still shows it at the phone's size until something resizes it. Giving a pane back to herdr's layout size when no tab is in use would need a new frame. The server also can't tell whether a native TUI is attached, so on a headless herdr that would resize panes to herdr's default layout. I left that for a separate discussion. ## Validation - New browser check `checkBackgroundTabKeepsTerminalSize` (`scripts/chat-size-regression.ts`, run from `ui-regression.ts`). It uses a desktop page and a phone page on one owned pane, and a second pane the desktop moves on to: - The phone takes the grid. - The desktop turns visible without the focus: no `resize`, and the pane keeps the phone's size (`stty size`). - The desktop reconnects in the background: the only frame is `attach` with `keep_size: true`, and the pane keeps the phone's size. - The desktop reloads in the background, with a chosen terminal font: the only frame is `attach` with `keep_size: true`, and the pane keeps the phone's size. - The pane closes in herdr while the desktop is in the background (the phone has left): the desktop moves on to herdr's focused pane with an `attach` with `keep_size: true`, and that pane keeps its size. - The desktop takes the focus: that pane goes to the desktop's size. - The check fails on the previous `PaneTerminal.tsx` ("a window shown without the focus sends no resize") and passes with this change. Each later step fails with its own fix undone (the attach, the font load, the lens effect). - `checkChatKeepsTerminalSize` and `checkPaneSwitchKeepsTerminalSize` still pass. `document.hasFocus()` is `true` in Playwright pages (focus emulation), so the existing browser checks keep their behavior. - `bun run check fast`: generated types, typecheck and build pass. Unit tests (with current `main` merged): 1964 pass, 3 fail. The 3 failures are `scripts/check.test.ts > isolate`, which fail on unchanged `main` on this Mac because the macOS temp dir makes the socket path too long (120 > 100). - `scripts/ui-regression.ts` via `bun run check run`: exit 0, 93 PASS lines. - I haven't tried it on a real iPhone. Returning to the app there should refit through `visibilitychange` with the focus, or through the window's `focus`. A page opened fresh while `document.hasFocus()` is `false` attaches with `keep_size` and waits for that `focus` too. --------- Co-authored-by: Hako <devswha@gmail.com> --------- Co-authored-by: Hako <devswha@gmail.com> Co-authored-by: e2e <e2e@example.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: studiood <studio-od@naver.com> Co-authored-by: studiood <253281802+od-studio-webagency@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The failure
"Fast checks" failed on main at 57d7663 (run 37730991278) on
parseClaudeTranscript > strips escapes in time linear in a malformed answer's length(server/conversation.test.ts, added by #588):The same code was green twice on the PR.
Why it flaked
The parse of either input takes well under a millisecond, so the assertion divided two sub-millisecond timings with a 0.05 ms floor. One scheduler pause in the large run, or a small run that hit the floor, moves the ratio past 3 with no change in the code. A required check that depends on that blocks every PR.
What the test asserts now
No clock is read. While
parseClaudeTranscriptruns, the test wraps the primitives the scanner reads the answer through and counts the characters each one touches:charCodeAt: 1indexOf: the distance scanned plus the needlestartsWith: the needleslice: the copy's lengthRegExp.prototype.exec, whichtestandreplacego through once it is wrapped): the square of its input. A regex's backtracking cannot be counted from outside, so it is charged its worst case. The scanner's/\s/over one character costs one step; a regex over the whole answer does not pass.It then asserts, for both input shapes (unterminated escapes
"\u001b]x"and closing tags the output prints itself), that doubling the input from 20,000 to 40,000 units grows the count by less than 2.5x. Measured on the current scanner: 240,044 to 480,044 steps (2.00x) and 2,424,046 to 4,844,046 (2.00x). The originals are put back in afinally. A count of zero gives NaN and fails too.Test-only:
server/conversation.tsis untouched, so no CHANGELOG line.Proof
localCommandOutputfrom 5979118 put back, the test fails:Expected: < 2.5, Received: 7.999. The source was then restored andcmpagainstHEADreported it identical.content.slice(next)at every closing tag to the current scanner:Expected: < 2.5, Received: 3.999, with the escape shape still passing. Restored,cmpidentical.HERDR_TEST_MODE=unit bun test ./server/conversation.test.ts -t "strips escapes"run 200 times, eight at a time: 200 passed.bun run check fast: ok (1726 pass, 0 fail).Note for review
The regex charge means this test also fails if the scanner goes back to running a regex over the whole answer, linear or not. That is deliberate (it is the only way a count can see the pre-#588 cost), and the comment in the test says so.