Skip to content

fix(desktop): Review 新鲜度绑定 dirty submodule 内部改动 - #2515

Open
fico-hub wants to merge 7 commits into
makecindy:mainfrom
fico-hub:fix/issue-2463-submodule-identity
Open

fix(desktop): Review 新鲜度绑定 dirty submodule 内部改动#2515
fico-hub wants to merge 7 commits into
makecindy:mainfrom
fico-hub:fix/issue-2463-submodule-identity

Conversation

@fico-hub

Copy link
Copy Markdown
Contributor

依赖说明:本分支叠在 #2512(fix #2460,staged index identity)之上,包含其 commit f75698c71,两者共用底层身份读取机制。建议先合 #2512;其合并后本 PR 将 rebase 到最新 main,只剩 submodule 部分的 commit。

这次改了什么

摘要

fix #2463。gitlink 是目录,文件指纹器只接受普通文件,submodule 被刻意排除在内容指纹之外;porcelain v2 对 submodule 只有一个「内部有修改」的布尔位,FileStatus 既不保留该位也没有任何 object id。于是 dirty submodule 内部的一份改动换成另一份时,Git 摘要看到的所有值都不变,verifyBeforeStart / verifyBeforePublish 两道新鲜度门都会放行过期结论。

新增 submodule 感知的身份读取通道(reviewSubmoduleIdentity.ts),与 #2460 的 staged index identity 共用底层机制:

  • 父仓侧绑定 index gitlink 记录(mode 160000 + oid)与 HEAD tree gitlink oid;已初始化子仓绑定当前 checkout HEAD。
  • dirty 子仓进入子仓读取:staged 侧复用 Review 新鲜度:staged 无内容 diff 未绑定 index blob #2460(path, mode, stage, oid) 身份;modified/untracked 工作树侧对具体普通文件复用 capped 指纹器做有界内容哈希(同一套路径守卫、敏感路径过滤、512MB 上限与哈希期间变更重读)。
  • 嵌套 submodule 以相同规则递归,深度封顶 5 层;单仓 dirty 条目封顶 1 万;超限、git 读取失败、目录形态(如 untracked 内嵌仓库)一律抛错 fail closed,不把目录塞回文件指纹器。
  • 已初始化判定校验 git toplevel 归属:deinit 后留下的空目录里 rev-parse 会静默落到父仓,不校验会把父仓 HEAD 错当子仓身份(集成测试当场抓到该缺陷)。
  • manifest 并入 workspace fingerprint;发生内层内容哈希时并入既有快照稳定性重读窗口。仅保存 submodule status 式的 commit oid + dirty 布尔不够——子仓 HEAD 不变、内部文件 A 换 B 时两者完全相同。

变更类型

  • fix 缺陷修复
  • feat 新功能
  • refactor / perf 重构或性能优化
  • docs / test / chore 文档、测试或工程维护
  • 其他:

范围

UI 变化

不涉及。

  • 引用的设计规范:不涉及

怎么验证的

自动验证

pnpm --filter desktop exec vitest run src/main/reviewer/__tests__/reviewEvidence.test.ts src/main/reviewer/__tests__/reviewSubmoduleIdentity.git-integration.test.ts --pool=forks
结果:全部通过(新增 2 条 reviewEvidence 回归:manifest 变化即指纹变化 / 读取失败 fail closed,既有 submodule 排除用例改为注入 stub 并保留断言;6 条真实 git 集成用例:内部改动 A→B / 内层 index blob 换身份 / clean 稳定 / 子仓 HEAD 移动 / deinit 后 uninitialized / 非仓库 fail closed)

pnpm --filter desktop run --if-present typecheck
结果:通过(0 错误)

desktop 全量单测(--pool=forks 规避 threads 池已知 SIGSEGV flake)
结果:约 2.44 万用例全部通过

根 pnpm test:unit
结果:通过(期间出现的失败均逐一隔离复跑核实为负载 flake,与本改动无关)

手工验证

不涉及(攻击路径与 deinit 边界均由真实 git 仓库集成测试复现)。

未执行的验证

无。

风险

风险分类

  • 无已知风险
  • 其他:

影响与回滚

  • 影响范围:review 新鲜度指纹新增 submodule 身份 manifest;clean 子仓只读 Git 已有对象身份,dirty 子仓内层文件走既有 capped 指纹器同口径有界哈希;全部失败路径 fail closed(拒绝发布而非放行)。递归深度 5 层、单仓 1 万条目封顶。
  • 回滚 / 降级方式:revert 本 PR 的 submodule commit 回到「submodule 不参与指纹」的原状,无状态、无数据迁移。

提交前检查

  • 已 review 完整 diff
  • 每个 commit 都带 DCO 签名(git commit -s,见 DCO)
  • UI 改动已在「UI 变化」注明引用的设计规范章节(不涉及 UI 则跳过)
  • 未提交凭证、令牌或授权文件
  • 已补充必要文档
  • 已确认测试结果或说明未执行原因

@fico-hub
fico-hub requested a review from a team as a code owner August 12, 2026 07:17
@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown

Greptile Summary

本 PR 为桌面端 Review 新鲜度指纹新增 submodule 感知的身份读取,并将 manifest 纳入启动前和发布前校验。

  • 绑定父仓 gitlink、子仓 HEAD、staged index 身份及 dirty 普通文件内容
  • 支持嵌套 submodule、共享内容预算和路径预算,并对无法完整读取的状态执行 fail closed
  • 新增真实 Git 集成测试,覆盖 dirty 内容替换、index blob 变化、deinit、类型变化、批处理与预算边界

Confidence Score: 4/5

当前仍不宜合并,因为子仓 HEAD 读取的非 unborn 错误仍可能被伪装成稳定身份并绕过 Review 新鲜度校验。

先前关于 rev-parse HEAD 的问题仍然存在:无条件 catch 会把超时、进程启动失败或其他 Git 读取错误统一转换为 "unborn";该值会进入启动前和发布前使用的 workspace fingerprint,因此重复读取失败可能产生一致指纹,而不是拒绝发布。

Files Needing Attention: apps/desktop/src/main/reviewer/reviewSubmoduleIdentity.ts

Important Files Changed

Filename Overview
apps/desktop/src/main/reviewer/reviewSubmoduleIdentity.ts 新增递归 submodule 身份 manifest,覆盖父仓 gitlink、子仓 HEAD、内层 index 和 dirty 文件内容。
apps/desktop/src/main/reviewer/reviewEvidence.ts 将 submodule manifest 并入 workspace fingerprint,并在发生内层内容哈希时启用稳定性重读。
apps/desktop/src/main/reviewer/reviewCappedWorkspaceFingerprint.ts 为 capped 文件指纹器增加跨多次调用共享且原地扣减的字节预算。
apps/desktop/src/main/git-review/indexIdentityReader.ts 导出支持只读路径数组的批处理辅助函数,供 submodule 身份读取复用。
apps/desktop/src/main/reviewer/tests/reviewSubmoduleIdentity.git-integration.test.ts 新增真实 Git 集成覆盖,验证 submodule manifest 的内容绑定、递归、预算和异常边界。
apps/desktop/src/main/reviewer/tests/reviewEvidence.test.ts 验证 submodule identity reader 的路由、manifest 指纹变化及读取失败时的 fail-closed 行为。

Reviews (8): Last reviewed commit: "fix(desktop): manifest 全体条目共耗路径预算,共享字节预算..." | Re-trigger Greptile

Comment thread apps/desktop/src/main/reviewer/reviewSubmoduleIdentity.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c7b413b22

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

Comment thread apps/desktop/src/main/git-review/indexIdentityReader.ts
Comment thread apps/desktop/src/main/reviewer/reviewSubmoduleIdentity.ts Outdated
fico-hub added a commit to fico-hub/cindy that referenced this pull request Aug 12, 2026
review P1(makecindy#2515):已初始化 dirty 子仓在 toplevel/realpath 读取持续出错
(权限等)时,无条件 catch 会把它记成稳定的 'uninitialized' 身份 —— 不同
的内层内容映射到同一 manifest,新鲜度门形同虚设,违反本函数 fail closed
契约。

「未初始化」收窄为两种可验证的合法形态:工作树目录整个缺席(stat ENOENT)
或 deinit 后空目录(toplevel 归属不是子仓自身);其余 stat / rev-parse /
realpath 失败一律向上抛。

新增 2 条集成用例:chmod 000 子仓 → rejects(win32 跳过);目录整个移除 →
uninitialized。

Signed-off-by: ficowang <fico@xd.com>
@fico-hub
fico-hub force-pushed the fix/issue-2463-submodule-identity branch from 3c7b413 to e7d4c87 Compare August 12, 2026 07:34
Comment thread apps/desktop/src/main/reviewer/reviewSubmoduleIdentity.ts
fico-hub added a commit to fico-hub/cindy that referenced this pull request Aug 12, 2026
review 反馈(makecindy#2515):

- 内容哈希预算改为整次 manifest 构建(全部子仓 + 嵌套递归)共享:此前每个
  子仓递归各自重置 capped 指纹器的 512MB 默认额度,N 个大型 dirty 子仓的
  快照总读取量没有上限。capped 指纹器新增 byteBudget 选项(以剩余额度为
  上限、按实际哈希字节原地扣减),路径数同样共享 1 万上限,耗尽 fail
  closed;单次调用的既有语义不变。
- submodule 路径入口移除 \n / \r 静默过滤,与 indexIdentityReader 的
  17c8fe9 同一裁决:全链路解析都是 -z + 首个 \t 分隔,控制字符路径安全,
  静默丢弃就是身份绕过口。

新增集成用例:两个 dirty 子仓在共享预算下合计超限 fail closed,各自单独
在同额度下通过(证明预算确实跨子仓累计而非按仓重置)。

Signed-off-by: ficowang <fico@xd.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7e0584ee4

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

Comment thread apps/desktop/src/main/reviewer/reviewEvidence.ts
Comment thread apps/desktop/src/main/reviewer/reviewSubmoduleIdentity.ts Outdated
fico-hub added a commit to fico-hub/cindy that referenced this pull request Aug 12, 2026
review 反馈(makecindy#2515 第二轮):

- gitlink 被普通文件 / 符号链接替换(typechange)时,porcelain 的 sub 字段
  仍标 S,statusReader 按 submodule 路由;此前把普通文件当 rev-parse 的
  cwd 直接 ENOTDIR,合法 typechange 场景无法启动 Review。现在 lstat 判型:
  非目录条目交给 capped 指纹器绑定文件字节(同一套路径守卫、敏感过滤与
  共享预算),subHead 记 'typechange';符号链接指向目录等超出表达能力的
  形态由指纹器 fail closed。
- readGitlinkPaths 与 indexIdentityReader 同规则分批(splitIntoBatches
  改导出复用),批间合并集合 —— 此前一次最多把 1 万条 pathspec 塞进单次
  spawn,Windows ~32K 命令行上限下声明的条目上限实际不可达。分批上限可经
  readReviewSubmoduleIdentity 的 limits.batch 注入,贯通 staged 身份与
  嵌套递归。

新增 2 条集成用例:typechange 绑定文件字节且同尺寸换字节被捕获 /
分批与单次调用 manifest 一致。

Signed-off-by: ficowang <fico@xd.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba616a46cf

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

Comment thread apps/desktop/src/main/reviewer/reviewSubmoduleIdentity.ts Outdated
fico-hub added a commit to fico-hub/cindy that referenced this pull request Aug 12, 2026
review 反馈(makecindy#2515 第三轮):readParentRecords 的 indexRecord 循环对
unmerged gitlink 的 stage 1/2/3 逐条覆盖、只留最后一条 —— 替换较早
stage 的 gitlink OID(保持末 stage、checkout 与 porcelain 不变)时
manifest 不变,旧 Review 结论仍过新鲜度门。与 readStagedIndexIdentity
的多 stage 表达同一裁决:全部记录并稳定排序,逗号连接进 indexRecord。

新增集成用例:子仓分叉两个互不为祖先的 commit(祖先关系会被 gitlink
合并自动收敛,构造不出冲突)→ 父仓 merge 冲突 → 断言 stage 1/2/3 三条
记录齐全且稳定排序。

Signed-off-by: ficowang <fico@xd.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cbc4592355

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

Comment thread apps/desktop/src/main/reviewer/reviewSubmoduleIdentity.ts Outdated
Comment thread apps/desktop/src/main/reviewer/reviewCappedWorkspaceFingerprint.ts Outdated
fico-hub added a commit to fico-hub/cindy that referenced this pull request Aug 12, 2026
review 反馈(makecindy#2515 第四轮,两条 P1):

- 共享路径预算此前只对 dirty 工作树文件扣减 —— N 个子仓仍可各自产生上限
  条 staged 身份记录或继续横向展开子仓(100 子仓 × 1 万 staged = 百万条
  记录,稳定性复查还会翻倍)。现在子仓条目自身、staged 记录、工作树文件
  统一经 consumePathBudget 消耗同一全局额度,耗尽 fail closed。
- 共享字节预算的 <= 0 判定会在较早子仓恰好吃满额度后,对只含零字节文件 /
  工作树删除的后续子仓误报超限。改为允许剩余 0(负值 = 账目损坏),是否
  真正超限交给逐文件的 size > remaining 判断。

新增 2 条集成用例:staged 记录计入共享路径预算(条目 1 + staged 2,预算
2 拒 3 过)/ 前仓恰好吃满字节预算后,零字节 dirty 文件的后仓正常通过。

Signed-off-by: ficowang <fico@xd.com>
@MagicLizi MagicLizi added the awaiting-discussion 等待维护者讨论(review-pr) label Aug 12, 2026
fix makecindy#2463:gitlink 是目录,文件指纹器只接受普通文件,submodule 被刻意
排除在内容指纹之外;porcelain v2 对 submodule 只有一个「内部有修改」的
布尔位,FileStatus 既不保留该位也没有任何 object id。于是 dirty submodule
内部的一份改动换成另一份时,Git 摘要看到的所有值都不变,verifyBeforeStart
/ verifyBeforePublish 两道新鲜度门都会放行过期结论。

新增 submodule 感知的身份读取通道(reviewSubmoduleIdentity.ts),与 makecindy#2460
的 staged index identity 共用底层机制:

- 父仓侧绑定 index gitlink 记录(mode 160000 + oid)与 HEAD tree gitlink
  oid;已初始化子仓绑定当前 checkout HEAD。
- dirty 子仓进入子仓读取:staged 侧复用 makecindy#2460 的 (path, mode, stage, oid)
  身份;modified/untracked 工作树侧对具体普通文件复用 capped 指纹器做有界
  内容哈希(同一套路径守卫、敏感路径过滤、512MB 上限与哈希期间变更重读)。
- 嵌套 submodule 以相同规则递归,深度封顶 5 层;单仓 dirty 条目封顶 1 万;
  超限、git 读取失败、目录形态(如 untracked 内嵌仓库)一律抛错 fail
  closed,不把目录塞回文件指纹器。
- 已初始化判定校验 git toplevel 归属:deinit 后留下的空目录里 rev-parse
  会静默落到父仓,不校验会把父仓 HEAD 错当子仓身份(集成测试当场抓到)。
- manifest 并入 workspace fingerprint;发生内层内容哈希时并入既有快照
  稳定性重读窗口。仅保存 submodule status 式的 commit oid + dirty 布尔
  不够——子仓 HEAD 不变、内部文件 A 换 B 时两者完全相同。

新增 2 条 reviewEvidence 回归(manifest 变化即指纹变化 / 读取失败 fail
closed;既有 submodule 排除用例改为注入 stub 并保留其断言)+ 6 条真实
git 集成用例(内部改动 A→B / 内层 index blob 换身份 / clean 稳定 / 子仓
HEAD 移动 / deinit 后 uninitialized / 非仓库 fail closed)。

Signed-off-by: ficowang <fico@xd.com>
CI runner 无全局 git 身份;git submodule add 产生的子仓克隆没有本地
user.name/email,在子仓内执行 commit 的用例(inner checkout 移动)在 CI
上以 exit 128 'Author identity unknown' 失败(本地因全局配置存在而通过)。
把身份配置抽成 configureRepo 并在 submodule 克隆完成后对子仓也执行。

验证:GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null 模拟无身份
环境,6/6 通过。

Signed-off-by: ficowang <fico@xd.com>
review P1(makecindy#2515):已初始化 dirty 子仓在 toplevel/realpath 读取持续出错
(权限等)时,无条件 catch 会把它记成稳定的 'uninitialized' 身份 —— 不同
的内层内容映射到同一 manifest,新鲜度门形同虚设,违反本函数 fail closed
契约。

「未初始化」收窄为两种可验证的合法形态:工作树目录整个缺席(stat ENOENT)
或 deinit 后空目录(toplevel 归属不是子仓自身);其余 stat / rev-parse /
realpath 失败一律向上抛。

新增 2 条集成用例:chmod 000 子仓 → rejects(win32 跳过);目录整个移除 →
uninitialized。

Signed-off-by: ficowang <fico@xd.com>
review 反馈(makecindy#2515):

- 内容哈希预算改为整次 manifest 构建(全部子仓 + 嵌套递归)共享:此前每个
  子仓递归各自重置 capped 指纹器的 512MB 默认额度,N 个大型 dirty 子仓的
  快照总读取量没有上限。capped 指纹器新增 byteBudget 选项(以剩余额度为
  上限、按实际哈希字节原地扣减),路径数同样共享 1 万上限,耗尽 fail
  closed;单次调用的既有语义不变。
- submodule 路径入口移除 \n / \r 静默过滤,与 indexIdentityReader 的
  17c8fe9 同一裁决:全链路解析都是 -z + 首个 \t 分隔,控制字符路径安全,
  静默丢弃就是身份绕过口。

新增集成用例:两个 dirty 子仓在共享预算下合计超限 fail closed,各自单独
在同额度下通过(证明预算确实跨子仓累计而非按仓重置)。

Signed-off-by: ficowang <fico@xd.com>
review 反馈(makecindy#2515 第二轮):

- gitlink 被普通文件 / 符号链接替换(typechange)时,porcelain 的 sub 字段
  仍标 S,statusReader 按 submodule 路由;此前把普通文件当 rev-parse 的
  cwd 直接 ENOTDIR,合法 typechange 场景无法启动 Review。现在 lstat 判型:
  非目录条目交给 capped 指纹器绑定文件字节(同一套路径守卫、敏感过滤与
  共享预算),subHead 记 'typechange';符号链接指向目录等超出表达能力的
  形态由指纹器 fail closed。
- readGitlinkPaths 与 indexIdentityReader 同规则分批(splitIntoBatches
  改导出复用),批间合并集合 —— 此前一次最多把 1 万条 pathspec 塞进单次
  spawn,Windows ~32K 命令行上限下声明的条目上限实际不可达。分批上限可经
  readReviewSubmoduleIdentity 的 limits.batch 注入,贯通 staged 身份与
  嵌套递归。

新增 2 条集成用例:typechange 绑定文件字节且同尺寸换字节被捕获 /
分批与单次调用 manifest 一致。

Signed-off-by: ficowang <fico@xd.com>
review 反馈(makecindy#2515 第三轮):readParentRecords 的 indexRecord 循环对
unmerged gitlink 的 stage 1/2/3 逐条覆盖、只留最后一条 —— 替换较早
stage 的 gitlink OID(保持末 stage、checkout 与 porcelain 不变)时
manifest 不变,旧 Review 结论仍过新鲜度门。与 readStagedIndexIdentity
的多 stage 表达同一裁决:全部记录并稳定排序,逗号连接进 indexRecord。

新增集成用例:子仓分叉两个互不为祖先的 commit(祖先关系会被 gitlink
合并自动收敛,构造不出冲突)→ 父仓 merge 冲突 → 断言 stage 1/2/3 三条
记录齐全且稳定排序。

Signed-off-by: ficowang <fico@xd.com>
review 反馈(makecindy#2515 第四轮,两条 P1):

- 共享路径预算此前只对 dirty 工作树文件扣减 —— N 个子仓仍可各自产生上限
  条 staged 身份记录或继续横向展开子仓(100 子仓 × 1 万 staged = 百万条
  记录,稳定性复查还会翻倍)。现在子仓条目自身、staged 记录、工作树文件
  统一经 consumePathBudget 消耗同一全局额度,耗尽 fail closed。
- 共享字节预算的 <= 0 判定会在较早子仓恰好吃满额度后,对只含零字节文件 /
  工作树删除的后续子仓误报超限。改为允许剩余 0(负值 = 账目损坏),是否
  真正超限交给逐文件的 size > remaining 判断。

新增 2 条集成用例:staged 记录计入共享路径预算(条目 1 + staged 2,预算
2 拒 3 过)/ 前仓恰好吃满字节预算后,零字节 dirty 文件的后仓正常通过。

Signed-off-by: ficowang <fico@xd.com>
@fico-hub
fico-hub force-pushed the fix/issue-2463-submodule-identity branch from 4ee1b37 to 1b8497e Compare August 12, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-discussion 等待维护者讨论(review-pr)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review 新鲜度未绑定 dirty submodule 的内部改动 Review 新鲜度:staged 无内容 diff 未绑定 index blob

2 participants