Skip to content

fix(codex): pin Darwin process utility path - #1362

Merged
Wibias merged 1 commit into
lidge-jun:devfrom
luvs01:agent/fix-darwin-app-server-ps-path
Aug 10, 2026
Merged

fix(codex): pin Darwin process utility path#1362
Wibias merged 1 commit into
lidge-jun:devfrom
luvs01:agent/fix-darwin-app-server-ps-path

Conversation

@luvs01

@luvs01 luvs01 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Pin every macOS Codex app-server process discovery and start-time probe to the system /bin/ps binary instead of resolving ps through PATH.
  • Preserve the existing arguments, timeouts, parsing, and fail-closed behavior.
  • Add a source guard that requires exactly four pinned Darwin call sites and rejects any bare execFileSync("ps", ...) invocation.

This prevents a modified local PATH from substituting another executable during Codex app-server synchronization and staleness checks.

Verification

  • bun test tests/codex-app-server-processes.test.ts — 31 passed, 0 failed on Bun 1.3.14.
  • bun run typecheck
  • bun run privacy:scan
  • git diff --check
  • Independent final diff review found no actionable P0–P3 findings.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were not needed for this internal process-launch hardening.
  • Security-sensitive changes require explicit maintainer review before merge.

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.

Summary by CodeRabbit

  • Bug Fixes

    • Improved process detection on Darwin systems by using a reliable system path when invoking process utilities.
  • Tests

    • Added regression coverage to ensure process checks consistently use the correct system command path.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b9cc7adf-75e7-4521-b411-c97768c2a407

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Darwin process enumeration and process start-time queries now invoke /bin/ps directly. A source-level regression test verifies four absolute-path invocations and rejects bare ps calls.

Changes

Darwin process command resolution

Layer / File(s) Summary
Absolute ps invocations and regression coverage
src/codex/app-server-processes.ts, tests/codex-app-server-processes.test.ts
Darwin process snapshot and start-time queries use /bin/ps. The test requires exactly four absolute-path calls and rejects PATH-resolved ps invocations.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • lidge-jun/opencodex#576: Directly modifies the same Darwin /bin/ps invocations in src/codex/app-server-processes.ts.

Suggested reviewers: lidge-jun

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes pinning the Darwin process utility path, which is the main change in the 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 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 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. If no CodeRabbit review appears, comment @coderabbitai review to request one.
Maintainers: @lidge-jun @Ingwannu @Wibias

Wibias commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Please rebase this PR onto the latest dev (e8ce2b93dd3ef8074d96dee407704c5dc06e2699) before merge, then rerun the exact-head CI checks.

@luvs01
luvs01 force-pushed the agent/fix-darwin-app-server-ps-path branch from 880b372 to e0f1388 Compare August 9, 2026 23:25
@Wibias
Wibias marked this pull request as ready for review August 9, 2026 23:45
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

@github-actions[bot] The PR gate reports that the PR is ready. I will review the current PR head.

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="lidge-jun/opencodex"
pr="1362"
required_dev_sha="e8ce2b93dd3ef8074d96dee407704c5dc06e2699"

echo "== Pull request metadata =="
gh pr view "$pr" --repo "$repo" \
  --json number,state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,mergeStateStatus,statusCheckRollup \
  --jq '{
    number,
    state,
    isDraft,
    headRefName,
    headRefOid,
    baseRefName,
    baseRefOid,
    mergeStateStatus,
    checks: [.statusCheckRollup[] | {name, status, conclusion, detailsUrl}]
  }'

echo
echo "== Exact-head check runs =="
head_sha="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq .headRefOid)"
gh api "repos/${repo}/commits/${head_sha}/check-runs?per_page=100" \
  --jq '.check_runs[] | {name, status, conclusion, details_url}'

echo
echo "== Required dev commit ancestry =="
git cat-file -e "${required_dev_sha}^{commit}"
git cat-file -e "${head_sha}^{commit}"
if git merge-base --is-ancestor "$required_dev_sha" "$head_sha"; then
  printf 'PASS: %s is an ancestor of %s\n' "$required_dev_sha" "$head_sha"
else
  printf 'FAIL: %s is not an ancestor of %s\n' "$required_dev_sha" "$head_sha"
  exit 1
fi

Length of output: 12945


@github-actions[bot] Verified PR #1362.

  • Head: e0f1388ad6daee902e57019db0454cd9192a73f1
  • Required dev commit: e8ce2b93dd3ef8074d96dee407704c5dc06e2699
  • The required dev commit is an ancestor of the PR head.
  • The PR merge state is CLEAN.
  • The exact-head ci check completed successfully.
  • The exact-head test, macOS, hygiene, keyring, npm-global, storage-policy, and gates checks completed successfully.
  • Cancelled duplicate label and enforce-target runs have successful replacement runs.

I will review the current PR head.

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Wibias
Wibias merged commit 98b29ce into lidge-jun:dev Aug 10, 2026
46 of 52 checks passed

Wibias commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Thanks @luvs01 — nice focused hardening here. Pinning the Darwin process probes to /bin/ps and locking it in with a source guard is exactly the kind of defensive cleanup we want. Merged! 🙏

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