From 477398ee61e47bf47d2b1e5f01959abba8951695 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Wed, 29 Apr 2026 16:34:19 -0400 Subject: [PATCH 1/4] Pin Copilot CLI version across all CI paths via pom.xml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, three different code paths determined the Copilot CLI version inconsistently: - Path A (build-test.yml) installed the CLI from the cloned reference impl pinned by .lastmerge — reproducible. - Path B (run-smoke-test.yml, update-copilot-dependency.yml via the setup-copilot action) ran `npm install -g @github/copilot` with no version pin — floated to whatever was latest on npm. - Path C (local dev) fell through to whatever `copilot` was on PATH (often whatever VS Code Insiders had installed). When the CLI protocol vocabulary changed between 1.0.35 and 1.0.39, this inconsistency made it hard to land reference-impl merges that required a specific CLI version. This change makes pom.xml the single source of truth for the @github/copilot version that all paths must use, while keeping .lastmerge as the source of truth for the reference implementation commit. Changes: - pom.xml: replace the PRIMER_TO_REPLACE placeholder in the property with the current value (^1.0.36-0) extracted from the cloned reference impl's nodejs/package.json. Add a doc-comment explaining that the property is auto-managed and is the canonical CLI version for all CI paths. - .github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh (new): reads dependencies."@github/copilot" from the reference impl's nodejs/package.json (via node) and rewrites the pom.xml property in place. Locates the repo root by walking up from the script's own location until it finds a pom.xml, so it is resilient to relocation and works from any CWD. Uses portable sed for the rewrite. - .github/scripts/reference-impl-sync/merge-reference-impl-finish.sh: call the new sync script right after writing .lastmerge, and stage pom.xml alongside .lastmerge in the same commit. .lastmerge and the CLI version property now always move together. - .github/actions/setup-copilot/action.yml: read the pinned version from pom.xml using sed and run `npm install -g "@github/copilot@VERSION"` instead of installing the latest. Fail fast if the property is unset or still the primer. Expose the resolved version as an action output. - CONTRIBUTING.md: document the local-dev workflow that mirrors what build-test.yml does in CI. The cloned reference impl contains two separate package.json files that both pin @github/copilot: * target/copilot-sdk/nodejs/package.json (^1.0.36-0) — the SDK-test pin; this is the CLI the tests must run against. * target/copilot-sdk/test/harness/package.json (^1.0.32) — the replay-harness pin; incidental, NOT the CLI under test. `mvn generate-test-resources` runs `npm install` only in the harness subtree, so a generic `find target -path '*/@github/copilot/index.js'` picks the wrong (older) copy. The instructions now: 1. Run `npm ci` in target/copilot-sdk/nodejs/ explicitly. 2. Scope the COPILOT_CLI_PATH lookup to '*/nodejs/node_modules/...' so the harness copy can never be selected, even if both are present. Both POSIX (find) and PowerShell (Get-ChildItem) forms are tightened. - .gitignore: minor housekeeping. Verified mvn clean package -Pskip-test-harness -DskipTests succeeds with the new POM, the sync script correctly rewrites the property when invoked from any CWD, and the tightened CONTRIBUTING.md sequence resolves COPILOT_CLI_PATH to the 1.0.36-0 CLI under target/copilot-sdk/nodejs. --- .github/actions/setup-copilot/action.yml | 25 +++++- .../merge-reference-impl-finish.sh | 14 +++- .../sync-cli-version-from-reference-impl.sh | 80 +++++++++++++++++++ .gitignore | 1 + CONTRIBUTING.md | 48 ++++++++++- pom.xml | 21 +++++ 6 files changed, 183 insertions(+), 6 deletions(-) create mode 100755 .github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh diff --git a/.github/actions/setup-copilot/action.yml b/.github/actions/setup-copilot/action.yml index dcc99b8a7..ef0d31ab5 100644 --- a/.github/actions/setup-copilot/action.yml +++ b/.github/actions/setup-copilot/action.yml @@ -4,15 +4,36 @@ outputs: cli-path: description: "Path to the Copilot CLI executable" value: ${{ steps.cli-path.outputs.path }} + cli-version: + description: "Pinned @github/copilot version installed (read from pom.xml)" + value: ${{ steps.cli-version.outputs.version }} runs: using: "composite" steps: - uses: actions/setup-node@v6 with: node-version: 22 - - name: Install Copilot CLI - run: npm install -g @github/copilot + - name: Read pinned @github/copilot version from pom.xml + id: cli-version shell: bash + # The version is the SINGLE SOURCE OF TRUTH for the Copilot CLI version + # used across all CI paths. It is kept in sync with the reference + # implementation pinned in .lastmerge by + # .github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh. + run: | + PROP="readonly-copilot-sdk-ref-impl-version-from-lastmerge-file-updated-by-weekly-reference-impl-sync" + VERSION=$(sed -n "s|.*<${PROP}>\(.*\).*|\1|p" pom.xml | head -n 1 | tr -d '[:space:]') + if [[ -z "$VERSION" || "$VERSION" == "PRIMER_TO_REPLACE" ]]; then + echo "::error::Could not read pinned @github/copilot version from pom.xml property <${PROP}>" >&2 + exit 1 + fi + echo "Pinned @github/copilot version: $VERSION" + echo "version=$VERSION" >> "$GITHUB_OUTPUT" + - name: Install Copilot CLI (pinned to pom.xml version) + shell: bash + env: + CLI_VERSION: ${{ steps.cli-version.outputs.version }} + run: npm install -g "@github/copilot@${CLI_VERSION}" - name: Set CLI path id: cli-path run: echo "path=$(which copilot)" >> $GITHUB_OUTPUT diff --git a/.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh b/.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh index 480d43bc0..4b01465cc 100755 --- a/.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh +++ b/.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh @@ -5,8 +5,10 @@ # Finalises a reference implementation merge: # 1. Runs format + test + build (via format-and-test.sh) # 2. Updates .lastmerge to reference implementation HEAD -# 3. Commits the .lastmerge update -# 4. Pushes the branch to origin +# 3. Syncs the @github/copilot version property in pom.xml from the +# cloned reference implementation's nodejs/package.json +# 4. Commits the .lastmerge + pom.xml updates +# 5. Pushes the branch to origin # # Usage: ./.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh # ./.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh --skip-tests @@ -48,7 +50,13 @@ echo "▸ Updating .lastmerge…" NEW_COMMIT=$(cd "$REFERENCE_IMPL_DIR" && git rev-parse origin/main) echo "$NEW_COMMIT" > "$ROOT_DIR/.lastmerge" -git add .lastmerge +# ── 2b. Sync pom.xml @github/copilot version ───────────────── +# Keeps the canonical CLI version in pom.xml aligned with what the +# reference implementation pinned in .lastmerge depends on. +echo "▸ Syncing @github/copilot version in pom.xml from reference implementation…" +"$ROOT_DIR/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh" "$REFERENCE_IMPL_DIR" + +git add .lastmerge pom.xml git commit -m "Update .lastmerge to $NEW_COMMIT" # ── 3. Push branch ─────────────────────────────────────────── diff --git a/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh b/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh new file mode 100755 index 000000000..4aa2e1256 --- /dev/null +++ b/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh @@ -0,0 +1,80 @@ +#!/usr/bin/env bash +# ────────────────────────────────────────────────────────────── +# sync-cli-version-from-reference-impl.sh +# +# Reads the @github/copilot version specifier from the cloned +# reference implementation's nodejs/package.json, and updates the +# corresponding property in pom.xml: +# +# +# +# This keeps the canonical Copilot CLI version (declared in pom.xml) +# in sync with whatever the reference implementation pinned in +# .lastmerge depends on. All workflows that install the Copilot CLI +# (build-test.yml — implicitly via cloned SDK, run-smoke-test.yml and +# update-copilot-dependency.yml — via the setup-copilot action) read +# this single property so every CI path uses the same CLI version. +# +# Usage: +# ./sync-cli-version-from-reference-impl.sh +# +# Or, when invoked from merge-reference-impl-finish.sh, sources +# REFERENCE_IMPL_DIR from the .merge-env file. +# ────────────────────────────────────────────────────────────── +set -euo pipefail + +# Locate the repo root by walking up from this script until we find a pom.xml. +# This is resilient to the script being moved to a different depth under +# .github/scripts/ in the future. +find_repo_root() { + local dir + dir="$(cd "$(dirname "$0")" && pwd)" + while [[ "$dir" != "/" ]]; do + if [[ -f "$dir/pom.xml" ]]; then + echo "$dir" + return 0 + fi + dir="$(dirname "$dir")" + done + echo "❌ Could not locate repo root (no pom.xml found above $(dirname "$0"))" >&2 + return 1 +} +ROOT_DIR="$(find_repo_root)" + +REFERENCE_IMPL_DIR="${1:-${REFERENCE_IMPL_DIR:-}}" +if [[ -z "$REFERENCE_IMPL_DIR" ]]; then + echo "❌ Usage: $0 " >&2 + echo " or set REFERENCE_IMPL_DIR in the environment." >&2 + exit 1 +fi + +PKG_JSON="$REFERENCE_IMPL_DIR/nodejs/package.json" +if [[ ! -f "$PKG_JSON" ]]; then + echo "❌ Cannot find $PKG_JSON" >&2 + exit 1 +fi + +# node is always available since the reference implementation uses npm. +CLI_VERSION=$(node -e \ + "const fs=require('fs');const p=JSON.parse(fs.readFileSync(process.argv[1],'utf8'));const v=(p.dependencies&&p.dependencies['@github/copilot'])||(p.devDependencies&&p.devDependencies['@github/copilot']);if(!v){process.exit(2);}process.stdout.write(v);" \ + "$PKG_JSON") + +if [[ -z "$CLI_VERSION" ]]; then + echo "❌ Could not extract @github/copilot version from $PKG_JSON" >&2 + exit 1 +fi + +POM="$ROOT_DIR/pom.xml" +PROP="readonly-copilot-sdk-ref-impl-version-from-lastmerge-file-updated-by-weekly-reference-impl-sync" + +if ! grep -q "<${PROP}>" "$POM"; then + echo "❌ Property <${PROP}> not found in $POM" >&2 + exit 1 +fi + +# Use a portable sed invocation (works on both BSD/macOS and GNU/Linux). +TMP="$(mktemp)" +sed -E "s|<${PROP}>[^<]*|<${PROP}>${CLI_VERSION}|" "$POM" > "$TMP" +mv "$TMP" "$POM" + +echo "▸ Updated pom.xml: <${PROP}> = ${CLI_VERSION}" diff --git a/.gitignore b/.gitignore index 35ea546f0..654d3278f 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ changebundle.txt* .settings scripts/codegen/node_modules/ *~ +*.sln diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f86976c01..1e7a1b939 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,9 +38,41 @@ If you have ideas for entirely new features, please post an issue or start a dis 1. Push to your fork and [submit a pull request][pr] 1. Pat yourself on the back and wait for your pull request to be reviewed and merged. -### Running tests and linters +### Running locally, including tests and linters ```bash +# Obtain the pinned version of Copilot CLI to the local workarea. +# This clones the reference implementation at the commit pinned in +# .lastmerge, but does NOT run `npm ci` inside its nodejs/ subdir. +mvn generate-test-resources + +# Install the pinned Copilot CLI into target/copilot-sdk/nodejs/node_modules +# so the SDK tests use the version declared in +# target/copilot-sdk/nodejs/package.json (the SDK-test pin), NOT the version +# in target/copilot-sdk/test/harness/package.json (the replay-harness pin, +# which is incidental and may be older). + +## POSIX + +(cd target/copilot-sdk/nodejs && npm ci --ignore-scripts) + +## PowerShell + +Push-Location target\copilot-sdk\nodejs; if ($?) { npm ci --ignore-scripts }; Pop-Location + +# Make it so the pinned Copilot CLI is used for the tests. The patterns +# below are scoped to the `nodejs/node_modules/` subtree so they cannot +# accidentally pick up the older harness copy under +# target/copilot-sdk/test/harness/node_modules/. + +## POSIX + +export COPILOT_CLI_PATH="$(find "$PWD/target" -type f -path '*/nodejs/node_modules/@github/copilot/index.js' | head -n 1)" + +## PowerShell + +$env:COPILOT_CLI_PATH = (Get-ChildItem -Path "$PWD\target" -Recurse -Filter 'index.js' -File | Where-Object { $_.FullName -match '[\\/]nodejs[\\/]node_modules[\\/]@github[\\/]copilot[\\/]index\.js$' } | Select-Object -First 1 -ExpandProperty FullName) + # Build and run all tests mvn clean verify @@ -54,6 +86,20 @@ mvn spotless:apply mvn spotless:check ``` +Assuming you are in the same shell you used to run the above commands, to run this exact Copilot CLI locally you can do the following. + +### POSIX + +```bash +node ${COPILOT_CLI_PATH} +``` + +### PowerShell + +```PowerShell +node $env:COPILOT_CLI_PATH +``` + Here are a few things you can do that will increase the likelihood of your pull request being accepted: - Write tests. diff --git a/pom.xml b/pom.xml index a35edacd4..718a6ad34 100644 --- a/pom.xml +++ b/pom.xml @@ -53,6 +53,27 @@ false + + ^1.0.36-0 + From 227a563f86f32271a98c934fcbfc1d0131cb2a7c Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Wed, 29 Apr 2026 20:38:23 -0400 Subject: [PATCH 2/4] On branch edburns/dd-2984436-make-copilot-cli-versions-consistent modified: CONTRIBUTING.md - Simplify this after realizing the POM can handle the `npm ci`. modified: pom.xml - Ensure the COPILOT_CLI_PATH is set to the built-in Copilot CLI instance. This ensures a user's local Copilot instance, which is not guaranteed to be the correct version, and thus the correct RPC protocol, will get in the way. --- CONTRIBUTING.md | 38 +++++--------------------------------- pom.xml | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1e7a1b939..38f2def77 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,39 +40,9 @@ If you have ideas for entirely new features, please post an issue or start a dis ### Running locally, including tests and linters -```bash -# Obtain the pinned version of Copilot CLI to the local workarea. -# This clones the reference implementation at the commit pinned in -# .lastmerge, but does NOT run `npm ci` inside its nodejs/ subdir. -mvn generate-test-resources - -# Install the pinned Copilot CLI into target/copilot-sdk/nodejs/node_modules -# so the SDK tests use the version declared in -# target/copilot-sdk/nodejs/package.json (the SDK-test pin), NOT the version -# in target/copilot-sdk/test/harness/package.json (the replay-harness pin, -# which is incidental and may be older). - -## POSIX - -(cd target/copilot-sdk/nodejs && npm ci --ignore-scripts) - -## PowerShell - -Push-Location target\copilot-sdk\nodejs; if ($?) { npm ci --ignore-scripts }; Pop-Location - -# Make it so the pinned Copilot CLI is used for the tests. The patterns -# below are scoped to the `nodejs/node_modules/` subtree so they cannot -# accidentally pick up the older harness copy under -# target/copilot-sdk/test/harness/node_modules/. - -## POSIX - -export COPILOT_CLI_PATH="$(find "$PWD/target" -type f -path '*/nodejs/node_modules/@github/copilot/index.js' | head -n 1)" - -## PowerShell - -$env:COPILOT_CLI_PATH = (Get-ChildItem -Path "$PWD\target" -Recurse -Filter 'index.js' -File | Where-Object { $_.FullName -match '[\\/]nodejs[\\/]node_modules[\\/]@github[\\/]copilot[\\/]index\.js$' } | Select-Object -First 1 -ExpandProperty FullName) +The POM has logic to ensure a known correct installation of Copilot CLI is used when executing the tests. If you want to test against a different installation of Copilot CLI, set the value of the `copilot.cli.path` maven property to the fully qualified path to the Copilot CLI. +```bash # Build and run all tests mvn clean verify @@ -86,17 +56,19 @@ mvn spotless:apply mvn spotless:check ``` -Assuming you are in the same shell you used to run the above commands, to run this exact Copilot CLI locally you can do the following. +## Running the known correct Copilot CLI installation ### POSIX ```bash +export COPILOT_CLI_PATH="$(find "$PWD/target" -type f -path '*/nodejs/node_modules/@github/copilot/index.js' | head -n 1)" node ${COPILOT_CLI_PATH} ``` ### PowerShell ```PowerShell +$env:COPILOT_CLI_PATH = (Get-ChildItem -Path "$PWD\target" -Recurse -Filter 'index.js' -File | Where-Object { $_.FullName -match '[\\/]nodejs[\\/]node_modules[\\/]@github[\\/]copilot[\\/]index\.js$' } | Select-Object -First 1 -ExpandProperty FullName) node $env:COPILOT_CLI_PATH ``` diff --git a/pom.xml b/pom.xml index 718a6ad34..087edff98 100644 --- a/pom.xml +++ b/pom.xml @@ -49,6 +49,17 @@ ${project.build.directory}/copilot-sdk ${copilot.sdk.clone.dir}/test + + ${copilot.sdk.clone.dir}/nodejs/node_modules/@github/copilot/index.js false @@ -261,6 +272,35 @@ + + + install-nodejs-cli-dependencies + generate-test-resources + + exec + + + ${skip.test.harness} + npm + ${copilot.sdk.clone.dir}/nodejs + + ci + --ignore-scripts + + + @@ -274,6 +314,15 @@ ${copilot.tests.dir} ${copilot.sdk.clone.dir} + + + ${copilot.cli.path} + From 14611064527b11cb4aab7cd448ba337721f0382e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 01:27:17 +0000 Subject: [PATCH 3/4] Address review comments: node exit 0, pin setup-node SHA, update commit message Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/a806a008-d6a9-4f91-9afd-746df3cc127a Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --- .github/actions/setup-copilot/action.yml | 2 +- .../scripts/reference-impl-sync/merge-reference-impl-finish.sh | 2 +- .../reference-impl-sync/sync-cli-version-from-reference-impl.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/actions/setup-copilot/action.yml b/.github/actions/setup-copilot/action.yml index ef0d31ab5..ec15f0d60 100644 --- a/.github/actions/setup-copilot/action.yml +++ b/.github/actions/setup-copilot/action.yml @@ -10,7 +10,7 @@ outputs: runs: using: "composite" steps: - - uses: actions/setup-node@v6 + - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v6 with: node-version: 22 - name: Read pinned @github/copilot version from pom.xml diff --git a/.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh b/.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh index 4b01465cc..0801f2548 100755 --- a/.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh +++ b/.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh @@ -57,7 +57,7 @@ echo "▸ Syncing @github/copilot version in pom.xml from reference implementati "$ROOT_DIR/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh" "$REFERENCE_IMPL_DIR" git add .lastmerge pom.xml -git commit -m "Update .lastmerge to $NEW_COMMIT" +git commit -m "Update .lastmerge to $NEW_COMMIT and sync pom.xml CLI version" # ── 3. Push branch ─────────────────────────────────────────── echo "▸ Pushing branch $BRANCH_NAME to origin…" diff --git a/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh b/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh index 4aa2e1256..5cdbd78e1 100755 --- a/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh +++ b/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh @@ -56,7 +56,7 @@ fi # node is always available since the reference implementation uses npm. CLI_VERSION=$(node -e \ - "const fs=require('fs');const p=JSON.parse(fs.readFileSync(process.argv[1],'utf8'));const v=(p.dependencies&&p.dependencies['@github/copilot'])||(p.devDependencies&&p.devDependencies['@github/copilot']);if(!v){process.exit(2);}process.stdout.write(v);" \ + "const fs=require('fs');const p=JSON.parse(fs.readFileSync(process.argv[1],'utf8'));const v=(p.dependencies&&p.dependencies['@github/copilot'])||(p.devDependencies&&p.devDependencies['@github/copilot']);process.stdout.write(v||'');" \ "$PKG_JSON") if [[ -z "$CLI_VERSION" ]]; then From d1b7091df2e6de6a3479fb1b2b9abab3a76b5b3b Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Wed, 29 Apr 2026 21:33:33 -0400 Subject: [PATCH 4/4] Skip CLI npm ci when tests are skipped Address review comment on PR #137 (discussion_r3165129538): the install-nodejs-cli-dependencies execution previously ran on every invocation of generate-test-resources because it was only guarded by ${skip.test.harness}. That made non-test builds (e.g. mvn package -DskipTests, mvn deploy -Dmaven.test.skip=true) require npm and network access unnecessarily. Introduce a new ${skip.cli.install} property that defaults to ${skip.test.harness} but is forced to true when -DskipTests=true or -Dmaven.test.skip=true is set, via two auto-activated profiles. Wire the install-nodejs-cli-dependencies execution's to this new property. --- pom.xml | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 087edff98..417d4c8ad 100644 --- a/pom.xml +++ b/pom.xml @@ -62,6 +62,17 @@ ${copilot.sdk.clone.dir}/nodejs/node_modules/@github/copilot/index.js false + + ${skip.test.harness} + + skip-cli-install-when-tests-skipped + + + skipTests + true + + + + true + + + + + skip-cli-install-when-maven-test-skip + + + maven.test.skip + true + + + + true + + debug