fix: resolve spurious dry-run diffs for rules, themes, and hooks - #1437
Open
ankita10119 wants to merge 6 commits into
Open
fix: resolve spurious dry-run diffs for rules, themes, and hooks#1437ankita10119 wants to merge 6 commits into
ankita10119 wants to merge 6 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1437 +/- ##
=======================================
Coverage 80.25% 80.26%
=======================================
Files 163 163
Lines 7553 7574 +21
Branches 1667 1673 +6
=======================================
+ Hits 6062 6079 +17
- Misses 801 804 +3
- Partials 690 691 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
harshithRai
reviewed
Jul 28, 2026
| return { del: [], create: [], conflicts: [], update: [] }; | ||
| } | ||
|
|
||
| const existing = await this.getType(); |
Contributor
There was a problem hiding this comment.
Small thing: getType() returns null when no-code isn't enabled (the 400 case in getThemes), and passing that straight into calculateDryRunChanges will blow up on [null]. The rules fix in this same PR guards it - could we do the same here?
const existing = await this.getType();
if (existing === null) {
return { del: [], create: [], conflicts: [], update: [] };
}
Contributor
Author
There was a problem hiding this comment.
Addressed and updated
harshithRai
approved these changes
Jul 29, 2026
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
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.
🔧 Changes
Fixes three sources of spurious dry-run diffs that caused a clean export-then-dry-run cycle to propose changes when none existed.
Rules (
handlers/rules.ts)dryRunChangeswas delegating tocalcChanges, which internally runs theduplicateItemssuffix-generation path. This caused every rule in the tenant to appear twice in the diff, once under its real name and once with a random suffix (e.g.Add voucherID to AccessToken-betv2).Fixed by calling
calculateDryRunChangesdirectly, which performs a clean name-based comparison without any renaming side-effects.Themes (
handlers/themes.ts)Themes are singletons, but a default export strips
themeIdfrom the local file (becauseAUTH0_EXPORT_IDENTIFIERSdefaults tofalse).Without a
themeId, the dry-run matcher found no match for the local theme and proposed a CREATE + DELETE on every run. Fixed by adding adryRunChangesoverride that backfillsthemeIdfrom the remote singleton before comparison, so the matcher resolves correctly without requiringAUTH0_EXPORT_IDENTIFIERS.Hooks (
handlers/hooks.ts)getTypewas destructuring{ data: secrets }from the secrets response, but the SDK returns the object directly. This meant secrets were alwaysundefinedlocally, causing hooks with secrets to appear changed on every dry run. Fixed by removing the.dataunwrap.📚 References
🔬 Testing
Unit tests added/extended for all three fixes:
test/tools/auth0/handlers/dryRun.tests.ts- new rules clean-cycle test (no creates/updates/deletes after a clean export), and two new themes regression tests (no themeId → no create/delete; content diff → update only)test/tools/auth0/handlers/themes.tests.js- new#themes dryRunChangesdescribe block with 5 cases covering thebackfill logic, content-diff detection, and null-guard behaviour
All 37 affected tests pass (
npm test).For end-to-end validation: export any tenant, immediately run
--dry-runagainst the same tenant with the same exported file - rules, themes, and hooks should all show zero proposed changes.📝 Checklist