Skip to content

Commit d5f98f8

Browse files
AgentEnderclaude[bot]FrozenPandaz
committed
feat(core): pass in progress task outputs to tui for non-direct child processes (#31655)
## Current Behavior Currently the TUI is disabled on windows due to poor support for the pseudoterminal and some lingering issues. ## Expected Behavior This PR starts tackling this by making the TUI more usable without the pty. The first step here is enabling processes created without the pty to display live outputs in the TUI, which was currently not possible. ## Copilot Summary This pull request introduces enhancements to task execution and output handling in the Nx task runner. The changes focus on improving the handling of progressive output for the TUI (Text User Interface), adding support for pseudo-terminal processes, and refining the orchestration of tasks. Below are the most important changes grouped by theme: ### Enhancements to Task Execution and Output Handling: * [`packages/nx/src/tasks-runner/running-tasks/node-child-process.ts`](diffhunk://#diff-8c0c3712ab796458d8f6fc6cb685f68a45c01fb94f2210d9b60eaabb07610a7cR12): Added a new `onOutput` method to allow streaming output to the TUI via callbacks. Updated `stdout` and `stderr` handlers to invoke these callbacks for progressive output. Introduced a `canProvideProgressiveOutput` method to indicate whether a task can stream output. [[1]](diffhunk://#diff-8c0c3712ab796458d8f6fc6cb685f68a45c01fb94f2210d9b60eaabb07610a7cR12) [[2]](diffhunk://#diff-8c0c3712ab796458d8f6fc6cb685f68a45c01fb94f2210d9b60eaabb07610a7cL55-R80) [[3]](diffhunk://#diff-8c0c3712ab796458d8f6fc6cb685f68a45c01fb94f2210d9b60eaabb07610a7cL88-R112) ### Support for Pseudo-Terminal Processes: * [`packages/nx/src/tasks-runner/forked-process-task-runner.ts`](diffhunk://#diff-9e7468f39e004b5e6087ab9a309150efa755b4f9f8047514b63fc71f8034c930L143-R143): Added comments to clarify when pseudo-terminal processes are used for interactive tasks and when non-interactive processes with piped output are used. These changes improve readability and understanding of the trade-offs involved. [[1]](diffhunk://#diff-9e7468f39e004b5e6087ab9a309150efa755b4f9f8047514b63fc71f8034c930L143-R143) [[2]](diffhunk://#diff-9e7468f39e004b5e6087ab9a309150efa755b4f9f8047514b63fc71f8034c930R155-R165) ### Improvements to Task Orchestration: * [`packages/nx/src/tasks-runner/task-orchestrator.ts`](diffhunk://#diff-e9bae83332b3d6e57c023959ab2e5f191c97e0a154a8c1d36dd81f8f869e1bdfL637-R637): Enhanced the registration of tasks in the TUI lifecycle. Added checks to ensure tasks that support progressive output but are not interactive (e.g., `NodeChildProcessWithNonDirectOutput`) are registered correctly. Introduced a fallback for tasks that don't support progressive output. [[1]](diffhunk://#diff-e9bae83332b3d6e57c023959ab2e5f191c97e0a154a8c1d36dd81f8f869e1bdfL637-R637) [[2]](diffhunk://#diff-e9bae83332b3d6e57c023959ab2e5f191c97e0a154a8c1d36dd81f8f869e1bdfL649-R655) <!-- Please link the issue being fixed so it gets closed when this is merged. --> Fixes # --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: Jason Jean <[email protected]> (cherry picked from commit 402d946)
1 parent d5bfc3a commit d5f98f8

File tree

4 files changed

+31
-7
lines changed

4 files changed

+31
-7
lines changed

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,8 @@ Fixes #ISSUE_NUMBER
177177
### Guidelines
178178

179179
- Ensure your commit message follows the conventional commit format (use `pnpm commit`)
180+
- Use `fix:`, `feat:`, `chore:`, etc. as appropriate types.
181+
- Scope is **required** for all commits. Possible scopes are listed in `scripts/commitizen.js`.
180182
- Read the submission guidelines in CONTRIBUTING.md before posting
181183
- For complex changes, you can request a dedicated Nx release by mentioning the Nx team
182184
- Always link the related issue using "Fixes #ISSUE_NUMBER" to automatically close it when merged

packages/nx/src/tasks-runner/forked-process-task-runner.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,17 @@ export class ForkedProcessTaskRunner {
153153
!disablePseudoTerminal &&
154154
(this.tuiEnabled || (streamOutput && !shouldPrefix))
155155
) {
156+
// Use pseudo-terminal for interactive tasks that can support user input
156157
return this.forkProcessWithPseudoTerminal(task, {
157158
temporaryOutputPath,
158159
streamOutput,
159160
taskGraph,
160161
env,
161162
});
162163
} else {
164+
// Use non-interactive process with piped output
165+
// Tradeoff: These tasks cannot support interactivity but can still provide
166+
// progressive output to the TUI if it's enabled
163167
return this.forkProcessWithPrefixAndNotTTY(task, {
164168
temporaryOutputPath,
165169
streamOutput,

packages/nx/src/tasks-runner/running-tasks/node-child-process.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export class NodeChildProcessWithNonDirectOutput implements RunningTask {
99
private terminalOutput: string = '';
1010
private exitCallbacks: Array<(code: number, terminalOutput: string) => void> =
1111
[];
12+
private outputCallbacks: Array<(output: string) => void> = [];
1213

1314
private exitCode: number;
1415

@@ -53,19 +54,31 @@ export class NodeChildProcessWithNonDirectOutput implements RunningTask {
5354
process.send(message);
5455
}
5556
});
56-
5757
this.childProcess.stdout.on('data', (chunk) => {
58-
this.terminalOutput += chunk.toString();
58+
const output = chunk.toString();
59+
this.terminalOutput += output;
60+
// Stream output to TUI via callbacks
61+
for (const cb of this.outputCallbacks) {
62+
cb(output);
63+
}
5964
});
6065
this.childProcess.stderr.on('data', (chunk) => {
61-
this.terminalOutput += chunk.toString();
66+
const output = chunk.toString();
67+
this.terminalOutput += output;
68+
// Stream output to TUI via callbacks
69+
for (const cb of this.outputCallbacks) {
70+
cb(output);
71+
}
6272
});
6373
}
64-
6574
onExit(cb: (code: number, terminalOutput: string) => void) {
6675
this.exitCallbacks.push(cb);
6776
}
6877

78+
onOutput(cb: (output: string) => void) {
79+
this.outputCallbacks.push(cb);
80+
}
81+
6982
async getResults(): Promise<{ code: number; terminalOutput: string }> {
7083
if (typeof this.exitCode === 'number') {
7184
return {
@@ -85,7 +98,6 @@ export class NodeChildProcessWithNonDirectOutput implements RunningTask {
8598
this.childProcess.send(message);
8699
}
87100
}
88-
89101
public kill(signal?: NodeJS.Signals) {
90102
if (this.childProcess.connected) {
91103
this.childProcess.kill(signal);

packages/nx/src/tasks-runner/task-orchestrator.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,6 @@ export class TaskOrchestrator {
635635
temporaryOutputPath,
636636
streamOutput
637637
);
638-
639638
if (this.tuiEnabled) {
640639
if (runningTask instanceof PseudoTtyProcess) {
641640
// This is an external of a the pseudo terminal where a task is running and can be passed to the TUI
@@ -646,11 +645,18 @@ export class TaskOrchestrator {
646645
runningTask.onOutput((output) => {
647646
this.options.lifeCycle.appendTaskOutput(task.id, output, true);
648647
});
649-
} else if ('onOutput' in runningTask) {
648+
} else if (
649+
'onOutput' in runningTask &&
650+
typeof runningTask.onOutput === 'function'
651+
) {
652+
// Register task that can provide progressive output but isn't interactive (e.g., NodeChildProcessWithNonDirectOutput)
650653
this.options.lifeCycle.registerRunningTaskWithEmptyParser(task.id);
651654
runningTask.onOutput((output) => {
652655
this.options.lifeCycle.appendTaskOutput(task.id, output, false);
653656
});
657+
} else {
658+
// Fallback for tasks that don't support progressive output
659+
this.options.lifeCycle.registerRunningTaskWithEmptyParser(task.id);
654660
}
655661
}
656662

0 commit comments

Comments
 (0)