Skip to content

fix(web): surface BYOK migration validation errors - #6280

Merged
AmyShang-alt merged 2 commits into
mainfrom
codex/fix-6279-byok-migration-error
Jul 31, 2026
Merged

fix(web): surface BYOK migration validation errors#6280
AmyShang-alt merged 2 commits into
mainfrom
codex/fix-6279-byok-migration-error

Conversation

@open-design-crew

Copy link
Copy Markdown
Contributor

Fixes #6279

Why

Use case: While validating the Codex plugin flow added in #6055, an existing legacy BYOK configuration failed its automatic secure-profile migration during startup.

Pain: The daemon returned a safe, actionable HTTP 400 validation reason, but the web client discarded it and told the user that the local daemon might be offline. That false diagnosis prevents the user from correcting the legacy field and leaves the credential in browser storage until migration can succeed. Because #6055 was backported by #6262, the same bug is present on the pre-tag release/v0.17.0 branch.

What users will see

When legacy BYOK migration receives an HTTP error from the daemon, the startup toast shows the daemon's concrete validation reason. The generic offline message is now reserved for network failures where the daemon could not be reached. Failed migrations continue to preserve the legacy browser credential until a later migration succeeds.

Surface area

  • UI — new page / dialog / panel / menu item / setting / empty state in apps/web or apps/desktop (including Electron menu bar)
  • Keyboard shortcut — new or changed
  • CLI / env var — new od subcommand or flag, new tools-dev / tools-pack flag, or new OD_* env var
  • API / contract — new /api/* endpoint, new SSE event, or changed shape in packages/contracts
  • Extension point — new entry under skills/, design-systems/, design-templates/, or craft/, or change to the skills protocol
  • i18n keys — added new translation keys (see TRANSLATIONS.md for the locale workflow)
  • New top-level dependency — adding any new entry to the root package.json (dependencies or devDependencies); workspace-package package.json files are out of scope. Include a paragraph on what we get vs. what bytes we ship (see CONTRIBUTING.md → Code style)
  • Default behavior change — changes what existing users experience without opting in (default model, default setting, file/SQLite schema, auto-network on startup, auto-install)
  • None — internal refactor, docs, tests, or translation update only

Screenshots

Not applicable.

Bug fix verification

  • Test path that reproduces the bug: apps/web/tests/state/config.test.ts
  • Red on main: yes
  • Green on this branch: yes
  • Explanation: On clean main at 7d7c56a, the focused migration spec expected the daemon's BYOK baseUrl... validation message and received Failed to save BYOK credential (400). The same spec and the HTTP-fallback/network-classification coverage pass on this branch.

Validation

  • mise exec -- pnpm --filter @open-design/web exec vitest run -c vitest.config.ts tests/state/config.test.ts -t "surfaces the daemon validation reason|keeps the HTTP status fallback|classifies fetch rejection"
  • mise exec -- pnpm --filter @open-design/web exec vitest run -c vitest.config.ts tests/state/config.test.ts
  • mise exec -- pnpm --filter @open-design/web exec vitest run -c vitest.config.ts tests/components/App.project-create-race.test.tsx
  • mise exec -- pnpm --filter @open-design/web typecheck
  • mise exec -- pnpm guard
  • mise exec -- pnpm typecheck
  • git diff --check

Adjacent issues

Preserve structured daemon errors during legacy credential migration and reserve offline guidance for fetch failures.

Keep legacy browser credentials until migration succeeds, and cover HTTP validation, malformed responses, and network failures.

Validated with focused web tests, web typecheck, workspace guard, and workspace typecheck.
@open-design-crew open-design-crew Bot added the backport release/v0.17.0 Backport this fix to release/v0.17.0 label Jul 30, 2026
@open-design-release-bot open-design-release-bot Bot removed the backport release/v0.17.0 Backport this fix to release/v0.17.0 label Jul 30, 2026
@open-design-release-bot

Copy link
Copy Markdown
Contributor

ℹ️ The backport release/v0.17.0 label requires write permission or above (@open-design-crew[bot] currently has none), so it was removed automatically. Please ask a maintainer to confirm whether this should ship in the current release.

@lefarcen
lefarcen requested a review from mrcfps July 30, 2026 09:44
@lefarcen lefarcen added size/M PR changes 100-300 lines risk/medium Medium risk: regular code changes type/bugfix Bug fix needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 30, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we'll loop QA in once it's merge-ready (and design/product have signed off, where applicable).

@mrcfps mrcfps 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.

@app/open-design-crew I verified the legacy BYOK migration path now preserves the daemon’s structured validation message, reserves the offline guidance for fetch failures, and keeps browser-stored credentials intact until migration succeeds. The full config test file passes (71 tests), along with web typecheck, repository guard, full workspace typecheck, and diff checks. Thanks for the careful error classification and focused regression coverage—this is a solid, user-helpful fix. 🙌

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Visual regression review

Head: fcbfa54 · Base: 7d7c56a

5 changed · 48 unchanged · 0 new without baseline · 0 failed

Changed cases

Case Main PR Diff
visual-settings-byok-model-dropdown
9,737 px (0.75%)
main pr diff
visual-settings-byok-openai
9,918 px (0.77%)
main pr diff
visual-topbar-byok-model-dropdown
4,920 px (0.38%)
main pr diff
visual-topbar-byok-switcher
4,765 px (0.37%)
main pr diff
visual-topbar-byok-switcher-popover
522 px (0.42%)
main pr diff
Unchanged cases
Case Main PR Diff
visual-avatar-local-agent-list
0 px (0.00%)
main pr diff
visual-avatar-local-agent-list-panel
0 px (0.00%)
main pr diff
visual-avatar-menu
0 px (0.00%)
main pr diff
visual-avatar-menu-panel
0 px (0.00%)
main pr diff
visual-avatar-open-design-account
0 px (0.00%)
main pr diff
visual-critical-settings
0 px (0.00%)
main pr diff
visual-critical-workspace
0 px (0.00%)
main pr diff
visual-critical-workspace-preview
0 px (0.00%)
main pr diff
visual-design-system-detail
0 px (0.00%)
main pr diff
visual-design-systems
0 px (0.00%)
main pr diff
visual-home
0 px (0.00%)
main pr diff
visual-home-catalog
0 px (0.00%)
main pr diff
visual-home-context-picker
0 px (0.00%)
main pr diff
visual-home-context-picker-popover
0 px (0.00%)
main pr diff
visual-home-plugin-filter
0 px (0.00%)
main pr diff
visual-home-plugin-use-staged
0 px (0.00%)
main pr diff
visual-home-plugin-use-with-query
0 px (0.00%)
main pr diff
visual-home-staged-attachment
0 px (0.00%)
main pr diff
visual-integrations
0 px (0.00%)
main pr diff
visual-integrations-mcp
0 px (0.00%)
main pr diff

Visual diff is advisory only and does not block merging.

@lefarcen
lefarcen requested a review from AmyShang-alt July 30, 2026 09:58
Map secure-profile HTTP and network failures to semantic error codes before emitting settings_byok_test_result, while preserving existing fallback behavior for unrelated connection-test errors.

Validated with the complete web test suite, web typecheck, workspace typecheck, guard, and git diff --check.
@AmyShang-alt

Copy link
Copy Markdown
Contributor

QA 验收通过

已在 PR head fcbfa546 的真实运行实例完成验证。

  • legacy BYOK 迁移请求实际命中 /api/byok/profiles
  • daemon 返回 HTTP 400 时,UI 展示结构化校验信息:
    BYOK baseUrl must be an absolute HTTP(S) URL without credentials, query, or fragment.
  • 未错误显示 Local daemon may be offline
  • 相关 CI、Web 全量测试、targeted regression tests、typecheck、UI P0 均通过
  • 自动化覆盖确认迁移失败时保留旧凭据

QA 结论:通过,可合并。

备注:本记录为 QA 验收结论,不代替 maintainer 的代码 review approval。

@AmyShang-alt AmyShang-alt 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.

QA verified on PR head fcbfa54. Legacy BYOK HTTP 400 errors surface the daemon validation message, offline guidance is reserved for network failures, and regression/CI checks are green.

@lefarcen lefarcen added validated Runtime change validated (via /explore Pass or manual QA). and removed needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 30, 2026
@AmyShang-alt
AmyShang-alt added this pull request to the merge queue Jul 31, 2026
Merged via the queue into main with commit 4a6c2da Jul 31, 2026
22 checks passed
@lefarcen lefarcen added backport release/v0.17.0 Backport this fix to release/v0.17.0 and removed backport release/v0.17.0 Backport this fix to release/v0.17.0 labels Jul 31, 2026
@open-design-release-bot

Copy link
Copy Markdown
Contributor

Successfully created backport PR for release/v0.17.0:

open-design-release-bot Bot added a commit that referenced this pull request Jul 31, 2026
* fix(web): surface BYOK migration validation errors

Preserve structured daemon errors during legacy credential migration and reserve offline guidance for fetch failures.

Keep legacy browser credentials until migration succeeds, and cover HTTP validation, malformed responses, and network failures.

Validated with focused web tests, web typecheck, workspace guard, and workspace typecheck.

* fix(web): stabilize BYOK persistence error telemetry

Map secure-profile HTTP and network failures to semantic error codes before emitting settings_byok_test_result, while preserving existing fallback behavior for unrelated connection-test errors.

Validated with the complete web test suite, web typecheck, workspace typecheck, guard, and git diff --check.

---------


(cherry picked from commit 4a6c2da)

Co-authored-by: open-design-crew[bot] <299007234+open-design-crew[bot]@users.noreply.github.com>
Co-authored-by: Cheems <94773058+itscheems@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport release/v0.17.0 Backport this fix to release/v0.17.0 risk/medium Medium risk: regular code changes size/M PR changes 100-300 lines type/bugfix Bug fix validated Runtime change validated (via /explore Pass or manual QA).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Legacy BYOK migration hides daemon validation errors

3 participants