Skip to content

feat(onboarding): the first global administrator is pinned to no learning path - #5077

Merged
meshweaver-cloud[bot] merged 5 commits into
mainfrom
feat/first-admin-no-learning-path
Sep 22, 2026
Merged

meshweaver-cloud[bot] merged 5 commits into
mainfrom
feat/first-admin-no-learning-path

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

"The learning path I don't need for first onboarding of global admin." — policy first-admin-no-learning-path (register: Doc/Architecture/PolicyNotProse).

That person is setting the instance up, not learning it. A Pinned tab full of doc landing pages in front of the setup work is noise at the one moment it costs most.

  • UserOnboardingDefaults.PinnedPathsFor(isPlatformBootstrap) — the one rule: [] for the first administrator, the four doc sections for everybody else. It is a create-time seed, never a value a later onboarding write re-imposes.
  • UserOnboardingRequest.IsPlatformBootstrap — an init-only property, default false. A caller's decision, not a form field, so it stays out of the primary constructor and the record's constructor signature is unchanged (the Public surface (binary compatibility) gate measured the first cut's positional parameter as an arity break; this shape has nothing to declare, no allow entry).
  • BootstrapController.FirstAdmin (the headless bootstrap path) sets it.
  • The first-user promotion in Onboarding.razor (MeshWeaver.Plugins#1961) sets the same flag once this seals — a one-line request with { IsPlatformBootstrap = admission.PromoteToPlatformAdmin }. Until then it behaves as today.

The review finding, and the fix

Copilot found a data-loss defect in the first cut, and it was right: FirstAdmin is documented as re-runnable ("already exists" is a successful step, because a re-run still wants the Admin grants), and CreateUser lands the profile through the full-instance upsert, whose update leg takes Content wholesale (UpdateAccordingToSourceNode). A second call on an existing user therefore replaced PinnedPaths with the seed — [] — and erased every pin added since. (The pre-existing code had the same shape with the four doc sections as the seed; the empty seed made it visible.)

CreateUser stays ONE owner-side upsert — the client-side create-vs-update split is exactly what its own comment records as removed (the exists-check lagged a concurrent create → patch onto a not-yet-materialised node → error storm → OOM wedge) — and now declares PinnedPaths a create-once member on it: CreateOrUpdateNodeRequest.WithFolds<User>(f => f.KeepExisting(u => u.PinnedPaths)) (FoldRule.KeepExisting, the shape #4928 added for exactly this and #5187 adopts for activity tracking). The seed lands when the profile is created; on an existing profile the owner keeps the pins it holds, whatever the request carries. Every other profile member still lands wholesale, deliberately. Issued off the router exactly as MeshService.CreateOrUpdateNode does, identity stamped on the request and the delivery; no IMeshService change.

Two-sided control — OnboardingPinsAreSeededOnceTest, real mesh, the second write's landing awaited on the node (its FullName change) before the pins are asserted:

case previous code this PR
first administrator, first call → [] green green
ordinary user, first call → the four sections green green
first administrator, pins something/i/pinned, re-run red — Expected {} to equal {"something/i/pinned"} green
ordinary user, unpins one section, pins one path, re-run red — the four sections re-imposed green

Local: dotnet build -c Release -warnaserror on Memex.Portal.Shared, Memex.Portal.Shared.Test, MeshWeaver.Documentation.Test — 0 Error(s) each. Memex.Portal.Shared.Test filtered to the onboarding + bootstrap tests: total: 12 failed: 0. Doc guards (link integrity, topic map, conflict markers): total: 6 failed: 0. check-record-signatures.py against the merge base: no binary-breaking primary-constructor changes.

Docs

Doc/Architecture/FirstRunSetupOnAProvisionedInstance → "The first administrator learns nothing here" carries the rule and the seed-not-reimposed reasoning. The two dated maintainer quotes the first cut added to that page become policy-register rows (first-admin-no-learning-path, setup-secrets-three-valued) cited from the page — policy-not-prose.

Companion to the merged #4474.

Pairs-with: none — nothing is removed from core's public surface; the record's primary constructor is unchanged and IsPlatformBootstrap is an added init-only property that MeshWeaver.Plugins (Onboarding.razor, two test suites — all ProjectReference into core source at the pin) compiles against unchanged.
Mirror-sync: none — no localization key is added or re-worded.

🤖 Generated with Claude Code

…ning path

Maintainer, 2026-09-21, from an attempt to onboard on partnerre.meshweaver.cloud:
'the learning path i don't need for first onboarding of global admin.' That
person is setting the instance up, not learning it — a Pinned tab full of doc
landing pages in front of the setup work is noise at the one moment it costs
most. Ordinary users keep the four sections.

UserOnboardingRequest gains IsPlatformBootstrap (default false, so no existing
caller's user loses a pin), UserOnboardingDefaults.PinnedPathsFor is the one
rule, and BootstrapController — the headless bootstrap path — passes true. The
first-user promotion in Onboarding.razor (MeshWeaver.Plugins) sets the same flag
once this seals; until then it behaves as today.

3 of 3 tests pass.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 21, 2026 13:27
…the first administrator

Two requirements from the 2026-09-21 PartnerRe onboarding attempt, recorded on
the design page: the dialog's third field state — a declared mapping whose
vault object is not there, probed from the instance with the read right it
already holds, three-valued so a permission error is never reported as either
answer — and the first global administrator being pinned to no learning path.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Repeated bootstrap calls can overwrite existing pins, potentially affecting arbitrary existing users.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

Adds bootstrap-aware onboarding so the first global administrator receives no learning-path pins, while ordinary users retain existing defaults.

Changes:

  • Adds IsPlatformBootstrap and conditional pin selection.
  • Updates bootstrap handling and documentation.
  • Adds tests for bootstrap and ordinary-user behavior.
File Summary
test/​Memex.Portal.Shared.Test/​FirstAdministratorHasNoLearningPathTest.cs Tests bootstrap and ordinary pin defaults.
src/​MeshWeaver.Documentation/​Data/​Architecture/​FirstRunSetupOnAProvisionedInstance.md Documents onboarding behavior.
memex/​Memex.Portal.Shared/​Authentication/​UserOnboardingService.cs Applies conditional pinned paths; service-level persistence coverage is still recommended.
memex/​Memex.Portal.Shared/​Authentication/​BootstrapController.cs Marks bootstrap requests; repeated calls may erase existing user pins and require correction.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

var request = new UserOnboardingRequest(user, email.Trim(), name ?? user);
// The first global administrator: no learning path pinned — they are setting the
// instance up, not learning it (UserOnboardingDefaults.PinnedPathsFor).
var request = new UserOnboardingRequest(user, email.Trim(), name ?? user, IsPlatformBootstrap: true);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct, and it is a real defect in this change. FirstAdmin is documented as re-runnable ("already exists" is a successful step, because a re-run still wants the Admin grants), and CreateUser lands the profile through CreateOrUpdateNode, whose update leg takes Content wholesale (UpdateAccordingToSourceNode: Content = sourceNode.Content ?? state.Content). So a second call on an existing user replaces their PinnedPaths with the seed — [] after this change — and erases anything they pinned since.

The fix follows on this branch: the pinned-paths seed applies only when the user is being created; on an existing profile the re-run must leave PinnedPaths exactly as it is, whatever the request says. It comes with a test that has a control on each side — a first call seeds nothing for the first administrator, and a second call after the user has pinned something leaves that pin intact — and the second case is proven red against the current code before the fix lands.

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

443 tests  ±0   443 ✅ ±0   54s ⏱️ +5s
  3 suites ±0     0 💤 ±0 
  3 files   ±0     0 ❌ ±0 

Results for commit 7b2cf72. ± Comparison against base commit 7ce4c43.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

  1 files  ±0    1 suites  ±0   2m 40s ⏱️ +10s
318 tests ±0  318 ✅ ±0  0 💤 ±0  0 ❌ ±0 
322 runs  ±0  322 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 7b2cf72. ± Comparison against base commit 7ce4c43.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

1 590 tests  ±0   1 590 ✅ ±0   3m 8s ⏱️ -4s
    2 suites ±0       0 💤 ±0 
    2 files   ±0       0 ❌ ±0 

Results for commit 7b2cf72. ± Comparison against base commit 7ce4c43.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

    3 files  ±0      3 suites  ±0   5m 57s ⏱️ +3s
1 938 tests ±0  1 938 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 939 runs  ±0  1 939 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 7b2cf72. ± Comparison against base commit 7ce4c43.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

725 tests  ±0   533 ✅ ±0   5m 33s ⏱️ -2s
  3 suites ±0   192 💤 ±0 
  3 files   ±0     0 ❌ ±0 

Results for commit 7b2cf72. ± Comparison against base commit 7ce4c43.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    5 files  ±0      5 suites  ±0   14m 45s ⏱️ -39s
3 915 tests +8  3 913 ✅ +8  2 💤 ±0  0 ❌ ±0 
3 919 runs  +8  3 917 ✅ +8  2 💤 ±0  0 ❌ ±0 

Results for commit 7b2cf72. ± Comparison against base commit 7ce4c43.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   17 files  ±0     17 suites  ±0   33m 1s ⏱️ -24s
8 929 tests +8  8 735 ✅ +8  194 💤 ±0  0 ❌ ±0 
8 938 runs  +8  8 744 ✅ +8  194 💤 ±0  0 ❌ ±0 

Results for commit 7b2cf72. ± Comparison against base commit 7ce4c43.

♻️ This comment has been updated with latest results.

rbuergi and others added 2 commits September 22, 2026 08:56
…eeps the user's pins

Review found a data-loss defect in the first cut: BootstrapController.FirstAdmin is
documented as re-runnable, and CreateUser lands the profile through the full-instance
upsert whose update leg takes Content wholesale, so a second call on an existing user
replaced PinnedPaths with the seed — [] for the first administrator — and erased every
pin added since. The two-sided control reproduces it verbatim against the previous code
(OnboardingPinsAreSeededOnceTest: first-call cases green, both re-run cases red).

The fix keeps CreateUser as ONE owner-side upsert (the client-side create-vs-update split
is the shape its own comment records as removed) and declares PinnedPaths a create-once
member on it (FoldRule.KeepExisting, #4928): the seed lands when the profile is created,
and on an existing profile the owner keeps the pins it holds. Every other member still
lands wholesale on purpose. Issued off the router exactly as MeshService.CreateOrUpdateNode
does, identity stamped on the request and the delivery.

IsPlatformBootstrap moves from the record's primary constructor to an init-only property:
the flag is the caller's decision, not a form field, and the constructor signature a
module compiled earlier calls stays exactly as it was — so the Public surface gate has
nothing to declare and no allow entry is needed.

Docs: the two dated maintainer quotes the first cut added become policy register rows
(first-admin-no-learning-path, setup-secrets-three-valued) cited from the page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi
rbuergi marked this pull request as ready for review September 22, 2026 07:14
…earning-path

# Conflicts:
#	src/MeshWeaver.Documentation/Data/Architecture/PolicyNotProse.md
@meshweaver-cloud
meshweaver-cloud Bot added this pull request to the merge queue Sep 22, 2026
Merged via the queue into main with commit 620a489 Sep 22, 2026
37 checks passed
@rbuergi
rbuergi deleted the feat/first-admin-no-learning-path branch October 10, 2026 13:10
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