fix(env): match real source commands, resolve reachability transitively (review)

Two P1s from the bot's review of #715:

- resolveActiveShellProfile's reachability check was a bare substring
  search on the active pick's content. A comment mentioning a filename
  (never executed) or a longer file sharing the same prefix
  (~/.bashrc.local) would both satisfy it, letting a stale block win
  the same way #682 did. Replaced with referencesCandidate(): strips
  full-line comments, splits each remaining line into statements on
  &&/||/;, and only counts a statement whose first word is literally
  `.` or `source` and whose second word is an anchored home-relative
  reference to exactly that candidate.

- The check only followed one hop: .bash_profile sourcing .profile
  sourcing .bashrc (the common Debian .profile pattern, sourcing
  .bashrc for interactive shells) would miss a block two hops away and
  inject a duplicate. Reworked into a loop that walks the chain of
  files the pick actually sources, with a visited set for cycle
  protection, stopping at the first one that carries the block.

Also fixed the P2: EnvHandler.detectShellProfile's doc comment still
claimed it "stays on whichever candidate already carries this scope's
block" unconditionally, which stopped being true once reachability was
required.

Verified end-to-end on a real Windows host:
- The new two-hop chain (.bash_profile -> .profile -> .bashrc, block
  in .bashrc): resolves to .bashrc, no duplicate, doctor fully clean.
- Re-ran the Git-for-Windows one-hop scenario and the #682 upgrade
  scenario from the prior round — both still correct, no regression.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

src/__tests__/shell-profile.test.ts

@@ -172,6 +172,45 @@ 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'));
+  });
 });
 
 describe('envBlockSourcesPath', () => {

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<string> {
     return resolveActiveShellProfile(envShPath, platform);

src/utils/shell-profile.ts

@@ -202,32 +202,61 @@ export function envBlockReferencesDataHome(block: string, envShPath: string): bo
   return false;
 }
 
+/**
+ * Whether `content` runs a `source`/`.` command on a home-relative reference
+ * to `name` (`~/.bashrc`, `$HOME/.bashrc`, `${HOME}/.bashrc`) — the shape a
+ * real forwarding line takes, e.g. Git for Windows' generated
+ * `test -f ~/.bashrc && . ~/.bashrc`.
+ *
+ * Deliberately narrower than a substring search (#693 review round 9): that
+ * matched a comment mentioning the filename (inert, never executed) and a
+ * same-prefixed but different file (`~/.bashrc.local` contains `~/.bashrc`
+ * as a substring). Comment lines are dropped outright; each remaining line
+ * is split on `&&`/`||`/`;` into statements, and a statement only counts
+ * when its first word is literally `.` or `source` and its second word is
+ * exactly the home-relative reference — anchored, so a longer filename
+ * cannot satisfy it by prefix.
+ */
+function referencesCandidate(content: string, name: string): boolean {
+  const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+  const target = new RegExp(`^["']?(?:~|\\$\\{?HOME\\}?)/${escaped}(?![\\w.-])["']?$`);
+  for (const rawLine of content.split('\n')) {
+    if (rawLine.trimStart().startsWith('#')) continue;
+    for (const statement of rawLine.split(/&&|\|\||;/)) {
+      const tokens = statement.trim().split(/\s+/);
+      if (tokens.length >= 2 && (tokens[0] === '.' || tokens[0] === 'source') && target.test(tokens[1])) {
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 /**
  * Resolve which shell profile file this scope's env block belongs in.
  *
  * Starts from `detectShellProfile`'s order-based pick — the file the current
- * environment actually reads — and only diverges from it in two cases, both
- * narrower than "any candidate with a block wins" (#693 review round 8: that
- * broader rule let a stale pre-#682 block in `.bashrc` outrank a genuinely
- * unwritten, currently-read `.profile`, silently reintroducing #682 for
- * exactly the installs upgrading through this fix, with `doctor` no longer
- * able to catch it since the stale block is well-formed where it sits):
+ * environment actually reads — and follows the chain of files it actually
+ * `source`s (transitively, with cycle protection) looking for one that
+ * already carries this scope's block. A candidate the chain never reaches is
+ * never preferred, regardless of what it contains: an earlier version
+ * 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) and 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`).
  *
- * 1. The order-based pick already carries this scope's block — the common
- *    steady state, unchanged from before.
- * 2. The order-based pick carries no block of its own, but its own content
- *    names another candidate that does — e.g. 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; injecting a
- *    second block there would leave the still-loading `.bashrc` one (loaded
- *    transitively through the generated forwarder) reported as a stray
- *    leftover, even though nothing ever stopped working.
- *
- * A candidate the order-based pick does not itself read is never preferred,
- * regardless of what it contains.
+ * 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,
@@ -236,24 +265,21 @@ export async function resolveActiveShellProfile(
   const home = getUserHome();
   const activePick = await detectShellProfile(platform);
 
-  const activeContent = await readFileSafe(activePick);
-  const activeBlock = activeContent ? extractEnvBlock(activeContent) : null;
-  if (activeBlock && envBlockReferencesDataHome(activeBlock, envShPath)) return activePick;
+  const visited = new Set<string>();
+  let current = activePick;
+  while (!visited.has(current)) {
+    visited.add(current);
+    const content = await readFileSafe(current);
+    const block = content ? extractEnvBlock(content) : null;
+    if (block && envBlockReferencesDataHome(block, envShPath)) return current;
+    if (!content) break;
 
-  if (activeContent) {
-    for (const name of SHELL_PROFILE_CANDIDATE_NAMES) {
+    const next = SHELL_PROFILE_CANDIDATE_NAMES.find((name) => {
       const candidate = path.join(home, name);
-      // A home-relative reference (`~/.bashrc`, `$HOME/.bashrc`), the shape a
-      // sourcing line actually takes — not a bare substring match, which a
-      // plain English comment mentioning the filename would also satisfy.
-      const referencesCandidate = activeContent.includes(`~/${name}`)
-        || activeContent.includes(`$HOME/${name}`)
-        || activeContent.includes('${HOME}/' + name);
-      if (candidate === activePick || !referencesCandidate) continue;
-      const content = await readFileSafe(candidate);
-      const block = content ? extractEnvBlock(content) : null;
-      if (block && envBlockReferencesDataHome(block, envShPath)) return candidate;
-    }
+      return candidate !== current && !visited.has(candidate) && referencesCandidate(content, name);
+    });
+    if (!next) break;
+    current = path.join(home, next);
   }
 
   return activePick;