Repository navigation
fix(maker-shared): fold Windows path spelling in project group keys - #8211
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
[Medium impact] The change affects project grouping and counts in the mobile device-session workflow. 本次修改可以合并;建议补充标题大小写断言。 Findings
Fix with agent prompt### Issue 1
packages/maker-shared/src/__tests__/sessionList.test.ts:1184-1186
新增用例只检查分区数、行数和 `projectCount`,没有检查本次修改的标题。即使把 `projectTitle(displayDir)` 改回 `projectTitle(workingDir)`,让 Windows 标题被强制小写,这些测试仍会通过。建议给首条会话使用 `C:\Repo\App` 的用例补上 `title === 'App'` 的断言,保护原始大小写的显示行为。
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
|
CI 说明(提交者自测记录,供 review 参考) head
本 PR 只碰 本机没有装这个 monorepo 的依赖, |
Signed-off-by: Eapp1e <163811520+Eapp1e@users.noreply.github.com>
cb3a0db to
e35ad04
Compare
|
Rebased onto Local re-verification on the new base: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e35ad04219
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The group key normalizes the directory, but the section title still read the raw session value, so a workingDir reported as ` /repo/app/ ` collapsed into the same section as `/repo/app` while projectTitle() saw nothing but a space and rendered a blank heading. Trim before folding the worktree path so the title keeps the user spelling without the padding. Signed-off-by: Eapp1e <163811520+Eapp1e@users.noreply.github.com>
MagicLizi
left a comment
There was a problem hiding this comment.
Review report — PR #8211 fix(maker-shared): fold Windows path spelling in project group keys
head 5f00bbfc11cb83781e3d09a419d11213d7341d0e · base 9649391a86104b0a860873f93faf67a79be44957 (main) · tier=standard
Findings
- [P1] packages/maker-shared/src/tests/sessionList.test.ts:1185 — 标题大小写保留行为缺必要测试,且已 Resolve 的 greptile 讨论要求在最终 diff 未落实。
事实证据:本 PR 的标题来源改为projectTitle(displayDir)(sessionList.ts:449,保留用户大小写与分隔符写法),但最终 head 的全部新增用例只断言分区数、行数与projectCount,全文件唯一标题断言是sessionList.test.ts:1218的'app'(trim 用例,不覆盖大小写)。greptile thread(#discussion_r4431034829)明确要求给首条会话C:\Repo\App的用例补title === 'App'断言,作者回复「成立,已补。两条用例现在都断言分区标题是App」并 Resolve;但 rebase 后的最终 diff 里这两个断言不存在(git show e35ad04219 -- sessionList.test.ts亦无)。
影响:把projectTitle(displayDir)改回projectTitle(workingDir)(Windows 标题被强制小写、相对 base 产生回退)不会有任何测试失败——实测 48/48 全绿;PR 声称并承诺保护的标题行为可被无声回退,讨论要求的修改未落实。
建议修复:给keeps one section when Windows sessions report the same project differently(首条会话C:\Repo\App)与folds a trailing separator and a projectDir/workingDir split into one section的 mixed 组各补expect(projectSections[0].title).toBe('App')(与 thread 中作者的承诺一致)。
验证方式:补断言后npx vitest run src/__tests__/sessionList.test.ts全绿;再临时回退projectTitle(displayDir)→projectTitle(workingDir)应有 2 条失败。
Rule coverage
AGENTS.md、docs/dev-rules/development-workflow.md(P0/P1/P2 口径、测试价值门槛):改动为共享 package 缺陷修复 + 测试,范围与描述一致;P1 口径按「缺必要测试」「讨论中要求的修改未落实」。.github/PULL_REQUEST_TEMPLATE.md:format 门formatPass=true(context 判定);正文「新增 2 条用例 / 47 passed」为 rebase 前口径,最终是 3 条用例 / 48 passed,属描述滞后(P2,不阻断,已在本报告注明)。docs/dev-rules/architecture-invariants.md(含 Review 清单):maker-shared 内sessionList.ts新增 importprojectOrderSync.ts为同包既有归一化事实源(projectAliases.ts/mobileHome.ts同款用法),未触 render/main、无 main 动态 import、无布局树改动——4 项清单均不适用或通过;projectOrderSync.ts只依赖worktreePaths.ts,无环。docs/dev-rules/desktop-development.md:按其验证口径跑了共享包全量单测与tsc --noEmit(见 Verification)。- rule-map 固定问题 1–9:单一目的(分组键归一 + 标题保真);共享符号调用方(
buildRemoteSessionSections/summarizeRemoteSessionOverview唯一消费方apps/mobile/app/devices/[deviceId].tsx)已核对,键格式变化只影响展示分组(React key / 排序),无持久化/协议/权限路径;无新依赖、无安全面变化(security hard/soft hits 均 0);POSIX 大小写语义不变(isWindowsStyleProjectPath只认盘符/UNC)、worktree 折叠不变(normalizeProjectOrderPath内部仍走collapseWorktreeDirForGrouping),与mobileHome.projectGroupKey、projectOrderSync、conversationSearch同口径的声明属实;无 UI/冷更/协议改动。第 9 问:存在 P1(见 Findings)。
Verification
node <SCRIPTS>/context.mjs 8211 …→ok:true(fresh 维护门/CI/thread 状态)。npx vitest run src/__tests__/sessionList.test.ts src/__tests__/mobileHome.test.ts→ 82/82 passed(exit 0)。npx vitest run(@cindy/maker-shared 全量)→ 109 files / 2076 tests passed(exit 0)。npx tsc --noEmit→ 2 个既有错误(src/botGroupProcess.ts:95,文件与 tsconfig 不在本 PR diff,与本 PR 无关)。- 回退实证(审后已还原):
projectTitle(displayDir)→projectTitle(workingDir)后 sessionList.test.ts 48/48 仍全绿,证实 Findings。 - 未执行:security-scan.mjs(standard 档不跑 heavy 专项);UI 渲染核对(
uiCodeFiles为空,不涉及)。 - 详细命令与退出码:roundDir
pr-8211-verification.md。
Intent alignment
材料来源:PR body(2026-11-01 经 live context 读取)、review thread #discussion_r4431034829(greptile)、#discussion_r443…(codex,sessionList.ts 文件级)、PR 评论(bot 汇总与作者自测记录)。初始实现 SHA:e35ad04219c45a86c023575bbbb7d7aba728bd1e(当前可见历史最早实现提交,基线 9649391a);作者提及的原始 head cb3a0dbf 已在 2026-10-31 rebase 中被改写、旧 revision 未取到,初始实现未能完整核实的部分:cb3a0dbf 上是否确有标题断言无法核验。待合并 head:5f00bbfc(diffHash aaaea8ed…)。
| 原目标/初始行为 | 最终行为与影响 | 代码/提交证据 | 变化依据或待确认问题 |
|---|---|---|---|
Windows 同一项目因大小写/分隔符写法拆成双分区、projectCount 翻倍 → 合并为一个分区 |
落实:buildProjectSections 两处建键与 summarizeRemoteSessionOverview 统一走 normalizeProjectOrderPath,Windows 归一、POSIX 大小写仍分开、worktree 折叠不变 |
e35ad04 sessionList.ts:265,404-405,422,439 |
与 mobileHome.projectGroupKey/projectOrderSync 既有口径一致,无偏离 |
| 分区标题保持用户自己的大小写与分隔符写法(相对 base 不变的显示规则) | 落实:projectTitle(displayDir)(fold worktree + trim 后的原始目录) |
e35ad04 sessionList.ts:444-449 |
行为保留属实,但缺测试保护 → Findings P1 |
codex thread:标题先去外围空白(/repo/app/ 不能出空白标题) |
落实:5f00bbfc 的 .trim() + trim whitespace before grouping 用例断言 title === 'app' |
5f00bbf sessionList.ts:447、sessionList.test.ts:1210-1219 |
已按讨论落实,无偏离 |
greptile thread:补 title === 'App' 断言保护大小写(作者已确认「成立,已补」并 Resolve) |
未落实:最终 diff 两个 Windows 用例均无标题断言 | sessionList.test.ts:1177-1203(无 title 断言)、rebase 后 e35ad04 测试 diff |
讨论要求未落实 → Findings P1(测试保护缺失 + thread 承诺与最终 diff 不符;非产品方向偏离) |
结论:原目标(合并 Windows 双分区、修正 projectCount、标题保真)与最终实现一致,无无关扩范围、无功能删减反转;已 Resolve 的 Codex 意见已落实。唯一偏离是已 Resolve 的 greptile 讨论要求(标题大小写断言)在最终 diff 缺失,按「讨论中要求的修改未落实 / 缺必要测试」记 P1,需要作者补 2 条断言后复审;不涉及产品方向变化,无需用户裁定。
Overall — changes-requested
1 条 P1(见 Findings),需要作者修复后重新审查;除此之外未发现 P0/P1。另注:bot 静默期按超时放行(copilot-pull-request-reviewer、chatgpt-codex-connector 未表态,意见可能在合并后才到);npx tsc --noEmit 的 2 处报错为本 PR 未触及文件的既有问题。
The grouping key lowercases Windows paths, so a test that only counts sections would pass even if the title fell back to the normalized key. Assert the title of the first session spelling instead. Signed-off-by: Eapp1e <163811520+Eapp1e@users.noreply.github.com>
MagicLizi
left a comment
There was a problem hiding this comment.
PR #8211 独立代码审查报告(auto,head 5e82751)
改动面:packages/maker-shared/src/sessionList.ts(+18/−4)与 packages/maker-shared/src/__tests__/sessionList.test.ts(+53)。审查档位 standard(review.tier=standard),未跑 security-scan.mjs(非 heavy 档);阶段一安全与隐私内容门 security.scanned=true, hardHits=0, softHits=0,格式门 formatPass=true。
Findings
没有 P0/P1。
逐项核过但不成案的点(仅记录,不阻断):
summarizeRemoteSessionOverview的projectCount与项目分区数在「会话被 pin」或「有projectDir但无workingDir的 dialogue 会话」两种输入下仍可能不相等(前者进置顶分区、后者只被概览计数)。两者都是改动前就存在的过滤口径,本 PR 的归一化让 Windows 写法差异导致的重复计数消失,未扩大该缺口,故不计 P1。automationGroupKey(sessionList.ts:930-937附近)的 legacy 兜底键仍用未归一化的原始dir,同类写法差异会拆自动化运行组。PR 描述已把它明确列为范围外并给了拆单理由(自动化任务分组的展示口径单独定),属可接受的显式延后,不影响本 PR 的项目分区正确性。displayDir的?? workingDir回退不可达(isDialogueSession已滤掉workingDir为空的会话),死分支无害。
Rule coverage
AGENTS.md、docs/dev-rules/development-workflow.md(P0/P1/P2 口径、验证真实性):改动只做一件事(项目分组键归一化 + 标题取原始写法),描述、diff、测试一致;验证声明可复核(见 Verification)。docs/dev-rules/engineering-conventions.md§3.2(测试价值门槛):两条新增 describe 共 3 条用例并入已有sessionList.test.ts,走真实生产入口buildRemoteSessionSections/summarizeRemoteSessionOverview,断言用户可观察结果(分区数、行数、projectCount、分区标题),红绿均已复验(见 Verification);PR 描述附了「生产入口 / 规则依据 / 覆盖缺口 / 最小失败输入」四项。未写实测耗时一项(实测单文件 23ms,远低于 5 秒预算),不构成阻断。docs/dev-rules/architecture-invariants.md(package 解耦 / 依赖方向):新增 import 为sessionList.ts → projectOrderSync.ts,同包内且projectOrderSync只依赖worktreePaths,无环、不引入 render/main 依赖。- 共享符号与调用方:
buildProjectSections/summarizeRemoteSessionOverview的唯一消费方是apps/mobile/app/devices/[deviceId].tsx与apps/mobile/src/session/sessionList.ts(wrapper 只覆写pinned/dialogue标题,项目标题透传),未见按旧键格式解析project:<key>的持久化或协议读方(SwipeableProjectHeader的project:前缀是首页滑动注册表的瞬时 key,与本键空间无关)。 - 跨端口径核验:
projectGroupKey走normalizeProjectOrderPath(projectOrderSync.ts:271),与首页mobileHome.ts的normalizePathKey(同函数)、项目顺序表foldProjectOrderKey、conversationSearch.ts的normalizeWorkingDirForGrouping同一语义(trim、Windows 统一正斜杠并小写、去尾分隔符、worktree 折 base repo),与 PR 声称的「与首页 projectGroupKey 同一口径」一致;POSIX 大小写敏感语义保持分开(normalizeProjectOrderPath只对 Windows 风格小写)。 docs/design-rules/DESIGN.md:不涉及视觉元素、布局、颜色与文案规则;分区标题仍是项目目录 basename,只是不再被归一化键的小写覆盖,无硬编码色/自造组件问题。- 冷更 / 协议 / 凭证 / 权限边界:未触碰原生配置、wire protocol、持久化 schema 或凭证路径,回滚仅需回退
sessionList.ts。
Verification
本机(worktree pr-8211,pnpm install --frozen-lockfile --prefer-offline --prod=false 后)实际执行,日志 pr-8211-verify.log:
npx vitest run src/__tests__/sessionList.test.ts src/__tests__/mobileHome.test.ts→ 82 passed (82),exit 0。npx vitest run(maker-shared 全量)→ 109 files / 2076 tests passed,无回归。- 红绿复验 A:把
projectTitle(displayDir)退回projectTitle(workingDir)→ 2 failed | 46 passed(两条expected 'app' to be 'App',即标题大小写断言确实保护该行为);还原后全绿。 - 红绿复验 B:去掉
displayDir的.trim()→ 1 failed | 47 passed(padding 用例expected ' ' to be 'app',即 Codex P2 的空白标题修复确实被钉住);还原后git diff干净。 npx tsc --noEmit(packages/maker-shared,即该包build脚本)→ 报src/botGroupProcess.ts的findLastIndex/隐式 any 两个错误;该文件不在本 PR 改动范围,错误属 tsconfiglib: ES2022的既有基线问题(与 diff 无关),非本次引入。- 未执行:
pnpm test:unit:related整仓选测(已用受影响包全量 2076 条 + 目标文件红绿替代);heavy 档security-scan.mjs(档位为 standard,不适用);真机/桌面界面目检(改动为分组键字符串归一化,无视觉面,PR 描述亦注明本机无客户端环境)。
Intent alignment
- 原目标(初始 commit
e35ad042,2026-10-31):手机端项目分区在 Windows 上把C:\Repo\App与c:/repo/app拆成两个分区、projectCount翻倍——把buildProjectSections/summarizeRemoteSessionOverview的分组键补成同包其它三处的同一归一化口径(Windows 统一正斜杠并小写、worktree 折 base repo),标题保留用户原始大小写与分隔符写法。 - 最终行为(head
5e827516):分组键统一走新增projectGroupKey()=normalizeProjectOrderPath(sessionProjectDirectory(session) ?? 'unknown'),两处建键点与概览projectCount同键空间;分区标题取组内首条会话原始目录(trim 后只折 worktree)经projectTitle()取 basename,大小写不被小写化。与原目标一致。 - 多轮意见核对:Codex P2(显示目录首尾空白会让标题变空格,
5f00bbfc以displayDir先 trim 修复并补trim whitespace before grouping用例)、Greptile P2 与 MagicLizi P1(标题大小写缺测试保护、已 Resolve 的讨论要求未落实,5e827516两条用例补title === 'App'断言)均已在当前 head 落实,且红绿复验确认断言真能拦住对应回退。三条 thread 全部isResolved=true,均为测试/边界加固,没有改变产品方向;无原意偏离。 - 非阻断提醒(交主 session/作者知悉,不作 P1):PR 描述「怎么验证的 / 范围」两处仍写「新增 2 条用例、47 passed」,而最终 head 实为 3 条新增(48 passed,第二 commit 追加了空白标题用例),描述计数滞后于 head;描述里的
titles = ["app"]示例依赖同组首条会话是哪个写法(实现按组内首条会话取标题,首条为C:\Repo\App时标题是App),属示例与排序有关的表述含糊,不算描述不实。
Overall
pass —— 没有 P0/P1;原意一致性复核通过,0 条未解决 thread,凭证/格式门通过,CI 全绿(pr-design-basis、client-ci 均 success)。注意:copilot-pull-request-reviewer / chatgpt-codex-connector 未对当前 head 表态(机器人静默期满 20 分钟上限放行),其意见可能在合并后到达。
这次改了什么
摘要
手机端「项目」分区在 Windows 上会把同一个真实项目拆成两个分区,概览里的项目数也跟着翻倍。
buildProjectSections和summarizeRemoteSessionOverview建分组键时只调了collapseWorktreeDirForGrouping,也就是只把 worktree 折到 base repo,没有做 Windows 路径的归一化。同包其它三处(mobileHome.ts的projectGroupKey、projectOrderSync.ts的normalizeProjectOrderPath、conversationSearch.ts的normalizeWorkingDirForGrouping)都是在折叠之上再统一分隔符并按 Windows 语义小写,只有这里漏了。于是
C:\Repo\App和c:/repo/app会落成两个分区键。这不是假想输入:同一台机器上报路径时正反斜杠混用、用户在不同位置手输大小写、以及一条会话带projectDir而另一条只有workingDir(sessionProjectDirectory = projectDir ?? workingDir),都会产生这种差异。改前的输出(真实模块跑出来的,
C:\Repo\App+c:/repo/app两条会话):改后:
同一个键还带出两个副作用,一并消失:同一项目在两个分区里各出现一次;
projectCount把一份项目数成两份。sessionList.ts:427-428那行注释本来就写着「与首页 projectGroupKey、桌面侧栏同一口径」,mobileHome.ts:672-674也写明设备详情页与首页用「同一套归一化语义」。这次是把代码补成注释承诺的样子,不是新定规则。实现上新增一个
projectGroupKey()小函数,三处建键的地方统一走它;normalizeProjectOrderPath内部已经调了collapseWorktreeDirForGrouping,所以 worktree 折叠行为不变。分区标题改成用组内首条会话的原始目录(只折 worktree),保留用户自己的大小写与分隔符写法——worktreePaths.ts:13-14明确说这个模块不做分隔符风格改写,标题不该被归一化。变更类型
feat新功能fix缺陷修复refactor/perf重构或性能优化docs/test/chore文档、测试或工程维护范围
packages/maker-shared/src/sessionList.ts的 3 处分组键 + 1 个标题来源;packages/maker-shared/src/__tests__/sessionList.test.ts新增 2 条用例。sessionList.ts:925附近的automationGroupKey同样是只折 worktree 的写法,但它属于自动化任务分组、会改变任务分组的展示口径,留作单独一单。桌面侧栏若另有同类建键点也不在本 PR。UI 变化
怎么验证的
自动验证
改动文件单独跑(vitest 1.6.0,本机):
把
sessionList.ts换回基线、两条新用例保留:两条新用例的价值(
docs/dev-rules/engineering-conventions.md§3.2):buildRemoteSessionSections/summarizeRemoteSessionOverview是手机端项目列表与概览的直接入口,没有绕过它们手写分组逻辑。workingDir只是写法不同(C:\Repo\App与c:/repo/app),断言的是用户能看到的「几个分区」与projectCount。sessionList.ts:427-428的注释、projectOrderSync.ts:267-270的 docstring、mobileHome.ts:672-674的说明三处都要求同一口径。session()构造器与buildRemoteSessionSections的既有调用方式,并入已有测试文件,没有新建文件。手工验证
不涉及。本机是 Windows,但没有可运行的 Cindy 客户端与设备联动环境,未做界面点击验证;上述结论来自真实模块的自动化调用。
未执行的验证
pnpm test:unit:related未执行:本机没有装这个 monorepo 的依赖(1 万级锁文件,node_modules缺失),只按改动文件单独跑了两个测试文件。风险
风险分类
跨平台差异一栏:本 PR 正是修跨平台差异,且只在 Windows 风格路径上改变结果。
影响与回滚
packages/maker-shared的项目分区键与概览projectCount。POSIX 路径行为不变(大小写敏感仍分开),worktree 折叠不变。sessionList.ts即可,没有数据写入、迁移或协议变化。提交前检查
git commit -s,见 DCO)