refactor(desktop): unify settings UI layout by SivanCola · Pull Request #2268 · esengine/DeepSeek-Reasonix

SivanCola · 2026-05-29T09:13:57Z

Summary

Split the desktop settings UI refactor out of feat(desktop): Native macOS Update Center｜原生 macOS 更新中心 #2231 so the native update-center PR stays focused.
Add scoped settings layout primitives/styles and a bilingual settings UI guideline document.
Merge theme mode/style into one flow: the selected mode shows only matching theme cards while preserving all 8 themes across modes.
Normalize credential rows, status badges, QQ connection state, and remove the duplicate General reasoning-effort control.

Verification

npm run test -- --run tests/web-tools.test.ts
npm run typecheck
npm --prefix desktop run build
pre-push npm run verify: 315 test files / 4050 tests passed, 9 skipped

Split From

Extracted from feat(desktop): Native macOS Update Center｜原生 macOS 更新中心 #2231 after review; update overlay positioning remains in feat(desktop): Native macOS Update Center｜原生 macOS 更新中心 #2231.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 515dd47673

ℹ️ 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".

esengine

Thanks for splitting the settings-UI refactor out of #2231 to keep that PR focused — that was the right move, and the settings-ui-guidelines.md doc is a nice addition. I haven't done the full behavior-preservation pass on the ~2000-line styles.css/settings.tsx rewrite yet, because the branch needs a rebase first (the diff will shift), but here's what's blocking:

CI is red — merge-skew with code I just merged. tests/web-tools.test.ts > web search applies 15s timeout to fetch fails with "expected fetch to be called with an AbortSignal". This collides with the searchSignal() 15s-timeout work that just landed in #2259/#2252 (every engine's fetch now gets AbortSignal.any([caller, timeout])). Your branch predates it, so the test-merge drops the timeout on searchBing and the assertion fails. Please rebase onto current main and reconcile against the merged searchSignal helper.
Scope creep (and it's the source of the red CI). A desktop settings-UI refactor shouldn't be touching src/tools/web.ts (the async function searchBing → export async function searchBing change) or adding tests/web-tools.test.ts. Please drop both from this PR. If exporting searchBing + a web-tools test is genuinely wanted, it belongs in a separate, clearly-scoped web PR — not bundled into the settings refactor. Removing them here also makes the rebase in (1) trivial.
CodeQL "failure" is not a security finding — the run is a build/process exit-1 (no flagged alert), so it should go green once the build and tests pass after the rebase. No action needed beyond fixing (1)/(2).
Once it's rebased and CI is green, I'll do the full review of the settings/styles refactor — specifically verifying the theme mode/style merge preserves all 8 themes across modes as the description claims, that no shipped settings rows/controls are silently dropped in the settings.tsx rewrite, and i18n parity across en/ja/zh-CN (please run tsc inside desktop/ — root CI doesn't typecheck desktop/, and ja.ts looked like it only added 1 key).

Get (1) and (2) sorted and ping me — happy to dig into the refactor itself right after.

…ayout # Conflicts: # desktop/src/styles.css

SivanCola · 2026-05-29T09:45:39Z

Fixed the CodeQL check failure from run 78474119016.

Root cause: the Perplexity missing-key test used a predictable temp config path with the process id, which flowed into readConfig and triggered CodeQL insecure temporary file detection.

What changed:

Replaced the predictable temp path with mkdtempSync(join(tmpdir(), ...)) plus a per-test config.json inside that secure temp directory.
Added cleanup with rmSync(..., { recursive: true, force: true }).
Synced the branch with latest upstream/main and resolved the styles.css conflict by preserving main animation cleanup.

Verification:

npm run test -- --run tests/web-tools.test.ts passed
npm run typecheck passed
pre-push npm run verify passed: 315 test files / 4051 tests passed, 9 skipped
GitHub checks on head c4f47fc are green, including CodeQL and both CI jobs.

esengine

Good split out of #2231 — settings layout primitives, theme mode/style merge, and credential normalization are all things I wanted. I'll trim CONTRIBUTING.md and docs/design/settings-ui-guidelines.md in a follow-up — the source code is the source of truth for component conventions in this repo, no separate design doc needed. Approving so CI can re-run.

SivanCola · 2026-05-29T10:01:52Z

Resolved the review blockers in #2268 (review).

What changed:

Dropped the web-search scope creep from this settings UI PR. The current PR files no longer include src/tools/web.ts or tests/web-tools.test.ts.
Kept the branch synced with current upstream/main (72e89ff7).
Left the settings UI/docs/i18n/test work intact.

Verification run locally:

npm run test -- --run tests/desktop-settings-ui.test.tsx tests/desktop-mcp-settings-layout.test.ts desktop/src/App.test.ts
npm run typecheck
npm --prefix desktop exec tsc -- --noEmit
npm --prefix desktop run build
pre-push npm run verify with a clean temporary HOME: 315 test files passed, 4051 tests passed, 9 skipped

Note: the first local pre-push run used my real ~/.reasonix/config.json, so the restored upstream tests/web-tools.test.ts picked up my Perplexity key and failed its missing-key expectation. I reran the same pre-push hook with only the hook environment isolated to a temporary HOME, without changing web files or bypassing the hook.

CI is now running on head a38e4832.

SivanCola · 2026-05-29T10:09:38Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a38e48320f

ℹ️ 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".

chatgpt-codex-connector · 2026-05-29T10:12:51Z

    setAppSecret(current.appSecret ?? "");
    setSandbox(current.sandbox ?? true);
-  }, [current.appId, current.appSecret, current.sandbox, configureOpen]);
+  }, [current.appId, current.appSecret, current.sandbox]);


