Publish Agent SDK packages - #1269
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds automated publishing for Python, Go, PHP, and Ruby Agent SDKs, extends the C# NuGet workflow for Agent SDK artifacts, updates SDK versions and dependencies, and documents release destinations and workflow behavior. ChangesAgent SDK release automation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant PyPI
participant GoOrigin
participant PHPRepository
participant NuGet
participant RubyGems
GitHubActions->>PyPI: Check and publish Python artifact
GitHubActions->>GoOrigin: Validate and push Go version tag
GitHubActions->>PHPRepository: Push PHP subtree split and tag
GitHubActions->>NuGet: Push C# Agent SDK packages
GitHubActions->>RubyGems: Check and publish Ruby gem
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency ReviewThe following issues were found:
License Issuespackages/sdk/agent-sdk-csharp/Phaseo.AgentSdk.csproj
OpenSSF ScorecardScorecard details
Scanned Files
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cdd0f89d4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Web previewCommit: |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
.github/workflows/publish-agent-sdks.yml (8)
48-49: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
persist-credentials: falsemissing on this checkout, unlike the pattern used elsewhere in this file.
publish-go(Line 155) andpublish-php(Line 218) both explicitly setpersist-credentials: falseonactions/checkout. This step doesn't, leaving the ambientGITHUB_TOKENpersisted in the local git config for the remainder of the job (which then runspip install -eandpytestagainst third-party-influenced code paths).🔒 Proposed fix
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-agent-sdks.yml around lines 48 - 49, Add persist-credentials: false to the actions/checkout step in the affected workflow job, matching the existing checkout configuration in publish-go and publish-php while preserving the pinned action reference.Source: Linters/SAST tools
1-39: 🩺 Stability & Availability | 🔵 TrivialConsider a
concurrencyguard.Multiple triggers (push to main + manual
workflow_dispatch) can overlap; concurrent runs ofpublish-go/publish-phpcould race on tag creation/subtree-split pushes. Consider adding a top-levelconcurrency: group: publish-agent-sdks-${{ github.ref }}to serialize runs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-agent-sdks.yml around lines 1 - 39, Add a top-level concurrency configuration for the “Publish Agent SDKs” workflow, using a group keyed by github.ref to serialize overlapping push and workflow_dispatch runs. Preserve the existing workflow triggers and permissions, and do not cancel an in-progress run so tag creation and subtree-split pushes complete safely.
141-151: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
app-sluginterpolated directly into the shell script instead of viaenv:.
bot="${{ steps.app-token.outputs.app-slug }}[bot]"expands a workflow expression directly inside therun:block, the same pattern zizmor'stemplate-injectionrule flags elsewhere (GH_APP_TOKEN/GIT_USER_NAMEon the same step are correctly passed viaenv:). Routeapp-slugthroughenv:too for consistency and to eliminate the injection surface.🔒 Proposed fix
- name: Resolve GitHub App identity id: app-identity env: GH_TOKEN: ${{ steps.app-token.outputs.token }} + APP_SLUG: ${{ steps.app-token.outputs.app-slug }} shell: bash run: | set -euo pipefail - bot="${{ steps.app-token.outputs.app-slug }}[bot]" + bot="${APP_SLUG}[bot]"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-agent-sdks.yml around lines 141 - 151, Update the “Resolve GitHub App identity” step to pass the app-slug through its env configuration, then read that environment variable inside the shell script when constructing bot. Remove the direct workflow-expression interpolation from the run block while preserving the existing GitHub API lookup and GITHUB_OUTPUT values.Source: Linters/SAST tools
278-279: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
persist-credentials: falsemissing here too.Same gap as
build-pythonLine 49 — this checkout precedesgem install/gem build, which can execute arbitrary code from the installed gem.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-agent-sdks.yml around lines 278 - 279, Update the checkout step in the publish-agent-sdks workflow to set persist-credentials to false, matching the secure configuration used by build-python, before the gem installation and build steps.Source: Linters/SAST tools
310-330: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
needs.build-ruby.outputs.versioninterpolated directly into therun:command.Same template-injection pattern flagged at Lines 148/211 — pass the version via
env:rather than inlining the expression into the shell command.🔒 Proposed fix
+ - env: + GEM_VERSION: ${{ needs.build-ruby.outputs.version }} - name: Publish to RubyGems - run: gem push "dist/ruby-agent/phaseo_agent_sdk-${{ needs.build-ruby.outputs.version }}.gem" + run: gem push "dist/ruby-agent/phaseo_agent_sdk-${GEM_VERSION}.gem"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-agent-sdks.yml around lines 310 - 330, Update the publish-ruby job’s “Publish to RubyGems” step to pass needs.build-ruby.outputs.version through an environment variable, then reference that variable in the gem push command instead of interpolating the GitHub expression directly in run.Source: Linters/SAST tools
160-183: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVersion regex allows non-semver Go module tags.
^[0-9]+\.[0-9]+\.[0-9]+([.-][0-9A-Za-z.-]+)?$permits a.-delimited suffix (e.g.1.2.3.4) after the patch version, but Go module tags require the canonicalvMAJOR.MINOR.PATCH[-PRERELEASE][+BUILD]form — a fourth.-separated segment isn't valid semver andgo getwould not resolve such a tag correctly. Restrict the optional suffix to-(pre-release) and+(build metadata) delimiters only.🐛 Proposed fix
- if ! [[ "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+([.-][0-9A-Za-z.-]+)?$ ]]; then + if ! [[ "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$ ]]; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-agent-sdks.yml around lines 160 - 183, Update the version validation regex in the “Test and tag module” workflow step to allow only canonical MAJOR.MINOR.PATCH versions with optional hyphen-delimited prerelease or plus-delimited build metadata suffixes; remove the dot delimiter so versions such as 1.2.3.4 are rejected.
204-214: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSame
app-sluginline-expansion pattern aspublish-go.Same as Line 148 — pass
app-slugviaenv:instead of inlining the expression directly in therun:script.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-agent-sdks.yml around lines 204 - 214, Update the “Resolve GitHub App identity” step using id app-identity to pass steps.app-token.outputs.app-slug through an environment variable, then read that variable in the bash script when constructing bot; remove the direct GitHub Actions expression from the run block while preserving the existing output values.Source: Linters/SAST tools
215-254: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSame non-semver-permissive regex as
publish-go.Line 233 uses the identical over-permissive pattern flagged at Line 169. Same fix applies for consistency (Composer versions are more lenient than Go, but tightening still guards against typos in the
VERSIONfile).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-agent-sdks.yml around lines 215 - 254, Update the version validation regex in the “Test, tag, and sync split repository” step to match the stricter semantic-version format already used by publish-go, while preserving support for the intended prerelease or build suffixes. Keep the existing invalid-version error and exit behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-agent-sdks.yml:
- Around line 197-203: Update the “Create GitHub App token” step identified by
id app-token to include a repositories restriction containing only Phaseo and
phaseo-php-agent-sdk, while preserving the existing owner and credential inputs.
- Around line 283-295: Update the “Check RubyGems release status” step
identified by id “registry” to distinguish an expected 404 from other HTTP or
network failures: use bounded curl connection/total timeouts, treat only 404 as
should_publish=true, and fail the workflow for 5xx, rate limits, or request
errors instead of publishing. Preserve the existing version extraction and
GitHub output assignments.
In `@packages/sdk/RELEASING.md`:
- Around line 83-90: Update the Agent SDK workflow documentation to remove C#
NuGet publishing and its NuGet trusted-publisher identity from the
publish-agent-sdks.yml description. Document those NuGet details under the C#
workflow entry associated with publish-sdk-csharp.yml, keeping the Python, Go,
PHP, and Ruby workflow description accurate.
---
Nitpick comments:
In @.github/workflows/publish-agent-sdks.yml:
- Around line 48-49: Add persist-credentials: false to the actions/checkout step
in the affected workflow job, matching the existing checkout configuration in
publish-go and publish-php while preserving the pinned action reference.
- Around line 1-39: Add a top-level concurrency configuration for the “Publish
Agent SDKs” workflow, using a group keyed by github.ref to serialize overlapping
push and workflow_dispatch runs. Preserve the existing workflow triggers and
permissions, and do not cancel an in-progress run so tag creation and
subtree-split pushes complete safely.
- Around line 141-151: Update the “Resolve GitHub App identity” step to pass the
app-slug through its env configuration, then read that environment variable
inside the shell script when constructing bot. Remove the direct
workflow-expression interpolation from the run block while preserving the
existing GitHub API lookup and GITHUB_OUTPUT values.
- Around line 278-279: Update the checkout step in the publish-agent-sdks
workflow to set persist-credentials to false, matching the secure configuration
used by build-python, before the gem installation and build steps.
- Around line 310-330: Update the publish-ruby job’s “Publish to RubyGems” step
to pass needs.build-ruby.outputs.version through an environment variable, then
reference that variable in the gem push command instead of interpolating the
GitHub expression directly in run.
- Around line 160-183: Update the version validation regex in the “Test and tag
module” workflow step to allow only canonical MAJOR.MINOR.PATCH versions with
optional hyphen-delimited prerelease or plus-delimited build metadata suffixes;
remove the dot delimiter so versions such as 1.2.3.4 are rejected.
- Around line 204-214: Update the “Resolve GitHub App identity” step using id
app-identity to pass steps.app-token.outputs.app-slug through an environment
variable, then read that variable in the bash script when constructing bot;
remove the direct GitHub Actions expression from the run block while preserving
the existing output values.
- Around line 215-254: Update the version validation regex in the “Test, tag,
and sync split repository” step to match the stricter semantic-version format
already used by publish-go, while preserving support for the intended prerelease
or build suffixes. Keep the existing invalid-version error and exit behavior
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 27962f62-7312-4d4a-b57b-85946b513044
⛔ Files ignored due to path filters (1)
packages/sdk/agent-sdk-go/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.github/workflows/publish-agent-sdks.yml.github/workflows/publish-sdk-csharp.ymlpackages/sdk/RELEASING.mdpackages/sdk/agent-sdk-csharp/Phaseo.AgentSdk.csprojpackages/sdk/agent-sdk-go/VERSIONpackages/sdk/agent-sdk-go/go.modpackages/sdk/agent-sdk-php/VERSION
Summary
Validation
go test ./...inpackages/sdk/agent-sdk-gopython -m pytest packages/sdk/agent-sdk-py/testspython -m build packages/sdk/agent-sdk-pydotnet test packages/sdk/agent-sdk-csharp/tests/Phaseo.AgentSdk.Tests/Phaseo.AgentSdk.Tests.csproj -c Releasedotnet pack packages/sdk/agent-sdk-csharp/Phaseo.AgentSdk.csproj -c Releasecomposer validate --strictand PHP Agent SDK testsgem buildactionlinton both modified publishing workflowsCreated with Codex
Summary by CodeRabbit
New Features
0.2.0for the Go, PHP, and C# Agent SDKs.Documentation