diff --git a/docs/usage-guide.md b/docs/usage-guide.md index b62bd87b..9cde3af6 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -745,7 +745,9 @@ teamai push On `pull`, when `injectShellProfile` is enabled (default), the env block goes into `~/.zshrc` if `$SHELL` is zsh, otherwise `~/.bashrc` — except on Windows: `$SHELL` is normally unset there, and Git Bash starts as a *login* shell that never reads `.bashrc`, so teamai instead prefers an existing `~/.bash_profile`, then `~/.bash_login`, then `~/.profile`, falling back to `~/.bashrc` only when none of them exist (a zsh installed via MSYS2/Cygwin, which does set `$SHELL`, still resolves to `.zshrc`). This matches Git for Windows' own fallback in `/etc/profile.d/bash_profile.sh`, whose guard is `[ -e ~/.bashrc -a ! -e ~/.bash_profile -a ! -e ~/.bash_login -a ! -e ~/.profile ]` — it only synthesizes a `.bash_profile` that sources `.bashrc` in that same one case, which is why a stray `~/.profile` (even one that just sources something else, e.g. `~/.local/bin/env`) is enough to make `.bashrc` alone go unread. Override the target file with `sharing.env.shellProfilePath` in `teamai.yaml`. -This preference order only decides where a *first* pull writes. Every pull after that sticks to whichever candidate already carries this scope's block, rather than re-running the order — otherwise Git for Windows' own bootstrap would move the target out from under it: the same `/etc/profile.d/bash_profile.sh` guard above also means that first pull satisfies its condition (`.bashrc` now exists, nothing else does yet), so the next Git Bash login shell auto-generates a `~/.bash_profile` that sources it. Without sticking to `.bashrc`, the next pull would prefer that newly-created file and inject a second block there, leaving the original — still working, just loaded one hop further away — reported as a dead leftover. +Every pull re-runs this order to find the file the current environment actually reads, then follows every reference from it to one of the other four candidate filenames — transitively, through as many hops as it takes — looking for a candidate that already carries the block, rather than duplicating it. A chain through a file outside that fixed set of five (e.g. a custom `~/.config/shell/profile` some setups source instead) is not followed. This is what keeps the Git-for-Windows bootstrap above from moving the target out from under it: that same guard condition means a first pull into `.bashrc` leaves the exact state that makes the next login shell auto-generate a `~/.bash_profile` sourcing it, and without following that forwarding relationship the next pull would prefer the newly-created file and inject a second block there, leaving the original — still working, just loaded further away — reported as a dead leftover. The same reasoning covers a plain `.profile` that flat-guards a source of `.bashrc` for interactive shells (`[ -f "$HOME/.bashrc" ] && . "$HOME/.bashrc"`), two hops from whatever a login shell reads first. + +Only two literal line shapes count as a real reference, though: a bare `source X` / `. X` on a line by itself, or the exact self-referential existence guard Git for Windows itself generates, `test -f X && . X` / `[ -f X ] && . X` (tested and sourced path the same file), also on a line by itself — in both cases `X` must be an unquoted `~/name` or an unquoted-or-double-quoted `$HOME/name` (never a quoted `~`, never a single-quoted `$HOME`: a shell does not expand either, so a reference that looks right there would source a literal, nonexistent path). Anything else — a trailing redirection or extra argument on the source itself, an `||` fallback, an unrelated `&&`-chained command, a condition this can't independently verify — is not recognized, and falls back to the order-based pick rather than being guessed at. This is a deliberately narrow, closed set of two forms rather than an attempt to parse arbitrary shell conditionals: matching everything a real shell script could do to make a line conditional (or to disguise one as inert text) needs an actual shell parser, and no fixed-size grammar ever finishes that job. Nothing inside an `if`, `for`/`while`/`until`, `case`, `select`, a function body, or a `(...)`/`{...}` group counts, however it's guarded — none of those are guaranteed to run (a subshell or brace group's body may always run, but its exports never reach the caller either way) — which also means the standard Debian/Ubuntu `.profile` template (the same source, but nested two `if`s deep, checking `$BASH_VERSION` on the way) is not recognized and falls back to the order-based pick. Nothing textually after an unconditional, top-level `return` or `exit` counts either, since control never reaches it. Anything this can't resolve one way or the other, and a block sitting in a candidate nothing in the chain actually reaches, is never preferred over the order-based pick — otherwise a stale block left by a pre-#682 install would outrank the correct file forever, silently reintroducing #682 on upgrade. `doctor` (and the check `pull` runs automatically afterward) also flags a teamai env block left behind in a *different* candidate file — e.g. a block a pre-#682 install wrote to `.bashrc` before this file-selection logic changed — even if that block is broken and was never functional. `teamai uninstall` removes it. diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index aee69483..773d387a 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -718,7 +718,9 @@ teamai push `pull` 时，若启用了 `injectShellProfile`（默认启用），`$SHELL` 为 zsh 时环境变量块会写入 `~/.zshrc`，否则写入 `~/.bashrc`——但 Windows 上例外：`$SHELL` 通常未设置，而 Git Bash 以*登录 shell*方式启动，从不读取 `.bashrc`，因此 teamai 会优先选择已存在的 `~/.bash_profile`、其次 `~/.bash_login`、再次 `~/.profile`，只有三者都不存在时才回退到 `~/.bashrc`（通过 MSYS2/Cygwin 安装、会设置 `$SHELL` 的 zsh 仍会解析到 `.zshrc`）。这与 Git for Windows 自身在 `/etc/profile.d/bash_profile.sh` 中的回退逻辑一致，其判断条件是 `[ -e ~/.bashrc -a ! -e ~/.bash_profile -a ! -e ~/.bash_login -a ! -e ~/.profile ]`——只有在这一种情况下它才会生成一个会 source `.bashrc` 的 `.bash_profile`；这也是为什么哪怕一个只 source 了其他内容（例如 `~/.local/bin/env`）的 `~/.profile` 存在，也足以让 `.bashrc` 单独失效。可通过 `teamai.yaml` 中的 `sharing.env.shellProfilePath` 覆盖目标文件。 -这个优先级顺序只决定*第一次* pull 写到哪里。此后的每次 pull 都会沿用已经承载着本作用域代码块的那个候选文件，而不会重新走一遍优先级判断——否则 Git for Windows 自身的引导逻辑会把目标文件从脚下换掉：上面那条 `/etc/profile.d/bash_profile.sh` 判断条件，在第一次 pull 之后同样会成立（`.bashrc` 已存在，其余候选文件都还不存在），于是下一次 Git Bash 登录 shell 启动时就会自动生成一个 source 它的 `~/.bash_profile`。如果不沿用 `.bashrc`，下一次 pull 就会转而偏好这个新出现的文件，在那里注入第二个代码块，而原来那个——依旧在正常工作，只是多绕了一跳——则会被误报为失效的遗留代码块。 +每次 pull 都会重新走一遍这个优先级判断，找到当前环境实际会读取的那个文件，然后沿着它对另外四个候选文件名的引用一路查下去——无论要经过多少跳——寻找一个已经带着代码块的候选文件，而不是重复注入。如果链条中间经过的是这五个候选文件名之外的文件（比如某些环境会改用 `~/.config/shell/profile` 这类自定义文件来 source），这条链就不会被继续跟踪。这正是为了不让 Git for Windows 自身的引导逻辑把目标文件从脚下换掉：上面那条 `/etc/profile.d/bash_profile.sh` 判断条件，在第一次 pull 写入 `.bashrc` 之后同样会成立，于是下一次 Git Bash 登录 shell 启动时就会自动生成一个 source 它的 `~/.bash_profile`；如果不沿着这条转发链去找，下一次 pull 就会转而偏好这个新出现的文件，在那里注入第二个代码块，而原来那个——依旧在正常工作，只是绕得更远了——则会被误报为失效的遗留代码块。同样的道理也适用于一个普通的 `.profile`：它用一条扁平的存在性守卫（`[ -f "$HOME/.bashrc" ] && . "$HOME/.bashrc"`）为交互式 shell source `.bashrc`——这时登录 shell 最先读到的文件，离实际代码块有两跳之遥。 + +不过，只有两种字面写法才算真正的引用：单独一行的裸 `source X` / `. X`，或者单独一行、与 Git for Windows 自己生成的写法完全一致的自引用存在性守卫 `test -f X && . X` / `[ -f X ] && . X`（被测试路径与被 source 路径完全相同）——两种情况下 `X` 都必须是不加引号的 `~/name`，或不加引号/用双引号包裹的 `$HOME/name`（绝不是加了引号的 `~`，也绝不是用单引号包裹的 `$HOME`：shell 不会展开这两种写法，看起来对的引用实际会 source 一个不存在的字面路径）。除此之外的写法——source 本身带了重定向或额外参数、`||` 回退、不相关的 `&&` 连接命令、任何这段逻辑无法独立验证的条件——一律不识别，直接回退到按优先级选出的文件，而不是去猜。这是刻意收窄到两种固定写法的封闭集合，而不是尝试解析任意的 shell 条件：真要匹配一个真实 shell 脚本能用来让某一行变成有条件执行（或者把可执行内容伪装成惰性文本）的所有手法，需要一个真正的 shell 解析器，任何固定规模的规则集合都不可能穷尽这件事。嵌在 `if`、`for`/`while`/`until`、`case`、`select`、函数体，或者 `(...)`/`{...}` 分组里的内容一律不算数，不管外层条件写的是什么——这些结构要么不保证一定会执行，要么即使一定会执行（比如子 shell 或大括号分组），它导出的环境变量也传不到调用它的 shell 里，这也意味着 Debian/Ubuntu 标准模板里那种嵌套两层 `if`、沿途还检查 `$BASH_VERSION` 的写法无法被识别，会回退到按优先级选出的文件。位于无条件的顶层 `return` 或 `exit` 之后的内容同样不算数，因为控制流根本不会执行到那里。凡是这套逻辑判断不了的情况，以及当前这条链条根本没触及到的候选文件——哪怕它本身带着代码块——都绝不会因此被优先选中，否则 #682 之前旧版本留下的失效代码块就会永远压过正确的文件，等于在升级后又悄悄把 #682 引入回来。 `doctor`（以及 `pull` 结束后自动运行的检查）还会标记出遗留在*其他*候选文件中的 teamai 环境变量块——例如 #682 之前的旧版本写入 `.bashrc` 的代码块，即便该代码块本身已损坏、从未生效。`teamai uninstall` 会清理它。 diff --git a/src/__tests__/shell-profile.test.ts b/src/__tests__/shell-profile.test.ts index ce80e7a2..947cbc9c 100644 --- a/src/__tests__/shell-profile.test.ts +++ b/src/__tests__/shell-profile.test.ts @@ -132,12 +132,30 @@ describe('resolveActiveShellProfile', () => { expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bashrc')); }); - it('sticks to a lower-priority candidate over a higher-priority one that exists but carries no block', async () => { - // Order-based detection would prefer .bash_profile over .profile; the - // sticky block living in .profile must still win. + // Regression (#693 review round 8): the original version of this resolver + // scanned every candidate for a matching block regardless of whether the + // order-based pick could ever reach it, so a stale pre-#682 block sitting + // in `.bashrc` outranked a genuinely unwritten, currently-read `.profile` + // — silently reintroducing #682 for exactly the installs this PR fixes, + // with `doctor` unable to catch it since the stale block is well-formed + // where it sits. The order-based pick's own content must name a candidate + // before that candidate's block is ever preferred over it. + it('does not stick to a stale block in a candidate the order-based pick never reads (#682 upgrade case)', async () => { + // The exact #682 repro: .bashrc present, .bash_profile/.bash_login absent, + // .profile present — order-based detection reads .profile, never .bashrc, + // and .profile does not itself source .bashrc. + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + await fse.writeFile(path.join(homeDir, '.profile'), '# just a profile, unrelated to .bashrc\n'); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.profile')); + }); + + it('prefers the order-based pick over an unrelated candidate that merely carries a block', async () => { + // .bash_profile exists (order-based winner) but has content unrelated to + // any other candidate; a block sitting in .profile must not be preferred + // just because it exists somewhere in the candidate list. await fse.writeFile(path.join(homeDir, '.bash_profile'), 'unrelated content\n'); await fse.writeFile(path.join(homeDir, '.profile'), teamaiBlock()); - expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.profile')); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); }); it('falls back to order-based detection when no candidate owns a block yet (first pull)', async () => { @@ -154,6 +172,395 @@ describe('resolveActiveShellProfile', () => { await fse.writeFile(path.join(homeDir, '.profile'), ''); expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.profile')); }); + + // Regression (#693 review round 9): a bare substring search matched a + // comment mentioning the filename (never executed) and a different, + // longer-named file sharing the same prefix. + it('does not stick to a candidate merely mentioned in a comment', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + '# source ~/.bashrc\nunrelated content\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not stick to a different, longer-named file sharing the same prefix', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'source ~/.bashrc.local\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + // Regression (#693 review round 9): the resolver only followed one hop of + // sourcing, so a chain like .bash_profile -> .profile -> .bashrc (the + // common Debian .profile pattern, sourcing .bashrc for interactive + // shells) missed a block two hops away and would have injected a + // duplicate into .bash_profile instead of reusing .bashrc. + it('follows a two-hop sourcing chain to reach a block (.bash_profile -> .profile -> .bashrc)', async () => { + await fse.writeFile(path.join(homeDir, '.bash_profile'), '. ~/.profile\n'); + await fse.writeFile(path.join(homeDir, '.profile'), '[ -f ~/.bashrc ] && . ~/.bashrc\n'); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bashrc')); + }); + + it('does not hang on a reference cycle and falls back to the order-based pick', async () => { + await fse.writeFile(path.join(homeDir, '.bash_profile'), 'source ~/.profile\n'); + await fse.writeFile(path.join(homeDir, '.profile'), 'source ~/.bash_profile\n'); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + // Regression (#693 review round 10): the resolver committed to the first + // referenced candidate in SHELL_PROFILE_CANDIDATE_NAMES's fixed order and + // gave up if that branch was a dead end, instead of trying every candidate + // the active pick actually references. .bash_profile sourcing both + // .bashrc and .profile is exactly Git for Windows' own generated content + // — .bashrc sorts earlier in the candidate list, so a dead .bashrc branch + // would previously stop the search before it ever reached .profile. + it('tries every referenced candidate, not just the first in priority order', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'test -f ~/.bashrc && . ~/.bashrc\ntest -f ~/.profile && . ~/.profile\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), '# no block here\n'); + await fse.writeFile(path.join(homeDir, '.profile'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.profile')); + }); + + // Regression (#693 review round 15): the scanner no longer recognizes `||` + // fallback chains at all — round 14's N-way generalization, and round 11's + // two-operand version before it, kept needing another regex for another + // adversarial shape (a target that exists but fails to source, mixed + // `&&`/`||`, three-plus operands...). Recognizing only the two literal, + // common forms (bare source, self-referential existence guard) means a + // `||` line of any shape now falls back to the order-based pick — a + // harmless duplicate block, never a wrong one. + it('does not treat either side of a || fallback as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'source ~/.profile || source ~/.bashrc\n', + ); + // .profile is deliberately absent, so a real shell would reach .bashrc — + // out of scope now regardless. + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + // Regression (#693 review round 11): `&&` only establishes reachability + // when this scanner can independently verify the guarding condition — the + // self-referential existence test. A condition testing anything else + // (here, an environment variable) is not verifiable, so a stale block + // behind it must not outrank a genuinely unwritten, currently-read file. + it('does not treat a non-existence && condition as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + '[ "$TERM_PROGRAM" = vscode ] && source ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + // Regression (#693 review round 11): a source line's own text looks + // identical whether it sits at top level or three lines inside an `if` + // block this scanner cannot evaluate. Nothing inside an `if` is trusted, + // conditional or not, so a block only reachable through one is not + // preferred over the order-based pick. + it('does not treat a source nested inside an if block as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'if [ -n "$BASH_VERSION" ]; then\n . ~/.bashrc\nfi\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + // Regression (#693 review round 12): a shell never tilde-expands inside + // any quotes and never variable-expands inside single quotes, so + // `source "~/.bashrc"` and `source '$HOME/.bashrc'` both source a + // literal, near-certainly nonexistent path — a reference that "looks + // right" but would never actually run must not be trusted. + it('does not treat an invalidly-quoted reference as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'source "~/.bashrc"\nsource \'$HOME/.bashrc\'\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does treat a double-quoted $HOME reference as reachable', async () => { + await fse.writeFile(path.join(homeDir, '.bash_profile'), 'source "$HOME/.bashrc"\n'); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bashrc')); + }); + + // Regression (#693 review round 15): only the self-referential existence + // guard is recognized as an `&&` form now — a bare source followed by an + // unrelated `&&`-chained command is no longer a special case, it is just a + // line that is not one of the two recognized forms, so it falls back. + it('does not treat a bare source followed by an unrelated && command as reachable', async () => { + await fse.writeFile(path.join(homeDir, '.bash_profile'), 'source ~/.bashrc && echo ready\n'); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + // Regression (#693 review round 12): only `if` nesting was tracked, so a + // source inside an uncalled function, a non-selected `case` arm, or a + // loop body — none of them guaranteed to run any more than an `if` body + // is — was wrongly treated as unconditional. + it('does not treat a source inside a function body as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'my_func() {\n . ~/.bashrc\n}\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a source inside a case arm as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'case "$-" in\n *i*) . ~/.bashrc ;;\nesac\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a source inside a loop body as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'for f in ~/.bashrc; do\n . ~/.bashrc\ndone\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a source after a backslash-continued unrelated condition as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + '[ "$TERM_PROGRAM" = vscode ] && \\\nsource ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not stop recognizing later unconditional sources after a one-line if/then/fi', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'if [ -f ~/.zshrc ]; then . ~/.zshrc; fi\nsource ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bashrc')); + }); + + it('does not stop recognizing later unconditional sources after a two-line function definition', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'my_func()\n{\n echo hi\n}\nsource ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bashrc')); + }); + + // Regression (#693 review round 15): the existence guard is only + // recognized as a complete line now — round 13's tolerance for further + // `&&`-chained commands after it is out of scope again, since it was part + // of the general trailing-content handling that also caused the pipe/ + // background false positive below. A guard with anything appended falls + // back to the order-based pick. + it('does not treat an existence-gated source with further &&-chained commands as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + '[ -f ~/.bashrc ] && . ~/.bashrc && export READY=1\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a source inside a comment after a semicolon as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + ': # old setup; source ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a source inside a heredoc body as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + "cat <<'EOF'\nsource ~/.bashrc\nEOF\n", + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a quoted separator as a real statement boundary', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + "printf '%s\\n' 'x; source ~/.bashrc; y'\n", + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a source inside a subshell as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + '(\n source ~/.bashrc\n)\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a source after an unconditional return as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'return\nsource ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a source after an unconditional exit as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'exit 0\nsource ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + // Regression (#693 review round 15): a bare source is only recognized as + // its own complete line now — trailing content of any kind (a redirection, + // a pipe, a background `&`) makes it not one of the two recognized forms, + // so it falls back rather than trying to reason about what the trailing + // content does to reachability. + it('does not treat a source with a trailing redirection as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'source ~/.bashrc 2>/dev/null\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + // Regression (#693 review round 15): `source ~/.bashrc | cat` and + // `source ~/.bashrc &` both run the source in a subshell — a pipeline + // member and a backgrounded job never propagate exports to the login + // shell — so trusting trailing content indiscriminately (as a prior round + // did, to recognize legitimate extra arguments and redirections) actively + // reintroduced a false "reachable" for these. The strict, line-only match + // rejects all trailing content uniformly, closing this without needing to + // special-case which trailing forms are safe. + it('does not treat a source piped or backgrounded as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'source ~/.bashrc | cat\nsource ~/.bashrc &\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat either side of a three-way || fallback as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'source ~/.profile || source ~/.bash_login || source ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not treat a source inside the second of two heredocs on one command as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + "cat < { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'cat << { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'cat < { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'cat < { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + '[ "$TERM_PROGRAM" = vscode ] &&\nsource ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('recognizes an existence guard split across an operator-end continuation', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + '[ -f ~/.bashrc ] &&\n. ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bashrc')); + }); + + it('does not treat a source inside a select body as reachable', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + 'select opt in a b; do\n . ~/.bashrc\ndone\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bash_profile')); + }); + + it('does not stop recognizing later unconditional sources after a compound closer with a redirection', async () => { + await fse.writeFile( + path.join(homeDir, '.bash_profile'), + '(\n echo hi\n) >/dev/null\nsource ~/.bashrc\n', + ); + await fse.writeFile(path.join(homeDir, '.bashrc'), teamaiBlock()); + expect(await resolveActiveShellProfile(envShPath, 'win32')).toBe(path.join(homeDir, '.bashrc')); + }); }); describe('envBlockSourcesPath', () => { diff --git a/src/resources/env.ts b/src/resources/env.ts index 507e9a9c..73a29090 100644 --- a/src/resources/env.ts +++ b/src/resources/env.ts @@ -414,9 +414,10 @@ export class EnvHandler extends ResourceHandler { * a second spelling of this choice would check `.bashrc` while the pull * wrote `.zshrc`, and report a correct install as broken. Delegates to the * shared `utils/shell-profile.js` so `teamai uninstall` resolves the same - * file too (#682), and stays on whichever candidate already carries this - * scope's block rather than re-deriving it from scratch every pull (#693 - * review round 7). + * file too (#682), and follows the chain of files the order-based pick + * actually `source`s to reuse a candidate that already carries this + * scope's block, rather than injecting a duplicate every time a new file + * enters that chain (#693 review rounds 7-9). */ detectShellProfile(envShPath: string, platform: NodeJS.Platform = process.platform): Promise { return resolveActiveShellProfile(envShPath, platform); diff --git a/src/utils/shell-profile.ts b/src/utils/shell-profile.ts index b602df3f..94eb2da2 100644 --- a/src/utils/shell-profile.ts +++ b/src/utils/shell-profile.ts @@ -202,34 +202,206 @@ export function envBlockReferencesDataHome(block: string, envShPath: string): bo return false; } +/** + * `~/name` (never quoted — a shell does not tilde-expand inside any quotes) + * or `$HOME/name` / `${HOME}/name` (unquoted or double-quoted — a shell + * does not variable-expand inside single quotes) as a token this scanner + * accepts as a reference to `name`. `source "~/.bashrc"` and + * `source '$HOME/.bashrc'` both source a literal, near-certainly + * nonexistent path, not `name` — a reference that "looks right" but would + * never actually reach the file must not be trusted (#693 review round 12). + */ +function homeRelativeRef(name: string): string { + const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const suffix = `${escaped}(?![\\w.-])`; + return `(?:~/${suffix}|\\$\\{?HOME\\}?/${suffix}|"\\$\\{?HOME\\}?/${suffix}")`; +} + +/** + * Whether `line` opens, or closes, a construct whose body either isn't + * guaranteed to run (`if`/`for`/`while`/`until`/`case`/`select`, a function) + * or runs in a subshell whose exports never reach the caller even when it + * always runs (`(...)`, a brace group) — content inside never counts as + * reaching a candidate, no matter how it looks (#693 review rounds 11-15). + */ +function opensUnverifiedBlock(line: string): boolean { + return /^(?:if|for|while|until|case|select)\b/.test(line) + || /^function\s+\S/.test(line) + || /^\S+\s*\(\)\s*\{?\s*$/.test(line) + || line === '{' + || line === '('; +} +function closesUnverifiedBlock(line: string): boolean { + return /^(?:fi|done|esac)\b/.test(line) || /^\}(?:\s|$)/.test(line) || /^\)(?:\s|$)/.test(line); +} + +/** + * Splits `content` into logical lines: joins a line ending in `\`, or in a + * dangling `&&`/`||` awaiting its next operand (both are real, unremarkable + * shell continuation — a trailing binary operator implicitly continues onto + * the next line with no backslash needed, and treating that next line as an + * independent, unconditional statement is a real false-"reachable" risk, not + * an edge case), joins a lone `{` onto the header line it opens, and drops + * heredoc bodies entirely — their text is data, never executed statements. + * A `<<<` here-string is not mistaken for a `<<` heredoc, a non-`-` heredoc's + * terminator is matched literally (only `<<-` strips leading tabs), and a + * heredoc delimiter may contain `-`/`_` as well as alphanumerics — all three + * were real detection gaps (#693 review round 15), not narrowed away. + */ +function logicalLines(content: string): string[] { + const result: string[] = []; + const rawLines = content.split('\n'); + const heredocQueue: { terminator: string; stripTabs: boolean }[] = []; + + for (let i = 0; i < rawLines.length; i += 1) { + if (heredocQueue.length > 0) { + const { terminator, stripTabs } = heredocQueue[0]; + const withoutCR = rawLines[i].replace(/\r$/, ''); + const candidate = stripTabs ? withoutCR.replace(/^\t+/, '') : withoutCR; + if (candidate === terminator) heredocQueue.shift(); + continue; + } + + let line = rawLines[i].trim(); + while (i + 1 < rawLines.length && ( + (line.endsWith('\\') && !line.endsWith('\\\\')) || line.endsWith('&&') || line.endsWith('||') + )) { + i += 1; + const next = rawLines[i].trim(); + line = line.endsWith('\\') ? `${line.slice(0, -1).trimEnd()} ${next}`.trim() : `${line} ${next}`.trim(); + } + if (!line) continue; + + // A self-contained one-liner (`if ...; then ...; fi`, `case ... esac`, + // `for ...; do ...; done`) opens and closes on the same line — net zero + // depth change, not an unclosed open that corrupts tracking for every + // real statement after it (#693 review round 13/15). + if (/^(?:if|for|while|until|case)\b.*;\s*(?:fi|done|esac)\s*$/.test(line)) continue; + + if (line === '{' && result.length > 0) { + result[result.length - 1] += ' {'; + continue; + } + + for (const heredoc of line.matchAll(/(? { + const ref = homeRelativeRef(name); + const bareSource = new RegExp(`^(?:\\.|source)\\s+${ref}$`); + const existenceGuard = new RegExp( + `^(?:test\\s+-f\\s+${ref}|\\[\\s+-f\\s+${ref}\\s*\\])\\s*&&\\s*(?:\\.|source)\\s+${ref}$`, + ); + + let depth = 0; + let halted = false; + for (const line of logicalLines(content)) { + if (opensUnverifiedBlock(line)) { depth += 1; continue; } + if (closesUnverifiedBlock(line)) { depth = Math.max(0, depth - 1); continue; } + if (depth > 0) continue; + + if (/^(?:return|exit)(?:\s+\S+)?$/.test(line)) { halted = true; continue; } + if (halted) continue; + + if (bareSource.test(line) || existenceGuard.test(line)) return true; + } + return false; +} + /** * Resolve which shell profile file this scope's env block belongs in. * - * Sticky by design: a candidate that already carries a block for this - * scope's `env.sh` is reused, rather than re-running `detectShellProfile`'s - * order-based fallback on every pull. Without this, Git for Windows' own - * `/etc/profile.d/bash_profile.sh` changes which candidate *exists* between - * two pulls out from under it: the first time a login shell starts with - * `~/.bashrc` present but none of `~/.bash_profile`, `~/.bash_login` or - * `~/.profile`, it auto-generates a `~/.bash_profile` that sources both — - * not a symlink, a plain file containing `test -f ~/.bashrc && . ~/.bashrc`. - * `detectShellProfile`'s order then prefers that newly-existing file on the - * *next* pull, injecting a second block there and reporting the still-loading - * `.bashrc` one (loaded transitively through the generated forwarder) as a - * stray leftover, even though nothing ever stopped working (#693 review - * round 7). Only when no candidate already owns a block — a genuinely first - * pull — does the order-based fallback decide. + * Starts from `detectShellProfile`'s order-based pick — the file the current + * environment actually reads — and searches every file it actually `source`s + * (transitively, breadth-first, with cycle protection) for one that already + * carries this scope's block. A candidate the search never reaches is never + * preferred, regardless of what it contains: earlier versions matched any + * candidate with a block anywhere (#693 review round 8: a stale pre-#682 + * block in `.bashrc` then outranked a genuinely unwritten, currently-read + * `.profile`, reintroducing #682 for exactly the installs upgrading through + * this fix), checked only one hop of sourcing (#693 review round 9: + * `.bash_profile` sourcing `.profile` sourcing `.bashrc` — the common Debian + * `.profile` pattern — would miss a block sitting in `.bashrc` two hops away + * and inject a duplicate into `.bash_profile`), and followed only the first + * referenced candidate in a fixed priority order rather than every one + * (#693 review round 10: `.bash_profile` sourcing both `.bashrc` and + * `.profile`, with the block actually sitting in `.profile`, would commit to + * the dead-end `.bashrc` branch first — earlier in `SHELL_PROFILE_CANDIDATE_ + * NAMES` — and give up without ever trying `.profile`). + * + * The common real case this exists for: Git for Windows' + * `/etc/profile.d/bash_profile.sh` auto-generates `~/.bash_profile` + * (`test -f ~/.bashrc && . ~/.bashrc`, a plain file, not a symlink) the + * first time a login shell starts with `~/.bashrc` present but none of + * `~/.bash_profile`, `~/.bash_login` or `~/.profile`. `detectShellProfile` + * then prefers that newly-existing file on the *next* pull; without + * following the chain it opens, injecting a second block there would leave + * the still-loading `.bashrc` one reported as a stray leftover, even though + * nothing ever stopped working. */ export async function resolveActiveShellProfile( envShPath: string, platform: NodeJS.Platform = process.platform, ): Promise { const home = getUserHome(); - for (const name of SHELL_PROFILE_CANDIDATE_NAMES) { - const candidate = path.join(home, name); - const content = await readFileSafe(candidate); + const activePick = await detectShellProfile(platform); + + const visited = new Set(); + const queue: string[] = [activePick]; + while (queue.length > 0) { + const current = queue.shift() as string; + if (visited.has(current)) continue; + visited.add(current); + + const content = await readFileSafe(current); const block = content ? extractEnvBlock(content) : null; - if (block && envBlockReferencesDataHome(block, envShPath)) return candidate; + if (block && envBlockReferencesDataHome(block, envShPath)) return current; + if (!content) continue; + + for (const name of SHELL_PROFILE_CANDIDATE_NAMES) { + const candidate = path.join(home, name); + if (candidate !== current && !visited.has(candidate) && await referencesCandidate(content, name)) { + queue.push(candidate); + } + } } - return detectShellProfile(platform); + + return activePick; }