chore(workflow): added release workflow#2940
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| print(f"Could not parse Bedrock response as JSON: {exc}", file=sys.stderr) | ||
| return 2 | ||
|
|
||
| bump = result.get("bump", "unknown") |
There was a problem hiding this comment.
Issue: bump from the model is written straight to bump.txt without validating it's one of major/minor/patch. The docstring promises that constraint, and the run summary presents it as the advisory bump. A malformed/hallucinated value would flow through unchecked.
Suggestion: Low priority since it's advisory-only, but a quick if bump not in {"major","minor","patch"}: bump = "unknown" keeps the output honest with the documented contract.
|
Issue: The PR description is the unfilled template — no summary, no linked issue, and the Testing section is blank. For a release pipeline that adds a Suggestion: Fill in the description with the why, link any tracking issue, and note how the pipeline was validated end to end (e.g. a dry-run in a fork/test repo). Even "couldn't fully exercise X because it needs the configured release environment" is useful to state explicitly. |
|
Assessment: Request Changes Well-structured release pipeline with a genuinely thoughtful security model — the Review themes
The security thinking behind the |
ae5d5af to
a833c83
Compare
|
Re-review after force-push (
Happy to dig into any of these if a recommendation is unclear — assessment remains Request Changes until the publish-gate and setup-doc items are addressed. |
a833c83 to
5c53369
Compare
|
Re-review after
Two optional, non-blocking nits remain — neither needs to hold up merge:
Assessment: Approve — the publish-gate fix in particular turns the most dangerous failure mode into an explicit error. The two remaining nits are yours to take or leave. |
5c53369 to
8ef8cb0
Compare
| - name: Install build tooling | ||
| run: pip install --no-cache-dir build | ||
|
|
||
| - name: Build wheel + sdist |
There was a problem hiding this comment.
Issue (Critical): This wheel will be built with the wrong version, and 8ef8cb0 now publishes that wheel to PyPI.
strands-py/pyproject.toml uses hatch-vcs ([tool.hatch.version] source = "vcs", tag_regex = "^python/v(?P<version>.+)$"), so the version is derived from git describe --match "python/v*". But in the new pipeline:
create-gh-release(which creates thepython/vX.Y.Ztag) runs last, afterpublish-pypi.- This checkout uses the default shallow depth with no
fetch-tags: true.
So at python -m build time the only reachable tag is the previous release, and hatch-vcs produces a dev/post version like 1.3.1.dev5+g<sha> — never the maintainer's typed inputs.version. That mis-versioned wheel is exactly what py-inspect → publish-pypi uploads.
Suggestion: Tag-last ordering is fundamentally incompatible with VCS-derived versioning. Either create the tag on the pinned SHA before the build/publish jobs (then build from the tag with fetch-tags: true), or inject inputs.version explicitly at build time (e.g. SETUPTOOLS_SCM_PRETEND_VERSION / HATCH_VCS_PRETEND_VERSION-style override) so the wheel carries the intended release version. Worth validating end-to-end with a dry build that asserts strands.__version__ == inputs.version.
| name: npm-pack-bundle | ||
| path: dist-npm | ||
|
|
||
| - name: Publish to NPM |
There was a problem hiding this comment.
Issue (Critical): This publishes strands-ts with the wrong version. strands-ts/package.json has "version": "0.0.1-development", and the new in-pipeline pack/inspect/publish path (ts-package-pack → ts-inspect → publish-npm) has no step that sets the release version into package.json before npm pack.
The old typescript-npm-publish-on-release.yml had an explicit npm version "$VERSION" --no-git-tag-version step (lines 94–99) precisely for this — that step has no equivalent in the new flow. As written, npm publish would push the literal 0.0.1-development tarball (or fail/collide), and the maintainer's typed inputs.version never reaches the published package.
Suggestion: Add an npm version ${{ inputs.version }} --no-git-tag-version step in ts-package-pack (working-directory strands-ts) before the build/pack, mirroring the fallback workflow, so the verified tarball carries the intended version. A guard in ts-inspect that asserts the embedded package.json version equals inputs.version would catch this class of bug going forward.
|
Re-review after First, the good news — the previously-blocking items are all still fixed, and the new direction is sound in concept:
However, the reordering introduced two new critical correctness bugs around version derivation (details in the two inline comments): Critical: published artifacts carry the wrong version
Common root cause: Two prior non-blocking nits also remain (advisory Assessment: Request Changes — the version-derivation bugs mean a release would publish a mis-versioned package to a public registry, which is worth fixing before merge. The overall pipeline shape is good; this is about getting the typed version onto the built artifact (and ideally an inspect-time assertion that catches it). Happy to dig in if any recommendation is unclear. |
8ef8cb0 to
eb0bf75
Compare
|
Re-review after What's new and good:
However, both critical version-derivation bugs from the last review are unchanged — this push didn't touch the build path:
Root cause is still that The two prior non-blocking nits (advisory Assessment: Request Changes — the fork-testing change is a good addition, but a real release still publishes a mis-versioned package, so the two version bugs remain blocking. Happy to clarify either fix. |
eb0bf75 to
ff7cc64
Compare
|
Re-review after What's new and good:
Unfortunately both critical version bugs are still present, because the fix targeted the wrong stage. The reorder addresses when the tag exists relative to publish, but the artifacts aren't built at publish time — they're built much earlier:
The fix still needs to land at the build step, one of:
Either way, passing The two prior nits (advisory Assessment: Request Changes — the tag-before-publish reorder is a genuine improvement, but a real release would still publish a mis-versioned artifact because the build precedes tagging and no version is injected. Happy to clarify either fix path. |
ff7cc64 to
3bba66b
Compare
|
Re-review after The fix landed exactly where it needed to (at the build step) and went a step further with inspect-time guards:
Only the two long-standing optional nits remain (no Assessment: Approve — every blocking issue raised across the review cycle is resolved and no new issues were introduced. The version-injection + assertion approach is robust and self-guarding. Nice work seeing this through. |
Description
Manually-triggered, per-package release pipeline. Pins
mainto a SHA, runs gates against it, drafts AI release notes via Bedrock, pauses on a 2-reviewer approval, creates the tag + GH release, then publishes to PyPI / NPM via OIDC trusted publishing. The existingpython-pypi-publish-on-release.yml/typescript-npm-publish-on-release.ymlworkflows are demoted toworkflow_dispatch-only manual fallback paths.Two design properties:
dry_run: false, and the whole pipeline runs end-to-end — onlypublish-pypi/publish-npmfails (registry rejects the fork's OIDC token). The build artifact is on the run page for local install + smoke testing. This is how we iterate onrelease.ymlitself.Pipeline (release.yml)
scan-commitspinsrelease_sha = origin/main, validates the typed version, rejects duplicate / non-monotonic /python/v2+tags.create-gh-releaseruns before publish so the tag is the source of truth — a publish failure leaves the tag intact and only the publish job needs re-running. On forks it runs but no-ops itsgh release createstep.run_integ_on_fork: truefor fork users with their own AWS credentials.draft-summary's AWS configure step iscontinue-on-error: true, so a fork without secrets falls back togit shortlogcleanly.Supporting changes
.github/scripts/draft-release-notes.py(new) — Bedrock-drafted{bump, notes}. Bump is advisory; maintainer types the version at dispatch.python-security-audit.yml(new) —pip-auditagainst the full dependency closure. Mirrorstypescript-security-audit.yml.python-test-package-build.yml(new) — wheel install + import in an out-of-tree venv. Uploads dist aspypi-build-outputfor the inspect step.typescript-test-package-pack.yml— also uploads the packed tarball.python-integration-test.yml/typescript-integration-test.yml— addedworkflow_call(ref)+ avalidate-call-refjob that rejectsrefs/pull/*/owner:branchrefs. Dropped the priorrepository.fork == truehard-reject; a fork using its own secrets on its own code is a legitimate use case (the ref-shape case statement is the real protection against ref-injection).python-pypi-publish-on-release.yml/typescript-npm-publish-on-release.yml— trigger flipped toworkflow_dispatch(tag). They no longer fire onrelease: published(that path is owned byrelease.yml). Header banner marks them as emergency fallback.One-time setup before merging
release-python,release-typescript:required_reviewers: 2, deployment branches →mainonly.pypi,npm: allowrelease.yml.release.ymlas a trusted publisher:strands-agents: workflowrelease.yml, environmentpypi.@strands-agents/sdk: workflowrelease.yml, environmentnpm.AWS_ROLE_ARNsecret needsbedrock:InvokeModel(already used bystrands-command.yml).Related Issues
Documentation PR
N/A — release pipeline is contributor-facing only.
Type of Change
Other: CI / release infrastructure.
Testing
actionlint+shellcheckpass on all modified workflows.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.