Skip to content

fix(worker): resolve setup and runtime regressions - #89

Merged
PIKACHUIM merged 1 commit into
mainfrom
fix-86-87-88
Sep 29, 2026
Merged

PIKACHUIM merged 1 commit into
mainfrom
fix-86-87-88

Conversation

@jyxjjj

@jyxjjj jyxjjj commented Sep 25, 2026

Copy link
Copy Markdown
Member

Summary / 摘要

  • Fix first-run deployments failing to show the initialization page when the published frontend does not implement /public/init_status and /@init.

  • Validate published frontend bundles before use and fall back to building the frontend main branch when the Worker initialization protocol is unavailable.

  • Fix AutoIndex parsing of malformed Apache directory pages containing duplicated, unclosed, or incorrectly nested HTML elements.

  • Parse remote pages with parse5 using HTML5 error-recovery rules before converting them to XML for the existing XPath implementation.

  • Declare parse5@7.3.0 as a direct dependency because production code imports it directly instead of relying on its existing transitive installation.

  • Keep the existing dependency graph unchanged; the lockfile only gains the direct importer entry for parse5.

  • Add a 30-second timeout to AutoIndex upstream requests to prevent unreachable mounts from occupying a Worker until the platform timeout.

  • Fix /api/auth/2fa/generate returning an unresolved QR-code Promise serialized as {}.

  • Prevent DB_DRIVER=auto from incorrectly detecting an EdgeOne KV proxy from an ordinary request origin and an HTML 200 fallback response.

  • Add regression tests for malformed Apache directory HTML and resolved 2FA QR-code data URLs.

  • Format all touched source files with Prettier.

  • This PR has breaking changes.
    / 此 PR 包含破坏性变更。

  • This PR changes public API, config, storage format, or migration behavior.
    / 此 PR 修改了公开 API、配置、存储格式或迁移行为。

  • This PR requires corresponding changes in related repositories.
    / 此 PR 需要关联仓库同步修改。

Related repository PRs / 关联仓库 PR:

  • OpenList: N/A
  • OpenList-Docs: N/A

Related Issues / 关联 Issue

Testing / 测试

  • pnpm lint
  • Driver test suite (112/112)
  • Server test suite (171/171)
  • Store test suite (12/12)
  • Model test suite (39/39)
  • node --import tsx scripts/_regress.mjs (71/71)
  • pnpm exec prettier --check on all changed files
  • Manual test / 手动测试:
    • Verified that frontend 4.2.6 is detected as missing the Worker setup protocol and falls back to building the frontend main branch.
    • Verified AutoIndex listing against https://archive.apache.org/dist/tomcat/.
    • Verified that git interpret-trailers --parse recognizes both commit trailers.

Checklist / 检查清单

  • I have read CONTRIBUTING.
    / 我已阅读 CONTRIBUTING。
  • I confirm this contribution follows the repository license, contribution policy, and code of conduct.
    / 我确认此贡献符合仓库许可证、贡献规范和行为准则。
  • I have formatted the changed code with gofmt, go fmt, or prettier where applicable.
    / 我已按适用情况使用 gofmt、go fmt 或 prettier 格式化变更代码。
  • I have requested review from relevant maintainers or code owners where applicable.
    / 我已在适用情况下请求相关维护者或代码所有者审查。

AI Disclosure / AI 使用声明

  • This PR includes AI-assisted content.
    / 此 PR 包含 AI 辅助内容。

Tools used / 使用工具:

  • ChatGPT
  • Codex
  • GitHub Copilot
  • Claude
  • Gemini
  • Other (please specify) / 其他(请注明):

Usage scope / 使用范围:

  • Code generation / 代码生成

  • Refactoring / 重构

  • Documentation / 文档

  • Tests / 测试

  • Translation / 翻译

  • Review assistance / 审查辅助

  • I have reviewed and validated all AI-assisted content included in this PR.
    / 我已审核并验证此 PR 中的所有 AI 辅助内容。

  • I have ensured that all AI-assisted commits include Co-Authored-By attribution.
    / 我已确保所有 AI 辅助提交都包含 Co-Authored-By 归属信息。

  • I can reproduce all AI-assisted content included in this PR without any AI tools.
    / 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。

Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Signed-off-by: jyxjjj <16695261+jyxjjj@users.noreply.github.com>
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
openlist-work 0f93ee9 Sep 25 2026, 06:46 PM

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
openlist-tsworkers 0f93ee9 Sep 25 2026, 06:47 PM

@pikachuren

Copy link
Copy Markdown
Collaborator

评审结论:需修改后合并(两处小改,几个字级别)

先说好的:两个核心修复我都实测确认是真的,这个 PR 的整体质量明显高于同期其它几个。

✅ 实测确认成立

1) 2FA 二维码 {} 修复是真的 —— buildQrImageUrl 确实是 async(pkg/totp.ts:172 返回
Promise<string>),所以 data: { qr: buildQrImageUrl(otpauth) } 被 JSON.stringify 成 {} 是必然的,
不是偶发。改成先 await 再放进响应体,并补了断言 ^data:image/png;base64, 的回归测试。

2) AutoIndex 那个修复也是真的,而且修的是硬失败 —— 我把补丁应用到副本、用真实 util.ts 做 5 场景 A/B:

场景 main(正则版) 本 PR(parse5 版)
PR 的目标畸形页 抛异常 Opening and ending tag mismatch: "pre" != "body",返回 0 条 2 条 ✅
普通页 / 文件名含 <(&lt;)/ &(&amp;)/ > 与引号 — 与 main 完全一致,无回归 ✅

(我原本怀疑 parse5 序列化会把文本里的 < 原样输出、破坏 XML 良构性,实测推翻了这个猜测:
它会转义回 &lt;,所以这里没有回归。)

3) 锁文件是干净的 —— parse5@7.3.0 本来就在 pnpm-lock.yaml 里(第 3551 / 7864 行两处快照),
只补 importers 条目即可,--frozen-lockfile 不会崩。描述里「锁文件仅新增 direct importer 条目」属实。

4) 超时覆盖完整 —— autoindex 驱动里只有一处 fetch((driver.ts:81),本次正是给它加的超时。

🔴 需要修改 1:KV 的门控只加了一半,/env_check 仍会误报 ready=true

kv.ts 里有两处调用 probeProxy:

调用点 所在方法 本次是否加了门控
kv.ts:261 isAvailable() ✅ 加了 if (!shouldProbeProxy(env)) return false
kv.ts:485 health() ❌ 未加(该分支本次只有格式调整)

而判定「是否 ready」走的恰恰是 health():

isPersistentStorageAvailable()  →  getStorageStatusSafe()  →  getStoreStatus()
   →  health = await driver.health(env)        // backend.ts:805-831,未门控的那条
   →  isPersistentStatus(): driverHealthy = (status.available !== false)

后果:在显式 DB_DRIVER=kv(或存在 EdgeOne 平台标记)的部署上,只要站点把未知路径回退成 HTML 200,
probeToHealth({ok:true}) 依然返回 true → available: true → /env_check 仍会报 ready=true,
正是本 PR commit message 声称修掉的症状。另外这也让 isAvailable() 与 health() 对同一问题给出不同答案,
是该文件注释自己警告过的「判定标准漂移」。

建议:把门控下沉到 probeProxy() 内部,两处调用自动同时生效,最不容易再漏;
或在 health() 的模式2分支同样加门控并返回 available: false + 明确 error 文案。

补充:本问题与 #79 互补而非重复 —— 本 PR 治「不该探测」,#79 治「探测到 200 就信」(要求返回合法
{keys:[...]},我已核验 functions/kv-list 确实返回该形状)。两者都建议保留。

🔴 需要修改 2:前端「自动回退到构建 main」会让 CI 产物不可复现

fetch-frontend.mjs 现在在发布版 dist 缺初始化协议时,静默回退到
git clone --depth 1 --branch main 现构建。而:

$ grep -n "OFFICIAL_REPO_REF" scripts/fetch-frontend.mjs
47:const OFFICIAL_REPO_REF = process.env.FRONTEND_GIT_REF || "main"   # 跟随移动的分支

$ grep -n "FRONTEND" .github/workflows/edgeone-artifact-guard.yml    # 无输出

