Skip to content

Fix flaky "Test export mpr-v2" CI job and lint job clobbering resources/rules - #154

Merged
xiwenc merged 2 commits into
mainfrom
cursor/fix-nondeterministic-app-yaml-82e5
Sep 10, 2026
Merged

Fix flaky "Test export mpr-v2" CI job and lint job clobbering resources/rules#154
xiwenc merged 2 commits into
mainfrom
cursor/fix-nondeterministic-app-yaml-82e5

Conversation

@xiwenc

@xiwenc xiwenc commented Sep 10, 2026

Copy link
Copy Markdown
Member

Summary

Investigation of recent CI runs found two issues in the pipeline; this PR fixes both.

1. Flaky "Test export mpr-v2" job (nondeterministic app.yaml)

CI run 34118183591 on PR #153 failed because the freshly exported resources/modelsource-v2/app.yaml contained originalName: Constant_2.Constants$Constant.yaml for Module2 instead of originalName: Module2 — while the exact same commit passed after merge to main.

Root cause: resolveOriginalPath in mpr/mpr.go derived a directory's original path by iterating over the path map with for ... range, whose order is randomized in Go. Module2 has one mapped child (Module2/_/Constant_2.Constants$Constant.yamlConstant_2.Constants$Constant.yaml) whose original path does not preserve the disk directory depth; whenever the iteration happened to visit that entry first, its prefix hijacked Module2's derived original name (~1-in-11 chance per run).

Fix: scan candidate children in sorted order and prefer structure-preserving mappings (same depth on disk and in the original path), falling back to depth-changing mappings only when no aligned child exists. Added regression tests; TestResolveOriginalPathIsDeterministic reliably fails against the old code with exactly the value seen in the CI failure.

2. "Test lint" job silently wipes resources/rules on the runner

The lint job's generated config set rules.path: ./resources/rules while inheriting rules.rulesets (the published mxlint-rules v3.3.0 zip) from the embedded default config. The lint subcommand then runs SyncRulesets, which deletes resources/rules and replaces it with the downloaded bundle — so the job lints against the remote bundle (matching its skip entries), but destroys the checked-in rules fixtures in the runner's workspace as a side effect.

Fix: point rules.path at a .ci/rules scratch directory so the sync cannot overwrite repository files. Behavior is otherwise unchanged (still lints with the published bundle plus the skip list).

Verification

  • go test ./... passes.
  • Both export jobs (mpr-v1 and mpr-v2) run 30 times back-to-back with zero diffs against the checked-in modelsource fixtures.
  • The updated lint config runs clean locally (same 24 rules evaluated, exit 0) and leaves the git worktree untouched.
  • New regression test fails against the pre-fix code and passes with the fix.

Other pipeline observations (no change needed)

  • Latest CI on main and the Release workflow (v3.17.2) are green.
  • All jobs emit Node.js 20 deprecation warnings (checkout@v4, setup-go@v4/v5, upload/download-artifact@v4 are forced onto Node 24); bumping action versions would silence these but is left out of scope here.
  • On dependabot PRs, the coverage-report job logs "No coverage report to output" and succeeds without posting a comment.
Open in Web Open in Cursor 

cursoragent and others added 2 commits September 10, 2026 20:29
resolveOriginalPath iterated over the path map in Go's randomized order
and derived a directory's original path from whichever mapped child it
saw first. A child whose original path does not preserve the disk
directory depth (e.g. Module2/_/Constant_2...) could hijack the prefix,
which made the 'Test export mpr-v2' CI job fail intermittently.

Scan candidates in sorted order and prefer structure-preserving
mappings, falling back to depth-changing ones only when no aligned
child exists.

Co-authored-by: Xiwen Cheng <xiwenc@users.noreply.github.com>
The lint job's config set rules.path to ./resources/rules while
inheriting the default remote ruleset, so SyncRulesets deleted the
checked-in rules fixtures on the runner and replaced them with the
downloaded bundle. Point rules.path at .ci/rules so the sync cannot
overwrite repository files.

Co-authored-by: Xiwen Cheng <xiwenc@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/mxlint/mxlint-cli/mpr 68.83% (+0.25%) 👍

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/mxlint/mxlint-cli/mpr/mpr.go 81.98% (+0.43%) 605 (+14) 496 (+14) 109 👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/mxlint/mxlint-cli/mpr/mpr_test.go

@xiwenc
xiwenc marked this pull request as ready for review September 10, 2026 20:31
@xiwenc
xiwenc merged commit 06b2b14 into main Sep 10, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants