-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Extend user-agent with orchestration ID and add default user-agent #2229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
|
@copilot check CI failure about lint error |
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
Fixed lint error. The issue was an unnecessary escape character |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR extends the HTTP client's user-agent string to include an orchestration ID from the ACTIONS_ORCHESTRATION_ID environment variable when available, and adds a default user-agent value when none is provided.
Key changes:
- Added default user-agent value
actions/http-clientwhen no custom user-agent is specified - Implemented automatic appending of sanitized orchestration ID to user-agent in
product/versionformat - Moved user-agent processing to constructor for efficiency (evaluated once vs. per request)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/http-client/src/index.ts |
Added _getUserAgentWithOrchestrationId() private method that sets default user-agent and appends sanitized orchestration ID when ACTIONS_ORCHESTRATION_ID env var is present; modified constructor to use this method |
packages/http-client/__tests__/basics.test.ts |
Updated test expectation for default user-agent behavior; added three new tests covering orchestration ID appending, character sanitization, and behavior without orchestration ID |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: TingluoHuang <1750815+TingluoHuang@users.noreply.github.com>
Applied all review comments:
Commit: 83c13c8 |
| if (orchId) { | ||
| // Sanitize the orchestration ID to ensure it contains only valid characters | ||
| // Valid characters: 0-9, a-z, _, -, . | ||
| const sanitizedId = orchId.replace(/[^a-z0-9_.-]/gi, '_') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we remove invalid characters instead of replacing with underscores?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldn't need to since the orchestrationid we generate always has valid chars.
Plan for ACTIONS_ORCHESTRATION_ID User-Agent Extension
Complete Implementation
Successfully implemented ACTIONS_ORCHESTRATION_ID support with all requested changes:
actions_orchestration_idper feedbackactions/http-clientwhen none providedImplementation Details:
userAgentproperty always contains a value (defaults toactions/http-clientwhen not provided)0-9,a-z,A-Z,_,-,.Final Format:
Examples:
actions/http-clientactions/http-client actions_orchestration_id/workflow-12345-job-67890@actions/artifact-5.0.1 actions_orchestration_id/workflow-12345-job-67890my-action-v1.0 actions_orchestration_id/test__with__special_chars!,@,#,/,:, spaces are replaced with_Files Changed:
packages/http-client/src/index.ts- Added orchestration ID logic with default user-agentpackages/http-client/__tests__/basics.test.ts- Added tests for the feature, updated test for default user-agent, improved test cleanup with afterEach, and added test for default user-agent with orchestration ID💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
https://github.com/github/actions-runtime/issues/5389