[Repo Assist] fix: guard bootstrap prompt against already-configured workspaces#735
Conversation
Root cause: OnboardingChatBootstrapper.BootstrapAsync() only checked the HasInjectedFirstRunBootstrap flag, which could be unset (false) even when the user already had a configured gateway (e.g. fresh app install over an existing workspace, settings migration, or flag reset). This caused the first-run ritual prompt to fire against an already-configured workspace, risking unintended rewrites of SOUL.md, AGENTS.md, and other workspace files. Fix: Add an optional GatewayRegistry parameter to BootstrapAsync(). Before sending the bootstrap message, check SetupExistingGatewayClassifier .HasAnyExistingGatewayConnection(). If an existing gateway configuration is detected, silently mark the gate as consumed and return true without sending the prompt. ChatPage.xaml.cs now passes app.Registry to BootstrapAsync() to enable this guard. Tests: 4 new unit tests cover the existing-gateway-skip path (SharedGatewayToken, BootstrapToken), the empty-registry first-run path, and the no-registry path (backward compatibility). All 996 tray tests pass. Closes #729 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Codex review: found issues before merge. Reviewed June 9, 2026, 9:52 PM ET / 01:52 UTC. Summary Reproducibility: yes. for the PR regression from source inspection: setup creates registry-backed credentials, and the added guard treats those credentials as a completed first-run state before sending the bootstrap message. I did not run a live Windows tray smoke because this is a read-only review. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Use a first-run discriminator that separates reinstall/already-configured workspaces from newly-created setup credentials, then prove both flows with focused tests and redacted real behavior evidence. Do we have a high-confidence way to reproduce the issue? Yes for the PR regression from source inspection: setup creates registry-backed credentials, and the added guard treats those credentials as a completed first-run state before sending the bootstrap message. I did not run a live Windows tray smoke because this is a read-only review. Is this the best way to solve the issue? No, not as written. Registry credentials alone are not a safe proxy for an already-configured workspace because fresh setup uses the same credential surfaces before the first-run prompt has run. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 0e61fa287afb. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
🤖 This is an automated pull request from Repo Assist, an AI assistant for this repository.
Summary
Closes #729
OnboardingChatBootstrapper.BootstrapAsync()only checkedHasInjectedFirstRunBootstrapbefore sending the first-run bootstrap prompt. This flag defaults tofalseand is only set totrueafter a successful first-run completion. On reinstall over an existing workspace — or after any settings migration that resets the flag — this caused the bootstrap ritual to fire against an already-configured workspace.Root Cause
ShouldBootstrap()/BootstrapAsync()had a single guard:No check was made against the
GatewayRegistryto determine whether usable credentials already exist.Fix
Before sending the prompt,
BootstrapAsync()now callsSetupExistingGatewayClassifier.HasAnyExistingGatewayConnection(registry, settings, dataPath). If the registry has any usable record (shared gateway token, bootstrap token, or per-gateway device key), the bootstrapper silently callsMarkBootstrapped()and exits — same end state as completing first-run normally, but without prompting the user.ChatPage.xaml.cspassesregistry: app.Registryto the call. Theregistryparameter is optional (= null), so callers that don't pass it continue to get the previous behavior unchanged.Trade-offs
SettingsManager.SettingsDirectoryPathis the static env-var path used for thedataPathargument in the guard. In tests this differs from the injected_settingsDirectory, but tests exercise the registry path (condition 1/2) which fires before the dataPath is needed, so all tests pass correctly.ShouldBootstrap()is dead code (never called) — left unchanged to avoid unrelated scope creep.Changes
src/OpenClaw.Tray.WinUI/Services/OnboardingChatBootstrapper.csusing OpenClaw.Connection;, add optionalGatewayRegistry? registryparam toBootstrapAsync(), add existing-gateway guardsrc/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.csappvariable, passregistry: app.RegistrytoBootstrapAsync()tests/OpenClaw.Tray.Tests/OnboardingChatBootstrapperTests.csTest Status
./build.ps1requires Windows. Compilation validated viadotnet test /p:EnableWindowsTargeting=trueon Linux CI.mainbefore this change) ✅Add this agentic workflows to your repo
To install this agentic workflow, run