diff --git a/.changeset/steady-jobs-resume.md b/.changeset/steady-jobs-resume.md new file mode 100644 index 000000000..23efd2b4a --- /dev/null +++ b/.changeset/steady-jobs-resume.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Keep suspended Hunk jobs alive so `fg` restores the TUI and its in-progress state. diff --git a/packages/hunk/src/core/process/jobControl.test.ts b/packages/hunk/src/core/process/jobControl.test.ts index 3a0170306..eb12c3a35 100644 --- a/packages/hunk/src/core/process/jobControl.test.ts +++ b/packages/hunk/src/core/process/jobControl.test.ts @@ -57,54 +57,6 @@ function createMockRenderer() { }; } -function createSignalHarness() { - const listeners = new Map void>>(); - const onceWrappers = new Map<() => void, () => void>(); - const removed: NodeJS.Signals[] = []; - - return { - emit(signal: NodeJS.Signals) { - const signalListeners = listeners.get(signal); - if (!signalListeners) { - return; - } - - const snapshot = Array.from(signalListeners); - for (const listener of snapshot) { - listener(); - } - }, - listenerCount(signal: NodeJS.Signals) { - return listeners.get(signal)?.size ?? 0; - }, - off(signal: NodeJS.Signals, listener: () => void) { - removed.push(signal); - listeners.get(signal)?.delete(listener); - const wrapped = onceWrappers.get(listener); - if (wrapped) { - listeners.get(signal)?.delete(wrapped); - onceWrappers.delete(listener); - } - }, - once(signal: NodeJS.Signals, listener: () => void) { - const wrapped = () => { - listeners.get(signal)?.delete(wrapped); - onceWrappers.delete(listener); - listener(); - }; - onceWrappers.set(listener, wrapped); - - let signalListeners = listeners.get(signal); - if (!signalListeners) { - signalListeners = new Set(); - listeners.set(signal, signalListeners); - } - signalListeners.add(wrapped); - }, - removed, - }; -} - describe("installJobControlInterruptSupport", () => { test("routes Ctrl-C through the provided shutdown callback", () => { const renderer = createMockRenderer(); @@ -185,45 +137,40 @@ describe("installJobControlSuspendSupport", () => { expect(sentSignals).toEqual([]); }); - test("suspends the foreground process group on Ctrl-Z and resumes on SIGCONT", () => { + test("suspends the foreground process group on Ctrl-Z and resumes once the job continues", () => { const renderer = createMockRenderer(); - const signals = createSignalHarness(); const sentSignals: Array<{ pid: number; signal: NodeJS.Signals }> = []; installJobControlSuspendSupport(renderer, { - kill: (pid, signal) => sentSignals.push({ pid, signal }), - off: signals.off, - once: signals.once, + kill: (pid, signal) => { + // Stands in for the stopped process: the renderer stays suspended until kill returns. + sentSignals.push({ pid, signal }); + expect(renderer.suspendCalls).toBe(1); + expect(renderer.resumeCalls).toBe(0); + }, platform: "linux", }); const ctrlZ = createTestKey({ ctrl: true, name: "z" }); renderer.emitKeypress(ctrlZ); + expect(ctrlZ.defaultPrevented).toBe(true); expect(ctrlZ.propagationStopped).toBe(true); - expect(renderer.suspendCalls).toBe(1); - expect(signals.listenerCount("SIGCONT")).toBe(1); expect(sentSignals).toEqual([{ pid: 0, signal: "SIGTSTP" }]); - - signals.emit("SIGCONT"); expect(renderer.resumeCalls).toBe(1); - expect(signals.listenerCount("SIGCONT")).toBe(0); }); - test("does not resume a destroyed renderer after SIGCONT", () => { + test("does not resume a destroyed renderer", () => { const renderer = createMockRenderer(); - const signals = createSignalHarness(); installJobControlSuspendSupport(renderer, { - kill: () => undefined, - off: signals.off, - once: signals.once, + kill: () => { + renderer.isDestroyed = true; + }, platform: "linux", }); renderer.emitKeypress(createTestKey({ ctrl: true, name: "z" })); - renderer.isDestroyed = true; - signals.emit("SIGCONT"); expect(renderer.suspendCalls).toBe(1); expect(renderer.resumeCalls).toBe(0); @@ -231,31 +178,24 @@ describe("installJobControlSuspendSupport", () => { test("restores the renderer if SIGTSTP cannot be sent", () => { const renderer = createMockRenderer(); - const signals = createSignalHarness(); installJobControlSuspendSupport(renderer, { kill: () => { throw new Error("unsupported signal"); }, - off: signals.off, - once: signals.once, platform: "linux", }); renderer.emitKeypress(createTestKey({ ctrl: true, name: "z" })); expect(renderer.suspendCalls).toBe(1); expect(renderer.resumeCalls).toBe(1); - expect(signals.listenerCount("SIGCONT")).toBe(0); }); - test("dispose removes the keypress listener and pending SIGCONT listener", () => { + test("dispose removes the keypress listener", () => { const renderer = createMockRenderer(); - const signals = createSignalHarness(); const support = installJobControlSuspendSupport(renderer, { kill: () => undefined, - off: signals.off, - once: signals.once, platform: "linux", }); @@ -263,7 +203,6 @@ describe("installJobControlSuspendSupport", () => { support.dispose(); expect(renderer.keypressListeners.size).toBe(0); - expect(signals.listenerCount("SIGCONT")).toBe(0); renderer.emitKeypress(createTestKey({ ctrl: true, name: "z" })); expect(renderer.suspendCalls).toBe(1); diff --git a/packages/hunk/src/core/process/jobControl.ts b/packages/hunk/src/core/process/jobControl.ts index e513c6463..11f412d50 100644 --- a/packages/hunk/src/core/process/jobControl.ts +++ b/packages/hunk/src/core/process/jobControl.ts @@ -1,6 +1,5 @@ import type { CliRenderer, KeyEvent } from "@opentui/core"; -type SignalListener = () => void; type KeypressListener = (key: KeyEvent) => void; type JobControlRenderer = Pick & { @@ -13,8 +12,6 @@ type JobControlRenderer = Pick unknown; - off?: (signal: NodeJS.Signals, listener: SignalListener) => unknown; - once?: (signal: NodeJS.Signals, listener: SignalListener) => unknown; platform?: NodeJS.Platform | string; /** Signal target passed to process.kill; defaults to 0 for the foreground process group. */ pid?: number; @@ -73,7 +70,16 @@ export function installJobControlInterruptSupport( * OpenTUI receives Ctrl-Z as a parsed keypress instead of letting the terminal driver turn it into * SIGTSTP. Match the common TUI pattern used by apps like opencode: treat Ctrl-Z as an app command, * ask OpenTUI to restore the terminal, then send SIGTSTP to the foreground process group so the - * shell can manage Hunk as a normal suspended job. SIGCONT resumes the renderer after `fg`. + * shell can manage Hunk as a normal suspended job. + * + * The stop takes effect before `kill` returns, because POSIX delivers a signal sent to the caller's + * own process group before the call completes. Suspend and resume are therefore one straight line: + * the statement after `kill` runs only once the shell continues the job with `fg`. Staying on that + * call stack is also what keeps the job alive, since OpenTUI's suspend drops its keep-alive timer + * and stops reading stdin, so a runtime that reached an idle event loop here could exit before + * being continued. A `kill` that returns without stopping — a runtime that refuses SIGTSTP, or an + * orphaned process group that discards it — reaches the same restore instead of waiting for a + * SIGCONT nobody will send. */ export function installJobControlSuspendSupport( renderer: JobControlRenderer, @@ -85,38 +91,21 @@ export function installJobControlSuspendSupport( } const kill = deps.kill ?? process.kill.bind(process); - const off = deps.off ?? process.off.bind(process); - const once = deps.once ?? process.once.bind(process); const pid = deps.pid ?? 0; let disposed = false; - let resumeOnContinue: SignalListener | null = null; - - const clearPendingContinue = () => { - if (resumeOnContinue) { - off("SIGCONT", resumeOnContinue); - resumeOnContinue = null; - } - }; const suspend = () => { - resumeOnContinue = () => { - resumeOnContinue = null; - if (!renderer.isDestroyed) { - renderer.resume(); - } - }; - renderer.suspend(); - once("SIGCONT", resumeOnContinue); try { + // Blocks until the shell continues this job; see the note above. kill(pid, "SIGTSTP"); } catch { - // If the platform/runtime refuses SIGTSTP, leave the app usable instead of half-suspended. - clearPendingContinue(); - if (!renderer.isDestroyed) { - renderer.resume(); - } + // A runtime that refuses SIGTSTP leaves the app usable instead of half-suspended. + } + + if (!renderer.isDestroyed) { + renderer.resume(); } }; @@ -135,7 +124,6 @@ export function installJobControlSuspendSupport( return { dispose: () => { disposed = true; - clearPendingContinue(); renderer.keyInput.off("keypress", keypressListener); }, }; diff --git a/test/pty/lifecycle.test.ts b/test/pty/lifecycle.test.ts index 3eefeff8d..187c5d7e0 100644 --- a/test/pty/lifecycle.test.ts +++ b/test/pty/lifecycle.test.ts @@ -210,6 +210,66 @@ function waitForStreamOutput(stream: NodeJS.ReadableStream, pattern: RegExp, tim } describe("PTY lifecycle", () => { + test.skipIf(process.platform === "win32")( + "resumes a suspended job after fg without losing app state", + async () => { + const fixture = harness.createTabbedFilePair(); + const hunkCommand = harness.buildHunkCommand([ + "diff", + "--files", + fixture.before, + fixture.after, + "--mode", + "stack", + ]); + const session = await harness.launchShellCommand({ + command: "exec /bin/bash --noprofile --norc -i", + cwd: fixture.dir, + }); + + try { + session.writeRaw("PS1='HUNK_SHELL> '\r"); + await session.waitForText(/HUNK_SHELL>/, { timeout: 5_000 }); + session.writeRaw(`${hunkCommand}\r`); + await session.waitForText(/before\.txt.*after\.txt/, { timeout: 15_000 }); + await Bun.sleep(1_000); + await session.press("c"); + await session.waitForText(/Draft note/, { timeout: 5_000 }); + await session.type("Keep this note after resume."); + await session.press(["ctrl", "s"]); + await session.waitForText(/Keep this note after resume\./, { timeout: 5_000 }); + + // OpenTUI parses Ctrl-Z in raw mode, so send its control byte instead of SIGTSTP. + session.writeRaw("\x1a"); + await session.waitForText(/\[\d+\][^\n]*(?:Stopped|suspended)/, { timeout: 5_000 }); + await Bun.sleep(5_000); + session.writeRaw("fg\r"); + await Bun.sleep(1_000); + expect(await session.text({ immediate: true })).not.toContain("HUNK_SHELL>"); + await harness.ensureKeyboardIsLive(session); + const resumed = await session.text({ immediate: true }); + expect(resumed).toMatch(/before\.txt.*after\.txt/); + expect(resumed).toContain("Keep this note after resume."); + + // A resumed job stays suspendable, so Ctrl-Z is not a one-shot escape hatch. An echoed + // shell command proves the stop really happened: the earlier job lines are still on the + // normal screen, so matching them again would pass even if Ctrl-Z did nothing. + session.writeRaw("\x1a"); + await session.waitForText(/\[\d+\][^\n]*(?:Stopped|suspended)/, { timeout: 5_000 }); + session.writeRaw("echo SECOND_SUSPEND_OK\r"); + await session.waitForText(/HUNK_SHELL> echo SECOND_SUSPEND_OK/, { timeout: 5_000 }); + + session.writeRaw("fg\r"); + await Bun.sleep(1_000); + const secondResume = await session.text({ immediate: true }); + expect(secondResume).not.toContain("SECOND_SUSPEND_OK"); + expect(secondResume).toContain("Keep this note after resume."); + } finally { + session.close(); + } + }, + ); + for (const signal of ["SIGHUP", "SIGQUIT", "SIGPIPE"] as const) { test.skipIf(process.platform === "win32")(`exits cleanly on ${signal}`, async () => { const fixture = harness.createLongWrapFilePair();