Skip to content

Fix Java builds with scoped npm registries on macOS - #2338

Merged
edburns merged 3 commits into
mainfrom
edburns/java-macos-test-changes-from-linux-x64-work
Aug 14, 2026
Merged

Fix Java builds with scoped npm registries on macOS#2338
edburns merged 3 commits into
mainfrom
edburns/java-macos-test-changes-from-linux-x64-work

Conversation

@edburns

@edburns edburns commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 14, 2026 22:15
@edburns
edburns requested a review from a team as a code owner August 14, 2026 22:15
Comment thread java/sdk/src/test/java/com/github/copilot/CliServerManagerTest.java Fixed
Comment thread java/sdk/src/test/java/com/github/copilot/JsonRpcClientTest.java Fixed
@github-actions

This comment has been minimized.

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.

Pull request overview

Updates Java test builds for npm registry portability and macOS process execution.

Changes:

  • Adds npm registry-related flags to Java build installs.
  • Resolves cat through PATH in Java tests.
Show a summary per file
File Description
java/sdk/pom.xml Adjusts npm install arguments.
JsonRpcClientTest.java Uses PATH-resolved cat.
CliServerManagerTest.java Uses PATH-resolved cat.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (1)

java/sdk/pom.xml:229

  • This flag cannot change which URLs this npm ci reads: npm ci treats the existing lockfile as frozen, while omit-lockfile-registry-resolved only controls writing new lockfiles. nodejs/package-lock.json still contains explicit public-registry URLs for the scoped CLI packages (for example, line 704), so a scoped @github registry remains bypassed here. Regenerate and commit the lockfile with registry resolved entries omitted, and persist that generation setting.
                                <argument>--omit-lockfile-registry-resolved=true</argument>
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread java/sdk/pom.xml
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR makes two Java-only changes:

  1. java/sdk/pom.xml — adds --omit-lockfile-registry-resolved=true to npm ci invocations to fix builds with scoped npm registries on macOS. This is a Java build-tooling concern (Maven invoking Node.js to set up the E2E test harness) with no equivalent in other SDKs.

  2. Java test refactoring — replaces the startBlockingProcess() helper (which spawned real OS processes like cat/more) with a new TestProcess mock class. This is internal Java test infrastructure with no public API surface.

Neither change affects the public SDK API or introduces feature asymmetry across languages. No cross-SDK consistency concerns.

Generated by SDK Consistency Review Agent for #2338 · sonnet46 16.5 AIC · ⌖ 5.36 AIC · ⊞ 6.6K ·

@edburns
edburns merged commit 18002a8 into main Aug 14, 2026
36 checks passed
@edburns
edburns deleted the edburns/java-macos-test-changes-from-linux-x64-work branch August 14, 2026 23:09
@edburns
edburns restored the edburns/java-macos-test-changes-from-linux-x64-work branch August 14, 2026 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants