fix(shared): tolerate missing git-info extension in child sessions - #340
Merged
tt-a1i merged 2 commits intoSep 2, 2026
Merged
Conversation
liwen-aaa
force-pushed
the
fix/child-session-gitinfo-realpath
branch
from
September 2, 2026 03:24
5a1c59c to
315cd17
Compare
tt-a1i
requested changes
Sep 2, 2026
tt-a1i
left a comment
Collaborator
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.
liwen-aaa
force-pushed
the
fix/child-session-gitinfo-realpath
branch
from
September 2, 2026 05:44
315cd17 to
c755bd1
Compare
tt-a1i
approved these changes
Sep 2, 2026
tt-a1i
left a comment
Collaborator
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.
tt-a1i
approved these changes
Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.