Skip to content

ci: lint and test on macOS - #2151

Merged
Benoît Cortier (CBenoit) merged 11 commits into
masterfrom
claude/macos-agent-ci-testing-86cd72
Oct 25, 2026
Merged

Benoît Cortier (CBenoit) merged 11 commits into
masterfrom
claude/macos-agent-ci-testing-86cd72

Conversation

@CBenoit

@CBenoit Benoît Cortier (CBenoit) commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

The Agent and the shared crates carry macOS-specific code, but CI compiled only jetsocat for macOS, so that code was never type-checked or tested there. Add a macos-15 (arm64) leg to the clippy and test jobs, let a failing leg finish without cancelling the other platforms, and bound the tests job at 60 minutes.

Running on macOS exposed these failures, now fixed:

  • The PEDM hook declared its Windows-only dependencies unconditionally, and retour does not compile on aarch64, so the workspace did not build on Apple Silicon.
  • The network scanner's async TCP connect treated only Linux's EINPROGRESS as a pending connection, so on macOS it never completed. It now fails with the error connect returns instead of waiting for the caller's timeout, and reads a failed connection's cause from the pending socket error, which kqueue needs and which lets callers tell a refused port from an unreachable host on every platform.
  • Gateway configuration failed to load on macOS without an explicit web app path, even with the web app disabled; it now defaults to webapp under the macOS configuration directory.
  • An agent stdin test outlasted the 5 s exit grace period on the macOS runner while draining its pipe.

The macOS Rust shim repair now runs before the Rust cache setup, including in the jetsocat job, because the cache action runs rustc first and a broken shim left those jobs uncached.

🤖 Generated with Claude Code

The Agent and shared crates carry macOS-specific code, but CI only
compiled jetsocat for macOS, so that code was never type-checked or
tested there. Add a macos-15 (arm64) leg to the clippy and test jobs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CBenoit
Benoît Cortier (CBenoit) marked this pull request as ready for review October 24, 2026 18:18
Copilot AI balanced review requested due to automatic review settings October 24, 2026 18:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Rust shim repairs should precede cache initialization in both macOS jobs so broken shims do not prevent dependency caching.

1 open issue
What changed in this PR

This PR extends CI coverage to macOS so platform-specific code in the Agent and shared crates is linted and tested, rather than only compiling jetsocat.

Changes:

  • Add a macos-15 leg to Clippy and workspace tests.
  • Configure tests for aarch64-apple-darwin while preserving Windows and Linux coverage.
  • Add macOS Rust shim repair steps before linting and testing.
File Description
.github/​workflows/​ci.yml Adds macOS lint/test matrix entries and Rust shim repairs.

Review effort: Balanced

Comment thread .github/workflows/ci.yml
The cache action runs rustc before restoring, so a broken shim left
the macOS legs uncached.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The cache action runs rustc before restoring, so a broken shim left
the jetsocat macOS builds uncached.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused workflow changes preserve existing platform coverage, address the prior cache-ordering feedback, and have no unresolved findings.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

The hook is Windows-only, but its dependencies were unconditional, and
retour fails to compile on aarch64, so the workspace did not build on
Apple Silicon. Also stop a failing lints or tests leg from cancelling
the other platforms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On macOS runners, forking sleep for every line left the lines still in
the pipe after the last write unread when the 5 s exit grace period
ran out. Reading slowly is only needed while the first frame backs up
the second, so read the rest at full speed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without DGATEWAY_WEBAPP_PATH or a static root path in the WebApp
section, the configuration failed to load on macOS even with the web
app disabled. Default to a webapp folder under the macOS configuration
directory, as Linux does under /usr/share.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The network scanner's connect future treated only Linux's EINPROGRESS
as a pending connection, so on macOS it returned Pending without
registering for readiness and never completed. Once it registers,
kqueue cannot report a failed connection, which the future expected,
so read the pending socket error instead.

Also bound the tests job, which ran into the 6 h job limit on this hang.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CBenoit
Benoît Cortier (CBenoit) requested a balanced review from Copilot October 25, 2026 02:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@CBenoit

Benoît Cortier (CBenoit) commented Oct 25, 2026 •

Copy link
Copy Markdown
Member Author

Lines changed at 9e6e1c544 (added/removed):

Area Files + −
Production code network-scanner-net/src/socket.rs, devolutions-gateway/src/config.rs, devolutions-pedm-hook/Cargo.toml 54 19
Tests network-scanner-net/src/tests.rs, devolutions-agent/src/psu_agent/process.rs (test module only) 43 6
Other (CI) .github/workflows/ci.yml 52 19

No UNTESTED: comments were added.

Note

LLM-assisted content (no human feedback).

A connect that failed on the spot stayed pending until the caller's
timeout, so an unreachable network was reported as a timeout, and a
failure seen through epoll or IOCP came back as a generic error, so a
closed port was not reported as refused. Fail with the error connect
returns, and with the pending socket error once readiness reports a
failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On Unix, EAGAIN from connect means no local resources, such as free
ephemeral ports, not a connection in progress, so it now fails the
connect instead of waiting for the caller's timeout. Also test that
an error connect returns right away fails the future.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CBenoit

Copy link
Copy Markdown
Member Author

Review record. Copilot could not review 16c296075 for lack of quota, so the rounds after it ran with fresh-context local reviewers.

  • Round 1 at 16c296075: fixed in 578bf5fc1 a connect that failed on the spot staying pending until the caller's timeout, failures seen through epoll or IOCP losing their cause, and two shim-repair steps missing their line continuation. Declined: sharing the shim-repair step in a composite action, because these jobs can check out an older commit that lacks it; the ci: squash title hiding the fix(dgw) changes from the changelog, since Gateway does not ship on macOS.
  • Round 2 at 578bf5fc1: fixed in 4743f654e the untested immediate-failure path, EAGAIN counting as a connection in progress on Unix, the closed-port oracle section, and comment layout. Declined: a re-poll on Windows can restart a refused connect, which delays the same refused result.
  • Round 3 at 4743f654e, final verdict: "I found no problems of low severity or above. The commit is correct and the new test should be deterministic on Linux, macOS and Windows." Its one nit, a missing Winsock oracle marker, is fixed in 9e6e1c544 (comment only, not re-reviewed).

Note

LLM-assisted content (no human feedback).

@CBenoit
Benoît Cortier (CBenoit) merged commit 6092cfa into master Oct 25, 2026
51 checks passed
@CBenoit
Benoît Cortier (CBenoit) deleted the claude/macos-agent-ci-testing-86cd72 branch October 25, 2026 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants