Repository navigation
Report release version the proxy is taken from in start-proxy telemetry - #4233
Conversation
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
🟡 Changes recommended
The primary telemetry payload lacks direct unit-test coverage for its field name and nesting.
1 open finding
What changed in this PR
Adds the originating proxy release version to successful start-proxy telemetry.
Changes:
- Returns both the proxy path and version.
- Adds
start_proxy.proxy_versionto telemetry. - Extracts proxy status reporting and updates tests.
| File | Description |
|---|---|
src/start-proxy/status-report.ts |
Defines proxy telemetry and reporting. |
src/start-proxy/status-report.test.ts |
Relocates failure-report tests. |
src/start-proxy.ts |
Returns proxy path and version. |
src/start-proxy.test.ts |
Verifies resolved proxy versions. |
src/start-proxy-action.ts |
Sends the version in success telemetry. |
lib/entry-points.js |
Generated bundle; content excluded from review. |
Files excluded by content exclusion policy (1)
- lib/entry-points.js
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| t.is(sendStatusReport.callCount, 1); | ||
| t.is(sendStatusReport.args[0].length, 1); | ||
|
|
||
| const arg = sendStatusReport.args[0][0]; | ||
| t.like(arg, { | ||
| action_name: statusReport.ActionName.StartProxy, | ||
| registry_types: "docker_registry,git_source", | ||
| start_proxy: { proxy_version }, | ||
| }); |
There was a problem hiding this comment.
Would calledOnceWithExactly work here?
There was a problem hiding this comment.
Yeah, but I don't like the sinon assertions because they only return true/false and so if the assertion fails we don't know why. The t.like will print a nice diff if the assertion fails.
There was a problem hiding this comment.
I think if we use sinon.assert.calledOnceWithExactly instead of sendStatusReport.calledOnceWithExactly we'll get a good error message if the assertion fails. It's non-blocking, though.

Extends
start-proxy's success telemetry with the version of the release that the proxy binary came from.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, Code Quality, ...).Products:
analysis-kinds: code-scanning.analysis-kinds: code-quality.Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist