Skip to content

reconstruct eeb8cb05 cleanup on current main - #1673

Open
cyfung1031 wants to merge 8 commits into
mainfrom
codex/reconstruct-eeb8cb05-pr
Open

reconstruct eeb8cb05 cleanup on current main#1673
cyfung1031 wants to merge 8 commits into
mainfrom
codex/reconstruct-eeb8cb05-pr

Conversation

@cyfung1031

Copy link
Copy Markdown
Collaborator

Summary

This PR resubmits the seven reconstructed commits from eeb8cb05 on the current main branch.

  • Suppresses the original eeb8cb05 with normal revert commit 5616639f.
  • Replays the requested commits in order:
    • 4e71781bbbdd6800 — preserve web-request install provenance
    • a98446002638187f — stabilize keep-alive E2E selectors
    • 8903bb3c0585db22 — prune redundant E2E smoke cases
    • 99f834da58115895 — exclude development kit from Vitest
    • e37b993eedd80320 — prune message and filesystem redundancies
    • dc537fb18f005b0a — prune redundant agent and UI cases
    • ce0b06e59cb5a122 — prune redundant utility cases

The branch is based on current origin/main at 88e73d8aeef2929e7fabf6f1552c8ea018dd1078.

Conflict and scope audit

  • The post-eeb8cb05 history changes three of the 46 original target files: src/app/service/service_worker/script.ts, src/pages/install/useInstallData.ts, and src/pages/install/useInstallData.test.ts.
  • The original revert applied cleanly while preserving newer MCP external-access and popup site-range changes.
  • All seven cherry-picks applied cleanly; no commit was skipped or made empty.
  • The final net diff against current main is limited to the five intentional reconstructed differences:
    e2e/keep-alive.spec.ts, src/pages/components/use-is-mobile.test.ts, src/pages/install/useInstallData.test.ts, src/pages/install/useInstallData.ts, and src/pkg/utils/skill-zip.test.ts.
  • The newer byWebRequest=1 redirect and MCP install flow remain present; the reconstructed install change additionally propagates provenance for the direct raw-URL path.

Related audit: #1672
Original cleanup PR: #1653

Validation

  • Focused Vitest: 3 files, 46 tests passed.
  • Full Vitest: 327 files, 3,721 tests passed.
  • pnpm run lint: passed, including TypeScript, formatting, i18n, issue-template checks, and ESLint.
  • pnpm run build: passed; existing bundle-size and Monaco dynamic-require warnings remain.
  • Keep-alive Playwright E2E remains environment-blocked on this macOS host: Chromium crashed before page load, so no browser assertion result is claimed.

cyfung1031 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

@CodFrm

结论

#1653 的原始承诺是“清理高置信冗余测试”,并且“不修改生产代码或生产行为”。原 eeb8cb05 实际把测试清理、测试基础设施和安装行为修改混在了一起;#1673 已经把历史拆成多个提交,但目前仍需要明确哪些内容属于行为修复、哪些内容属于测试清理。

按真实提交关系:

  • eeb8cb05 已经是当前 main 的祖先;
  • 6c2c43d0 又直接建立在 eeb8cb05 之上,并扩展了同一个安装页的 MCP 审批流程;
  • reconstruct eeb8cb05 cleanup on current main #1673 先用 5616639f 回滚,再用后续提交重放部分工作。

因此要同时看“提交历史中的职责”和“相对当前 main 的最终净 diff”,不能只看其中一个。

一、原 eeb8cb05 混入了哪些实现/配置修改

原提交一共改了 46 个文件,其中下列 4 个不是普通测试文件:

1. DNR 安装入口改变了生产行为

文件:src/app/service/service_worker/script.ts

重定向从:

.../install.html?url=\\1

改成:

.../install.html?byWebRequest=1&url=\\1

这新增了网页请求来源标记,改变了 Service Worker 生成的安装 URL,不是删除测试。

2. 安装页离开逻辑改变了用户可见行为

文件:src/pages/install/useInstallData.ts

原来按:

window.history.length > 1

决定 history.back()window.close();原提交改为读取 byWebRequest

  • 从 URL 查询参数读取 byWebRequest=1
  • uuid 暂存信息读取 cached?.[2]?.byWebRequest
  • 保存到 byWebRequestRef
  • 安装成功、关闭、技能安装完成、技能取消都改用这个来源状态。

这改变了页面离开方式,属于生产行为修改。

3. 为 E2E 增加了生产组件选择器

文件:src/pages/options/routes/Setting/sections/RuntimeSection.tsx

给两个 keep-alive Switch 增加:

data-testid="keep-alive-switch"

虽然它通常不改变功能,但仍是生产 JSX 修改,是测试接缝,不是测试删除。若要遵守 #1653 的“不得修改生产代码”,应单独处理。

4. 改变了 Vitest 测试发现范围

文件:vitest.config.ts

增加:

"**/.dev-kit/**"

这是测试基础设施配置行为,不是生产运行时代码,也不是单纯删除某一个冗余测试,应单独归类并说明它改变了哪些验证范围。

二、这些修改在当前 main 和 #1673 中的真实状态

当前 main@88e73d8a 已经包含原提交的大部分结果。#1673 中:

  • 5616639f 先撤销原提交的相关 hunk;
  • bbdd6800 重放 DNR 标记和 useInstallDatabyWebRequest 逻辑;
  • 2638187f 重放 RuntimeSectiondata-testid
  • 58115895 重放 Vitest 的 .dev-kit 排除。

所以这几项在 #1673 的提交历史中仍然是实现/配置内容,但最终大多与当前 main 抵消了。不能把它们全部描述成 #1673 相对当前 main 新增的生产行为。

当前 #1673 相对 main@88e73d8a 的最终净 diff 是 5 个文件、67 additions、11 deletions;唯一的生产文件是:

src/pages/install/useInstallData.ts

具体新增的是 raw URL 入口的来源传播:

// 原来
loadFromInfo(..., false, {});

// 当前 #1673
loadFromInfo(..., false, {
  byWebRequest: byWebRequestRef.current,
});

这是当前 PR 仍然超出 #1653 测试清理范围的直接证据。

三、这个新增实现修改为什么是实际行为修复

当前来源链分为两条。

DNR 直接 URL 入口

DNR 规则
  -> install.html?byWebRequest=1&url=...
  -> useInstallData 读取 byWebRequest
  -> raw URL 分支
  -> loadFromInfo
  -> prepareScriptByCode(..., options.byWebRequest)

eeb8cb05 只完成了前面的标记和读取,但 raw URL 分支仍传入空 options:

  • byWebRequestRef.current 虽然是 true
  • prepareScriptByCode 却收不到 byWebRequest
  • searchExistingScript 不会执行网页安装来源需要的 origin 匹配;
  • 当脚本改名、名称空间相同,或回收站存在同名脚本时,可能无法复用正确的现有脚本身份。

UUID 暂存入口

openInstallPageByUrl(..., { byWebRequest: true })
  -> createTempCodeEntry 保存 options
  -> getInstallInfo(uuid)
  -> cached[2].byWebRequest
  -> prepareScriptByCode

UUID 路径已经能保留来源;缺口只在不经过 temp storage、直接从 ?url= 读取的路径。因此 #1673 的 raw URL 修改是一个有明确 producer→consumer seam witness 的行为修复,不是测试清理。

四、必须考虑的后续 MCP 依赖

6c2c43d0eeb8cb05 之后扩展了相同的安装页:

  • useInstallData.ts 增加 externalAccess 展示和审批决定;
  • external_access/approval.tscreateTempCodeEntry(..., {}) 暂存 MCP 安装/更新;
  • MCP 确认页通过 openInCurrentTab 打开新的 install.html?uuid=... 标签;
  • 这类页面的 byWebRequest 应为 false,并应走新标签关闭路径。

所以不能为了回滚 eeb8cb05,把整个 useInstallData.ts 恢复成 65121faa 的旧文件;这样会覆盖 MCP 审批逻辑。任何行为回滚都必须只撤销 eeb 自己的 hunk,并保留 externalAccess 代码。

普通更新入口也仍然是:

openUpdatePage(..., { source })
  -> install.html?uuid=...
  -> byWebRequest = false
  -> window.close()

因此 #1669 的“脚本更新后点击关闭不生效”仍未由 #1673 解决,不能把来源标记修复当成 #1669 的修复。

五、具体应该怎样修正

方案 A:把 #1673 收敛成纯测试清理 PR

