[PM-37965] Milestone 2 - Staged Users by sven-bitwarden · Pull Request #1163 · bitwarden/directory-connector

sven-bitwarden · 2026-07-13T19:37:25Z

🎟️ Tracking

📔 Objective

The - final - ticket for Milestone 2, this allows BWDC to specify whether or not to automatically send email invitations after provisioning. Disabling this setting will provision users in the Staged state.

📸 Screenshots

github-actions · 2026-07-13T19:38:09Z

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Re-reviewed the inviteUsersAfterProvisioning toggle after the state-migration follow-up commits. The StateVersion.Eight migration backfills the flag to true only when an existing sync config omits it, leaves explicit true/false values untouched, does not create a config where none exists, and is wired into the migrate() switch so 3→5→6→7→8 chains land at Latest while fresh installs are stamped at Latest and keep the false default — this resolves the upgrade-behavior question raised on the earlier pass. The flag threads through RequestBuilderOptions, both request builders (including every partitioned batch request), OrganizationImportRequest, the GUI checkbox (matching the existing sync.update() signal pattern used by removeDisabled/largeImport), and bwdc config sync.inviteUsersAfterProvisioning with true/false validation, with all directory config fixtures updated. Unit, integration, and migration tests cover forwarding, the absent-flag fallback, and each backfill branch; no new findings this pass.

Code Review Details

♻️ : Test comment still references the wrong migration (v6->v7 / migrateStateFrom6To7) for the backfill added here — already raised in an open thread
- libs/services/sync.service.integration.spec.ts:169

codecov · 2026-07-13T19:40:00Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (951d96c) to head (599c56c).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@     Coverage Diff      @@
##   main   #1163   +/-   ##
============================
============================

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

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JaredScar · 2026-07-14T20:02:28Z

Add labels... @sven-bitwarden

claude · 2026-08-06T15:15:15Z

    this.overwriteExisting = model.overwriteExisting;
    this.largeImport = model.largeImport;
+    // Defaults to false so users are not invited unless invitations are explicitly enabled.
+    this.inviteUsersAfterProvisioning = model.inviteUsersAfterProvisioning ?? false;


❓ QUESTION: Defaulting the absent flag to false silently stops invitations for every existing install on upgrade — is that the intended rollout?

Details

Saved data.json configs written before this PR have no inviteUsersAfterProvisioning key, and stateService.getSync() returns the stored object rather than a new SyncConfiguration(), so the class-level default never applies. The ?? false fallback here means the first sync after upgrade posts inviteUsersAfterProvisioning: false, and newly provisioned users land in Staged instead of receiving invitation emails. Because the added field also changes the request JSON, the sync hash differs and the import is guaranteed to be re-posted on that first run.

Admins who never touch the new checkbox get a behavior change they did not opt into, and it fails silently — the sync reports success while new users simply never get invited. The commit message for b1da474 acknowledges this, so if it is a deliberate product decision for the Staged Users milestone it would help to note it in the PR description so it makes the release notes; otherwise defaulting the absent-key case to true (while keeping false as the default for newly created configs) preserves existing behavior on upgrade.

Is this valid? @sven-bitwarden

Kind of. We want to default to false, the only time the setting should be true is if there was a prior installation, which is precisely what the migration is for

…ning

Flip the default for inviteUsersAfterProvisioning from true to false so users are not invited after provisioning unless invitations are explicitly enabled. - SyncConfiguration: new-config default is now false - OrganizationImportRequest: field default and absent-flag fallback now resolve to false (a saved config that predates this setting no longer sends invitations on upgrade) - GUI checkbox: unset now displays as unchecked - Config fixtures updated to match the new default - Tests updated: "unset" cases expect false; forwarding tests push true to confirm a non-default value is still carried through

The previous commit made inviteUsersAfterProvisioning default to false. That is correct for new installs, but would silently stop sending invitations for existing users who provisioned before the setting existed (their persisted sync config has no such key). Add a v6->v7 state migration that backfills inviteUsersAfterProvisioning to true on any sync config that already exists but does not define the flag. Existing installs therefore keep their prior invite-on-provision behavior, while fresh installs report stateVersion Latest, skip the migration, and retain the new false default. - StateVersion: add Seven, make it Latest - StateMigrationService: migrateStateFrom6To7 backfills the flag onto an existing sync config; explicit values are left untouched and no config is created when none exists - Update migration specs for the new Latest version and add coverage for the v6->v7 backfill - Clarify the sync-service fallback test comment now that real upgraded configs are backfilled before reaching the request builder

claude · 2026-08-12T18:29:07Z

+      // safe default). Real upgraded configs are backfilled to true by the v6->v7 state
+      // migration before they ever get here (see StateMigrationService.migrateStateFrom6To7).


♻️ DEBT: Comment points at the wrong migration — the backfill is v7→v8 (migrateStateFrom7To8), not v6→v7.

Details and fix

migrateStateFrom6To7 is the keytar credential migration; the inviteUsersAfterProvisioning backfill added in this PR lives in migrateStateFrom7To8 (libs/services/state-service/stateMigration.service.ts:364). Anyone tracing invite behavior from this test lands in the wrong method.

Suggested change

// safe default). Real upgraded configs are backfilled to true by the v6->v7 state

// migration before they ever get here (see StateMigrationService.migrateStateFrom6To7).

// safe default). Real upgraded configs are backfilled to true by the v7->v8 state

// migration before they ever get here (see StateMigrationService.migrateStateFrom7To8).

sven-bitwarden requested a review from a team as a code owner July 13, 2026 19:37

sven-bitwarden requested a review from JaredScar July 13, 2026 19:37

sven-bitwarden added the t:feature Change Type - Feature Development label Jul 21, 2026

JaredScar previously approved these changes Jul 21, 2026

View reviewed changes

sven-bitwarden dismissed JaredScar’s stale review via b1da474 July 24, 2026 16:51

sven-bitwarden added needs-qa ai-review-vnext Request a Claude code review using the vNext workflow labels Aug 6, 2026

claude Bot reviewed Aug 6, 2026

View reviewed changes

sven-bitwarden removed the needs-qa label Aug 12, 2026

sven-bitwarden added 3 commits August 12, 2026 12:56

adds ability to specify whether or not to invite users after provisio…

a8f0737

…ning

sven-bitwarden force-pushed the ac/pm-37965/milestone-2-bwdc-edition branch from c4c8f81 to a85ceaf Compare August 12, 2026 18:24

JaredScar previously approved these changes Aug 12, 2026

View reviewed changes

claude Bot reviewed Aug 12, 2026

View reviewed changes

fix lint

599c56c

sven-bitwarden dismissed JaredScar’s stale review via 599c56c August 12, 2026 20:05

sven-bitwarden enabled auto-merge (squash) August 12, 2026 20:07

sven-bitwarden requested a review from JaredScar August 12, 2026 20:37

JaredScar requested changes Aug 13, 2026

View reviewed changes

sven-bitwarden requested a review from JaredScar August 13, 2026 15:55

JaredScar approved these changes Aug 13, 2026

View reviewed changes

sven-bitwarden merged commit d9afa86 into main Aug 13, 2026
42 checks passed

sven-bitwarden deleted the ac/pm-37965/milestone-2-bwdc-edition branch August 13, 2026 22:15

Navigation Menu

Uh oh!

[PM-37965] Milestone 2 - Staged Users - #1163

[PM-37965] Milestone 2 - Staged Users#1163
sven-bitwarden merged 4 commits into
mainfrom
ac/pm-37965/milestone-2-bwdc-edition

sven-bitwarden commented Jul 13, 2026 •

edited by atlassian Bot

Loading

Uh oh!

github-actions Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

JaredScar commented Jul 14, 2026

Uh oh!

claude Bot Aug 6, 2026

Uh oh!

JaredScar Aug 13, 2026

Uh oh!

sven-bitwarden Aug 13, 2026

Uh oh!

claude Bot Aug 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// safe default). Real upgraded configs are backfilled to true by the v6->v7 state
		// migration before they ever get here (see StateMigrationService.migrateStateFrom6To7).

Uh oh!

Conversation

sven-bitwarden commented Jul 13, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

📸 Screenshots

Uh oh!

github-actions Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 Bitwarden Claude Code Review

Uh oh!

codecov Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JaredScar commented Jul 14, 2026

Uh oh!

claude Bot Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

JaredScar Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

sven-bitwarden Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sven-bitwarden commented Jul 13, 2026 •

edited by atlassian Bot

Loading

github-actions Bot commented Jul 13, 2026 •

edited

Loading

codecov Bot commented Jul 13, 2026 •

edited

Loading