Skip to content

test(chat): the pending-input recorder polls its screen instead of trusting fs.watch - #756

Merged
devswha merged 2 commits into
mainfrom
test/pending-input-screen-flake
Oct 11, 2026
Merged

devswha merged 2 commits into
mainfrom
test/pending-input-screen-flake

Conversation

@devswha

@devswha devswha commented Oct 11, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes a recurring CI flake: server/pending-input.contract.test.ts failed with "recorder screen did not update" (main Website deploy run 38011364808, and 38044126400), unrelated to the PRs it hit.

  • The recorder fixture redrew its screen from fs.watch. Bun's watcher can drop the change that follows writeFileSync's truncation, leaving the empty file it read in between on screen. Locally, truncate-then-write lost the final event 32/200 times (3 ended on the empty read). The recorder now polls its screen file every 20 ms and redraws on change.
  • showScreen now also waits for the pane's ANSI rendering to change: two screens (lines 271, 323) end with the same footer once the trailing-space marker is trimmed, so the wait returned before the redraw and the next submit raced it. The poll made that race visible.
  • scripts/ui-regression.ts's pending-agent fixture used the same watcher and gets the same poll.

Test-only change; no changelog entry.

Verification

  • Before: the targeted tests pass 40/40 on an idle machine (the flake needs CI load).
  • After: bun run check run with the full pending-input.contract.test.ts repeated under 28 CPU burners on an isolated herdr session (results in the PR thread).
  • bun run typecheck passes.

…usting fs.watch

The recorder fixture redrew its screen from fs.watch. Bun's watcher can
drop the change event that follows writeFileSync's truncation, so the
recorder sometimes drew the empty file it read in between and never drew
the new screen: "recorder screen did not update" (CI runs 38011364808 and
38044126400). Locally, truncate-then-write lost the final event 32 times
in 200, three of them left on the empty read.

The recorder now polls the file every 20 ms and redraws on any change, so
a read during the truncation is corrected on the next tick. showScreen
also waits for the pane's rendering to change: two screens end with the
same footer (the trailing-space marker is trimmed away), so it returned
before the redraw and the next submit raced it.

The browser suite's pending-agent fixture had the same watcher and gets
the same poll.
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 398be589-958c-4b2a-82c4-c040b50b5c9a

📥 Commits

Reviewing files that changed from the base of the PR and between 3fddbf7 and 4bd9c2e.


📒 Files selected for processing (2)
  • scripts/ui-regression.ts
  • server/pending-input.contract.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Summary

Summary by CodeRabbit

  • Tests
    • Improved terminal-screen update checks across regression and pending-input tests. Screens now refresh when their contents change, and checks wait for both a screen update and the expected text to appear before confirming success. This makes test results more reliably reflect what is visibly displayed.

Walkthrough

The pending-agent fixture and contract test now poll the screen file every 20 ms and redraw when its contents change. The contract test also waits for a changed ANSI screen and the expected visible text suffix.

Changes

Pending-agent screen updates

Layer / File(s) Summary
Poll and redraw screen
scripts/ui-regression.ts, server/pending-input.contract.test.ts
Both screen recorders poll every 20 ms and redraw when the screen file contents change.
Wait for visible screen change
server/pending-input.contract.test.ts
showScreen waits for a changed ANSI screen and visible text ending with the expected suffix.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: yoonwoo-ha


Merge Risk: ⚪ Minimal · up to 4bd9c

The test fixtures now poll screen files and wait for visible updates. In the inspected flows, required screen states are not replaced before they reach the pane, leaving no identified merge-blocking risk.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: replacing fs.watch with polling for the pending-input recorder.
Description check Passed The description explains the CI flake, resulting behavior, affected fixtures, test-only scope, and validation results. It uses Summary and Verification headings instead of the template's Change and Va…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR





🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

…reen-flake

# Conflicts:
#	server/pending-input.contract.test.ts
@devswha
devswha merged commit cb49d69 into main Oct 11, 2026
13 checks passed
@devswha
devswha deleted the test/pending-input-screen-flake branch October 11, 2026 06:10
devswha added a commit that referenced this pull request Oct 11, 2026
## Change

Release 0.4.7, from main at 80ca9bb (47 PRs since v0.4.6).

- `package.json` and `herdr-plugin.toml`: 0.4.7.
- CHANGELOG: `[Unreleased]` becomes `[0.4.7] - 2026-10-12`, with the
compare links.
- #513's entry moves out of the 0.4.0 section, where its branch's merge
left it, and gets its credit; it says the pace is a setting, off by
default (#834).
- A Maintenance section covers the PRs that had no entry: #743, #756,
#767, #795, #805, #810, #813, #821, #826, #831.
- The note under the Hermes entry that asked for a runtime bump at
release now says what a remote PC needs.
- `release-summaries.json`: 0.4.7 in en/ko/ja/zh/tr (6 new, 6 improved,
5 fixed). The Turkish lines are not checked by a native speaker.
- `REMOTE_BUNDLE_VERSION` 25 -> 26. Since v0.4.5's runtime the bridge
gained Hermes transcripts (#604), conversation pushes (#689) and
Grok/Antigravity activity (#816), so `remote-v26` is published before
the app release.

Not in this release: #606.

## Validation

- `bun scripts/release-notes.ts 0.4.7` renders the summaries and the
full changelog (235 lines).
- `bun run typecheck` and `bun run build`: pass.
- `TMPDIR=/tmp bun run test:unit`: one failure, `server/voice.test.ts`
("audio.webm" vs "clip.webm"), which fails the same way on unmodified
`main` on this Mac (Bun 1.4.0); CI runs Bun 1.4.2.
- Every PR merged since v0.4.6 was checked against the changelog
section: all 47 are linked.

## Release

After this merges and its CI on `main` passes: push `remote-v26`, wait
for its release, then run the Release workflow with `0.4.7`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant