Consolidate home directory path expansion

[rumpl]

No description provided.

[docker-agent]

Assessment: 🟢 APPROVE

The consolidation of home-directory path expansion into a single pkg/path.ExpandHomeDir function is well-structured and semantically equivalent to the three separate implementations it replaces. No high or medium severity issues were found.

Two minor quality observations are left as inline comments.

[docker-agent]

[LOW] Fragile tilde-detection via identity comparison in AbsolutePath

The check if expanded == p && p != "~" detects unsupported tilde forms like ~user/foo by relying on ExpandHomeDir returning the input unchanged when it doesn't expand it. This is correct today, but it is brittle: if a future change to ExpandHomeDir normalises or cleans an unrecognised tilde path before returning (e.g. calls filepath.Clean), the check would silently pass and fall through to path resolution with a literal ~ in the path.

Consider adding an explicit check instead:

if strings.HasPrefix(p, "~") && !strings.HasPrefix(p, "~/") && p != "~" {
    return "", fmt.Errorf("unsupported tilde expansion format: %s", p)
}

This makes the intent clear regardless of what ExpandHomeDir does internally.

[docker-agent]

[LOW] Error path of ExpandHomeDir is not tested

The test suite covers all the happy-path expansions, but never exercises the case where os.UserHomeDir() fails (e.g. HOME and USERPROFILE are both unset). This means the error message "failed to get user home directory" and the homeDir == "" guard are untested.

Consider adding a test that unsets both env vars and asserts that ExpandHomeDir("~/foo") returns an error.

[docker-agent]

Assessment: 🟢 APPROVE

This PR cleanly consolidates multiple local tilde-expansion implementations into a single pkg/path.ExpandHomeDir function. The refactoring is well-structured and behaviorally consistent with the code it replaces.

What was reviewed: 460 diff lines across 10 files, covering the new ExpandHomeDir implementation, its adoption in cmd/root, pkg/environment, and pkg/tools/builtin/filesystem, plus the relocated and improved test suite.

Key findings:

• Behavioral parity with all three old local implementations is maintained
• Cross-platform correctness (compile-time filepath.Separator check) is consistent with prior behavior
• Error handling strategy is preserved: resolvePath intentionally swallows errors (by design, documented in the old code), while expandPathToken propagates them
• Test improvements: using t.TempDir() + t.Setenv to control the home directory is a solid upgrade from relying on the real home dir
• No behavioral regressions detected

Minor observations (not blocking):

• In pkg/environment/env_files.go, the guard expanded == p && p != "~" is logically correct — the p != "~" arm is unreachable in the success path (if ExpandHomeDir returns nil error and p == "~", then expanded will be the home dir, not "~"), but it is harmless.
• In filesystem_paths.go, the || token == "~" in the early-return condition is redundant since a successful ExpandHomeDir("~") always returns a different string than "~", but again, harmless.