Skip to content

Commit bd072c9

Browse files
edburnsedburns
authored
Merge pull request #137 from github/edburns/dd-2984436-make-copilot-cli-versions-consistent
Edburns/dd 2984436 make copilot cli versions consistent
2 parents 68beab9 + d1b7091 commit bd072c9

6 files changed

Lines changed: 250 additions & 8 deletions

File tree

‎.github/actions/setup-copilot/action.yml‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,36 @@ outputs:
44
cli-path:
55
description: "Path to the Copilot CLI executable"
66
value: ${{ steps.cli-path.outputs.path }}
7+
cli-version:
8+
description: "Pinned @github/copilot version installed (read from pom.xml)"
9+
value: ${{ steps.cli-version.outputs.version }}
710
runs:
811
using: "composite"
912
steps:
10-
- uses: actions/setup-node@v6
13+
- uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v6
1114
with:
1215
node-version: 22
13-
- name: Install Copilot CLI
14-
run: npm install -g @github/copilot
16+
- name: Read pinned @github/copilot version from pom.xml
17+
id: cli-version
1518
shell: bash
19+
# The version is the SINGLE SOURCE OF TRUTH for the Copilot CLI version
20+
# used across all CI paths. It is kept in sync with the reference
21+
# implementation pinned in .lastmerge by
22+
# .github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh.
23+
run: |
24+
PROP="readonly-copilot-sdk-ref-impl-version-from-lastmerge-file-updated-by-weekly-reference-impl-sync"
25+
VERSION=$(sed -n "s|.*<${PROP}>\(.*\)</${PROP}>.*|\1|p" pom.xml | head -n 1 | tr -d '[:space:]')
26+
if [[ -z "$VERSION" || "$VERSION" == "PRIMER_TO_REPLACE" ]]; then
27+
echo "::error::Could not read pinned @github/copilot version from pom.xml property <${PROP}>" >&2
28+
exit 1
29+
fi
30+
echo "Pinned @github/copilot version: $VERSION"
31+
echo "version=$VERSION" >> "$GITHUB_OUTPUT"
32+
- name: Install Copilot CLI (pinned to pom.xml version)
33+
shell: bash
34+
env:
35+
CLI_VERSION: ${{ steps.cli-version.outputs.version }}
36+
run: npm install -g "@github/copilot@${CLI_VERSION}"
1637
- name: Set CLI path
1738
id: cli-path
1839
run: echo "path=$(which copilot)" >> $GITHUB_OUTPUT

‎.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
# Finalises a reference implementation merge:
66
# 1. Runs format + test + build (via format-and-test.sh)
77
# 2. Updates .lastmerge to reference implementation HEAD
8-
# 3. Commits the .lastmerge update
9-
# 4. Pushes the branch to origin
8+
# 3. Syncs the @github/copilot version property in pom.xml from the
9+
# cloned reference implementation's nodejs/package.json
10+
# 4. Commits the .lastmerge + pom.xml updates
11+
# 5. Pushes the branch to origin
1012
#
1113
# Usage: ./.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh
1214
# ./.github/scripts/reference-impl-sync/merge-reference-impl-finish.sh --skip-tests
@@ -48,8 +50,14 @@ echo "▸ Updating .lastmerge…"
4850
NEW_COMMIT=$(cd "$REFERENCE_IMPL_DIR" && git rev-parse origin/main)
4951
echo "$NEW_COMMIT" > "$ROOT_DIR/.lastmerge"
5052

51-
git add .lastmerge
52-
git commit -m "Update .lastmerge to $NEW_COMMIT"
53+
# ── 2b. Sync pom.xml @github/copilot version ─────────────────
54+
# Keeps the canonical CLI version in pom.xml aligned with what the
55+
# reference implementation pinned in .lastmerge depends on.
56+
echo "▸ Syncing @github/copilot version in pom.xml from reference implementation…"
57+
"$ROOT_DIR/.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh" "$REFERENCE_IMPL_DIR"
58+
59+
git add .lastmerge pom.xml
60+
git commit -m "Update .lastmerge to $NEW_COMMIT and sync pom.xml CLI version"
5361

5462
# ── 3. Push branch ───────────────────────────────────────────
5563
echo "▸ Pushing branch $BRANCH_NAME to origin…"

‎.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
#!/usr/bin/env bash
2+
# ──────────────────────────────────────────────────────────────
3+
# sync-cli-version-from-reference-impl.sh
4+
#
5+
# Reads the @github/copilot version specifier from the cloned
6+
# reference implementation's nodejs/package.json, and updates the
7+
# corresponding property in pom.xml:
8+
#
9+
# <readonly-copilot-sdk-ref-impl-version-from-lastmerge-file-updated-by-weekly-reference-impl-sync>
10+
#
11+
# This keeps the canonical Copilot CLI version (declared in pom.xml)
12+
# in sync with whatever the reference implementation pinned in
13+
# .lastmerge depends on. All workflows that install the Copilot CLI
14+
# (build-test.yml — implicitly via cloned SDK, run-smoke-test.yml and
15+
# update-copilot-dependency.yml — via the setup-copilot action) read
16+
# this single property so every CI path uses the same CLI version.
17+
#
18+
# Usage:
19+
# ./sync-cli-version-from-reference-impl.sh <reference-impl-dir>
20+
#
21+
# Or, when invoked from merge-reference-impl-finish.sh, sources
22+
# REFERENCE_IMPL_DIR from the .merge-env file.
23+
# ──────────────────────────────────────────────────────────────
24+
set -euo pipefail
25+
26+
# Locate the repo root by walking up from this script until we find a pom.xml.
27+
# This is resilient to the script being moved to a different depth under
28+
# .github/scripts/ in the future.
29+
find_repo_root() {
30+
local dir
31+
dir="$(cd "$(dirname "$0")" && pwd)"
32+
while [[ "$dir" != "/" ]]; do
33+
if [[ -f "$dir/pom.xml" ]]; then
34+
echo "$dir"
35+
return 0
36+
fi
37+
dir="$(dirname "$dir")"
38+
done
39+
echo "❌ Could not locate repo root (no pom.xml found above $(dirname "$0"))" >&2
40+
return 1
41+
}
42+
ROOT_DIR="$(find_repo_root)"
43+
44+
REFERENCE_IMPL_DIR="${1:-${REFERENCE_IMPL_DIR:-}}"
45+
if [[ -z "$REFERENCE_IMPL_DIR" ]]; then
46+
echo "❌ Usage: $0 <reference-impl-dir>" >&2
47+
echo " or set REFERENCE_IMPL_DIR in the environment." >&2
48+
exit 1
49+
fi
50+
51+
PKG_JSON="$REFERENCE_IMPL_DIR/nodejs/package.json"
52+
if [[ ! -f "$PKG_JSON" ]]; then
53+
echo "❌ Cannot find $PKG_JSON" >&2
54+
exit 1
55+
fi
56+
57+
# node is always available since the reference implementation uses npm.
58+
CLI_VERSION=$(node -e \
59+
"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||'');" \
60+
"$PKG_JSON")
61+
62+
if [[ -z "$CLI_VERSION" ]]; then
63+
echo "❌ Could not extract @github/copilot version from $PKG_JSON" >&2
64+
exit 1
65+
fi
66+
67+
POM="$ROOT_DIR/pom.xml"
68+
PROP="readonly-copilot-sdk-ref-impl-version-from-lastmerge-file-updated-by-weekly-reference-impl-sync"
69+
70+
if ! grep -q "<${PROP}>" "$POM"; then
71+
echo "❌ Property <${PROP}> not found in $POM" >&2
72+
exit 1
73+
fi
74+
75+
# Use a portable sed invocation (works on both BSD/macOS and GNU/Linux).
76+
TMP="$(mktemp)"
77+
sed -E "s|<${PROP}>[^<]*</${PROP}>|<${PROP}>${CLI_VERSION}</${PROP}>|" "$POM" > "$TMP"
78+
mv "$TMP" "$POM"
79+
80+
echo "▸ Updated pom.xml: <${PROP}> = ${CLI_VERSION}"

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ changebundle.txt*
1313
.settings
1414
scripts/codegen/node_modules/
1515
*~
16+
*.sln

