Skip to content

Revert "fix: solve #3028 — forward DA1 query responses during shell init" #3127

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.

Sign up for GitHub

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

Merged
Kitenite merged 1 commit into from
Apr 3, 2026
Merged

Revert "fix: solve #3028 — forward DA1 query responses during shell init" #3127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 0 additions & 51 deletions apps/desktop/src/main/terminal-host/session-shell-ready.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,57 +301,6 @@ describe("Session shell-ready: kill/exit before readiness", () => {
});
});

/** Wait for the emulator write queue to drain (uses setImmediate internally). */
function waitForEmulatorFlush(): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, 50));
}

describe("Session shell-ready: DA1 query response forwarding (#3028)", () => {
it("forwards headless emulator DA1 response to subprocess during pending state", async () => {
// Fish shell sends DA1 (ESC[c) at startup to detect terminal capabilities.
// The headless emulator generates a response (ESC[?1;2c) via xterm.js.
// This response MUST be forwarded to the subprocess even during the
// "pending" shell-ready state, otherwise fish waits 10s and then
// disables optional features like cursor shape and reflow detection.
const { session, proc } = createTestSession("/usr/local/bin/fish");
spawnAndReady(session, proc);

// Simulate PTY output containing a DA1 query from fish.
// When the headless emulator processes this, xterm.js generates
// a DA1 response via its onData callback.
sendData(proc, "\x1b[c");

// The emulator write queue processes via setImmediate, so we need
// to let the event loop tick for xterm to process the query.
await waitForEmulatorFlush();

// The emulator's DA1 response should have been forwarded to the
// subprocess (written to stdin) even though shell is still pending.
const writes = getWrittenData(proc);

// The response should contain a DA1 reply (ESC[?...c format)
expect(writes.length).toBeGreaterThan(0);
const da1Response = writes.join("");
// biome-ignore lint/suspicious/noControlCharactersInRegex: matching ESC in terminal protocol data
expect(da1Response).toMatch(/\x1b\[\?[\d;]+c/);
});

it("forwards DSR response to subprocess during pending state", async () => {
const { session, proc } = createTestSession("/bin/zsh");
spawnAndReady(session, proc);

// DSR (Device Status Report): ESC[5n → ESC[0n (terminal OK)
sendData(proc, "\x1b[5n");

await waitForEmulatorFlush();

const writes = getWrittenData(proc);
expect(writes.length).toBeGreaterThan(0);
const response = writes.join("");
expect(response).toContain("\x1b[0n");
});
});

describe("Session shell-ready: supported shells", () => {
for (const shell of [
"/bin/zsh",
Expand Down
15 changes: 6 additions & 9 deletions apps/desktop/src/main/terminal-host/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,21 +224,18 @@ export class Session {
// Set initial CWD
this.emulator.setCwd(options.cwd);

// The headless emulator responds to terminal queries (e.g. DA1,
// DSR) when no renderer client is attached. These responses must
// be forwarded to the subprocess immediately — even during shell
// init — because shells like fish send DA1 at startup and wait
// up to 10 seconds for a reply before disabling optional features.
// Unlike renderer-generated responses (which go through write()
// and are correctly dropped during init to avoid appearing as
// typed text), headless emulator responses are written directly
// to the PTY and consumed by the shell as protocol data.
// The headless emulator responds to terminal queries (e.g. DA)
// when no renderer client is attached. During shell init we drop
// these — they'd land in the pre-ready stdin queue and appear as
// typed text like "?62;4;9;22c" once flushed. After a client
// attaches the renderer's xterm handles all terminal queries.
Comment on lines +227 to +231

@greptile-apps greptile-apps Bot Apr 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Inaccurate comment — sendWriteToSubprocess bypasses the preReadyStdinQueue

The comment justifies dropping headless-emulator responses during pending by saying they'd "land in the pre-ready stdin queue and appear as typed text like ?62;4;9;22c once flushed." This is factually incorrect.

The onData callback calls this.sendWriteToSubprocess(data) directly, not this.write(data). Only write() routes through preReadyStdinQueue. The emulator's response would be sent straight to the subprocess's stdin — it would never touch the pre-ready queue. If the real motivation is something else (e.g. the headless emulator generates an incorrect or incomplete DA1 response that confuses fish/zsh), the comment should say so.

this.emulator.onData((data) => {
if (
this.attachedClients.size === 0 &&
this.subprocess &&
this.subprocessReady
) {
if (this.shellReadyState === "pending") return;

@greptile-apps greptile-apps Bot Apr 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Reintroduces fish shell 10-second startup delay (#3028)

By returning early when shellReadyState === "pending", this change drops all headless-emulator responses (DA1, DSR, etc.) during the critical shell-init window. Fish shell sends a DA1 query (\x1b[c) at startup and waits up to 10 seconds for a response before disabling cursor-shape changes and reflow detection.

The SHELL_READY_TIMEOUT_MS is 15 seconds, so when a session starts without a renderer client attached, fish's 10-second DA1 timeout will always fire before the shell-ready marker arrives, degrading the shell experience every single headless startup.

The original PR #3030 was specifically introduced to fix this. If this revert is needed because #3030 introduced a separate regression, that regression should be documented here (and ideally addressed separately) so the trade-off is clear.

this.sendWriteToSubprocess(data);
}
});
Expand Down
Loading