Skip to content

fix(scripts): run ocx-run commands in the requested workdir - #2474

Open
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/ocx-run-workdir
Open

fix(scripts): run ocx-run commands in the requested workdir#2474
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/ocx-run-workdir

Conversation

@luvs01

@luvs01 luvs01 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Run bounded ocx-run jobs from the requested <workdir> instead of inheriting the SSH caller's current directory.
  • Preserve the existing timeout, process-group, and argv behavior by changing directory inside the background subshell before exec setsid.
  • Ignore caller-controlled CDPATH during that directory change so relative workdirs cannot resolve to a different matching directory.
  • Add a Linux regression that uses a relative workdir containing spaces and a conflicting CDPATH entry.

Verification

Focused checks were run on exact head e6cd33cf09aef3fd2230b9b55895a141ee792c98 with Bun 1.4.0-canary.1 (9fcdea80b):

  • bun test tests/ocx-run.test.ts — 0 failed; the one GNU/Linux-only execution case was skipped on the Windows test host and will execute in Linux CI
  • bun run typecheck — passed
  • bun run privacy:scan — passed
  • git diff --check and staged whitespace validation — passed
  • two independent focused reviews — no remaining actionable findings

The repository-wide suite was not duplicated locally; this change is isolated to the Linux-only maintenance runner, and the focused regression plus relevant static gates are green.

Exact base: c44e43f00f1b8001f30292067324fb419e5ffc86
Exact head: e6cd33cf09aef3fd2230b9b55895a141ee792c98

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. Existing scripts/OCX-RUN.md already describes the requested workdir as the execution directory, so no wording change is needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. The command remains an argv array, no request or credential data is added, and caller-controlled CDPATH is neutralized.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes

    • Commands launched with a specified working directory now execute from that directory.
    • Working directories containing spaces are handled correctly.
    • Configured CDPATH values no longer interfere with command execution.
  • Tests

    • Added coverage verifying working-directory behavior and logged execution paths.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5a0d5761-718b-4c28-9ec4-fa6ff9225fcf

📥 Commits

Reviewing files that changed from the base of the PR and between c44e43f and e6cd33c.

📒 Files selected for processing (2)
  • scripts/ocx-run
  • tests/ocx-run.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

scripts/ocx-run now applies the workdir argument before launching the timed job. A Linux integration test verifies paths with spaces, CDPATH isolation, successful execution, logged working-directory output, and cleanup.

Changes

Workdir execution

Layer / File(s) Summary
Workdir execution and integration validation
scripts/ocx-run, tests/ocx-run.test.ts
At scripts/ocx-run:128, the job runs in a subshell that clears CDPATH, changes to $workdir, and executes the existing setsid timeout command. The Linux integration test uses a workdir containing spaces and verifies the exit code and logged working-directory path. Temporary files are removed during cleanup.

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

Merge Risk: ⚪ Minimal · up to e6cd3

The PR changes the runner to execute commands from the requested workdir and adds regression coverage for relative paths with spaces and CDPATH conflicts. Focused validation passes, and no actionable merge-blocking risk remains.

Suggested reviewers: ingwannu, lidge-jun

🚥 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: running ocx-run commands in the requested workdir.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions github-actions Bot added bug Something isn't working review-ready labels Aug 24, 2026
@github-actions
github-actions Bot marked this pull request as ready for review August 24, 2026 07:14
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 68 / 80

설명: 이 풀 리퀘스트는 ocx-run 이 받은 작업 폴더에서 명령을 돌리게 고친다. 작성자는 luvs01 이다. 오늘 열여섯 시 십사 분에 열렸다. 포크 저장소에서 왔다. 초안이 아니다. 점검 네 칸이 다 채워졌다. 위생은 통과다. 베이스는 지금 HEAD c44e43f00 다. 파일 둘이다. scripts/ocx-run 한 줄, tests/ocx-run.test.ts 마흔네 줄이 새로 들어온다. 더하기 45, 빼기 1 이다. 연결 이슈는 없다. 지금 CURRENT dev HEAD 는 c44e43f00 이다. 이번 시간에 origin/dev 는 그대로다. 새 머지는 없다. package.json 은 2.27.0 이다. src/config.ts 는 3238줄이다. src/runtime 폴더는 지금 HEAD 에 없다. combo-stream-preflight.ts 는 171줄이고 HEAD 에 있다. gui/src/combo-workspace-data.ts 는 589줄이다. src/providers/default-aliases.ts 와 model-presets.ts 는 아직 없다. src/codex/history-provider.ts 는 1557줄이다. src/server/responses/fetch-helpers.ts 는 137줄이다. src/codex/history-manifest.ts 는 112줄이다.

지금 HEAD 의 scripts/ocx-run 을 열었다. 16줄 쓰임새는 이름, 작업 폴더, 시간 한도, 명령이다. 23줄 보기에도 작업 폴더가 실행 자리로 적혀 있다. scripts/OCX-RUN.md 도 같은 뜻이다. 104줄에서 workdir 를 받는다. 105줄에서 그 폴더가 있는지만 본다. 128줄은 setsid timeout 으로 명령을 띄운다. 그 줄에 cd 가 없다. 그래서 명령은 호출한 쪽의 현재 폴더에서 돈다. 비대화 ssh 는 대개 집 폴더다. bun run test 가 저장소가 아니라 집에서 패키지를 찾는다. 작업 폴더 칸은 죽은 칸이다.

이 PR 은 128줄을 괄호로 감싼다. 먼저 CDPATH 를 비운다. 그 다음 cd -- 로 받은 작업 폴더로 들어간다. 그 다음 exec setsid timeout 이다. 부모 스크립트는 자리를 안 옮긴다. 로그와 상태와 잠금 파일은 절대 경로라서 그대로다. exec 가 중요하다. 빼면 백그라운드 아이디가 껍데기 번호가 된다. 88줄 cmd_stop 의 kill -TERM -$pid 가 타임아웃 무리 대신 껍데기 무리를 친다. 작업 제어가 꺼진 스크립트에서는 그 무리가 부모와 같다. 부모까지 죽을 수 있다. exec 는 반드시 남겨야 한다. CDPATH 를 비우는 것도 맞다. 상대 작업 폴더가 다른 맞는 폴더로 새면 안 된다. cd -- 는 이름 앞에 빼기가 있어도 버틴다.

시험은 리눅스에서만 돈다. 상대 경로에 빈칸이 있고, 같은 이름의 CDPATH 가짜 폴더가 있다. 명령은 pwd -P 다. 로그가 진짜 작업 폴더와 같아야 한다. 구멍의 중심을 잠근다. 다만 절대 경로, 빼기 이름, 시간 초과, 멈춤은 안 잠근다. exec 가 있으면 멈춤은 이론상 예전과 같다. 시험이 없을 뿐이다. 작성자 로컬은 윈도라서 이 시험이 건너뛰어졌다. 깃허브 점검 목록에는 위생, 라벨, 타깃, 토끼만 있다. Cross-platform CI 는 안 돌아갔다. 포크 첫 기여라서 승인 전에는 본 시험이 안 뜬다. 리눅스 CI 가 이 새 시험을 실제로 돌리기 전에는 머지하지 말 것.

운영 스크립트 구멍이라서 제품 사용자 길이보다는 낮다. 그래도 문서와 칸이 거짓말하는 상태라서 68. 점검 네 칸은 채워졌다. 베이스는 지금 HEAD 와 같다. 범위는 이 한 줄과 시험이다. types.ts/config.ts 가르기와 상관없다. 닫고 다시 밑지 말 것. 프리뷰 배포가 아니다. 2426 은 2473 이 착지하기 전에는 닫지 않는다. 2473 은 아직 초안이다. 2472 는 연다. 2463 과 2464 과 2465 는 구현 파일이 없어서 닫지 않는다. 2468 은 메인이라서 메인에 넣지 말 것. 2451 과 2467 과 2471 은 다시 열지 말 것. 카탈로그 팁은 Ox Alpha x-preview-f-free + deepseek-v4-flash-vision-exp. Cursor 정적 카탈로그는 opus-4-8-fast / opus-5-fast. 2334 CursorCredentialRouter 는 여전히 src/providers/cursor-pool.ts 모듈+테스트만 있고 어댑터에 연결되지 않았다. 2332 H2 는 discovery 전용. 2320 overflow + 2342 는 이미 dev. 2188 사이드카는 이미 dev. 2361 __omit__ 는 이미 HEAD 다. src/lib/bounded-body.ts 128줄은 이미 죽은 신호에서 취소 없이 던진다. 2453 이 착지해서 wait 허용 목록은 이제 밑줄 yield_time_ms 와 max_tokens 이다. types.ts/config.ts 가르기는 이 구멍을 바꾸지 않는다. 닫고 다시 밑지 말 것.

scripts/ocx-run 라인 104 - workdir 를 받는다. 이 칸이 실행 자리여야 한다
scripts/ocx-run 라인 105 - 폴더가 있는지만 본다. 들어간 적은 없다
scripts/ocx-run 라인 128 - HEAD 는 여기서 명령을 띄운다. cd 가 없다. 이 PR 이 CDPATH 비우기, cd --, exec setsid 로 고친다
scripts/ocx-run 라인 88 - cmd_stop 은 마이너스 아이디로 무리를 죽인다. exec 를 빼면 이 줄이 깨진다
tests/ocx-run.test.ts - HEAD 에 없다. 이 PR 이 넣는다. 리눅스만. 빈칸 상대 경로와 CDPATH 가짜 폴더를 잠근다
scripts/OCX-RUN.md - 이미 작업 폴더를 실행 자리로 적었다. 글 고칠 필요는 없다
Cross-platform CI - 이 커밋에 점검이 없다. 포크라서 본 시험이 아직 안 돌았다. 지금 머지하지 말 것

메인테이너의 판단이 필요한 지점

  • exec 를 남길지. 남겨야 한다. 빼면 멈춤이 부모를 죽일 수 있다
  • 포크 CI 를 승인할지. 승인해야 한다. 리눅스 시험이 이 새 파일을 돌려야 한다
  • 멈춤과 시간 초과 시험을 이 기차에 넣을지. 넣지 않아도 된다. 다음 기차여도 된다
  • 절대 경로와 빼기 이름 시험을 넣을지. 넣지 않아도 된다. cd -- 와 따옴표가 이미 있다
  • types.ts/config.ts 가르기로 이 PR 을 닫을지. 해당 없다. 스크립트 한 줄이다
  • 2426 을 이 PR 로 닫을지. 닫지 말 것. 웹소켓 구멍이 아니다

너의 추천
리눅스 CI 가 tests/ocx-run.test.ts 를 실제로 돌리고 초록이면 머지한다. 지금은 포크라서 그 점검이 없다. 지금 머지하지 말 것. exec 를 빼지 말 것. 멈춤 시험은 다음이어도 된다. 라벨은 그대로 둔다. 2426 과 2472 와 2463 과 2464 과 2465 는 닫지 않는다. 2473 은 초안이니 머지하지 말 것. 2468 은 메인에 넣지 말 것. 호출 길을 넓히지 말 것. 프리뷰 배포가 아니다.

이 댓글은 grok-bot이 작성했습니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants