Skip to content

fix(home): resolve ~ through getUserHome everywhere - #1046

Open
Jay0130-a wants to merge 1 commit into
Tencent:mainfrom
Jay0130-a:fix/consistent-user-home
Open

Jay0130-a wants to merge 1 commit into
Tencent:mainfrom
Jay0130-a:fix/consistent-user-home

Conversation

@Jay0130-a

@Jay0130-a Jay0130-a commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

getUserHome() (HOME → USERPROFILE → os.homedir()) is the resolver the rest of teamai already uses for ~/.teamai state and credentials, but several call sites still expanded ~ with os.homedir() directly. On any machine where the two disagree — a native Windows PowerShell session that exposes only USERPROFILE, or Git Bash on Windows, which sets HOME to a path os.homedir() never reports — the same state was written and read in two different homes. This routes every remaining expansion through getUserHome().

Background

src/utils/home.ts defines the canonical home — HOME → USERPROFILE → os.homedir(), throwing when none resolve — because a native Windows PowerShell session commonly exposes only USERPROFILE, and Git Bash on Windows sets HOME to a path os.homedir() never reports. The rest of teamai already resolves ~/.teamai state and credentials through getUserHome(), but several call sites still expanded ~ with os.homedir() directly, so on any machine where the two disagree the same state is written and read in two different homes.

The change

Every remaining os.homedir() expansion now goes through getUserHome():

file what it resolved
src/providers/gitcode/gitcode-api.ts netrcPath() — ~/.netrc
src/providers/tgit/gf-cli.ts gfGetOAuthToken() — ~/.netrc
src/utils/path-safety.ts resolveReal() and defaultAllowedRoots()
src/utils/repo-cache.ts, src/utils/cache-index.ts getCacheRoot()
src/push.ts push --skill ~/… (resolveRequestedSkill())

os.homedir() now appears only inside home.ts, where it is the documented last-resort fallback.

Why it matters

  • The netrc token was in the file git does not read. writeNetrcToken() wrote os.homedir()/.netrc while git reads $HOME/.netrc on POSIX — and %HOME% before %USERPROFILE% on Windows, the same order getUserHome() uses. The credential teamai stored was not the one git authenticated with, and readNetrcToken() could return a different account's token from the other file.
  • The repo cache was split from the rest of ~/.teamai. Credentials and team-repo state live under $HOME/.teamai; the cache went to os.homedir()/.teamai/cache/repos.
  • The path-safety allowlist rejected the home teamai writes to. defaultAllowedRoots() returned os.homedir(), so a path under $HOME — where teamai had just written — resolved outside every allowed root.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature causing existing behavior to change)
  • Documentation only
  • Refactor / internal cleanup

Test Plan

New src/__tests__/user-home-resolution.test.ts holds $HOME and the platform home apart (a real HOME with os.homedir() mocked elsewhere) and asserts the netrc token, the cache root and the default allowed roots all land in $HOME. It fails on main and passes with this change:

× stores the GitCode token in the netrc file git itself reads
× keeps the repo cache under the same home as the rest of ~/.teamai
× allows the home it actually writes to by default

src/__tests__/gitcode-provider.test.ts also needed HOME set in its beforeEach, because getUserHome() reads HOME before the os.homedir() spy it already installed.

  • npx tsc --noEmit passes
  • npm run lint passes
  • npx vitest run passes
  • Added/updated tests for the change

npm run lint and npx vitest run are left unticked because both hit host-specific problems on the machine this was prepared on, not findings in the change:

  • npm run lint is oxlint --type-aware --deny-warnings --report-unused-disable-directives, and its tsgolint subprocess could not spawn there (Failed to spawn tsgolint … os error 231). The non-type-aware pass was run instead and is clean: npx oxlint --deny-warnings --report-unused-disable-directives src/ → 0 warnings, 0 errors (895 files).
  • The full vitest suite has pre-existing host-specific failures on that machine (safe-delete shim and Windows path separators). For the files this PR touches, the failure set is byte-identical before and after — src/__tests__/push-skill-flag.test.ts reports the same 5 failures / 3 passes on main and on this branch — with the 3 new tests in user-home-resolution.test.ts as the only delta. On Linux CI, where path.join uses / and HOME agrees with os.homedir(), those cases are not affected.

Related Issues

Fixes #1045

Notes for Reviewers

Rebased onto main. main moved this code while the branch was open: push --skill now resolves its path in a new resolveRequestedSkill() helper that still called os.homedir(). The rebase keeps main's structure and moves the ~ expansion onto getUserHome() there; the helper is now synchronous, since dropping the dynamic import('node:os') removed its only await.

One deliberate non-change. getUserHome() still returns $HOME verbatim, so a Git Bash session on Windows that sets HOME=/c/Users/… gets a POSIX-style path. That behaviour is pre-existing and shared with every current getUserHome() caller — normalising MSYS paths would be a separate change. This PR only makes the remaining call sites agree with the resolver the project already standardised on, rather than introducing a third rule.

Follow-ups worth a decision, not hidden here:

  • Existing credentials written to os.homedir()/.netrc are not migrated and not read as a fallback, so on a machine where the two homes differ a previously stored token becomes invisible until the user authenticates again. If a migration is wanted, it belongs in netrcPath()'s read path.
  • getUserHome()'s POSIX-style value is passed straight into path.join() on Windows, so HOME=/c/Users/alice resolves to \c\Users\alice. The new tests set HOME to a native path, so that combination is not yet pinned by a test.

@jeff-r2026 jeff-r2026 self-assigned this Oct 12, 2026
@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/providers/gitcode/gitcode-api.ts:74 moves credential reads from the legacy os.homedir()/.netrc without migration or fallback. An existing user with HOME != os.homedir() who authenticated before upgrading has their token only in the old file; after upgrading, getGitCodeToken() returns null and GitCode operations fail until they re-authenticate.
  • [P1 blocking] src/providers/gitcode/gitcode-api.ts:74 passes getUserHome() directly to Windows path.join(). In the acknowledged Git Bash case where HOME=/c/Users/alice, Node resolves this as a drive-root path such as \c\Users\alice, not C:\Users\alice; netrc, cache, path-safety, and push --skill ~/… therefore use invalid/wrong locations. The new tests use native temporary paths and do not exercise this case.
  • [P1 blocking] The PR changes runtime behavior but the Verification section contains only type-check, lint, and test-suite results. Per the repository’s Code Review Rules, it must document at least one representative end-to-end/real-CLI invocation before merge.

@STiFLeR7 STiFLeR7 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the HOME vs os.homedir() split is a real problem and the fix is mechanical and low-risk. I read the full diff against src/utils/home.ts and checked out the branch (Windows, Git Bash, npm ci, vitest).

  1. The PR says "os.homedir() now appears only inside home.ts". That is not quite true on the branch head. src/hermes-home.ts:2,16 still does import { homedir } from 'node:os' and path.join(homedir(), '.hermes'). A grep for os.homedir misses it because it is a named import (grep -rn homedir src --include=*.ts shows it). It is the same bug class: with only USERPROFILE set, or Git Bash's HOME differing from os.homedir(), teamai would look for ~/.hermes in a different home than the rest of ~/.teamai. Suggest path.join(getUserHome(), '.hermes') there too. src/__tests__/hermes-config.test.ts mocks os.homedir, so it will need the same HOME handling you added to gitcode-provider.test.ts.

  2. Test teardown nit in src/__tests__/user-home-resolution.test.ts (afterEach): process.env.HOME = savedHome; assigns the string "undefined" when HOME was unset at start (Node coerces; verified with process.env.X = undefined). HOME unset is exactly the native-PowerShell case this PR is about. savedCacheDir already uses the === undefined ? delete : assign pattern, so HOME should too. It only leaks within that test file's worker, so low severity.

  3. Coverage: the new tests exercise HOME vs os.homedir() only. The USERPROFILE-only path (the Windows PowerShell case in the description) is not asserted, and nothing covers push --skill ~/... (push.ts around L1371 and L1394) or gfGetOAuthToken. push-skill-flag.test.ts "expands ~ in skill path" builds expectations from os.homedir(). Optional follow-up, not a blocker.

Test run notes, for transparency: push-skill-flag.test.ts has 4 failures on this branch on Windows and 5 on current main (same cause, POSIX-style paths in mocks), so they are not caused by this PR. path-safety.test.ts passes (26/26).

Once 1 is addressed I would be happy to approve; the rest is optional.

@STiFLeR7

Copy link
Copy Markdown
Collaborator

Triage pass (Triage access only, no merge authority on my end) — this PR has 3 open P1-blocking findings from the bot with no response yet:

  • src/providers/gitcode/gitcode-api.ts:74: credential reads moved off the legacy os.homedir()/.netrc with no migration/fallback — an existing user with HOME != os.homedir() who authenticated pre-upgrade loses their token (getGitCodeToken() returns null) until they re-auth.
  • Same line: getUserHome() piped straight into Windows path.join(). Under Git Bash where HOME=/c/Users/alice, that resolves to a drive-root path (\c\Users\alice), not C:\Users\alice — netrc, cache, path-safety and push --skill ~/... would all land on the wrong path. The new tests use native temp paths, so this case isn't covered yet.
  • The Verification section only has typecheck/lint/test results; the repo's review rules ask for at least one representative real-CLI/e2e run for a runtime-behavior change like this.

Also showing CONFLICTING against main. Let me know if you're still working this or need anything clarified — not trying to rush, just making sure these don't get buried.

`getUserHome()` (HOME -> USERPROFILE -> os.homedir()) is the resolver the
rest of teamai already uses for ~/.teamai state and credentials, but
several call sites still expanded ~ with os.homedir() directly. On any
machine where the two disagree -- a native Windows PowerShell session that
exposes only USERPROFILE, or Git Bash on Windows, which sets HOME to a path
os.homedir() never reports -- teamai wrote and read the same state in two
different homes:

- the GitCode/TGit netrc token was written to os.homedir()/.netrc while
  git reads $HOME/.netrc, so the stored credential was not the one git
  uses, and the provider reported unauthenticated;
- the repo cache landed under os.homedir()/.teamai/cache/repos while the
  rest of ~/.teamai stayed under $HOME;
- defaultAllowedRoots() allowed os.homedir(), so a path under $HOME --
  where teamai had just written -- was judged outside every allowed root.

Route every remaining expansion through getUserHome() so one machine has
one home, and add regression tests that hold HOME and the platform home
apart.
@Jay0130-a
Jay0130-a force-pushed the fix/consistent-user-home branch from 7c5b0b4 to 603e959 Compare October 13, 2026 15:09
@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/providers/gitcode/gitcode-api.ts:74 changes the credential location without migration or fallback. An existing user whose HOME differs from os.homedir() and whose token exists only in the old .netrc loses GitCode authentication after upgrading. Preserve access to the existing credential or migrate it safely.
  • [P1 blocking] src/providers/gitcode/gitcode-api.ts:74 now feeds an MSYS-style HOME directly into native Windows path handling. With HOME=/c/Users/alice, this targets \c\Users\alice\.netrc, not C:\Users\alice\.netrc; the cache and tilde-expansion changes likewise target the wrong location. Although other resolver callers already have this problem, this diff introduces it into previously native-home-based operations. Normalize the path before switching these callers.
  • [P1 blocking] The PR description still documents only type-check, lint, and unit-test results, despite changing runtime behavior. This violates the first Code Review Rules item requiring an end-to-end/real-CLI verification record. Document one representative real-CLI run after building; a full provider × agent matrix is unnecessary.

None of the three earlier findings is resolved in the current diff. No PR code was run, built, or installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants