test: isolate codex hook sync env

[affaan-m]

Summary

• make codex hook sync tests hermetic by setting explicit Codex/package-manager env
• stop from inheriting ambient runner state
• restore full-suite stability for the shared CI matrix

Testing

• node tests/scripts/codex-hooks.test.js
• CI=true node tests/run-all.js

---

Summary by cubic

Make Codex hook sync tests run in a hermetic environment by setting explicit Codex and package manager env. Prevents CI runner state from leaking in and stabilizes the suite.

• Bug Fixes
  • Added makeHermeticCodexEnv() to build an isolated env (HOME/USERPROFILE, CODEX_HOME, .agents, ECC_GLOBAL_HOOKS_DIR, CLAUDE_PACKAGE_MANAGER, CLAUDE_CODE_PACKAGE_MANAGER=npm).
  • Applied the helper to both sync and check steps in tests/scripts/codex-hooks.test.js.

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

Summary by CodeRabbit

• Tests
  • Enhanced test environment configuration for shell script integration testing to ensure more consistent and reliable test execution.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 585feff1-1886-4f9a-8e50-3cd64fd8f3b3

📥 Commits

Reviewing files that changed from the base of the PR and between 118e57e and d8c940f.

📒 Files selected for processing (1)

• tests/scripts/codex-hooks.test.js

---

📝 Walkthrough

Walkthrough

Added a makeHermeticCodexEnv helper function to construct a self-contained environment object for Codex shell scripts, setting HOME, CODEX_HOME, USERPROFILE, AGENTS_HOME, ECC_GLOBAL_HOOKS_DIR, and package manager variables. Updated script invocations in tests to use this helper instead of manually passing individual environment variables.

Changes

Cohort / File(s)	Summary
Codex test environment helper tests/scripts/codex-hooks.test.js	Added makeHermeticCodexEnv(homeDir, codexDir, extraEnv) helper function to generate a comprehensive environment object for Codex scripts. Updated invocations of sync-ecc-to-codex.sh and check-codex-global-state.sh to use this helper for consistent environment setup.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: harden codex sync CI hermeticity #1020: Modifies how Codex sync and its tests construct and pass hermetic environments, directly aligning with the environment variable setup changes in this PR.
• fix(codex): broaden context7 config checks #1000: Updates to tests/scripts/codex-hooks.test.js that change script invocations and environment handling, complementary to the helper function approach.
• fix: harden Trae install ownership #1013: Modifies the same test file to adjust how Codex sync/global scripts are invoked with environment and argument setup.

Poem

🐰 A hermetic home for every test we weave,
One helper function makes environments cleave,
No more scattered vars in disarray,
The Codex scripts now have their way! 🚀

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/codex-hooks-hermetic-env

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

No issues found across 1 file

[greptile-apps]

Greptile Summary

This PR tightens test isolation for the Codex hook sync suite by introducing a makeHermeticCodexEnv helper that stamps explicit values for HOME, USERPROFILE, CODEX_HOME, AGENTS_HOME, ECC_GLOBAL_HOOKS_DIR, CLAUDE_PACKAGE_MANAGER, and CLAUDE_CODE_PACKAGE_MANAGER on every subprocess spawned by the affected tests. The key motivation is that sync-ecc-to-codex.sh delegates to a Node.js script (merge-mcp-config.js) that reads CLAUDE_PACKAGE_MANAGER for auto-detection; without overriding that var, test results could vary depending on which package manager is available on the CI runner.

• The hermetic env correctly covers all variables consumed by sync-ecc-to-codex.sh and check-codex-global-state.sh (CODEX_HOME, AGENTS_HOME, ECC_GLOBAL_HOOKS_DIR, and the package-manager hints).
• The first test (install-global-git-hooks.sh) still uses the old ad-hoc env object and does not receive the new hermetic treatment — a minor inconsistency given the stated goal of full-suite stability.
• CLAUDE_CODE_PACKAGE_MANAGER is included in the hermetic env but is not actively read by any script or library in the repo (only CLAUDE_PACKAGE_MANAGER is consumed); it is harmless but unexplained.

Confidence Score: 5/5

Safe to merge — changes are test-only and all findings are minor style/consistency suggestions.

All findings are P2: the first test not using the helper is a low-risk inconsistency (the script it exercises doesn't read the newly-guarded vars today), and the extra CLAUDE_CODE_PACKAGE_MANAGER entry is harmless. No production code is touched, and the hermetic env correctly covers the variables that actually matter for the sync/check test.

No files require special attention.

Important Files Changed

Filename	Overview
tests/scripts/codex-hooks.test.js	Adds makeHermeticCodexEnv helper to isolate sync/check tests from ambient runner env; first test (install-global-git-hooks.sh) is left using the old ad-hoc env object and does not receive the same hermetic treatment.

Sequence Diagram

sequenceDiagram
    participant Test as codex-hooks.test.js
    participant Helper as makeHermeticCodexEnv()
    participant Sync as sync-ecc-to-codex.sh
    participant MCP as merge-mcp-config.js (Node)
    participant Check as check-codex-global-state.sh

    Test->>Helper: makeHermeticCodexEnv(homeDir, codexDir)
    Helper-->>Test: { HOME, USERPROFILE, CODEX_HOME, AGENTS_HOME, ECC_GLOBAL_HOOKS_DIR, CLAUDE_PACKAGE_MANAGER, CLAUDE_CODE_PACKAGE_MANAGER }

    Test->>Sync: runBash(syncScript, ['--update-mcp'], hermeticEnv)
    Sync->>MCP: node merge-mcp-config.js (inherits CLAUDE_PACKAGE_MANAGER=npm)
    MCP-->>Sync: merged config.toml
    Sync-->>Test: exit 0

    Test->>Check: runBash(checkScript, [], hermeticEnv)
    Check-->>Test: exit 0 + stdout with context7 MCP section match

Loading

Comments Outside Diff (1)

1. tests/scripts/codex-hooks.test.js, line 72-75 (link)

   First test not given hermetic env

   The new makeHermeticCodexEnv helper is used for the sync/check test but the first test (install-global-git-hooks.sh handles quoted hook paths) still passes a plain ad-hoc object. If CODEX_HOME, CLAUDE_PACKAGE_MANAGER, or other ambient CI vars are set on the runner, this test could still be affected by that state — contradicting the stated goal of making the full suite hermetic.

   install-global-git-hooks.sh currently only cares about HOME and ECC_GLOBAL_HOOKS_DIR, so the risk is low today, but it's an inconsistency worth closing for long-term stability:

_{Reviews (1): Last reviewed commit: "test: isolate codex hook sync env" | Re-trigger Greptile}

[greptile-apps]

CLAUDE_CODE_PACKAGE_MANAGER is not consumed by any script

CLAUDE_PACKAGE_MANAGER is the env var read by scripts/lib/package-manager.js (line 167). CLAUDE_CODE_PACKAGE_MANAGER only appears as a commented-out example in .env.example and is never read at runtime. Including it is harmless and defensive, but worth a comment so future readers understand the intent rather than wondering whether it's a typo of the active var.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}