Uh oh!
There was an error while loading. Please reload this page.
fix(shared): tolerate missing git-info extension in child sessions - #340
Conversation
5a1c59c to
315cd17Compare
tt-a1i
left a comment
There was a problem hiding this comment.
方向和价值都认可,但有两个边界需要在合并前收紧:
当前把权威 git-info 目标路径交给 canonicalExistingPath,而该函数会吞掉所有 realpath 错误。这里应只把 ENOENT 解释为扩展确实不存在;权限错误、循环链接或其他无法验证身份的错误应继续 fail closed,避免子会话在不可验证时保留 Git 轮询扩展。
回归测试会临时重命名仓库中的真实 extensions/git-info/index.ts。测试被中断时可能留下损坏工作区,也可能与并行测试产生文件系统竞态。请改为 mock 目标 realpath 调用,或在临时副本/fixture 中验证,不要变更被跟踪的源码文件。
当前 exact head 315cd17;生产修复方向正确,完成这两项后可重新 review。
excludeOpenPiGitInfoExtension computed the git-info path with a bare realpathSync.native at module top level, which threw ENOENT and failed the whole child-session module when extensions/git-info/index.ts was absent. That module is the child-session core for both subagents/ and workflows/, so a single missing file took down every Direct/Workflow child with no degradation path. Compute the path lazily inside the function via canonicalExistingPath (try/catch, returns undefined) and treat undefined as "git-info absent, nothing to exclude": a trimmed fork, partial install, or packaging drift now degrades to one benign extra extension instead of total outage. Add a regression test that re-imports the module with git-info temporarily renamed away and asserts the core exports still resolve.
315cd17 to
c755bd1Compare
tt-a1i
left a comment
There was a problem hiding this comment.
Re-reviewed exact head c755bd1. The prior blockers are resolved: only ENOENT degrades, non-ENOENT path failures remain fail-closed, and the regression test no longer renames tracked source. Focused checks and current CI are green; repeated unrelated timing failures passed on isolated rerun.
Uh oh!
There was an error while loading. Please reload this page.
excludeOpenPiGitInfoExtension computed the git-info path with a bare realpathSync.native at module top level, which threw ENOENT and failed the whole child-session module when extensions/git-info/index.ts was absent. That module is the child-session core for both subagents/ and workflows/, so a single missing file took down every Direct/Workflow child with no degradation path.
Compute the path lazily inside the function via canonicalExistingPath (try/catch, returns undefined) and treat undefined as "git-info absent, nothing to exclude": a trimmed fork, partial install, or packaging drift now degrades to one benign extra extension instead of total outage.
Add a regression test that re-imports the module with git-info temporarily renamed away and asserts the core exports still resolve.