feat(core): guarantee terminal output files and add an agent-friendly summary output style - #36703
Open
AgentEnder wants to merge 45 commits into
Open
feat(core): guarantee terminal output files and add an agent-friendly summary output style#36703AgentEnder wants to merge 45 commits into
AgentEnder wants to merge 45 commits into
Conversation
Contributor
|
View your CI Pipeline Execution ↗ for commit ffa5c74
☁️ Nx Cloud last updated this comment at |
AgentEnder
force-pushed
the
feat/nxc-4694-4698-agent-output
branch
7 times, most recently
from
August 24, 2026 19:44
3a3b65b to
a012527
Compare
![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.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
AgentEnder
force-pushed
the
fix-nxc-4693-4692
branch
from
August 25, 2026 02:06
78e3643 to
8b95443
Compare
AgentEnder
force-pushed
the
feat/nxc-4694-4698-agent-output
branch
from
August 25, 2026 02:07
39d6603 to
a0d3ce3
Compare
AgentEnder
force-pushed
the
feat/nxc-4694-4698-agent-output
branch
from
August 25, 2026 11:43
a0d3ce3 to
2a587f6
Compare
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
AgentEnder
force-pushed
the
feat/nxc-4694-4698-agent-output
branch
from
August 25, 2026 11:51
2a587f6 to
b0f5779
Compare
AgentEnder
marked this pull request as ready for review
August 25, 2026 18:29
This was referenced Aug 25, 2026
AgentEnder
force-pushed
the
feat/nxc-4694-4698-agent-output
branch
3 times, most recently
from
August 27, 2026 11:49
88afdce to
4effb76
Compare
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.
AgentEnder
force-pushed
the
feat/nxc-4694-4698-agent-output
branch
2 times, most recently
from
August 29, 2026 17:03
89dc3c5 to
af7545b
Compare
…ad it Three defects with one root cause: the `specified`/`resolved` split left call sites reading the wrong one of two now-similar fields. The two tests gating NXC-4698 could not fail. Both asserted `result.outputStyle`, which holds what yargs parsed and which resolution never writes -- so making `summary` the default for every user, and making the agent default override an explicit `--output-style`, both left the suite green. They assert `resolvedOutputStyle` now, and each of those mutations fails a test. The non-agent case also passed `NX_TUI` as a boolean, which unsets the variable rather than setting `'false'`, so `shouldUseTui` fell through to defaulting ON and the assertion never reached the non-agent branch. It is the string now. `nx release publish` set only the deprecated `outputStyle`, so its hardcoded `static` was silently dropped: the renderer guard reads `specifiedOutputStyle`, which was undefined, and `shouldUseDynamicLifeCycle` then selected the dynamic run-many renderer, which buffers a successful task's output and deletes it. Under `--dry-run` every task succeeds by definition, so the whole publish report went missing with no agent involved. It sets all three fields, after the spread rather than before -- the middleware always writes `resolvedOutputStyle`, so a value placed before it was overwritten on every run. A style the user actually named still wins. `VSCODE_AGENT` and `CODEX_THREAD_ID` were missing from the e2e stripper, and each alone satisfies `is_ai_agent`, so running e2e from Codex or the VS Code agent resolved every `runCLI` to `summary` -- which prints no `Successfully ran target` line, the string ~96 e2e files assert on. CLAUDE.md tells agents to run `nx affected -t e2e-local`, so this repo's own documented loop hit it. The list is now at parity with `ai.rs`, which the comment names.
A dead batch worker has one log explaining every task in the batch, and it is unbounded by design -- a Gradle batch spans the whole command. Under `summary` that whole string was concatenated into every task's `terminalOutput`, so `persistTerminalOutputs` wrote a byte-identical unbounded file per task and reported each copy's full size, charging it against `maxCacheSize` and evicting genuine cache entries. It also recorded one task's bytes as another's in task history and Nx Cloud. The first task in the batch carries the log now and the rest address that copy by path, so the bytes exist once and each task's own file holds its own stack. The summary no longer prints `(exit N)`. `completeTasks` rebuilds `TaskResult.code` from the status, so every failure arrives as 1: a task killed with 137, a missing binary's 127 and a test runner's 2 all rendered `(exit 1)`. This was the first renderer to display an exit code at all, so an always-1 internal value was becoming user-visible, labelled as the task's exit code, to a consumer whose job is to machine-read that line. The PR's own e2e asserted `(exit 1)` for a task exiting 3 and so pinned the wrong number. Carrying the real code through `TaskResult` would change what Nx Cloud and both task-history life cycles record, which is why that is not the fix here. Also corrects the batch-fold comment, whose two load-bearing claims this stack falsified: the read is `resolvedOutputStyle` while the streaming decision deliberately reads `specifiedOutputStyle`, and `init-tasks-runner` no longer passes no style argument.
`record_terminal_outputs` said "on conflict only `accessed_at` moves ... leaving `has_artifacts` and `size` alone", but the SQL nineteen lines below moves `size` too, for a row that is still output-only. The function's own inline comment said the opposite of its doc. A maintainer trusting the doc would drop the CASE as dead weight and an output-only hash would keep its first-write size forever, undercounting against `maxCacheSize`. Corrected in `cache.rs` and in the declaration it is copied into verbatim. `printsFullTaskOutput` said exactly one style collapses and none may quietly withhold output. Two collapse now, and `summary` withholds even under `--verbose` -- the PR's own spec asserts it. `init-tasks-runner`'s new comment claimed the style is no longer discarded on that path. It still is: `createOrchestrator` builds its args from `nx.json`'s runner options, which carry neither argv-only field, so both expressions evaluate to undefined and `static-failures-only`. It now says what holds. The new `@deprecated` on `NxArgs.outputStyle` named a replacement but no removal version, which the repo's own comment rules require and both neighbouring deprecations have. `NxArgs` is what third-party task-runner integrations type against, so a deprecation with no deadline leaves them nothing to plan against and the major-release sweep no version string to grep for.
… write The capture file was named from a static counter, incremented inside the lazy open. A failed write closes the fd, so the next open minted a NEW name and repointed `capturedOutputPath` at it -- orphaning everything captured so far on disk and leaving the reader only the bytes that arrived after the last failure. On a persistently unwritable data directory that was one file, and one warning, per chunk. The name comes from the batch id now (`<executor> <n>`, already unique per run and already carrying the executor) plus the pid, so a reopen returns to the same file. The first open truncates and later ones append, so a stale file left by a dead run whose pid was recycled is overwritten rather than appended to. A failed write no longer ends the capture. Under `summary` this file is the only copy the reader ever sees, so giving up on it after one momentarily full disk costs the whole rest of the batch. It retries, and gives up after three consecutive failures -- this runs inside a stream 'data' handler where an escape is an uncaught exception, so the retries are bounded rather than unlimited. The warning fires on the first failure and on giving up, not on every chunk. The bytes that missed the file still go to the terminal, which leaves a hole in the middle of the file. That hole is now named in place rather than left for a reader to not notice: an agent reading this file never saw the bytes scroll past, so a log that reads as continuous prose with a chunk missing is worse than one that says what it lost.
… write [Self-Healing CI Rerun]
…crashed one The previous round fixed the crash path and left the results path with the same defect. Under `summary`, `forkProcessForBatch` is passed `printsTaskOutput(resolvedOutputStyle)`, so `BatchProcess` captures every chunk the worker writes. The results path folds that file only under `shouldGroupBatchOutput()`, and `printGroupedBatchOutput` requires `printsTaskOutput` on top of that, which is false for `summary`. Neither route ran, and `discardCapturedOutput` then unlinked the file. `readCapturedBatchLog` had exactly one caller, inside the catch. `@nx/maven` and `@nx/gradle` both declare `preferBatch`, so this needed no `--batch` flag, and an agent-driven run resolves `summary` on its own. A batch runner failure printed `full log: <path>` holding only `Error: Maven batch runner exited with code 1`, while the stderr naming the real failure was captured and deleted. The same run under `--output-style=static` printed it. The handoff is now one method both paths call, since keeping two copies of it is what let them drift. It attaches only when a task actually failed: a batch whose tasks all succeeded has nothing the worker's chatter would explain, and the style's contract is that a failure's log is readable, not that every byte the run produced is. The capture counter now resets on a successful write, so it is consecutive as its own doc says. Counting cumulatively meant three hiccups spread across a long healthy batch gave up on capture permanently. The capture filename carries a per-instance sequence again. Deriving it from the batch id alone lost the uniqueness the old counter guaranteed, because `batchId` falls back to the executor name for callers that do not pass one. Assigned in the field initializer rather than at open time, which is what made the original counter mint a new file on every reopen. `COPILOT_CLI` and `SUPERSET_AGENT_ID` join the e2e strip list. Both are new in `ai.rs` since this branch last checked parity, and each alone satisfies `is_ai_agent`. The comment no longer claims the list is in sync; it gives the grep that answers whether it is, because the claim has now gone stale twice.
…out to change The pointer was built from `holder.task.hash` as it stands when `runBatch` returns. `applyFromCacheOrRunBatch` then clears and recomputes that hash for every depsOutputs task that ran, failures included, and the holder is a failure by construction. `persistTerminalOutputs` writes under the new hash, so the pointer addressed a file nothing ever wrote. Under `summary` the reader is an agent told the output is on disk: it follows the path, gets ENOENT, and the log it needed sits one hash away. It names the task now instead of a path. Nothing is lost, because the holder failed too and so gets its own `full log:` line in the same summary, and the pointer no longer depends on when it is minted. The crash path had the same defect, from the same round that introduced the handoff. It goes through the shared method now rather than keeping a second copy of the logic, which is what let the two drift in the first place. A stopped batch passes no captured path, so its tasks still carry no output and its partial log still reaches the fold. Both wirings are pinned. Every existing crash-path test runs under grouping, where the handoff is a no-op, so none of them noticed when the crash path stopped calling it; there is a summary-mode test for each path now, and reverting either call fails one. Three comments described the capture filename as derived from the batch id alone. `captureSeq` is what makes it unique, since the id falls back to the executor name for callers that do not pass one.
A batch stopped by a signal carried its worker's partial log nowhere. Its tasks were given no output, and the fold that would otherwise render it is gated on `printsTaskOutput`, which is false for exactly `summary` - so the `finally` unlinked the only record of what got through. A CI step timeout is the ordinary way to reach this, and the same run under `--output-style=static-failures-only` renders those bytes. The stopped path now goes through the same handoff as the crashed one. The exit error still stays out, since it only restates the cancellation. Persisting it is not enough on its own: `summary` listed a stopped task as a bare id, so the log would have been written with nothing addressing it. A stopped task that produced output now gets the same `full log:` line a failure gets, and one that produced none still gets just its id. Three comments were wrong. The stopped-batch comment said the fold below was where that log belonged, which is what this change makes true. `batchId` was described as the whole capture filename when it is one of three components. And `captureSeq` was justified by a `batchId` fallback that never fires in production, since the only non-spec construction site always passes one - the real reason is that `TasksSchedule.batchCounters` is per instance, so a second schedule in the same process mints the same id and the pid does not separate them.
`has_artifacts` only ever holds two values and is only ever tested as a boolean, so declare it as one. `is_cache_entry` names what the reads actually ask, and the CHECK makes the two-state invariant structural rather than assumed at every call site. Cheap to do here: DB_VERSION already goes 3 -> 4 on this branch, so the table is created fresh and no migration is needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment advertised an `nx.json` escape hatch that cannot fire: `getRunnerOptions` merges a configured style in as `outputStyle`, while these arguments read `specifiedOutputStyle`/`resolvedOutputStyle`, so the fallback is the only branch that ever runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The capture hand-rolled what a WriteStream already does: partial writes, a bounded retry, a reopen-and-append, and a gap marker naming the bytes that missed the file. Piping into a stream drops all of it, and gives backpressure to the worker rather than an unbounded buffer here. A stream reports write failures as an 'error' event, and an unhandled one is an uncaught exception, so the listener is required rather than defensive. It stops capturing and keeps what reached the file. The live-stream fallback that followed a failed write is gone with it, so a capture that fails now loses the rest rather than breaking the format. Buffered writes have to be sequenced against the reads, so `runBatch` flushes before the fold and the worker-log handoff read the file. Not verified on Windows.
The comment said a `WriteStream` gives backpressure, but `write()`'s return value was discarded, so a slow disk grew the stream's buffer instead - the unbounded in-process growth the file-on-disk design exists to avoid. Pause the source until it drains. Also drops comments the write-stream change left false: the capture stops on failure as well as release, the stream is closed by the flush rather than left open, and there is no fd any more. The test double was a bare EventEmitter, so it had no pause/resume and could not reach this path; it is a real stream now.
The log was smuggled into one failed task's terminal output because a task hash was the only address the GC knew. That put a whole batch's bytes in a file cache replay reads back as that task's own output, gave every other failure a pointer sentence instead of a log, and left a stopped batch's tasks holding files whose entire contents was that sentence. It lives in <cacheDir>/batchOutputs/<batchId>-<pid>-<seq>.log now, and the life cycle is told where it is. Every failed task keeps only its own output, and summary prints one line per worker rather than N cross-references. Swept from disk rather than the database: nothing looks a batch log up by key, so a row would be write-only bookkeeping that a hard-killed process could skip, orphaning the file forever. The file is appended to while the batch runs, so a size recorded anywhere but the filesystem is wrong until it ends. A readdir+stat pass gives age and size together, and the 7-day threshold is what makes it safe while other Nx processes are live. Drops attachBatchWorkerLog and readCapturedBatchLog, which read an unbounded log into a JS string - the growth the file-on-disk design exists to avoid.
The comment implied a colliding batch id was routine. It only happens off the CLI path, where `runDiscreteTasks`/`runContinuousTasks` build a second orchestrator in one process. Names that, and the consequence that changed when these files started outliving their batch.
The age pass deletes at 7 days, where nothing is running, and the docblock claimed that made the whole sweep safe. It only covered that pass: the size eviction sorted by mtime and deleted oldest-first with no age gate, so once the directory passed its budget a batch still appending to its log - possibly in another Nx process - could have it removed. Going over budget recovers on the next sweep; losing a live batch's only log does not. Also corrects what the write-stream rework left false: `output.ts` still described the crash path folding the file into task results, `getCapturedOutputPath` still promised a late chunk would append when the flush has ended the stream, the capture file's directory name was stale, and the deleted `attachBatchWorkerLog` docblock was still stacked above its replacement. The late-chunk spec flushed after the chunk, the reverse of production order, so it asserted appending that cannot happen. It runs the real order now.
The backpressure pause released only on 'drain', and a write stream that errors never emits one. Once the capture failed with a chunk buffered, the worker's stdout stayed paused for the life of the process: it blocked as soon as its pipe filled, never sent its results, and `getResults` waited on a batch that could no longer finish. Verified against /dev/full - the source is still paused after the error and the next chunk never arrives. Paused sources are tracked and resumed when the capture stops, so a dead capture costs the log rather than the run. Also corrects the crash-path comment, which said the fold covers a style that prints nothing when the gate requires `printsTaskOutput`; that case is served by the announcement above it. Reported in four consecutive reviews. The sweep now takes regular files from the dirent rather than a stat, so a symlink is aged by itself rather than its target and a stray directory is not invisible to the size accounting.
`tasks-schedule.ts` drops a task from batching without a diagnostic when its batch implementation does not resolve, and the single-task path already wrote the output file before this PR. The fixture printed the same marker either way, so the acceptance test for NXC-4694's headline case would have stayed green on unfixed code. The two implementations emit distinct markers now and the test asserts the batched one. Pins the discard guard too: replacing `!announcedBatchLogs.has(batch.id)` with `true` left all 45 orchestrator tests passing, so nothing covered the one thing keeping an announced log on disk. Corrects two comments that outlived their code: `capture()` still promised appending until the fold, which the flush now precedes, and the sweep docblock claimed neither pass can touch a live batch's log when the eviction window is measured from last write - a batch silent through a long quiet phase is not protected.
It was the only one of the three sweeps written in TypeScript. The 7-day record sweep and the size eviction both live in cache.rs, and this one walks a directory under the same cache path for the same reason, so it belongs beside them rather than pulling readdir/stat/unlink into the tasks runner. The directory name is now defined in cache.rs and mirrored by `batchOutputPathForKey`, the way `terminalOutputPathForHash` already mirrors `get_task_outputs_path_internal`. Behaviour is unchanged: age pass at 7 days, then oldest-first while over budget, skipping anything written within the last hour so a live batch's log is not evicted. Regular files come from the dirent, so a symlink is neither followed for its age nor counted, and a stray directory is not invisible to the size accounting.
`existsSync` then `statSync` asks the filesystem the same question twice. One try block rather than `throwIfNoEntry` plus a catch: the catch has to be there regardless, since that option only suppresses ENOENT and an unreadable cache directory would still throw. This runs inside the process exit handler, where anything thrown is an unhandled rejection rather than something the caller can act on, so the record call sits inside it too - the whole method is best-effort bookkeeping for the GC.
The backpressure fix covered the error and discard paths but not `flushCapturedOutput`, which is the normal one. `end()` means a pending 'drain' never arrives, so a source paused at flush time had nothing left to resume it - measured 20/20 runs reaching 'finish' with the source still paused. That is exactly the announced-log path, where the file is deliberately kept and the discard never runs, so the worker sat blocked on a full pipe until cleanup killed it. No test covered that branch: removing the resume from the error handler failed a test, removing it from the flush did not. There is one now, and the failure mock no longer performs the real write - a genuine 'drain' could otherwise resume the source and let both tests pass on unfixed code. Also restores the sweep as a free function. Routing it through the cache object left the legacy `Cache` with an empty body while `BatchProcess` writes those logs whichever implementation is active, so `batchOutputs/` grew forever wherever the DB cache is off. It was unconditional before the move to Rust. And narrows a comment that claimed the dirent check keeps a stray directory visible to the size accounting; a non-file entry is skipped, exactly as before.
It deleted files with no test in either language. Adding one exposed that the obvious tests do not bind: a stray directory survives whether or not the dirent check is there, because `remove_file` on a directory fails silently, and a symlink's own mtime is fresh so the age pass never reaches it. So the thresholds are parameters again, as they were before the move to Rust, and the tests drive the paths that matter: the age pass, oldest-first eviction stopping at the budget, and a log written within the hour being off limits even far over budget. Each is mutation-verified. Wiring that up found the seam was half-dead - the retain still read the constant rather than the parameter, so the live-log test had been passing on the hardcoded hour and the mutation targeting it silently matched nothing.
The `writeSync` -> `WriteStream` rewrite dropped the guarantee `printsTaskOutput` still documents: a style withholds task output on the promise that it is readable somewhere else. Three ways the bytes went nowhere instead. - A failed capture returned early with no `else`, so every later chunk was neither written, printed nor warned about. Base forwarded it to the terminal on the stream it arrived on and warned once; that is restored, including for the chunk whose own `mkdir` threw. - `flushCapturedOutput` resumed a backpressured source and ended the stream in the same turn. `resume()` delivers on the next tick, so the tail the source still held was written to an ended stream and dropped - the end of the log, where a build tool puts what went wrong. It now drains first, bounded. - The capture key was `<batchId>-<pid>-<captureSeq>`, and `captureSeq` is a per-process static. These files outlive their batch now and `cacheDir` can be shared across checkouts, so a recycled pid could truncate a log an earlier run's summary still pointed at. A per-instance nonce separates them. The four tests that pinned the old forwarding were deleted alongside it, so the regression had nothing to fail. Two replace them, and both were confirmed to fail with their fix reverted. The write-error test also no longer races the asynchronous open - it waited one `setImmediate` and failed about 9% of the time under load.
…xist `cache.rs` is `#[cfg(not(target_arch = "wasm32"))]`, so the wasi module registers no cache symbol at all and `sweepBatchOutputs` resolves to `undefined`. `postRunSteps` called it unconditionally, so a WebContainer run ended in `TypeError: nativeSweepBatchOutputs is not a function` even when every task had passed. Every other native cache entry point is kept off that path by `dbCacheEnabled()` returning false under WASM. This one is a free function - deliberately, so it runs whichever cache implementation is active - so it needs its own guard. The logs are collected by the next run on a platform that has the sweep. Also corrects the mirrored symbol this doc names: `batch_outputs_path`, not `get_batch_outputs_path_internal`, which does not exist. Its sibling reference is real, so a reader grepping for the native half had no cue the first was wrong - the exact drift the comment is there to prevent.
`openCapturedOutput` assigns `capturedOutputPath` when it opens the file,
before a byte reaches it, and `announceBatchWorkerLog` gated only on the path
being set. A capture that failed on its first write therefore left a 0-byte
file that was still announced, so `summary` printed
Batch worker logs:
- @nx/gradle:batch 1: <path>
and the reader opened nothing. Checked after the flush, so the size is what a
reader will actually find.
Also notes at the sweep call site that it no-ops under WASM, since the comment
above it explains why the call is unguarded by a cache implementation.
`hasOutput` tested `terminalOutput !== undefined`, but `persistTerminalOutputs`
skips only an undefined output - an empty string is written as a 0-byte file.
Every task of a stopped batch carries `''`, so under `summary` a Ctrl-C or a CI
step timeout printed
Stopped before finishing:
- proj:build
full log: <cacheDir>/terminalOutputs/<hash>
for a file with nothing in it, directly beneath a comment promising "no output,
no file, no address". Truthiness is what that comment already described.
The batch worker's own partial log is not lost by this: it is announced as the
batch's artifact and rendered once under `Batch worker logs:`.
Also stops nesting `output.dim` inside `output.dim` on that block - the inner
reset closed the outer one early, so the line rendered undimmed while its
sibling above did not.
`read_dir` opens through `opendir(2)`, which follows a symlink on the directory itself. The per-entry handling already refuses to follow a link - a symlinked entry is neither aged by its target nor counted - but the root it is handed was never checked, so a `batchOutputs` planted as a symlink had the sweep delete that directory's files older than seven days instead. `~/.nx` is writable by anything sharing our uid, which is why `probeWritable` opens with `wx` for the same reason; this is the same adversary one directory up. Refusing a non-directory or symlinked root is the right failure, since symlinking it is not a supported configuration.
AgentEnder
force-pushed
the
feat/nxc-4694-4698-agent-output
branch
from
September 11, 2026 01:28
d448425 to
ffa5c74
Compare
Contributor
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
We guard the writeTerminalOutput call with mkdirSync({ recursive: true }) so that a deleted terminalOutputs directory (e.g. after nx reset runs while a long-running task is still active) is recreated on the fly before writeFileSync is called. Without this, the ENOENT crash in the onExit handler caused the daemon to fail to reconnect and the nest serve test to hang until its 120 s timeout was hit.
Note
⏳ We are verifying this fix by re-running a subset of the 2 failed tasks that were analyzed.
diff --git a/packages/nx/src/tasks-runner/forked-process-task-runner.ts b/packages/nx/src/tasks-runner/forked-process-task-runner.ts
index 2268dbf2..68c45c99 100644
--- a/packages/nx/src/tasks-runner/forked-process-task-runner.ts
+++ b/packages/nx/src/tasks-runner/forked-process-task-runner.ts
@@ -1,6 +1,6 @@
import { fork, Serializable } from 'child_process';
-import { writeFileSync } from 'fs';
-import { join } from 'path';
+import { mkdirSync, writeFileSync } from 'fs';
+import { dirname, join } from 'path';
import { ProjectGraph } from '../config/project-graph';
import { Task, TaskGraph } from '../config/task-graph';
@@ -408,6 +408,7 @@ export class ForkedProcessTaskRunner {
}
private writeTerminalOutput(outputPath: string, content: string) {
+ mkdirSync(dirname(outputPath), { recursive: true });
writeFileSync(outputPath, content);
}
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
Or Apply changes locally with:
npx nx-cloud apply-locally YN6o-PFHg
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
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.
Two tickets, in one PR at Jason's request — the second is meaningless without the first.
Current Behavior
Terminal output files are not guaranteed. The
writeFileSyncthat persists task output lives only in the non-streaming single-task branch. Batch output arrives over IPC in memory and only lands on disk viacache.put, which is gated ontask.cache. So acache:falsebatch task — the main CI path underNX_BATCH_MODE— leaves no terminal output file, and anything pointing at<cacheDir>/terminalOutputs/<hash>dangles exactly there.Agents read transcripts to find one failure. With the TUI off they get the static renderer and thousands of lines of passing output.
Expected Behavior
NXC-4694 — every task leaves its output on disk
Every task that reaches a terminal state writes to
<cacheDir>/terminalOutputs/<hash>, regardless ofcachesetting, streaming mode, or batch membership. No double-write whencache.putalso stores it.Outputs written without a cache entry (uncacheable tasks, and cacheable ones under
--skip-nx-cache) get a row via a new napirecord_terminal_outputs, carryinghas_artifacts = 0. Without a row the file is invisible toremove_old_cache_records, which only walks hashes it finds in the database, so these files would accumulate forever. The flag means such a row can never be served as a cache hit — there are no artifacts behind it.Bonus interop: Nx Cloud's upload fallback already reads this exact path when in-memory output is undefined, so guaranteed files make that path real for streamed and continuous tasks.
NXC-4698 —
--output-style=summaryBecause the files are now guaranteed, a run no longer has to carry that text:
Nothing is printed for a successful task, and no task output is inlined at all — so the size of a run's output depends on how many tasks failed, never on how much they logged.
It is the default when
isAiAgent()and no style was given. That resolution sits in the existing yargs middleware chain, afterNX_DEFAULT_OUTPUT_STYLEand before the TUI check, so an explicitly requested style always wins and the TUI correctly declines. Deciding it at life-cycle selection instead would have skippedNX_DEFAULT_OUTPUT_STYLEand landed after--output-stylewas already normalized.Decisions worth reviewing
The summary points at the raw log, not a stripped-ANSI sibling. That keeps NXC-4696 out of scope. It only works because nothing is inlined — an inlined excerpt would carry escape codes.
An earlier cut inlined the last ~20 lines of each failure. It was dropped. The argument for it is one fewer round trip; the arguments against won: every consumer of this style can read the file (it is chosen for agents or asked for explicitly), a fixed tail is as likely to catch a runner's footer as its error, and with 8 failures a 21-line-each bound is a transcript again.
DB_VERSIONgoes 3 → 4 rather than adding the column in place. Conventional here, and task hashes include the nx version so the cache is cold on upgrade anyway — but it does start an empty DB, orphaning existing cache dirs from both GCs and firing one "Unrecognized Cache Artifacts" warning.Continuous tasks still stream under
summary. The style prints nothing untilendCommand, which never runs for a task that does not end — so suppressing them would leavenx servesilent with no completed log to read instead. A deliberate exception to the style's contract.The inline fallback for an unhashed task was removed as unreachable.
processTaskhashes before it schedules, and the onlytask.hash = undefinedin the codebase is a transient clear immediately followed by a re-hash — so a task with terminal output always has a hash. The remainingif (task.hash)is a type guard against printing a bogus path, and says so.Verification
tscclean onpackages/nxande2e/nx; 195 unit tests green;cargo checkpasses on the Rust change.e2e/nx/src/terminal-outputs.test.ts):cache:falseunderNX_BATCH_MODEand under--output-style=streameach leave a readable file; the cached replay path is unchanged; a--skip-nx-cacherecord is never served as a hit. Plus the summary cases — a passing run stays under 10 lines, a failing run under 30, an explicit style beats the agent default, and the printed path is opened and asserted to contain the task's output, which is the seam between the two tickets.cache.spec.ts(the committed.nodepredatesrecord_terminal_outputs). CI covers both.Related Issue(s)
Tracked in Linear rather than as GitHub issues:
--output-style=summary, auto-selected underisAiAgent()Both belong to the Terminal Logs available from disk milestone of Agent Friendly Task Output. NXC-4694 unblocks NXC-4696 and NXC-4695; NXC-4698 unblocks NXC-4699.
View Polygraph session ↗