fix(cli): stabilize packaged PTY smoke test - #13474
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 files)
Previous Review Summary (commit e84e232)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit e84e232)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Reviewed by grok-4.6 · Input: 150.3K · Output: 6.1K · Cached: 70.1K Review guidance: REVIEW.md from base branch |
| const exited = Promise.withResolvers<number>() | ||
| const data = proc.onData((chunk) => { | ||
| state.output += chunk | ||
| if (/(?:^|[\r\n])KILO_PTY_READY(?:\r?\n|$)/.test(state.output)) output.resolve() |
There was a problem hiding this comment.
OK, if it's multiline I think the /m at the end of the regex is needed, I would even go /gm to be sure on multiple lines.
If this output is meant to be a "maybe with new lines around" situation, but the only thing that should be there is KILO_PTY_READY, then I'd use state.output.trim() === "KILO_PTY_READY" instead and remove the regex complexity.
| const data = proc.onData((chunk) => { | ||
| state.output += chunk | ||
| if (/(?:^|[\r\n])KILO_PTY_READY(?:\r?\n|$)/.test(state.output)) output.resolve() | ||
| if (marker(state.output)) output.resolve() |
There was a problem hiding this comment.
looks like this should be an else if as if already resolved in previous if there's no point in re-resolving and checking marker at all.
3 participants
What Problem This Solves
The v7.5.1 prerelease run failed in CLI artifact validation after PR #13472 merged. The packaged binaries built successfully, but the new PTY smoke test produced false failures on Windows x64, macOS x64, and glibc Linux runners.
Why This Change Was Made
PowerShell decorates the marker output with terminal control sequences, so the exact line matcher did not detect output that was visibly present in the log. Standard release runners also need more than 15 seconds to cold-start the larger non-split binary and reach the visible TUI frame.
This strips CSI, OSC, DCS, and related escape sequences before matching the PTY marker, while keeping the shell check at 15 seconds. Only the packaged TUI cold-start deadline increases to 60 seconds.
User Impact
No runtime behavior changes. The prerelease gate continues to require a visible packaged TUI frame without failing on terminal formatting or slower cold starts.
Evidence
98204019815.--pure __pty-smokereached the visible TUI prompt.git diff --checkpassed.Failed run: https://github.com/Kilo-Org/kilocode/actions/runs/32976503200