.github/workflows/edgeone-artifact-guard.yml 没有 env: 块,不固定 FRONTEND_VERSION /
FRONTEND_DIST / FRONTEND_GIT_REF,直接跑 node scripts/fetch-frontend.mjs。于是:

  1. 该工作流的核心步骤是「重新构建 cloud-functions/[[default]].js 并与已提交版本比对」;回退后产物里嵌的是
    main 分支构建出的内容哈希资源,每次运行都不一样 → Check artifact freshness 会长期失败,
    且在兼容的发布版前端出现之前,维护者无法通过重新构建让产物收敛。
  2. 脚本文件头自己就警告过:main 构建产物的哈希与 CDN 不一致,会让 ASSET_URLS 路径失效、
    npmmirror 等镜像不可用 —— 这个回退把当初被刻意避开的坑又作为静默路径引入了。
  3. 回退只有一行 console.warn,构建失败/超时的排查成本很高。

建议:回退改成响亮告警(或在 CI 里要求显式开关),并在 edgeone-artifact-guard.yml 里固定
FRONTEND_VERSION 或 FRONTEND_GIT_REF,保证守卫判定确定。另外建议确认 stampFrontendVersion
在源码构建路径下会把未发布的前端版本号写进 index.html,进而让 $version 指向 CDN 上不存在的版本。

非阻塞建议

  • 协议校验只覆盖「自动下载」这一条路径:supportsWorkerSetup() 只在 fetchPublishedDist() 里调用,
    FRONTEND_DIST / FRONTEND_REPO 路径不校验。显式配置优先是合理的,但建议在文档/日志里说明这个边界。
  • 字节串启发式有脆性:code.includes("/public/init_status") + code.includes("/@init") 依赖打包后仍保留这两个字面量。
    若未来前端带 base path 前缀或拆分了字符串常量,会出现假阴性 → 静默回退到 main(即上面那个坑)。
    建议至少把判定结果打进日志。
  • shouldProbeProxy 读 DB_DRIVER 的约定与 readEnv()(backend.ts:38,env 为 falsy 时会回退 process.env)
    不完全一致。影响面很窄(env 为 {} 时两者一致),但复用 readEnv 成本为零。
  • auth.ts 的 diff 约 95% 是 Prettier 格式化噪音,真正的修复只有 3 行。建议拆成独立 commit,
    否则会与同样基于 e903f475 的 fix(security): enforce base_path boundary in getActualPath and add S3 gateway authorization #82/fix: resolve P2-level issues found in code review (28 fixes across pkg/server/internal/edge/drivers) #85 产生额外冲突(这两个已关闭,但仍值得保持这个习惯)。

未复核

PR 自述的测试计数(driver 112 / server 171 / store 12 / model 39 / regress 71)与 pnpm lint ——
本机无 node_modules,未复核。


本评论由 AI 辅助的自动化评审生成,基于对该 PR 当前 head 提交的源码核对。标注「实测复现」的结论已在本地用真实源码(打补丁后)跑脚本验证;其余为代码走查结论。请以人工复核为准。

@jyxjjj

jyxjjj commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

感谢复核。重新沿调用链检查后,我认为两点需要修正:

  1. KV gate 这一条不成立。/env_check 调用 health() 前会先经 getStorageBackend() 解析驱动;DB_DRIVER=auto 下,KV 会先执行 isAvailable(),而本 PR 新增的 shouldProbeProxy() 会让普通部署直接返回 false,因此根本不会进入 kvDriver.health()。

评论举的 DB_DRIVER=kv / EdgeOne 标记场景本身就是明确允许 probe 的情况,即使把同样 gate 加到 health() 也不会改变结果。

HTML 200 假阳性确实仍可能存在,但原因是 probeProxy() 只检查 HTTP 状态,没有校验 /kv-list 的响应结构;这是另一个独立问题。

  1. frontend fallback 到移动的 main 确实存在构建可复现性风险,建议 pin FRONTEND_GIT_REF。但“每次运行都不同”“长期失败”“无法收敛”表述过强;准确说法应是不同时间构建不保证一致,并存在 main 在本地构建与 CI 之间移动导致 artifact mismatch 的竞态。

因此建议移除 KV blocking issue,保留 frontend reproducibility issue。

@PIKACHUIM
PIKACHUIM merged commit 29f7a0a into main Sep 29, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants