fix(core): support skipped batch tasks end-to-end and fix TUI double logs#35617
Merged
Conversation
- Swap appendTaskOutput / printTaskTerminalOutput order in the batch result handler so the second call no-ops in the TUI instead of writing the same terminalOutput twice into the per-task PTY. - Add optional `status` to TaskResult so batch executors can mark unyielded peers as `skipped` rather than `failure` when they never got to run because a sibling failed. - Clear skipped tasks from the in-progress set in tui-summary so a failed batch isn't reported as `Cancelled`. - Skip skipped tasks in the run-one summary so we don't print a misleading `> nx run <task>` header for tasks that never ran.
When a Gradle task fails (or the batch runner exits non-zero), the peers that never got to run are now reported with `status: 'skipped'` and an empty terminalOutput. Previously they all surfaced with a generic `Gradlew batch failed` message and a `failure` status, which hid the actual root cause and noisily duplicated the same line in every dependent task's pane. Fixes NXC-4439.
Mirrors the Gradle batch executor: when a Maven task fails or the batch runner exits non-zero, peers that never got to run are now reported with `status: 'skipped'` and an empty terminalOutput instead of `failure` with a generic error message.
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
View your CI Pipeline Execution ↗ for commit d418f5a
☁️ Nx Cloud last updated this comment at |
Adds tests that mock the batch-runner JAR's spawn and verify: - When a sibling reports failure and the runner exits non-zero, unyielded peers are emitted with `status: 'skipped'` and an empty terminalOutput. - When the runner exits non-zero with no per-task results, all peers fall back to `failure` (no skipped status), since there is no observed sibling failure to attribute the skip to. - Successful results pass through unchanged. Equivalent tests added for the Maven batch executor.
ResultEmitter is the protocol boundary between the Kotlin batch runner and the TypeScript executor — these tests pin the on-the-wire shape that the TS-side spec depends on: - emits a single NX_RESULT line per call with the expected JSON - dedupes repeated emissions for the same task id - handles distinct task ids independently - terminates each emission with a newline so readline can split
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Reverts 39f7401. These tests pinned the NX_RESULT protocol shape but didn't exercise any code changed in this PR — keeping them only as scaffolding wasn't worth the maintenance surface.
CI surfaced a regression in the maven batch e2e test: the previous post-loop fallback yielded a `failure` result for any unyielded task, which prevented Nx's task-orchestrator from retrying tasks that the batch runner failed to report on. The Maven batch runner's stderr can interleave concurrent task output such that one task's NX_RESULT line ends up captured inside another task's terminalOutput string, leaving the inner task unyielded on the TS side. With a clean exit code, Nx's retry loop is the right recovery path — yielding a fallback failure here turns a transient parser miss into a permanent failure. The catch-block fallback (for non-zero exit codes) is preserved so real maven failures still get the skipped/failed peer treatment.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Instead of inferring skipped status on the TS side after a sibling fails, the Kotlin batch runner now emits an explicit `status: "skipped"` NX_RESULT for any requested Nx task that Gradle never reached. - TaskResult gains a `status` field; ResultEmitter writes it on the wire alongside `success` for back-compat. - runBuildLauncher and runTestLauncher walk the requested task set at end-of-batch and emit `skipped` for any task without a TaskFinishEvent (e.g. a peer compilation failed and the build aborted before this task could be scheduled). - gradle-batch.impl.ts is now a thin relay: parse NX_RESULT (passing through `status` if present), yield. The only TS-side fallback is a generic crash recovery path that fires when the runner exits before reporting on every task. This eliminates a class of TS-side inference bugs (the `sawFailure` heuristic was fragile when the runner emitted partial output) and mirrors the Maven runner's existing skipped-task tracking.
Mirrors the Gradle runner change. The Maven batch runner already tracked TaskState.SKIPPED for tasks whose dependency failed, but silently dropped them — only success/failure tasks were emitted as NX_RESULT, leaving the TS side to infer skipped peers. - TaskResult gains a `status` field surfaced on the wire. - The work-stealing scheduler's skipped-task branch now emits an explicit `skipped` NX_RESULT (with empty terminalOutput) for each task removed due to a failed dependency. - maven-batch.impl.ts becomes a thin relay: parses NX_RESULT, passes `status` through, no inference. The only TS-side fallback is a generic crash recovery for non-zero exits with no per-task results. The previous TS-side `sawFailure` inference combined with a post-loop fallback was the source of a CI regression: if the runner's stderr interleaved concurrent task output and one task's NX_RESULT got captured inside another's terminalOutput, the missing task got marked as a permanent failure instead of letting Nx retry. With the runner now authoritative, that inference path goes away entirely.
FrozenPandaz
changed the title
Drop the silent default on `TaskResult.status` and add `success`, `failure`, `skipped`, `fromBoolean` factory functions. Every call site is now explicit about the outcome it represents, and the TS side can drop the `data.result.status ?? …` defensive fallback — the runner always sends `status` on the wire. Same shape for the Maven runner. Also tightens stale comments throughout the diff.
Apply review cleanup: - Extract `resolveBatchTaskStatus` helper in task-orchestrator instead of repeating `result.status ?? (result.success ? 'success' : 'failure')` in two spots. - Skip `printTaskTerminalOutput` for skipped batch tasks with no terminal output — otherwise the TUI allocates a per-task PTY just to write a cursor-hide escape, which scales linearly with skipped peers in large batches. - Move maven `emitResult` for cascaded skips out of the graph `synchronized` block — JSON serialization and stderr I/O don't need the graph monitor and would otherwise stall every other worker on cascading failures. Skipped-task results are collected locally and emitted after the lock is released. - Drop the redundant "Kotlin runner emits NX_RESULT" preamble comment in both TS executor files. - Drop `= 0` defaults from Maven `TaskResult` factory startTime/endTime params — every real call site passes both, and the default would silently produce zero-duration tasks. - Reword the `TaskResult.status` doc-comment to call out that it's the contract for batch executors, not an opt-in escape hatch.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: FrozenPandaz <FrozenPandaz@users.noreply.github.com>
…Y state The TS-side ordering swap that fixed double-logged batch terminal output also moved batch tasks from the no-PTY branch of `print_task_terminal_output` (which appended `\x1b[?25l`) to the PTY-exists branch (which previously returned early). Finished batch panes ended up showing a blinking virtual cursor at the end of the captured output. Make `print_task_terminal_output` emit the cursor-hide escape on both branches: the no-PTY branch keeps writing `output + cursor-hide` when it creates the PTY, and the PTY-exists branch now writes only the cursor-hide. The escape is idempotent so it's safe regardless of how the PTY's content got there. Two unit tests pin the contract — verified to fail without the fix: - batch-style flow (append creates the PTY, print finalizes) - cache-hit flow (print creates the PTY)
Pins the negative half of the streaming/finalize contract: while chunks are still arriving via append_task_output, the cursor must remain visible. Only print_task_terminal_output (called at end of life) hides the cursor.
…Y state [Self-Healing CI Rerun]
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…elf-Healing CI Rerun]
Contributor
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.
🎓 Learn more about Self-Healing CI on nx.dev
AgentEnder
approved these changes
May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current Behavior
When running a batch executor (Gradle or Maven) under the TUI:
terminalOutputis written into the per-task PTY twice —printTaskTerminalOutputlazily creates the PTY with the terminalOutput, thenappendTaskOutputimmediately writes the same content again, producing repeated text in the pane (e.g.> Task :foo:bar UP-TO-DATE> Task :foo:bar UP-TO-DATE).success: falsewith a genericGradlew batch failed(orMaven batch runner exited with code N) message — both in the streaming output and in every dependent task's TUI pane. The actual root failure is hidden in noise. Same applies for Maven.Cancelledeven though the failure was real.> nx run Xheaders in run-one. Tasks that never actually ran still got a header printed in the streaming output, suggesting they were executed.Expected Behavior
terminalOutputfor a batch task is written to its PTY exactly once.status: 'skipped'and empty terminal output. The actual failed task keeps its full error in its own pane (✖). Dependents show as ⏭ in the task list with empty panes — the user navigates to the failed task to see why.Ran target build … N/M failedrather thanCancelled.Implementation
This PR changes the batch executor protocol so the Kotlin batch runners are the source of truth for per-task outcomes — the TS executors are a thin relay instead of inferring missing results.
Wire protocol
TaskResultgains an optionalstatus?: 'success' | 'failure' | 'skipped'field. When set, the orchestrator and lifecycle honor it instead of inferring fromsuccess: boolean. Existing batch executors that don't emitstatusare unaffected — the orchestrator falls back to the boolean.NX_RESULT:{json}lines now carrystatusalongsidesuccessfor back-compat with older Nx versions.Kotlin runners (Gradle and Maven)
skippedNX_RESULTfor any task without a finish event (e.g. a peer compilation failed and the build aborted before this task could be scheduled).runBuildLauncherandrunTestLauncher.TaskState.SKIPPEDfor tasks removed due to a failed dependency — it now emits anNX_RESULTfor each instead of silently dropping them.TS executors (
gradle-batch.impl.ts,maven-batch.impl.ts)Become thin relays:
NX_RESULT, passstatusthrough.sawFailure-based inference is gone — it was fragile (a regression in this PR's CI revealed that Maven's stderr can interleave concurrent task output and stash one task'sNX_RESULTinside another'sterminalOutputstring, defeating the inference).Nx core (orchestrator + TUI lifecycle)
runBatch'sonTaskResultshonorsresult.status ?? (success ? 'success' : 'failure').appendTaskOutputruns beforeprintTaskTerminalOutput— the latter then no-ops in the TUI because the PTY is already populated.inProgressTasksonsetTaskStatus(Skipped)(so the run summary doesn't say "Cancelled"), and skips them inprintRunOneSummary(so we don't print a misleading> nx run Xheader for tasks that never ran).Tests
tui-summary-life-cycle.spec.ts: snapshot test that aSkippedtask does not print a> nx runheader and the run summary reports as a real failure (not cancelled).gradle-batch.impl.spec.tsandmaven-batch.impl.spec.ts: spawn-mocked tests verify the executor relaysstatus: 'skipped'from the runner unchanged, and backfills asfailureonly when the runner crashes before reporting.Related Issue(s)
Fixes NXC-4439 (Linear) — Show root Gradle failure for dependent tasks in TUI.
Fixes NXC-4449 (Linear) — TUI shows duplicate terminal output for batch tasks.