Reset QQ drafts when reopening configuration

When a user opens the QQ configuration, edits the app id/secret or sandbox value, then clicks Back without saving and opens Configure again, the draft state is no longer reset because this effect only depends on the persisted current values. The previous configureOpen dependency refreshed the fields on each reopen, but now stale unsaved values remain and can be saved or connected accidentally; reset the drafts when configureOpen changes or when opening the panel.

Useful? React with 👍 / 👎.

refactor(settings): unify settings UI layout

515dd47

SivanCola mentioned this pull request May 29, 2026

feat(desktop): Native macOS Update Center｜原生 macOS 更新中心 #2231

Open

chatgpt-codex-connector Bot reviewed May 29, 2026

View reviewed changes

Comment thread desktop/src/ui/settings.tsx Outdated

fix(settings): preserve Escape close shortcut

2a7b25b

esengine requested changes May 29, 2026

View reviewed changes

SivanCola added 2 commits May 29, 2026 17:33

fix(test): avoid predictable Perplexity config path

52e3400

Merge remote-tracking branch 'upstream/main' into codex/settings-ui-l…

c4f47fc

…ayout # Conflicts: # desktop/src/styles.css

esengine approved these changes May 29, 2026

View reviewed changes

chore(settings): drop web search changes from ui pr

a38e483

chatgpt-codex-connector Bot reviewed May 29, 2026

View reviewed changes

esengine added the v1 Legacy TypeScript line (0.x) — v1 branch, maintenance only label May 31, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(desktop): unify settings UI layout#2268

refactor(desktop): unify settings UI layout#2268
SivanCola wants to merge 5 commits into
esengine:v1from
SivanCola:codex/settings-ui-layout

SivanCola commented May 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

esengine left a comment

Uh oh!

SivanCola commented May 29, 2026

Uh oh!

esengine left a comment

Uh oh!

SivanCola commented May 29, 2026

Uh oh!

SivanCola commented May 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

SivanCola commented May 29, 2026

Summary

Verification

Split From

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

esengine left a comment

Choose a reason for hiding this comment

Uh oh!

SivanCola commented May 29, 2026

Uh oh!

esengine left a comment

Choose a reason for hiding this comment

Uh oh!

SivanCola commented May 29, 2026

Uh oh!

SivanCola commented May 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants