fix(onboard): validate reset options before deleting state#111348
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 626ab893f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| snapshot.exists && | ||
| !snapshot.valid && | ||
| !snapshot.sourceConfig |
There was a problem hiding this comment.
Detect unreadable snapshots without testing object truthiness
When --reset-scope full encounters malformed JSON5 or an unreadable config, this guard never runs: ConfigFileSnapshot.sourceConfig is required, and the snapshot reader supplies {} on parse failure (src/config/io.snapshot.ts:90-99) and read failure. Because {} is truthy, onboarding falls through to DEFAULT_WORKSPACE and moves that unrelated workspace to Trash even though the actual configured workspace is unknown. Check parse/read failure state such as raw, readError, or snapshot issues instead.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
What Problem This Solves
Fixes an issue where users running onboarding with
--resetcould have their config, credentials, sessions, or workspace moved to Trash before invalid options were rejected. It also fixes full reset targeting the default workspace when a schema-invalid but parseable config pointed at a custom workspace.AI-assisted change; the implementation and proof were reviewed locally before submission.
Why This Change Was Made
Reset is now the final step before setup dispatch, after deterministic mode, option, gateway, provider-auth, credential-storage, setup-token, and workspace checks complete. Provider-owned non-interactive auth methods expose a side-effect-free validation hook; migration import fails closed when combined with reset because migration providers do not yet expose a reusable read-only import plan.
For full reset, the parsed source config remains authoritative even when schema validation fails. OpenClaw deletes only the configured workspace, and refuses the reset when the workspace cannot be determined safely.
User Impact
Invalid onboarding invocations no longer destroy existing state before reporting their error. Users with a schema-invalid config and a custom workspace can safely request a full reset without risking deletion of the unrelated default workspace.
Evidence
Reproduction before this change:
openclaw onboard --reset --non-interactive --accept-risk --mode typowith existing state.After this change, the same invocation reports the invalid mode and does not call reset. A second regression test supplies a schema-invalid parsed config whose
agents.defaults.workspaceis custom; full reset targets that custom path and never the default workspace.Focused proof:
node scripts/run-vitest.mjs src/commands/onboard.test.ts src/commands/onboard-non-interactive.gateway.test.ts src/commands/onboard-remote.test.ts src/plugins/provider-api-key-auth.test.ts— 64 tests passed across four files.node scripts/run-vitest.mjs extensions/anthropic/index.test.ts— 47 tests passed.pnpm tsgopnpm check:test-typesnode scripts/run-oxlint.mjs <touched files>pnpm plugin-sdk:surface:checkpnpm deadcode:exportsgit diff --checkorigin/main: clean, no accepted/actionable findings.