Repository navigation
test(mcpx): darwin test-compile portability for internal/daemon (#237) - #3288
Conversation
unixPair: drop the Linux-only syscall.SOCK_CLOEXEC from shared test code; create the socketpair plainly and set close-on-exec via syscall.CloseOnExec (portable across linux/darwin). exeBytes: split into per-OS helpers — linux keeps the /proc/<pid>/exe inode read; darwin resolves the child exec path via kern.procargs2 (x/sys/unix SysctlRaw) and reads that file. The shared exec_watch_test.go no longer assumes /proc. darwin-compile-check.sh: DARWIN_COMPILE_EXCEPTIONS is now empty (the internal/daemon entry was the last one). Gate verified on the laptop: darwin/arm64 test-compile scan passes with 0 exceptions skipped; internal/daemon compiles for darwin/arm64; targeted linux tests pass.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 29 minutes. View limit details
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. |
![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: 23a73750bb
ℹ️ 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".
|
🤖 Agent note · parked — queue task 4805 parked on this PR's queued CI (fleet runner backlog). Verified on the laptop before opening: darwin/arm64 test-compile of internal/daemon OK, full darwin-compile-check.sh passed with 0 exceptions, targeted linux tests pass. Resume: checks green → Codex disposition → merge → dezren39/nix downstream PR (pin bump + postPatch removal + one mcpxDarwinSkip entry drop) per the park note on #237. |
…ex P2 on #3288) kern.procargs2 is handler-backed: name2oid rejects the PID formatted into the OID string with ENOENT, so the darwin exec-change test would fail at runtime on macOS. SysctlRaw takes the PID as a numeric MIB arg.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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 review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…-compile # Conflicts: # pkgs/mcpx/scripts/darwin-compile-check.sh
What
Darwin test-compile portability for
pkgs/mcpx/internal/daemon— the compile half of #237 (queue task 4805; triage verdict by task 2054 on #237).unixPair(takeover_test.go): drop the Linux-onlysyscall.SOCK_CLOEXECfrom shared test code. The socketpair is created plainly and close-on-exec is set viasyscall.CloseOnExec, which exists on both linux and darwin.exeBytes(exec-change test): split into per-OS helpers. Linux keeps the/proc/<pid>/exeinode read (exe_bytes_linux_test.go); darwin, which has no /proc, resolves the child's exec path viakern.procargs2(golang.org/x/sys/unix.SysctlRaw) and reads that file (exe_bytes_darwin_test.go). The shared test no longer assumes /proc or skips off-linux.scripts/darwin-compile-check.sh:DARWIN_COMPILE_EXCEPTIONSis now empty — the internal/daemon entry was the last one.Verification (laptop, this branch)
GOOS=darwin GOARCH=arm64 CGO_ENABLED=0 go test -c ./internal/daemon/— compiles (previously:undefined: syscall.SOCK_CLOEXEC).bash scripts/darwin-compile-check.sh— passed, exceptions skipped: 0 (full darwin test-compile scan, 36s).go vet ./internal/daemon/(linux + darwin) — clean;gofmtclean.TestExecChangeRestartsThatPoolAndNoOther,TestExecWatchOffWhenTheIntervalIsZero,TestAFrameCarriesItsDescriptorWithItsBody.Full linux suites for internal/daemon, internal/pool, internal/e2e, internal/mcpclient ride CI on this PR.
Set
Downstream member: dezren39/nix
mcpxDarwinSkipcompile-driven entries — separate PR in that repo referencing #237 (same set, per the cross-repo rule; no new per-repo issue).Refs #237 (does not close it: the darwin runtime takeover decision is the companion packet, preserved at hidden_files/packet-237-runtime.json).
Need help on this PR? Tag
@codesmith-botwith what you need.