‎CONTRIBUTING.md‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ If you have ideas for entirely new features, please post an issue or start a dis
3838
1. Push to your fork and [submit a pull request][pr]
3939
1. Pat yourself on the back and wait for your pull request to be reviewed and merged.
4040

41-
### Running tests and linters
41+
### Running locally, including tests and linters
42+
43+
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.
4244

4345
```bash
4446
# Build and run all tests
@@ -54,6 +56,22 @@ mvn spotless:apply
5456
mvn spotless:check
5557
```
5658

59+
## Running the known correct Copilot CLI installation
60+
61+
### POSIX
62+
63+
```bash
64+
export COPILOT_CLI_PATH="$(find "$PWD/target" -type f -path '*/nodejs/node_modules/@github/copilot/index.js' | head -n 1)"
65+
node ${COPILOT_CLI_PATH}
66+
```
67+
68+
### PowerShell
69+
70+
```PowerShell
71+
$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)
72+
node $env:COPILOT_CLI_PATH
73+
```
74+
5775
Here are a few things you can do that will increase the likelihood of your pull request being accepted:
5876

5977
- Write tests.

‎pom.xml‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,53 @@
4949
<!-- Directory where the copilot-sdk repo will be cloned for tests -->
5050
<copilot.sdk.clone.dir>${project.build.directory}/copilot-sdk</copilot.sdk.clone.dir>
5151
<copilot.tests.dir>${copilot.sdk.clone.dir}/test</copilot.tests.dir>
52+
<!--
53+
Path to the Copilot CLI entry point used by the SDK tests. Defaults
54+
to the CLI installed under target/copilot-sdk/nodejs by the
55+
install-nodejs-cli-dependencies execution. Surefire injects this
56+
into the test JVM as the COPILOT_CLI_PATH environment variable, so
57+
`mvn verify` is self-contained and the developer never has to set
58+
it manually. Override on the command line to point at a different
59+
CLI build, e.g.:
60+
mvn verify -Dcopilot.cli.path=/some/other/copilot/index.js
61+
-->
62+
<copilot.cli.path>${copilot.sdk.clone.dir}/nodejs/node_modules/@github/copilot/index.js</copilot.cli.path>
5263
<!-- Set to true (via -Pskip-test-harness) to skip git-clone + npm install of test harness -->
5364
<skip.test.harness>false</skip.test.harness>
65+
<!--
66+
Whether to skip the install-nodejs-cli-dependencies execution
67+
(npm ci of the @github/copilot CLI). Defaults to ${skip.test.harness}
68+
so it tracks the rest of the test-harness setup, but is also
69+
forced to true when Maven tests are skipped (-DskipTests or
70+
-Dmaven.test.skip) via the skip-cli-install-when-tests-skipped
71+
and skip-cli-install-when-maven-test-skip profiles below, so that
72+
non-test builds (e.g. package/deploy with tests skipped) do not
73+
require npm or network access.
74+
-->
75+
<skip.cli.install>${skip.test.harness}</skip.cli.install>
5476
<!-- Extra JVM args for Surefire; overridden by the jdk21+ profile -->
5577
<surefire.jvm.args />
78+
<!--
79+
The version of the @github/copilot npm package that the reference implementation
80+
commit pinned in .lastmerge depends on. Mirrors the value of dependencies."@github/copilot"
81+
in target/copilot-sdk/nodejs/package.json after the reference impl is cloned/reset to the
82+
commit in .lastmerge.
83+
84+
The previously mentioned package.json contains the SINGLE
85+
SOURCE OF TRUTH for the Copilot CLI version that all paths
86+
(build-test.yml, run-smoke-test.yml,
87+
update-copilot-dependency.yml, setup-copilot action) must
88+
pin to. It is updated automatically by
89+
.github/scripts/reference-impl-sync/sync-cli-version-from-reference-impl.sh,
90+
which is called from merge-reference-impl-finish.sh
91+
whenever .lastmerge is updated.
92+
93+
DO NOT EDIT MANUALLY. To update, run the
94+
reference-impl-sync workflow and deal with the subsequent
95+
PR.
96+
-->
97+
<readonly-copilot-sdk-ref-impl-version-from-lastmerge-file-updated-by-weekly-reference-impl-sync>^1.0.36-0</readonly-copilot-sdk-ref-impl-version-from-lastmerge-file-updated-by-weekly-reference-impl-sync>
98+
5699
</properties>
57100

