fix(test): isolate environment in non-interactive-env hook tests

[devxoul]

Summary

• Fix flaky test failures in non-interactive-env hook that occurred on GitHub Actions CI runners
• Add environment isolation in beforeEach() to ensure tests start with clean Unix-like environment

Changes

• src/hooks/non-interactive-env/index.test.ts:
  • Clear PSModulePath environment variable in beforeEach() to prevent PowerShell detection on Linux CI runners
  • Set SHELL=/bin/bash to establish baseline Unix-like environment
  • Add explanatory comment documenting why this isolation is necessary
  • Tests can still override these values for PowerShell-specific test cases

Context

GitHub Actions CI runners have PSModulePath set even on Linux systems, causing the non-interactive-env hook to incorrectly detect PowerShell as the shell. This led to test failures because tests expecting Unix behavior were getting PowerShell environment variables instead.

The fix ensures all tests start from a known, clean state by explicitly clearing platform-specific variables in beforeEach(), while still allowing individual tests to set these values when testing PowerShell-specific behavior.

Testing

bun run typecheck
bun test

All tests pass locally and on CI after this fix.

Related Issues

• Unblocks PR feat: add file:// URI support in agent prompt_append #821 which was failing due to this test regression

---

Summary by cubic

Fixes flaky non-interactive-env hook tests on CI by resetting shell-related env vars before each test. Ensures Unix-like defaults so tests run consistently while still allowing PowerShell-specific cases.

• Bug Fixes
  • Clear PSModulePath in beforeEach to avoid accidental PowerShell detection on Linux CI.
  • Set SHELL=/bin/bash as the baseline; tests can override when needed.

^{Written for commit 6015fce. Summary will update on new commits.}

[code-yeongyu]

@sisyphus-dev-ai review this ulw ulw

[cubic-dev-ai]

No issues found across 1 file

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

[sisyphus-dev-ai]

PR Review: #822 - fix(test): isolate environment in non-interactive-env hook tests

Status: ✅ APPROVE

This PR correctly fixes a test isolation issue that could cause failures in CI environments where PowerShell is installed (even on Linux/macOS).

---

What This Fixes

The non-interactive-env hook uses detectShellType() which checks for PSModulePath and SHELL environment variables to determine which shell syntax to use. Without baseline environment isolation in beforeEach:

• Tests expecting Unix behavior could fail if CI has PowerShell installed
• Individual tests already set these variables explicitly for platform-specific testing (lines 175-177, 197-198, 217, 237-239, etc.)
• But tests relying on "default" Unix behavior had no guaranteed clean state

Example scenario where this fails:

• GitHub Actions runner on Linux has PowerShell Core installed
• PSModulePath is set in the CI environment
• Test at line 30 expects export syntax but gets $env: syntax instead
• Test fails ❌

---

Changes Verified

Check	Status	Evidence
Environment isolation	✅	Adds baseline Unix state (SHELL=/bin/bash, no PSModulePath) in beforeEach
State restoration	✅	Original env properly saved/restored in afterEach (lines 18-26)
Tests pass	✅	bun test output: 16 pass, 0 fail, 47 expect() calls
Individual tests unaffected	✅	Tests needing Windows/PowerShell still explicitly set variables
Other files need fix?	❌	Checked shell-env.test.ts and env-cleaner.test.ts - both already properly isolated

---

Code Quality Assessment

Strengths:

• ✅ Clear BDD comment explaining intent (lines 16-18)
• ✅ Follows existing codebase patterns
• ✅ Minimal, surgical fix (only 5 lines added)
• ✅ No breaking changes
• ✅ Proper test isolation principle

Location in codebase: src/hooks/non-interactive-env/index.test.ts:16-18

---

Additional Context

I verified that no other test files in the codebase need this fix:

1. src/shared/shell-env.test.ts: Each test explicitly sets environment variables within the test case itself. No baseline state needed.

2. src/features/skill-mcp-manager/env-cleaner.test.ts: Tests environment variable filtering, not shell detection. Each test sets what it needs explicitly.

This PR is the only place where baseline environment isolation is needed because:

• Some tests don't explicitly set SHELL/PSModulePath (e.g., tests at lines 30-47, 49-63, 65-77)
• Those tests expect Unix-like behavior by default
• Without this fix, they could fail in CI environments with PowerShell

---

Recommendation

LGTM! Ship it! 🚢

This is a textbook example of proper test isolation. The fix is:

• Necessary (prevents CI flakiness)
• Correct (follows best practices)
• Complete (no other files need changes)
• Verified (tests pass)