opc/soft_quay
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
36d43cdf166c17f3623c3d6829d8f8c1911769b4
soft_quay/docs/review/phase0-skeleton-review.md
T

17 KiB
Raw Blame History

Phase 0 工程骨架审查(T-001 ~ T-004)

审查范围:提交 cf9fc01(T-001)、0e20dd7(T-002)、6f920eb(T-003)、45c242e(T-004)所落地的 monorepo 骨架、domain 状态模型、事件 runtime、Gio 空窗口、平台 stub 与验证闸门。 审查视角:全栈开发工程师。仓库在审查时已推进到 T-301,本文只覆盖 Phase 0;安全关键模块(T-101~T-103、T-301)另行专项审查。 日期:2026-07-16

总体判断

Phase 0 质量在平均线以上。最难的部分——现代版 / Win7 双工具链隔离——处理正确且稳健,这是最容易翻车的环节。存在几处值得优化的点,但没有返工级硬伤。M1 验收(双目标可编译、core 无头可测)成立。

做得好的地方

  1. 分层边界是机器强制的,不是口头约定。 scripts/check_core_boundaries.py 用 go list -json 解析 core 的真实 import 图,拒绝 Gio / Windows / SQLite / app 层依赖,并自带 --self-test 防匹配器失效。core 确实零 UI、零 Windows 依赖。

  2. 平台层 build-tag stub 模式正确。 app-*/platform/windows/platform_windows.go(//go:build windows)+ platform_stub.go(//go:build !windows),保证 core 与平台契约能在无头 Linux CI 跑测试。

  3. 工具链隔离稳健。 独立的 app-win7/go.work;scripts/verify_phase0.sh 对每个目标显式设置 GOWORK=... 而非依赖 cwd 自动发现;GOTOOLCHAIN 双钉(go1.25.0 / go1.20.14);scripts/check_go_versions.py 扫描 win7 构建列表里每个模块的 go 指令,拦截任何 >1.20 的依赖。这是防"依赖被隐式升级到要求 Go 1.21+"的正确做法。

  4. 验证闸门可一键复现。 verify_phase0.sh 串起治理检查 + 边界检查 + 版本检查 + core vet/test + 双目标构建;dist/ 中两个 EXE 均真实产出。

  5. 事件 runtime 是生产级并发质量。 core/application/runtime.go 使用 closeOnce 防重复关闭、双 select 让 done 通道优先于入队、context 取消,非占位骨架。

  6. 分发构建参数正确。 CGO_ENABLED=0 -trimpath -ldflags="-H=windowsgui":单文件、无控制台窗口、可复现路径。

  7. 测试有真实断言。 状态迁移、事件 runtime(含 context 取消)、AppShell 布局尺寸、平台 stub 契约均有覆盖。

需优化项(按优先级)

P1 · app-win7 不应同时出现在根 go.work

现状: 根 go.work 同时 use ./app-modern、./app-win7、./core,于是同一 workspace 混入 Gio v0.6.0 与 v0.10.1,MVS 统一为 v0.10.1。同时 app-win7/go.work 又单独存在(只含 win7 + core)。

影响: 不是致命问题——gate 通过、两个 EXE 都能出,因为构建按目标显式指定 GOWORK,win7 走自己的 workspace。但代价具体存在:

  • IDE / gopls 打开仓库根时,用 v0.10.1 类型检查 app-win7 代码,而其实际编译用 v0.6.0,会产生假报错或漏报真实的 v0.6.0 API 不兼容。
  • 从根目录跑 go build ./... 或 go work sync 时,app-win7 被拉进 v0.10.1 解析,是潜在雷。

建议: 根 go.work 只保留 ./app-modern 与 ./core,win7 完全交给 app-win7/go.work。这样没有任何一个 workspace 会混两个 Gio 版本,隔离彻底。两个 workspace 本是该项目不可避免的结构,问题只在于把 app-win7 重复塞进了根。

P2 · 两个 ui/gio/shell.go 约 95% 重复

现状: app-modern/ui/gio/shell.go 与 app-win7/ui/gio/shell.go 的 shellColors 调色板、NewTheme、Layout 结构几乎一字不差。

影响: 一部分重复是固有的(core 不能 import Gio,两版 Gio API 不同);但调色板(纯数据)与布局骨架(结构)目前是硬拷,每新增一个视图重复就翻倍。

建议: 到 T-203+ 真正落地"共享 ViewModel / 设计 token + 隔离 Gio 适配"的纪律。已观察到 T-203 把列表状态放进 core/application.CatalogListModel,方向正确,需持续保持;纯数据的调色板值得抽到一处共享来源。

P3 · 状态机迁移表偏宽松

现状: core/domain/status.go 的 allowedTransitions 允许 Downloading→Installed、Queued→Installed、Failed→Installed 等直接跳转,绕过 verifying → extracting → installing 序列。

影响: 状态机只是 UI 状态、非真正安装门禁,但对签名更新器,允许状态模型直接跳到 installed 会让"跳过校验的已安装"变得可表达,增加误表征风险。

建议: 若这些是"已最新、无操作"的快路径,加注释说明其语义;否则收紧迁移表,强制经过校验序列。

P4 · CI 目前只有本地脚本

现状: 03-tech-stack.md 记为"Gitea Actions 模板 + 远端 runner 可用性待确认",闸门只在本地跑。

影响: 对签名分发 / 自更新产品,只在本地跑的 gate 会腐化。Phase 0 验收写的是"CI 或本地脚本",算达标,但不应停留于此。

建议: 近期把 verify_phase0 真正挂到 push / PR。

P5 · 杂项

  • 未跟踪的 soft_quay.code-workspace(编辑器文件)应加入 .gitignore。
  • 确认 go1.25.0 这个 pin 在 CI 环境可获取(执行记录显示 win7 工具链是靠 golang.org/dl/go1.20.14 现场下载的)。

后续

真正的风险不在 Phase 0 骨架,而在已提交的 T-101(清单验签)、T-102(ZIP 安全解压)、T-103(原子切换回滚)、T-301(下载队列)——这些是"错了等于远程代码执行 / 装机损坏 / 数据丢失"的模块。从 api.md 与 04-architecture.md 的执行记录看实现相当细致,但正因如此需要逐行安全审计,不能只凭其自带测试通过判定。建议单独开一轮针对这四个模块的安全审查(实际读解压与验签代码、寻找绕过路径),另存 docs/review/。

全栈二次复核

复核视角:Codex 全栈开发工程师。 复核方式:对照 Phase 0 原始提交、当前代码、workspace 实际模块解析结果和验证脚本逐条核验。

复核结论

原审核的总体判断成立:Phase 0 没有返工级硬伤,M1 验收有效。双工具链构建、core 无头测试、平台 build tag 和本地完整闸门都已实际工作。

但原审核约有三类内容需要校正:

  1. P1 是真实且优先级最高的工程结构问题。
  2. P2/P3 的现象存在,但不能直接按原建议实施,否则可能破坏现有双 Gio 隔离或合法的状态恢复语义。
  3. 原审核高估了 application Runtime 的关闭并发质量,并遗漏了边界检查范围与 vendor 文档不一致。

对原优化项的修订

P1 · 接受:彻底隔离根 workspace 与 Win7 workspace

实际执行模块解析:

  • 根 go.work 将 app-win7 使用的 gioui.org 解析为 v0.10.1。
  • app-win7/go.work 将同一模块正确解析为 v0.6.0。

根 workspace 下的 win7 测试目前仍能通过,但这不能证明 v0.6.0 兼容,反而可能掩盖仅在旧 Gio API 下出现的问题。因此建议采纳原 P1,但实施必须同时完成:

  1. 根 go.work 只保留 app-modern 与 core。
  2. app-win7 继续只由 app-win7/go.work 管理。
  3. init / verify_phase0 对两个 workspace 分别执行 go work sync,不能移除 win7 后只同步根 workspace。
  4. 更新 docs/03-tech-stack.md 中“三模块根 workspace”的旧描述。
  5. 验证 gopls / IDE 打开仓库根时能正确识别嵌套 win7 workspace,避免从“版本污染”变成“win7 模块不在 workspace”。

该问题主要影响 IDE、根目录命令和兼容性假阴性。正式构建闸门已显式指定不同 GOWORK,因此当前发布构建仍是隔离的。

P2 · 部分接受:共享业务模型,不贸然共享 Gio 控件

Phase 0 两个 shell.go 的文本相似度约为 95.6%;推进到 T-204 后仍存在大量相似结构,维护成本判断正确。

但 T-003 明确要求 modern 与 win7 不共享 Gio 控件代码,而 Gio v0.6 与 v0.10 会继续产生 API 和布局能力差异。整改纪律应为:

  • 搜索、分类、选择、状态组合等无 UI 逻辑继续放在 core/application ViewModel。
  • modern/win7 的 Gio widget、Layout 和交互适配保持独立。
  • 不为了调色板把 gioui.org、UI 控件或平台表现概念引入 core。
  • 只有颜色/间距等纯数据出现明显漂移时,再评估生成式 design token 或跨 adapter 一致性测试;当前不值得为少量 token 新增共享模块。

因此 P2 是持续关注项,不是 Phase 0 必须立即重构的问题。

P3 · 修正风险解释:不要直接收紧恢复到 installed 的迁移

Queued/Downloading/Failed → Installed|UpdateAvailable|NotInstalled 可表示:

  • 已安装软件更新取消/失败后恢复原基础状态。
  • 活跃操作结束后重新按磁盘事实纠正 UI 状态。
  • 启动恢复时清除过期操作态。

当前 ValidateTransition / CanTransition 只在状态测试中使用,并未成为下载、验签或安装流程的安全门禁。因此“可直接跳过校验成为 installed”目前是模型语义不清,不是实际安全绕过。

建议:

  1. 为这些迁移补充“恢复基础状态”的注释和测试。
  2. T-302 安装编排必须独立强制 download → verify → extract → install → health check,不得依赖 UI 状态迁移表提供安全保证。
  3. 长期可把“安装事实”(not_installed/installed/update_available)与“操作状态”(queued/downloading/verifying/...)拆为两个维度,避免一个枚举同时表达持久事实和瞬时操作。

在没有拆分基础状态前直接删除这些迁移,会破坏取消更新和失败恢复语义。

P4 · 改为运维确认项

仓库已经存在 .gitea/workflows/phase0-build.yml,并复用 verify_phase0.sh;不能表述为“没有 CI”。

本地仓库只能确认 CI 定义存在,无法证明远端 runner 已启用和实际跑绿。准确结论应是:

  • 本地完整闸门已成立。
  • 远端 push/PR 强制执行状态尚未验证。
  • 后续应在 Gitea 确认 Actions、runner、日志留存和失败阻断策略。

P5 · 杂项修订

  • soft_quay.code-workspace 如果确认只是个人编辑器配置,加入 .gitignore 合理;如果团队需要共享多 workspace/gopls 配置,则应先审查内容再决定提交。
  • 当前机器已实际验证 go1.25.0 与 go1.20.14 可用;剩余问题是远端 runner 能否获取对应工具链,不是本地 pin 是否存在。

原审核遗漏的问题

新 P1 · Runtime Close/Publish 语义没有达到“生产级并发”结论

core/application.Runtime.Publish 的两个 select 不能保证 done 优先。以下交错是合法的:

  1. Publish 第一个 select 观察到 Runtime 尚未关闭。
  2. 另一个 goroutine 执行 Close 并关闭 done。
  3. Publish 进入第二个 select;若事件通道同时可写,done 与发送分支都 ready。
  4. Go select 随机选择 ready 分支,Publish 可能发送成功并返回 nil。

现有测试只覆盖“Close 完成后再 Publish”,没有覆盖 Publish 与 Close 并发。因此需要先明确契约:

  • 若允许已开始的 Publish 与 Close 竞争成功,应在注释和测试中明确。
  • 若要求 Close 取得线性化点后绝不再接受事件,则需要调整同步设计并增加并发/重复稳定性测试。

在该契约未明确前,应把原文“生产级并发质量”改为“基础并发骨架可用,关闭竞态仍需定义”。

新 P2 · core 边界脚本只检查直接 import

scripts/check_core_boundaries.py 使用 go list -json ./...,然后检查 core 包的 Imports/TestImports/XTestImports。它能可靠阻止 core 源码直接 import Gio、Windows、SQLite 和 app 层,但没有遍历 go list -deps 的完整传递依赖图。

当前 core 没有第三方模块依赖,因此现实边界仍然安全;原文“解析真实 import 图”的描述略宽。未来 core 引入第三方依赖时,应决定是否把传递依赖也纳入检查。

新 P3 · 技术栈写了 vendor,仓库现实没有 vendor

docs/03-tech-stack.md 将“版本锁定 + vendor”标为已定,依赖纪律也要求 Gio 相关依赖 vendor,但仓库没有 vendor 目录。

这不属于 T-001～T-004 的验收失败,但属于文档与现实不一致。后续需要二选一:

  • 正式增加适配双 workspace 的 vendor 流程和 CI 验证。
  • 在未实施前把技术栈状态改为“版本锁定;vendor 待实现”,避免宣称已有离线依赖保障。

建议处理顺序

  1. 修正根 go.work 的 Win7 版本污染,同步调整双 workspace sync 和文档。
  2. 明确并测试 Runtime 的 Close/Publish 并发契约。
  3. 注释状态机中的“恢复基础状态”迁移,不要直接收紧为单一路径。
  4. 在远端 Gitea 验证 Actions runner 与 push/PR 阻断。
  5. 处理 vendor 文档差异,再决定 workspace ignore 和 UI token 是否需要独立任务。

交叉复核裁定(定稿)

复核视角:Claude 全栈开发工程师,对 Codex 二次复核逐条核验后裁定。 核验方式:grep 生产调用者、检查工作流文件与 vendor 目录、对照 runtime.go / status_resolver.go 实际代码。 日期:2026-07-16

已确认的事实(可作为后续任务前提)

项 结论 证据
根 go.work Win7 版本污染 属实,未修复 go.work 仍 use ./app-win7;根解析 Gio v0.10.1,app-win7/go.work 解析 v0.6.0
CI 工作流存在 属实(原审核 P4 表述不准) .gitea/workflows/phase0-build.yml 触发 push/pull_request,跑 verify_phase0.sh;远端 runner 实跑状态未验证
状态迁移表非安全门禁 属实 CanTransition/ValidateTransition 无生产调用者,仅自身与测试引用;真实推导在 core/domain/status_resolver.go:ResolveAppStatus(facts 驱动,已分离操作态与安装事实)
Runtime Close/Publish 竞态 属实 core/application/runtime.go 第二个 select:done 已关闭且 buffer 有空位时,<-done 与 events<-event 同时 ready,select 随机选,Publish 可能在 Close 后仍成功返回 nil;测试未覆盖并发
core 边界脚本只查直接 import 属实,现实安全 check_core_boundaries.py 检查各 core 包 Imports/TestImports/XTestImports,不遍历第三方传递依赖;core 目前无第三方依赖,故无实际泄漏
vendor 文档与现实不符 属实 无 vendor/ 目录;03-tech-stack.md 将“版本锁定 + vendor”标为已定

对 Codex 二次复核的采纳与修正

  • 采纳 P1(workspace 隔离)及其实施细则:移除 app-win7 后 verify_phase0 必须对两个 workspace 分别 go work sync;须验证 gopls 对嵌套 win7 workspace 的识别,避免从“版本污染”变成“win7 不在 workspace”。
  • 采纳 新 P1(Runtime 竞态)、新 P2(边界传递依赖)、新 P3(vendor 漂移)三项遗漏。
  • 修正 P2 论述:调色板是 image/color.NRGBA(标准库),共享它不需要 import Gio,可放独立非 Gio 包;“不值得为少量 token 建共享模块”的结论保留,但约束不成立于“会污染 core”。
  • 补充(双方均遗漏):allowedTransitions + CanTransition/ValidateTransition 现为测试脚手架/死代码,与 ResolveAppStatus 并行且可能漂移,构成第二个合法状态来源。须接进 resolver/安装编排做校验,或显式标注“仅规格测试”。
  • 补充 CI 工具链隐藏依赖:phase0-build.yml 只 setup-go 1.25.0,win7 构建靠 GOTOOLCHAIN=go1.20.14 自动联网下载才能绿,与“无 vendor”是同一可复现性缺口。须显式预装 go1.20.14 或知情接受该联网依赖。

最终处理顺序(定稿)

  1. [结构 · 最高优先 · T-604] 修根 go.work Win7 版本污染:根仅 use ./app-modern + ./core;verify_phase0 / init 对根与 app-win7/go.work 分别 go work sync;更新 03-tech-stack.md “三模块根 workspace” 描述;验证 gopls 嵌套 workspace 识别。
  2. [低成本 · 一并做] 状态模型收口:给“恢复基础状态”迁移补注释与测试;同时裁定 allowedTransitions/CanTransition 去留(接入 resolver 校验 或 标注仅测试);不在拆分基础状态维度前直接删迁移。
  3. [可复现性 · 一组收口] vendor 与 CI 工具链一起处理:确定 vendor 落地或把技术栈状态改为“vendor 待实现”;phase0-build.yml 显式预装 go1.20.14 或知情接受自动下载;在远端 Gitea 确认 Actions runner、push/PR 阻断与日志留存。
  4. [契约 · 可延后至 Phase 2-3] 定义并测试 Runtime 的 Close/Publish 并发契约(接上真实事件消费者与关停语义后再定,含并发/重复稳定性测试)。
  5. [观察项 · 非重构] UI 适配持续保持“共享 ViewModel + 隔离 Gio 控件”纪律;边界脚本待 core 引入第三方依赖时再评估是否纳入传递依赖检查;soft_quay.code-workspace 确认为个人配置后加入 .gitignore。

裁定:Codex 二次复核整体成立且部分优于原审核;上述顺序为双方交叉复核后的共识。P1 为唯一“结构级、应尽快处理”项,其余为低成本收口、可复现性收口与可延后契约。

Reference in New Issue View Git Blame Copy Permalink