58101
<dependencies>
@@ -240,6 +283,35 @@
240283
</arguments>
241284
</configuration>
242285
</execution>
286+
<!--
287+
Install the @github/copilot CLI declared by
288+
target/copilot-sdk/nodejs/package.json. This is the CLI
289+
version the SDK tests must run against (independent of
290+
the older pin in test/harness/package.json which is
291+
incidental to harness internals). Mirrors what
292+
.github/workflows/build-test.yml does manually so that
293+
`mvn clean verify` is self-contained: the prior
294+
target/copilot-sdk/nodejs/node_modules is wiped by
295+
clean, but this re-creates it before tests run.
296+
Uses npm ci with the ignore-scripts flag, matching
297+
build-test.yml.
298+
-->
299+
<execution>
300+
<id>install-nodejs-cli-dependencies</id>
301+
<phase>generate-test-resources</phase>
302+
<goals>
303+
<goal>exec</goal>
304+
</goals>
305+
<configuration>
306+
<skip>${skip.cli.install}</skip>
307+
<executable>npm</executable>
308+
<workingDirectory>${copilot.sdk.clone.dir}/nodejs</workingDirectory>
309+
<arguments>
310+
<argument>ci</argument>
311+
<argument>--ignore-scripts</argument>
312+
</arguments>
313+
</configuration>
314+
</execution>
243315
</executions>
244316
</plugin>
245317
<plugin>
@@ -253,6 +325,15 @@
253325
<copilot.tests.dir>${copilot.tests.dir}</copilot.tests.dir>
254326
<copilot.sdk.dir>${copilot.sdk.clone.dir}</copilot.sdk.dir>
255327
</systemPropertyVariables>
328+
<!--
329+
Set COPILOT_CLI_PATH for the forked test JVM so the SDK
330+
tests transparently use the pinned CLI under
331+
target/copilot-sdk/nodejs/. See the copilot.cli.path
332+
property above for the override mechanism.
333+
-->
334+
<environmentVariables>
335+
<COPILOT_CLI_PATH>${copilot.cli.path}</COPILOT_CLI_PATH>
336+
</environmentVariables>
256337
</configuration>
257338
</plugin>
258339
<!-- Add src/generated/java as an additional source root -->
@@ -587,6 +668,39 @@
587668
<skip.test.harness>true</skip.test.harness>
588669
</properties>
589670
</profile>
671+
<!--
672+
Skip the install-nodejs-cli-dependencies (npm ci) execution when
673+
tests are skipped via -DskipTests, so non-test builds do not
674+
require npm or network access. Activates automatically; no manual
675+
-P needed.
676+
-->
677+
<profile>
678+
<id>skip-cli-install-when-tests-skipped</id>
679+
<activation>
680+
<property>
681+
<name>skipTests</name>
682+
<value>true</value>
683+
</property>
684+
</activation>
685+
<properties>
686+
<skip.cli.install>true</skip.cli.install>
687+
</properties>
688+
</profile>
689+
<!--
690+
Same as above, but for -Dmaven.test.skip=true.
691+
-->
692+
<profile>
693+
<id>skip-cli-install-when-maven-test-skip</id>
694+
<activation>
695+
<property>
696+
<name>maven.test.skip</name>
697+
<value>true</value>
698+
</property>
699+
</activation>
700+
<properties>
701+
<skip.cli.install>true</skip.cli.install>
702+
</properties>
703+
</profile>
590704
<!-- Debug profile for FINE logging during tests -->
591705
<profile>
592706
<id>debug</id>

0 commit comments

Comments
 (0)