fix(core): only report recursive task invocations from ancestor processes - #36481
Draft
AgentEnder wants to merge 4 commits into
Draft
fix(core): only report recursive task invocations from ancestor processes#36481AgentEnder wants to merge 4 commits into
AgentEnder wants to merge 4 commits into
Conversation
✅ 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 5d9025b
☁️ Nx Cloud last updated this comment at |
![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.
…sses The loop detector keyed `task_invocations` on `(root_pid, task_id)`, and `root_pid` is inherited by every nested process. That made the table a set of every task seen anywhere in the process tree rather than a chain, so two *sibling* Nx processes running the same task collided and the collision was reported as recursion. This is easy to hit: N atomized `e2e-ci--*` specs each spawning the same `serve-static` web server, or two parallel tasks that each shell out to `nx run lib:build`. Sibling discrete tasks reproduced it every run. - Re-key `task_invocations` on `(root_pid, pid, task_id)` so siblings can each hold the same task id, and bump DB_VERSION accordingly. - Plumb `NX_INVOCATION_ANCESTOR_PIDS` down through the task env and scope the check to those pids, so only a genuine ancestor re-invoking a task is a loop. - Scope `unregisterTask` by pid so a sibling finishing the same task cannot drop another process's record. - Return the chain from `registerTask` instead of signalling through an exception, so a DB failure is no longer misreported as recursion. - Build the reported chain from the ancestry rather than `created_at`, whose one-second granularity cannot order rows written in the same second.
`startContinuousTask` checked `getRunningTasks()` to decide whether another Nx process already owns a continuous task, but only wrote its own claim via `addRunningTask()` after forking the child. A sibling process asking the same question inside that window is told the task is not running and starts a duplicate of it — two `serve-static` servers racing for the same port. Move the claim to immediately after the check so the `SharedRunningTask` path wins reliably. With six siblings this makes the shared wait deterministic instead of dependent on fork timing. This is load-bearing alongside the ancestry fix for recursive task invocation: once siblings are allowed past the loop detector, this claim is what keeps them sharing one server instead of starting several.
Co-authored-by: AgentEnder <AgentEnder@users.noreply.github.com>
AgentEnder
force-pushed
the
feature/nxc-4732-investigate-recursive-task-invocation-with-serve-static
branch
from
July 27, 2026 21:50
3b5a3aa to
f893f11
Compare
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.
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
🎓 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
1 participant
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
Nx reports
Recursive task invocation detectedfor task graphs that contain no recursion.The loop detector keys the
task_invocationstable onPRIMARY KEY (root_pid, task_id), androot_pidis inherited by every nested process viaNX_INVOCATION_ROOT_PID. That makes the table a set of every task seen anywhere in the process tree rather than a chain — there is aparent_pidcolumn, but the check never reads it. So any two sibling Nx processes that run the same task collide on the primary key, and the collision is reported as recursion followed byprocess.exit(1).This is easy to hit. Reproduced two ways, with no Cypress and no ports involved:
Discrete tasks — fails every run. Two sibling tasks each running
nx run app-e2e:build:Continuous tasks — fails intermittently (~1 in 3). Two sibling
e2e-ci--*specs each spawningnx run app-e2e:serve-static:In both cases neither process is an ancestor of the other.
The continuous case is additionally a check-then-act race.
startContinuousTaskasksgetRunningTasks()whether another process already owns the task — and takes the correctSharedRunningTaskpath when it does — but only writes its own claim viaaddRunningTask()after forking the child. A sibling entering that window is told the task is not running and falls through into the collision. (With six siblings, five correctly printWaiting for ... in another nx process; the race just occasionally skips it.)Two further problems in the same code path:
registerTaskas a loop, so a locked or full database would print the recursion error and exit 1.created_at, but SQLite'sCURRENT_TIMESTAMPhas one-second granularity — so same-second rows order arbitrarily and an unordered set is printed as though it were a call path.Expected Behavior
Only a task re-invoked by an actual ancestor process is a loop. Sibling Nx processes running the same task are legitimate and proceed normally.
task_invocationsis re-keyed on(root_pid, pid, task_id)so siblings can each hold the same task id, withDB_VERSIONbumped accordingly.NX_INVOCATION_ANCESTOR_PIDSis plumbed through the task env, and the check is scoped to those pids.unregisterTaskis scoped by pid, so a sibling finishing the same task cannot drop another process's record.registerTaskreturns the chain instead of signalling through an exception, so a DB failure is no longer misreported as recursion — loop detection fails open, since it is diagnostic only.created_at, so it is a truthful, ordered call path.addRunningTasknow claims the task immediately after thegetRunningTasks()check rather than after the fork. This is load-bearing: once siblings are allowed past the detector, the claim is what keeps them sharing one server instead of starting several racing for the same port.Verification
really-recursive -> really-recursivetasks-runnerjestnx build,lint -p nxFive Rust unit tests cover the semantics directly: siblings allowed, ancestor rejected, loop through an intermediate process, sibling cleanup isolation, and re-registration after completion.
Known remaining gap
Moving the claim narrows the check-then-act window to two adjacent statements but does not make it atomic. Closing it properly needs the read and claim as one operation in
RunningTasksService— anINSERT ... ON CONFLICT DO NOTHINGwhose affected-row count decides ownership. That is a separate change and is not attempted here. Separately, discrete tasks have noSharedRunningTaskequivalent at all, so two sibling processes running the same discrete task both run it; that is pre-existing behaviour and out of scope.Related Issue(s)
Investigated as Linear NXC-4732 (customer report of
Recursive task invocation detectedbetweene2e-ci--*andserve-staticin a self-serving Cypress e2e project).Regression from #34820, which introduced the loop detector.
View session information ↗