如果目标是严格兑现 #1653 的 cleanup-only 承诺:

  1. bbdd6800 拆开:
    • 保留 vi.restoreAllMocks() 等测试质量修复;
    • useInstallData.ts 的 raw URL 生产 hunk移出;
    • 把对应的“直接 URL 来源传播”回归测试和生产修复一起移到独立行为 PR。
  2. 不要在 cleanup PR 中重做或删除 byWebRequest 生产逻辑。当前 main 已经有这项行为;如果要撤销它,应另开“行为回滚”PR。
  3. 2638187f 中的 RuntimeSection.tsx 生产 data-testid 不应作为 cleanup 内容重新引入。若测试必须依赖它:
    • 要么保留当前 main 的现状,不在 cleanup PR 改动;
    • 要么改用可访问名称/role 等测试端定位方式,并把移除生产选择器作为独立变更。
  4. 58115895.dev-kit 排除属于测试基础设施,应单独提交并说明它排除了哪些测试;不要把它和“删除冗余测试”混成一个逻辑。
  5. 如果需要真正从当前 main 撤销 eeb 带来的生产行为,应先开一个独立行为回滚 PR,且用手术式修改保留 6c2c43d0 的 MCP 代码;测试清理 PR 再基于回滚后的结果提交。

该方案的验收标准是:cleanup PR 的最终净 diff 不包含 src/app/service/**、页面生产代码或新增生产行为;raw URL 修复及其专属测试不在其中。

方案 B:承认 #1673 是“实现修复 + 测试清理”

如果原意就是同时纠正 eeb 的实现缺陷并重建测试清理,则可以保留当前 raw URL 修复,但必须:

  1. 把标题和正文改成“回滚并重建 ✅ 清理高置信冗余测试 #1653 的混合提交”,不要再称为纯 cleanup replacement;
  2. 明确拆分提交职责:
    • 实现行为:DNR 来源标记、byWebRequest 消费、raw URL 传播;
    • 测试清理和 E2E 稳定性;
    • Vitest 配置单独作为测试基础设施;
  3. 补齐行为测试:
    • DNR producer 直接断言生成 byWebRequest=1
    • UUID/temp-storage 入口保留来源;
    • raw URL 入口把来源传给 prepareScriptByCode
    • 普通手动安装、普通更新和 MCP 安装不误标为 byWebRequest
  4. 保留 MCP 的 externalAccess 审批链;
  5. [BUG] 脚本更新后点击关闭不生效 #1669 作为独立问题处理。

最终判断

当前 #1673 的范围问题不是“所有原 eeb 实现都仍然出现在最终 diff 中”;更准确的判断是:

请先选择其中一个定位,再按上面的文件和提交拆分,不要只删除或保留整个 bbdd6800

@CodFrm

CodFrm commented Aug 13, 2026

Copy link
Copy Markdown
Member

所以这个pr是revert一些之前的改动和修复那个问题?

AI给的一大长串感觉都没说到重点,感官很乱


推荐做法(轻量,代价小):

  1. 让作者把标题和正文改成如实描述——「修复 DNR 直接 URL 安装丢失 byWebRequest 来源 + 重建 ✅ 清理高置信冗余测试 #1653 的测试清理」,不要再叫纯 cleanup replacement。commit 层面 bbdd680 fix: preserve web-request install provenance 已经分得很干净,问题只在 PR 标题。
  2. 确认 [BUG] 脚本更新后点击关闭不生效 #1669(更新后关闭不生效)不由本 PR 解决——审计里已明确两者无关,别在合并时误关 issue。
  3. 然后正常合入。

@cyfung1031

cyfung1031 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

所以这个pr是revert一些之前的改动和修复那个问题?

AI给的一大长串感觉都没说到重点,感官很乱

本身我給的指示是重做commit不是PR

所以它沒做到重點部份

這pr有一些測試補回,同時要你處理那些已經merge了的改動
這個AI自己也很混亂吧。 commit跟pr要求不一樣


#1672 提到了5個問題
你都看一下吧
要改的在這個pr改掉就行

因為merge了後又有其他comment
最好應該是完全revert merged那個然後重做那個PR

@CodFrm

CodFrm commented Aug 13, 2026

Copy link
Copy Markdown
Member

所以这个pr是revert一些之前的改动和修复那个问题?
AI给的一大长串感觉都没说到重点,感官很乱

本身我給的指示是重做commit不是PR

所以它沒做到重點部份

這pr有一些測試補回,同時要你處理那些已經merge了的改動 這個AI自己也很混亂吧。 commit跟pr要求不一樣

#1672 提到了5個問題 你都看一下吧 要改的在這個pr改掉就行

因為merge了後又有其他comment 最好應該是完全revert merged那個然後重做那個PR

说得我有点不知道怎么处理了 😂

我先另外开pr去修复那个bug吧

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