[WRONG BRANCH] fix(scripts): run ocx-run commands in the requested workdir - #287
[WRONG BRANCH] fix(scripts): run ocx-run commands in the requested workdir#287luvs01 wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe job launcher now changes to the requested ChangesJob execution
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The change makes commands run in the validated requested workdir while preserving existing process handling; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
✅ Deterministic PR hygiene checks passed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 298d625629
ℹ️ 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".
| # setsid gives the job its own process group so a timeout kills the children too; | ||
| # --kill-after upgrades to SIGKILL for a process that ignores SIGTERM. | ||
| setsid timeout --signal=TERM --kill-after=60s "$limit" "$@" > "$log" 2>&1 & | ||
| (cd "$workdir" && exec setsid timeout --signal=TERM --kill-after=60s "$limit" "$@") > "$log" 2>&1 & |
There was a problem hiding this comment.
When scripts/ocx-run receives a valid relative workdir named -, the preceding -d check succeeds, but Bash interprets cd "-" as a request to switch to $OLDPWD; the wrapped command therefore succeeds in the wrong directory. Other hyphen-leading directory names can instead be parsed as invalid options. Use cd -- "$workdir" (or canonicalize the path) so every validated workdir is treated as a directory operand.
Useful? React with 👍 / 👎.
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
1 participant
Motivation
ocx-runso the validatedworkdiris actually used when launching the job, because previously the wrapped command inherited the caller's current directory and could fail for project-relative commands likebun run test.Description
scripts/ocx-runto change into the validatedworkdirbefore executing the job by replacing the directsetsid timeout ... "$@"invocation with(cd "$workdir" && exec setsid timeout --signal=TERM --kill-after=60s "$limit" "$@"), while preserving backgrounding, process-group timeout behavior, logging, PID tracking, and status writes.Testing
workdir, and ranbun run typecheck,bun run privacy:scan, andgit diff --check, all of which completed successfully.Codex Task
Summary by CodeRabbit