fix(release): scope ambiguous-scope check to active release group's projects#35745
Conversation
…rojects The conventional-commit scope ambiguity check in getCommitsRelevantToProjects fires unconditionally on the global project graph, blocking unrelated release groups whose tag-range includes a commit whose scope happens to substring-match multiple projects outside the active group. Filter the per-pattern match set to the active release group's projects before counting; in-group ambiguity still throws, cross-group matches fall through to the existing file-affectedness path. Also restrict scopedProjects to the active group so cross-group matches don't bleed into isProjectScopedCommit. Closes nrwl#35744
👷 Deploy request for nx-docs pending review.Visit the deploys page to approve it
|
👷 Deploy request for nx-dev pending review.Visit the deploys page to approve it
|
|
Reproduces using the minimal repo at https://github.com/jmclellan-crexi/nx-release-scope-ambiguity-repro — |
|
View your CI Pipeline Execution ↗ for commit 1dc83ad
☁️ Nx Cloud last updated this comment at |
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, the solution is to rerun CI. Because this branch comes from a fork, it is not possible for us to push directly, but you can rerun by pushing an empty commit:
git commit --allow-empty -m "chore: trigger rerun"
git push
🎓 Learn more about Self-Healing CI on nx.dev
leosvelperez
left a comment
There was a problem hiding this comment.
Thanks for taking the time to contribute!
The changes are in the right direction. Let's address one issue with projectsRelationship: 'independent'.
| projectGraph.nodes | ||
| ); | ||
| const inGroupMatches = perPatternMatches.filter((p) => | ||
| projectSet.has(p) |
There was a problem hiding this comment.
For release groups with projectsRelationship: "independent", projectSet only contains the single/independent project currently being processed, not all the projects in the release group (see packages/nx/src/command-line/release/version/derive-specifier-from-conventional-commits.ts:37). So, we need to forward the projects in the release group to this function so we can account for independently released projects and properly match against the release group here.
Let's fix that and add unit tests to cover that scenario as well.
There was a problem hiding this comment.
Good catch — fixed in 1dc83ad.
The full release group is now forwarded down the chain as a separate releaseGroupProjects argument and used for scope matching, while projects continues to drive which projects get commits assigned:
getCommitsRelevantToProjects(shared.ts) — addedreleaseGroupProjects: string[] = projects. The per-pattern ambiguity check and thescopedProjectsfilter now intersect againstreleaseGroupProjectSetinstead ofprojectSet. The commit-assignment loop still usesprojectSet. Defaults toprojects, so fixed-group behavior is unchanged.resolveSemverSpecifierFromConventionalCommits— threaded the param through (defaults toprojectNames).deriveSpecifierFromConventionalCommits— passesreleaseGroup.projectsregardless of relationship, so even when a single independent project drivesaffectedProjects, scopes are matched against the whole group.
Added two unit tests for the independent scenario:
- intra-group ambiguity (
projects: ['@foo/graph'],releaseGroupProjects: ['@foo/graph', '@bar/graph'], scopegraph) → still throws. - collision only with a project outside the group (release group is just
['@foo/graph']) → no throw, treated as scoped to@foo/graph.
shared.spec is now 40 passing; the full release suite is 505 passing / 1 skipped.
…roup For release groups with projectsRelationship: 'independent', only the single project being processed was passed to getCommitsRelevantToProjects, so the scope ambiguity check could not see sibling projects in the same group. Forward the full release group's projects separately so genuine intra-group ambiguity is still detected for independent groups, while cross-group collisions continue to fall through to file-affectedness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Current Behavior
nx release --group=<group>walks the full commit range since the last matching tag and resolves each commit's conventional-commit scope against the entire project graph (findMatchingProjectsoverprojectGraph.nodes). If a scope is ambiguous against any projects in the workspace, Nx throws unconditionally — even when those projects aren't in the release group currently being processed.In workspaces with slash-style subpath project names (e.g. a meta package
@scope/cuiplus subpath packages@scope/cui/forms,@scope/cui/select, …), a single commit with a short-form scope likefix(cui): …permanently blocks every release group whose tag-range includes it — including unrelated groups that don't contain any of the matching projects.Closes #35744.
Expected Behavior
The ambiguity check should be scoped to the active release group's projects. A scope that only collides with projects outside the group should fall through to the file-affectedness path that's already used when a commit has no scope at all.
Fix
In
getCommitsRelevantToProjects,projects: string[](already in the function signature as the active release group's projects, materialized asprojectSetat the top) is now applied to the per-pattern ambiguity check and to thescopedProjectsset:inGroupMatches = perPatternMatches.filter(p => projectSet.has(p)). Throw only wheninGroupMatches.length > 1— ambiguity within the group is still a real error.scopedProjectsis intersected with the group. Cross-group matches no longer bleed intoisProjectScopedCommitdownstream.When the filtered set is empty or singleton, the commit is treated identically to a commit with no scope for this group's processing —
isProjectScopedCommitfalls back tofalsefor projects whoseaffectedGraphincludes them via files, matching the existing scope-less behavior.Tests
packages/nx/src/command-line/release/utils/shared.spec.ts:should throw when commit scope matches multiple projects (ambiguous scope)) — converted from atry { … } catch (err) { expect(…) }pattern (which silently passed if no throw happened) toawait expect(…).rejects.toThrow(…). Existing behavior preserved: throws when@foo/graph+@bar/graphare both in the active group with scopegraph.['lib-a'], scopegraphmatches@foo/graph+@bar/graph(both outside the group). No throw. Commit is included via file-affectedness,isProjectScopedCommit: false.['@foo/graph'], scopegraphmatches@foo/graph(in group) +@bar/graph(out). Filtered to one match. No throw, treated as scoped to@foo/graph(isProjectScopedCommit: true).Also extended
createMockCommitto accept an explicitscopeargument so tests actually exercise the scope-parsing path. The existing ambiguity test relied onfeat(graph): …in the commit message, butcommit.scopeitself was hardcoded to'', soscopePatternswas always empty and the throw never fired in the test — the assertion lived in a catch block that was never reached. The conversion torejects.toThrowplus the explicitscopeargument makes the original test actually verify what its name says.Verification
Minimal real-world repro repo: https://github.com/jmclellan-crexi/nx-release-scope-ambiguity-repro
PR Checklist