Skip to content

fix(dashboard): add missing unit-test for validate proxy before account creation#92

Closed
smeinecke wants to merge 2 commits intodwgx:masterfrom
smeinecke:feat/add-account-proxy
Closed

fix(dashboard): add missing unit-test for validate proxy before account creation#92
smeinecke wants to merge 2 commits intodwgx:masterfrom
smeinecke:feat/add-account-proxy

Conversation

@smeinecke
Copy link
Copy Markdown
Contributor

@smeinecke smeinecke commented Apr 29, 2026

改了什么 / What changed

Added test/account-add-proxy.test.js with coverage for:

  • bad proxy -> 400 and no account created
  • private proxy (default policy) -> 400 and no account created
  • valid proxy -> account created and per-account proxy bound
  • no proxy -> account created and no per-account proxy bound

Frontend:

  • centralize error i18n via App.translateError(code, fallback) - replaces 6 inline error.${err.message} lookups.

为什么 / Why

As mentioned in #90 (review) we need to validate the proxy before adding the account.

Checklist

  • 代码风格和现有文件一致 / Code style matches existing files
  • 没有引入 npm 依赖 / No new npm dependencies (project is zero-dep)
  • 涉及 LS binary 协议改动时 在 PR 描述里注明字段号来源 / If touching LS protocol, document field-number source in the PR description
  • 涉及 dashboard UI 用 App.confirm / App.prompt 不用浏览器原生 alert/confirm / Uses App.confirm / App.prompt, not native dialogs (if dashboard)

…flow

Reorder POST /accounts validation - parseProxyUrl + host checks (validateHostFormat vs assertPublicUrlHost per ALLOW_PRIVATE_PROXY_HOSTS) now run before addAccountByKey/addAccountByToken. Prevents orphaned accounts on proxy validation failure. Gate ensureLsForAccount + probeAccount behind !isTestEnv. Frontend: centralize error i18n via App.translateError(code, fallback) - replaces 6 inline `error.${err.message}` lookups.
@smeinecke
Copy link
Copy Markdown
Contributor Author

Whops, you already fixed this in main :D

@smeinecke smeinecke marked this pull request as draft April 29, 2026 09:56
…ounts

Move api_key/token presence check to top of POST /accounts handler - runs before proxy validation + account creation. Prevents proxy network checks when credentials are missing. Simplify account creation to ternary expression (api_key vs token path). No functional change to validation order from dwgx#90 (proxy still validated before account creation).
@smeinecke smeinecke changed the title fix(dashboard): validate proxy before account creation in single-add flow fix(dashboard): add missing unit-test for validate proxy before account creation Apr 29, 2026
@smeinecke smeinecke marked this pull request as ready for review April 29, 2026 10:08
@dwgx
Copy link
Copy Markdown
Owner

dwgx commented Apr 29, 2026

@smeinecke 谢谢这个 PR,i18n 那部分真的很赞——我把 App.translateError(code, fallback) helper 摘出来 cherry-pick 进 master 了(commit 294f5fd),把分散在 dashboard 里 6 处 r.error || I18n.t(...) 全统一过 helper 走,credit 给你了。

测试那部分(test/account-add-proxy.test.js)只能 close,原因是 v2.0.30 那一波 audit 我自己已经独立加了一份覆盖几乎一样的 case 的测试 test/account-add-proxy-ordering.test.js(commit 41fad0e),而且更全(7 条 vs 4 条,包含 prefers api_key when both api_key+tokenreturns non-ERR_* errorscreates account when no proxy 这几个边角)。两份并存的话只是多跑一倍 fixture 创建删除,没多覆盖什么。

handleDashboardApideps = {} 注入这部分我没采纳——我们的现有测试直接通过真实 path 跑 handleDashboardApi 也没问题(spark-B 的测试就是这么写的),加 deps 注入会引入一个不再被生产代码用到的"测试专用入参",不偏向加这层。

总之 i18n refactor 已经在 v2.0.32 里了,谢谢这个 cleanup!

@dwgx dwgx closed this Apr 29, 2026
@smeinecke smeinecke deleted the feat/add-account-proxy branch April 29, 2026 12:51
dwgx added a commit that referenced this pull request Apr 29, 2026
… sanitizer (#86 follow-up)

Two independent fixes bundled:

1. Dashboard error i18n centralization (cherry-picked from PR #92 by @smeinecke)
   - Adds App.translateError(code, fallbackKey) helper
   - Replaces 6 inline `error.${err.message}` lookups with a single helper
   - Same behavior, less duplication; future server error codes only need
     to be added in one place

2. Windows-style workspace path sanitization (#86 follow-up)
   oaskdosakdoakd reported `C:\home\user\projects\workspace-devinxse`
   leaking despite the existing Unix-only regex catching the matching
   `/home/user/projects/workspace-skxwsx01` form. GLM running on Windows
   clients hallucinates Windows-prefixed forms of the workspace path.

   Defensive coverage:
   - C:\home\user\projects\workspace-x (Windows backslash + drive prefix)
   - \home\user\projects\workspace-x  (Windows backslash, no drive)
   - C:\home/user/projects/workspace-x   (mixed separators)
   - d:\... lowercase drive

   Path body char class also extended to keep matching across backslash
   tails (previously the first `\` would terminate the match and leave
   the path tail visible).
dwgx added a commit that referenced this pull request Apr 29, 2026
… follow-ups

- bump version to 2.0.32
- release notes covering #94, #87, Windows path sanitizer, PR #92 i18n cherry-pick
- 373/373 tests pass (+8 from v2.0.31)
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