feat(core): add TaskFileResolver primitive; refactor show target --check to use it#35583
feat(core): add TaskFileResolver primitive; refactor show target --check to use it#35583polygraph-snapshot-app[bot] wants to merge 15 commits into
Conversation
✅ 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. |
|
View your CI Pipeline Execution ↗ for commit 98a1780
☁️ Nx Cloud last updated this comment at |
9ec92af to
8378d0e
Compare
| const taskId = t.configuration | ||
| ? `${t.projectName}:${t.targetName}:${t.configuration}` | ||
| : `${t.projectName}:${t.targetName}`; |
There was a problem hiding this comment.
We have utils to create the task id already, don't do it ad-hoc like this
| function parseTaskId(taskId: string): { | ||
| project: string; | ||
| target: string; | ||
| configuration?: string; | ||
| } { | ||
| const [project, target, configuration] = splitByColons(taskId); | ||
| if (!project || !target) { | ||
| throw new Error( | ||
| `Invalid taskId "${taskId}" — expected "project:target[:configuration]"` | ||
| ); | ||
| } | ||
| return { project, target, configuration }; | ||
| } |
There was a problem hiding this comment.
Nope, we have dedicated utils for this stuff
| // The result key is usually the same as taskId but may include a | ||
| // defaultConfiguration suffix when none was explicitly given. | ||
| let inputs: HashInputs | undefined = planResult[taskId]; | ||
| if (!inputs) { | ||
| const prefix = `${project}:${target}`; | ||
| for (const [key, val] of Object.entries(planResult)) { | ||
| if (key === prefix || key.startsWith(prefix + ':')) { | ||
| inputs = val; | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
We can read the target's default configuration instead of guessing here
| function findCanonicalTaskId(taskId: string, tg: TaskGraph): string | null { | ||
| if (tg.tasks[taskId]) return taskId; | ||
| const { project, target } = parseTaskId(taskId); | ||
| const prefix = `${project}:${target}`; | ||
| for (const id of Object.keys(tg.tasks)) { | ||
| if (id === prefix || id.startsWith(prefix + ':')) return id; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
This is kinda crazy, we've got better utils for this. Don't duplicate.
| function getDepsOutputs(taskId: string): ExpandedDepsOutput[] { | ||
| if (depsOutputsCache.has(taskId)) return depsOutputsCache.get(taskId)!; | ||
|
|
||
| const tg = getTaskGraphFor(taskId); | ||
| if (!tg) { | ||
| depsOutputsCache.set(taskId, []); | ||
| return []; | ||
| } | ||
| const canonical = findCanonicalTaskId(taskId, tg); | ||
| if (!canonical) { | ||
| depsOutputsCache.set(taskId, []); | ||
| return []; | ||
| } | ||
| const task = tg.tasks[canonical] as Task; | ||
| let result: ExpandedDepsOutput[] = []; | ||
| try { | ||
| result = | ||
| getInputs(task, options.projectGraph, getNxJson()).depsOutputs ?? []; | ||
| } catch { | ||
| result = []; | ||
| } | ||
| depsOutputsCache.set(taskId, result); | ||
| return result; | ||
| } |
| function getUpstreamTaskIds(taskId: string, transitive: boolean): string[] { | ||
| const tg = getTaskGraphFor(taskId); | ||
| if (!tg) return []; | ||
| const canonical = findCanonicalTaskId(taskId, tg); | ||
| if (!canonical) return []; | ||
| const direct = tg.dependencies[canonical] ?? []; | ||
| if (!transitive) return [...direct]; | ||
| const visited = new Set<string>(); | ||
| const queue = [...direct]; | ||
| while (queue.length) { | ||
| const id = queue.shift()!; | ||
| if (visited.has(id)) continue; | ||
| visited.add(id); | ||
| queue.push(...(tg.dependencies[id] ?? [])); | ||
| } | ||
| return [...visited]; | ||
| } |
There was a problem hiding this comment.
We almost certainly have a better util
| function pathMatchesOutputPattern( | ||
| normalizedPath: string, | ||
| pattern: string | ||
| ): boolean { | ||
| const np = pattern.replace(/\\/g, '/'); | ||
| return ( | ||
| normalizedPath === np || | ||
| normalizedPath.startsWith(np + '/') || | ||
| minimatch(normalizedPath, np, { dot: true }) | ||
| ); | ||
| } |
| function isOutputImpl(taskId: string, path: string): boolean { | ||
| const normalized = path.replace(/\\/g, '/'); | ||
| return getOutputs(taskId).some((p) => | ||
| pathMatchesOutputPattern(normalized, p) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Why these weird Impl suffixes? Also, why are we even doing the filtering like this? This sucks. Getting bespoke listts of outputs per path instead of some bulk query
| const depsOutputs = getDepsOutputs(taskId); | ||
| if (depsOutputs.length === 0) return false; | ||
| for (const { dependentTasksOutputFiles, transitive } of depsOutputs) { | ||
| if (!minimatch(normalized, dependentTasksOutputFiles, { dot: true })) { |
There was a problem hiding this comment.
We almost certainly have a util
31a7d28 to
adae585
Compare
a6b9fe5 to
989f262
Compare
7387ad4 to
54d2fe3
Compare
| const matched: string[] = []; | ||
| const unmatched: string[] = []; | ||
| for (const file of files) { | ||
| if (isInput(taskId, file, ctx)) { |
There was a problem hiding this comment.
Pass more information into this function.. it should.. maybe just get all the files?
|
|
||
| function isInput(taskId: string, path: string, ctx: LoadedContext): boolean { | ||
| const normalized = normalizePath(path); | ||
| const raw = getRawInputs(taskId, ctx); |
There was a problem hiding this comment.
This should be done once per task
| if (raw.files.includes(normalized)) return true; | ||
| if (raw.depOutputs.includes(normalized)) return true; | ||
| } | ||
| return matchesDependentTaskOutputs(taskId, normalized, ctx); |
There was a problem hiding this comment.
Why do we need this if we already checked if raw.depOutputs has it?
| const matched: string[] = []; | ||
| const unmatched: string[] = []; | ||
| for (const file of files) { | ||
| if (isOutput(taskId, file, ctx.projectGraph)) { |
There was a problem hiding this comment.
Same as how isInputs is used.. this should probably receive all the files
| resolvePrompt, | ||
| PromptResolutionError, | ||
| } from './command-line/migrate/prompt-files'; | ||
| export { HashPlanInspector } from './hasher/hash-plan-inspector'; |
There was a problem hiding this comment.
We don't need to export this.
54d2fe3 to
5d8d533
Compare
29e9034 to
80390cc
Compare
…cking Adds a programmatic API that lets external consumers verify whether sandbox-violation file paths from a prior task run would be considered legitimate inputs/outputs in the current workspace configuration. The API mirrors the logic that powers nx show target --check: inputs are reconciled via HashPlanInspector.inspectTaskInputs, outputs via getOutputsForTargetAndConfiguration with glob matching. HashPlanInspector and the new verifier are now exported from devkit-exports for use by the nx-cloud light client.
…evkit-internals Conflict resolution: added PR exports (HashPlanInspector, TaskFileResolver, createTaskFileResolver) after master's new resolveImplementation/resolvePrompt exports in devkit-internals.ts.
…ally
Previously isInput() only matched against the materialized HashInputs.files
list — when an upstream task hadn't run yet, depOutputs was empty and any
file declared via { dependentTasksOutputFiles: '...', transitive?: bool }
was reported as not-an-input even when the path obviously matched both the
glob and a declared upstream output.
The new logic walks the task graph from the inspected task, pulls each
upstream's declared output globs, and reports the path as an input when
it matches the dependentTasksOutputFiles glob AND lies inside one of those
upstream outputs. Honors transitive: true/false. The check is exposed
separately as resolver.matchesDependentTaskOutputs so consumers (e.g. the
nx-cloud check-sandbox-report command) can reason about why a path was
considered an input.
Adds 6 unit tests covering: materialized depOutputs, static glob+output
match, glob-without-output mismatch, output-without-glob mismatch,
transitive=true walk, transitive=false short-walk, and the standalone
matchesDependentTaskOutputs accessor.
…uts public functions Conflict resolution: removed TaskFileResolver/createTaskFileResolver exports from devkit-internals.ts (between master's new resolveImplementation/resolvePrompt exports and the PR's HashPlanInspector export).
Replaces splitByColons (low-level lexer) with the project-graph-aware splitTarget helper so taskIds whose project name contains colons (e.g. some:scoped:project:build) parse correctly.
… [Self-Healing CI Rerun]
Output patterns were evaluated one at a time, so a negated pattern like
'!{projectRoot}/.next/cache/**' acted as a standalone minimatch matcher
(matching everything outside the cache dir) instead of excluding files
from the positive patterns beside it.
- isOutput now uses whole-list semantics: a path matches when it matches
a positive pattern and no negated pattern, mirroring the task runner's
expand_outputs behavior
- also fixes 'nx show target outputs --check' and upstream-output
matching for dependentTasksOutputFiles, which share this matcher
…ts --check 'nx show target inputs <task> --check <file>' only consulted HashInputs (resolved files + materialized depOutputs), so a dependent task's output was reported as 'not an input' until the upstream task had actually run. - the --check path now also consults checkFilesAreInputs, which statically matches dependentTasksOutputFiles globs against upstream tasks' declared outputs via the task graph - static matches are reported under the depOutputs category
check-task-files matched output patterns with minimatch plus hand-rolled directory-containment and negation logic, which can drift from the globset-based engine the task runner uses when expanding and hashing outputs. Matching is now equivalent by construction: - add matchOutputPaths: static path-vs-output-patterns matching that mirrors expand_outputs semantics (non-glob entries match themselves and anything nested under them, negated entries exclude from the whole set, a list with no positive entries matches nothing) without touching the file system - add matchGlobPaths: build_glob_set matching for arbitrary globs, used for dependentTasksOutputFiles the same way hash_task_output filters dep outputs - check-task-files now delegates all pattern matching to these, dropping its minimatch-based matcher
matchOutputPaths returned no matches when the pattern list had no positive entries, diverging from expand_outputs, which keeps everything the negated globs do not exclude. Static matching now reproduces that fallback so --check answers exactly what the cache would capture; an empty list still matches nothing, as expand_outputs returns nothing for it as well.
75653f5 to
31c9117
Compare
'coerce' was imported as a named value from 'yargs' but never used as a value — every 'coerce:' in this file is an object-literal key. The named import only typechecked as a property lookup on the Argv object that @types/yargs exports via 'export =', and ts-jest's nodenext handling rejects that form with TS2614. This was latent until this PR: the new devkit re-exports (checkFilesAreInputs/Outputs, HashPlanInspector) connect @nx/devkit to command-line-utils and shared-options, pulling this file into the e2e ts-jest type-check set for the first time and failing every e2e-ci task at transform time. Argv and ParserConfigurationOptions are only used as types, so the import becomes type-only.
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
Summary
Adds a small generic
TaskFileResolverprimitive (exported fromdevkit-internals,not
devkit-exports) that lets callers ask whether a workspace-relative path is adeclared input or output of a given task. Refactors
nx show target ... --check(both inputs and outputs) to use it instead of duplicating the input/output
reconciliation logic.
This supersedes the earlier
verifySandboxViolationsAPI: that high-level helperhas been removed in favor of this lower-level primitive. Consumers (the nx-cloud
light client) own their own report schemas and call the primitive per file.
API
Exported from
packages/nx/src/devkit-internals.ts.isInput()accepts a path as an input if any of the following hold:HashInputs.files(resolved self-inputs).HashInputs.depOutputs(materialized — only populated afterupstream tasks have run).
dependentTasksOutputFilesglob declared on the task ANDlies inside the declared outputs of an upstream task in the task graph
(honors
transitive: true|false). This is the static check that letssandbox-report verification work without first running the dependency.
Files
packages/nx/src/hasher/task-file-resolver.tspackages/nx/src/hasher/task-file-resolver.spec.tspackages/nx/src/hasher/verify-sandbox-violations.tspackages/nx/src/hasher/verify-sandbox-violations.spec.tspackages/nx/src/devkit-internals.tscreateTaskFileResolverpackages/nx/src/devkit-exports.tsverifySandboxViolationsexportspackages/nx/src/command-line/show/show-target/inputs.tspackages/nx/src/command-line/show/show-target/outputs.tsTest plan
6 new tests covering
dependentTasksOutputFilesstatic validation).show target inputs|outputs(25 tests passing locally).Linked PR
Consumed by nrwl/ocean#11134, which owns the
SandboxReportschema anditerates the report calling
resolver.isInput/resolver.isOutputper file.