-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(cli): stabilize packaged PTY smoke test #13474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,16 @@ import { KiloPtyTermination } from "./termination" | |
| import { spawn } from "#pty" | ||
|
|
||
| const TIMEOUT = 15_000 | ||
| const RENDER_TIMEOUT = 60_000 | ||
|
|
||
| export function marker(output: string) { | ||
| const text = output | ||
| .replace(/\x1b\](?:[^\x07\x1b]|\x1b(?!\\))*(?:\x07|\x1b\\)/g, "") | ||
| .replace(/\x1b[P^_](?:[^\x1b]|\x1b(?!\\))*\x1b\\/g, "") | ||
| .replace(/\x1b\[[0-?]*[ -/]*[@-~]/g, "") | ||
| .replace(/\x1b[@-_]/g, "") | ||
| return text.split(/\r?\n/).some((line) => line.trim() === "KILO_PTY_READY") | ||
| } | ||
|
|
||
| async function render() { | ||
| const proc = spawn(process.execPath, ["--pure"], { | ||
|
|
@@ -34,15 +44,18 @@ async function render() { | |
| state.exited = true | ||
| ready.reject(new Error(`TUI exited before rendering (code ${event.exitCode}): ${JSON.stringify(state.output)}`)) | ||
| }) | ||
| const timeout = AbortSignal.timeout(TIMEOUT) | ||
| const timeout = AbortSignal.timeout(RENDER_TIMEOUT) | ||
|
|
||
| try { | ||
| await Promise.race([ | ||
| ready.promise, | ||
| new Promise<never>((_, reject) => | ||
| timeout.addEventListener( | ||
| "abort", | ||
| () => reject(new Error(`TUI produced no rendered frame within ${TIMEOUT}ms: ${JSON.stringify(state.output)}`)), | ||
| () => | ||
| reject( | ||
| new Error(`TUI produced no rendered frame within ${RENDER_TIMEOUT}ms: ${JSON.stringify(state.output)}`), | ||
| ), | ||
| { once: true }, | ||
| ), | ||
| ), | ||
|
|
@@ -67,7 +80,7 @@ export async function smoke() { | |
| 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() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, if it's multiline I think the 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 |
||
| if (marker(state.output)) output.resolve() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like this should be an |
||
| }) | ||
| const exit = proc.onExit((event) => { | ||
| state.exited = true | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import { describe, expect, test } from "bun:test" | ||
| import { marker } from "../../src/kilocode/pty/smoke" | ||
|
|
||
| describe("PTY smoke output", () => { | ||
| test("detects a marker after PowerShell formatting", () => { | ||
| const output = | ||
| "\x1b[93mecho KILO_PTY_READY\r\n\x1b[mKILO_PTY_READY\r\n\x1b]0;Administrator: PowerShell\x07PS> " | ||
|
|
||
| expect(marker(output)).toBe(true) | ||
| }) | ||
|
|
||
| test("does not accept the echoed command", () => { | ||
| expect(marker("\x1b[93mecho KILO_PTY_READY\r\n\x1b[mPS> ")).toBe(false) | ||
| }) | ||
|
|
||
| test("detects a marker around OSC and DCS sequences", () => { | ||
| const output = "\x1b]133;A\x07\x1bP+q4d73\x1b\\KILO_PTY_READY\r\n" | ||
|
|
||
| expect(marker(output)).toBe(true) | ||
| }) | ||
| }) |
Uh oh!
There was an error while loading. Please reload this page.