Skip to content

fix: prevent false enabled_clients diff in database dry-run - #1479

Merged
harshithRai merged 4 commits into
masterfrom
DXCDT-2281
Sep 3, 2026
Merged

fix: prevent false enabled_clients diff in database dry-run#1479
harshithRai merged 4 commits into
masterfrom
DXCDT-2281

Conversation

@harshithRai

Copy link
Copy Markdown
Contributor

🔧 Changes

Fixes a false positive in --dry-run mode where database connections with enabled_clients were incorrectly reported as changed.

In the database dry-run path (dryRunChanges), the local connection had its enabled_clients populated, but the remote connection it was compared against was fetched from connections.list, which no longer returns enabled_clients inline. The field now lives behind the dedicated enabled-clients endpoint. Because the remote object lacked the key entirely, the diff reported a false difference:

Key [<connection>.enabled_clients] found in 'localObj' but not in 'remoteObj'.

The fix enriches the remote connections with their enabled_clients via getConnectionEnabledClients before diffing, mirroring what the real import path already does in getType(). The change is scoped entirely to dryRunChanges, so real import and export behavior is untouched.

📚 References

🔬 Testing

Added a regression test in test/tools/auth0/handlers/dryRun.tests.ts that fails on the old code (reports a spurious update) and passes with the fix.

Verified end to end against live tenants: exported a tenant to a directory, then ran a dry-run against the identical export. On tenants whose connections.list omits enabled_clients inline, the old code produced the false positive and the fix removes it. Confirmed the real import and export paths are unaffected.

To reproduce manually:

npx a0deploy export -c=config.json --format=directory --output_folder=local
npx a0deploy import -c=config.json --input_file=local --dry-run --debug

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@harshithRai
harshithRai force-pushed the DXCDT-2281 branch 2 times, most recently from fe7bcf7 to f5fd72a Compare August 28, 2026 09:11
@codecov-commenter

codecov-commenter commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.85%. Comparing base (4955e4b) to head (a0a8601).

Files with missing lines Patch % Lines
src/tools/auth0/handlers/databases.ts 81.81% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1479      +/-   ##
==========================================
+ Coverage   80.81%   80.85%   +0.03%     
==========================================
  Files         163      163              
  Lines        7805     7807       +2     
  Branches     1741     1742       +1     
==========================================
+ Hits         6308     6312       +4     
+ Misses        797      796       -1     
+ Partials      700      699       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@harshithRai
harshithRai marked this pull request as ready for review August 31, 2026 10:17
@harshithRai
harshithRai requested a review from a team as a code owner August 31, 2026 10:17

@ankita10119 ankita10119 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't calcChanges also enrich with enabled_clients the same way dryRunChanges does after this fix? If connections.list no longer returns enabled_clients inline, then getEnabledClients in calcChanges would see no enabled_clients on the remote connection, which means excluded clients wouldn't be preserved and could get silently disabled on a real import. Am I reading this correctly?

@ankita10119

Copy link
Copy Markdown
Contributor

getType(), calcChanges, and dryRunChanges all independently call connections.list and enrich (or forget to enrich) enabled_clients separately. Would it be worth extracting the enrichment into a shared helper so these paths can't drift out of sync? I understand this is out of scope, just flagging it while we're touching this workflow

@harshithRai

Copy link
Copy Markdown
Contributor Author

Shouldn't calcChanges also enrich with enabled_clients the same way dryRunChanges does after this fix? If connections.list no longer returns enabled_clients inline, then getEnabledClients in calcChanges would see no enabled_clients on the remote connection, which means excluded clients wouldn't be preserved and could get silently disabled on a real import. Am I reading this correctly?

Yes that understanding is correct. And it's a valid catch too. Fixed both in the last 2 commits.

calcChanges now enriches too, so excluded-but-enabled clients are preserved on a real import instead of getting silently disabled. I confirmed it live on a test tenant: before the fix the excluded client was disabled, after it stays enabled.

getType(), calcChanges, and dryRunChanges all independently call connections.list and enrich (or forget to enrich) enabled_clients separately. Would it be worth extracting the enrichment into a shared helper so these paths can't drift out of sync? I understand this is out of scope, just flagging it while we're touching this workflow

I also pulled the enrichment into a single shared helper (enrichConnectionsWithEnabledClients) that getType, calcChanges, and dryRunChanges all use, so the paths can't drift. Small bonus: calcChanges reuses getType's cached result rather than re-fetching, which also dropped a duplicate connections.list call.

@harshithRai
harshithRai merged commit 460e7a3 into master Sep 3, 2026
9 checks passed
@harshithRai
harshithRai deleted the DXCDT-2281 branch September 3, 2026 12:09
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.

3 participants