Skip to content

fix(ci): stabilize nvm shell checks - #255

Merged
astyfx merged 1 commit into
mainfrom
fix/nvm-shell-pr-checks
Jul 27, 2026
Merged

fix(ci): stabilize nvm shell checks#255
astyfx merged 1 commit into
mainfrom
fix/nvm-shell-pr-checks

Conversation

@heath-s

@heath-s heath-s commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • keep terminal-runtime.ts within the max-lines ratchet after the project NVM environment change
  • make the provider timeout explicit and allow cold provider-runtime imports enough time in isolated tests
  • isolate NVM executable discovery from the developer's login shell and private dotfiles

Root cause

The merged project-NVM change pushed terminal-runtime.ts six lines past its 1,170-line ratchet. Local CI-equivalent runs also exposed tests whose five-second timeout included cold provider-runtime imports, plus an NVM discovery fixture that invoked the user's real login-shell initialization.

Verification

  • bun install --frozen-lockfile
  • bun run check:reliability-gates
  • bun run check:max-lines-ratchet
  • bun run check:switch-exhaustiveness
  • bun run typecheck
  • bun run test:isolated (300/300 files, Bun 1.3.12)
  • affected test files rerun independently (83/83 tests, Bun 1.3.13)

@heath-s
heath-s marked this pull request as ready for review July 27, 2026 08:36
@heath-s
heath-s requested a review from astyfx July 27, 2026 08:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 74bf980706

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

{
CODEX_HOME: path.join(fakeNvmRoot, "codex-home"),
NVM_DIR: fakeNvmRoot,
SHELL: "/bin/sh",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Isolate /bin/sh from the developer profile

When a Unix developer's ~/.profile exposes a newer codex than the fixture's 9.9.9 binary, forcing SHELL to /bin/sh does not isolate this test: resolveLoginShellCommandPath invokes that shell with -ilc, so it loads the profile and discovers the real executable, and the resolver then selects the higher version, causing the codexPath assertions to fail. Give the probe a temporary HOME/PATH or mock login-shell discovery instead of only changing SHELL.

AGENTS.md reference: AGENTS.md:L13-L16

Useful? React with 👍 / 👎.

@astyfx
astyfx merged commit ac2ad6c into main Jul 27, 2026
4 checks passed
@astyfx
astyfx deleted the fix/nvm-shell-pr-checks branch July 27, 2026 08:47
@astyfx astyfx mentioned this pull request Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants