Skip to content

test(link): isolate the remote-prelude tests from a host-installed ocx - #6787

Merged
lidge-jun merged 2 commits into
devfrom
codex/l7-link-test-isolation
Oct 9, 2026
Merged

lidge-jun merged 2 commits into
devfrom
codex/l7-link-test-isolation

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

Carries #6726 by @sungyongcho so it runs under repository CI (fork CI needs maintainer approval).

Two tests in tests/clients/link-ssh-argv.test.ts fail on any host that already has ocx in /usr/bin, such as a machine with the OpenCodex desktop .deb. Both simulate the remote PATH as /usr/bin:/bin and expect the stub in ~/.bun/bin to run. The remote prelude appends ~/.bun/bin after the remote PATH on purpose, so a host ocx wins and the stub never runs.

The two tests now use a remote PATH that holds only an sh symlink to /bin/sh. The first test's expected PATH line follows the new base. Production code and the other tests are unchanged; the ordering contract stays enforced by the literal prelude assertion and the "an ocx the remote PATH already resolves wins" test.

Closes #6726 once landed (the source PR is closed manually after this merges).

Verification

  • bun test tests/clients/link-ssh-argv.test.ts on macOS with broken ocx shims on PATH: 18 pass, 1 skip (the Windows-only PowerShell case), 0 fail.
  • Regression guard: changing REMOTE_OCX_SCRIPT in src/link/ssh-argv.ts to prepend the fallback directories makes 3 tests fail (prelude literal, append ordering, remote-PATH ocx wins); restored afterwards.
  • Independent gpt-6.1-sol review of c15037b324...1878576851: PASS.
  • Full suite not run locally (test-only change to one file); hosted Cross-platform CI on this head is the remaining evidence.

Checklist

  • Scope stays focused and avoids unrelated cleanup. (One test file.)
  • Docs or release notes were updated when needed. (None needed for a test-only change.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (No runtime, credential or workflow changes.)

Co-authored-by: Sungyong Cho dev@sungyongcho.com

Summary by CodeRabbit

  • Tests
    • Improved the reliability of checks for remote command paths and argument handling by isolating their test environment. No user-facing behavior changes are included.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 9, 2026 01:37
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T01:39:15.032954Z 1878576 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Oct 9, 2026
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 311e02a1-9358-4ca3-a7bd-1216af3c893e
📥 Commits

Reviewing files that changed from the base of the PR and between c15037b and 1878576.

📒 Files selected for processing (1)
  • tests/clients/link-ssh-argv.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

Two SSH argument tests now use a temporary remote PATH containing only a sh symlink. The prelude test checks the updated PATH and retained arguments. Production code and other tests are unchanged.

Changes

Remote PATH test isolation

Layer / File(s) Summary
Isolated remote PATH for SSH tests
tests/clients/link-ssh-argv.test.ts
A helper creates a temporary directory containing a sh symlink. The remote-prelude test uses this PATH and checks the resulting PATH and retained arguments. The POSIX argument-bytes test uses the same isolated PATH.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 18785

The change isolates the SSH argument tests from host-installed ocx binaries without removing their shell or PATH-ordering checks. No actionable merge risk remains; normal CI should still run.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: isolating the remote-prelude link tests from host-installed ocx binaries. It matches the scope in tests/clients/link-ssh-argv.test.ts.
Linked Issues check Passed The change meets #6726. In tests/clients/link-ssh-argv.test.ts, shOnlyRemotePath() creates a temporary directory with only an sh symlink to /bin/sh. The two affected tests use this path instea…
Out of Scope Changes check Passed The changes stay within #6726. The diff is limited to tests/clients/link-ssh-argv.test.ts. It adds the required symlinkSync import, documents and implements the isolated remote PATH helper, and up…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

@lidge-jun
lidge-jun merged commit 2e0d607 into dev Oct 9, 2026
34 checks passed
@lidge-jun
lidge-jun deleted the codex/l7-link-test-isolation branch October 9, 2026 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants