Skip to content

fix(ci): stabilize visual regression runner and flaky PTY and TUI tests - #14047

Merged
marius-kilocode merged 4 commits into
mainfrom
fix-kilo-vscode-visual-regression-ci
Sep 11, 2026
Merged

marius-kilocode merged 4 commits into
mainfrom
fix-kilo-vscode-visual-regression-ci

Conversation

@marius-kilocode

@marius-kilocode marius-kilocode commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

What Problem This Solves

Three unrelated CI failures kept the pipeline red:

  1. Both visual regression jobs failed in Install Playwright system deps on blacksmith-4vcpu-ubuntu-2404. apt read mirror+file:/etc/apt/blacksmith-ubuntu-mirrors.txt and then could not connect to archive.ubuntu.com or security.ubuntu.com on port 80, so bunx playwright install-deps chromium exited with code 100 before any test ran (run 34574732611).
  2. unit (macos) timed out in packages/tui/test/cli/tui/question-custom-answer.test.tsx. The test pressed keys after a fixed 50 ms sleep, but the provider tree mounts the prompt asynchronously, so under load the keys hit nothing and the textarea focus wait expired.
  3. unit (macos) timed out in packages/core/test/pty/pty-session.test.ts (replays exited output and ends when enabled, terminates background descendants, and others) with missing exit events or empty output. This also failed on main (runs 34509521842 and 34507254245).

Why This Change Was Made

Runner. The Playwright deps step cannot be removed; the failing log lists nine packages, including fonts, that are not on the image. Retries do not help a persistent connectivity failure and browser caching does not cover system libraries. The two jobs now run on GitHub-hosted ubuntu-24.04, which takes a different network path to the Ubuntu package servers. This is marked as temporary in the workflow, with the failing run referenced, so the Blacksmith expression can be restored once the mirror path is confirmed working. The regenerated baselines in this PR come from the new runner.

TUI test. The test now renders until the options are on screen before pressing keys. Under 10x parallel load the original failed 2 of 10 runs; the fixed test passed 10 of 10.

PTY. bun-pty fires onData and onExit exactly once from its read loop and drops events while no listener is attached. Two races lost them for short-lived children such as sh -c 'printf x; exit 7', and also affect real Kilo terminals:

  • Pty.create registers its listeners after yield* Effect.sync(spawn). Measured gaps of 20-100 ms at that point under load (the fiber yields via setImmediate), long enough for the child to exit. packages/core/src/kilocode/pty/latch.ts buffers early events and replays them once a listener attaches; pty.bun.ts wraps the adapter with it.
  • bun-pty's Terminal constructor runs the first bun_pty_read synchronously. If the JS thread is descheduled between bun_pty_spawn and that read, the Rust reader thread has already queued the child's output and the first iteration fires with zero listeners. Diagnostics showed the shell alive in wait with status running and output "". patches/bun-pty@0.4.8.patch defers the first read by one microtask; this is a candidate for an upstream bun-pty fix.

Under 6x parallel load of the affected pty tests: 8 of 24 runs failed before, 1 of 24 with the latch only, 36 of 36 with both changes. The full pty suites passed 72 of 72 under the same load. test/kilocode/pty-latch.test.ts reproduces the late-attach case deterministically and fails without the latch.

User Impact

Short-lived commands in Kilo terminals (for example Agent Manager scripts) no longer lose their output or fail to report an exit. Everything else is CI only.

Evidence

  • bun run script/check-workflows.ts: ok (29 workflows)
  • actionlint .github/workflows/visual-regression.yml: passed
  • bun run script/check-opencode-annotations.ts --worktree: passed
  • bun run typecheck in packages/core and packages/tui: passed
  • Visual Regression (kilo-ui) and (kilo-vscode webview) passed on the new runner in run 34579076231

Both visual regression jobs failed in bunx playwright install-deps on
blacksmith-4vcpu-ubuntu-2404 because apt could not reach the Ubuntu
package servers through the Blacksmith mirror list (run 34574732611).
Switch these two jobs to ubuntu-24.04 until Blacksmith confirms a fix.
@kilo-code-bot

kilo-code-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (8 files)
  • .changeset/pty-early-output.md
  • bun.lock
  • package.json
  • packages/core/src/kilocode/pty/latch.ts
  • packages/core/src/pty/pty.bun.ts
  • packages/core/test/kilocode/pty-latch.test.ts
  • packages/tui/test/cli/tui/question-custom-answer.test.tsx
  • patches/bun-pty@0.4.8.patch
Previous Review Summaries (2 snapshots, latest commit 38d4728)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 38d4728)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • .github/workflows/visual-regression.yml
  • packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/agentmanager/full-screen-diff-with-changes-chromium-linux.png (Git LFS baseline pointer)

Previous review (commit b8a8ae2)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)
  • .github/workflows/visual-regression.yml

Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

kilo-maintainer Bot and others added 3 commits September 11, 2026 08:24
The custom answer test pressed keys after a fixed 50 ms sleep, but the
provider tree mounts the prompt asynchronously. Under load the options were
not on screen yet, the keys hit nothing, and the textarea focus wait timed
out on the macOS runner. Render until the options are visible instead.
bun-pty fires onData and onExit once from its read loop and drops events
that arrive while no listener is attached. Two races lost them for short
lived children such as sh -c 'printf x; exit 7':

- The Pty service registers its listeners after yield* Effect.sync(spawn),
  and the fiber can yield there for 20-100 ms under load. Buffer early
  events in a latch and replay them when a listener attaches.
- The bun-pty Terminal constructor runs the first read synchronously, so
  output the reader thread queued while the child started fired with no
  listeners at all. Patch bun-pty to defer the first read by a microtask.

Under 6x parallel load the pty tests failed 8 of 24 runs before and pass
36 of 36 with both changes.
@marius-kilocode marius-kilocode changed the title chore(ci): use GitHub-hosted runner for visual regression fix(ci): stabilize visual regression runner and flaky PTY and TUI tests Sep 11, 2026
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.

2 participants