Repository navigation
Add the new-package skill: 0.0.0 placeholder publish and OIDC trusted publisher setup - #2130
mishushakov wants to merge 1 commit into
Conversation
|
PR SummaryLow Risk Overview Reviewed by Cursor Bugbot for commit 37b2de9. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Package ArtifactsBuilt from a9b1304. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-dockerfile-utils-0.1.2-new-package-skill.0.tgz ./e2b-2.63.1-new-package-skill.0.tgzCLI ( npm install ./e2b-cli-2.23.1-new-package-skill.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.9.2-new-package-skill.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.5.1-new-package-skill.0.tgzID JS ( npm install ./e2b-id-0.0.1-new-package-skill.0.tgzPython SDK ( pip install ./e2b_dockerfile_utils-0.1.1+new.package.skill-py3-none-any.whl ./e2b-2.63.0+new.package.skill-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.11.9+new.package.skill-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.7.2+new.package.skill-py3-none-any.whlID Python ( pip install ./e2b_id-0.0.0+new.package.skill-py3-none-any.whl |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fef604c93
ℹ️ 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".
| ```md | ||
| --- | ||
| '@e2b/<name>': minor | ||
| --- |
There was a problem hiding this comment.
When this skill scaffolds both packages, the shown changeset schedules only @e2b/<name>. Consequently changeset version leaves @e2b/python-<name> at 0.0.0, and publish_selected.cjs does not select its postPublish script, so the first release publishes only the JavaScript package while the PyPI project remains a placeholder. Add the Python workspace to this changeset or instruct authors to create a separate Python changeset, as the existing ID packages do.
Useful? React with 👍 / 👎.
| - PyPI checks `job_workflow_ref`, which names the **called** workflow, so it wants | ||
| `publish_packages.yml` (this is how `e2b`, `e2b-code-interpreter` and | ||
| `e2b-desktop` are registered, see PR #1828). |
There was a problem hiding this comment.
For a release started by release.yml, PyPI matches the caller's workflow_ref; it does not support registering the reusable publish_packages.yml via job_workflow_ref. PyPI's current troubleshooting documentation explicitly says reusable workflows cannot be used as trusted publishers, so the values given here produce invalid-publisher when uv publish runs. Register release.yml, or move the upload into a non-reusable workflow.
Useful? React with 👍 / 👎.
| > | Workflow filename | `release.yml` | | ||
| > | Environment name | leave empty | |
There was a problem hiding this comment.
New trusted-publisher configurations now default to allowing staged publishing, while permission for direct npm publish must be selected separately, according to the npm trusted-publishing documentation. Because this table is presented as the exact configuration but omits the allowed-action field, a newly configured package can reject the release workflow's npm publish; explicitly tell the author to enable direct publishing.
Useful? React with 👍 / 👎.
| Registry settings are not visible from any CLI, so stop here and hand the author the | ||
| exact values. Do not open the PR as ready, and do not merge, until they confirm both | ||
| are registered. The two registries check different OIDC claims, so the workflow |
There was a problem hiding this comment.
This ordering requires registering the npm publisher before the PR is even ready, but npm expires a new trusted-publisher configuration unless its first successful publish occurs within two days. Any PR that spends more than two days in review will therefore reach its first release with an expired configuration and fail; instruct authors to register or recreate it within two days of the planned release instead.
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| PyPI. A project that does not exist yet cannot have a project-scoped token, so this | ||
| needs an account-scoped API token from the account that owns the other `e2b-*` | ||
| projects. Read it into `UV_PUBLISH_TOKEN` from a password manager; never paste it | ||
| on the command line or echo it. The placeholder is a wheel only, like `e2b-id`. | ||
|
|
||
| ```sh | ||
| dir=$(mktemp -d) && cd "$dir" | ||
| mkdir e2b_<name> | ||
| printf '"""Placeholder for <description>. The first real release follows shortly."""\n' > e2b_<name>/__init__.py | ||
| cat > pyproject.toml <<'EOF' | ||
| [project] | ||
| name = "e2b-<name>" | ||
| version = "0.0.0" | ||
| description = "Placeholder for <description>. The first real release follows shortly." | ||
| authors = [{ name = "e2b", email = "hello@e2b.dev" }] | ||
| license = "Apache-2.0" | ||
| requires-python = ">=3.10" | ||
| dependencies = [] | ||
|
|
||
| [build-system] | ||
| requires = ["uv_build>=0.10.0,<0.11.0"] | ||
| build-backend = "uv_build" | ||
|
|
||
| [tool.uv.build-backend] | ||
| module-name = ["e2b_<name>"] | ||
| module-root = "" | ||
| EOF | ||
| uv build --wheel | ||
| # The author exports UV_PUBLISH_TOKEN in their own shell beforehand. | ||
| uv publish dist/*.whl |
There was a problem hiding this comment.
🟡 (optional) Authors following the PyPI placeholder step leave a token that can publish any e2b-* project, including e2b, in the agent session. The skill never says to revoke it afterwards. The agent's uv publish dist/*.whl (SKILL.md:162) only sees UV_PUBLISH_TOKEN if it is exported in the environment that launched the agent. Every later subprocess inherits it, and line 135 implies a token kept in a password manager. Fix: make the token-free pending-publisher route the default. Otherwise use a token minted for this one upload, have the author run uv publish in their own terminal, and delete the token right after.
Why this was flagged
Step 4 (SKILL.md:133-136) tells the author to get an account-scoped token "from the account that owns the other e2b-* projects" and read it into UV_PUBLISH_TOKEN from a password manager. The agent then runs uv publish dist/*.whl itself (SKILL.md:161-162). For that call to see the token, the author must export it in the shell that starts Claude Code. From then on the token is in the env of every command the session runs: pnpm install lifecycle scripts, tests, and npm publish. Nothing in the skill says to scope it to this upload or delete it on PyPI afterwards. An account-scoped token can upload new releases of e2b, e2b-code-interpreter and e2b-desktop. Before this change there was no documented flow that put such a token in front of an agent. The "never paste it on the command line or echo it" warning does not help, because the exposure comes from the env inheritance itself. The pending-publisher alternative at SKILL.md:166-169 avoids the token completely but is offered only as an opt-out.
Verification: SKILL.md:133-135 asks for an account-scoped token read into UV_PUBLISH_TOKEN, and line 162 runs uv publish dist/*.whl. For line 162 to see the token, it has to be exported in the environment that started the session. No line says to unset it after publishing or revoke the account-wide token.
| Each file below names every package explicitly. Grep for `js-id` / `python-id` / | ||
| `@e2b/id` / `@e2b/python-id` and add the new package next to each hit: | ||
|
|
||
| - `.github/workflows/<name>_tests.yml`: a `workflow_call` test workflow, modelled on | ||
| `id_tests.yml` (no secrets) or `js_sdk_tests.yml` / `python_sdk_tests.yml` | ||
| (needs `E2B_API_KEY`). | ||
| - `.github/workflows/sdk_tests.yml`: a path filter under `changes` and a job that | ||
| `uses:` the new test workflow. |
There was a problem hiding this comment.
🟡 (optional) If an agent follows this skill, the new package's PR tests can be skipped silently on every PR, or fail without turning the required check red. The sdk_tests.yml step asks only for a path filter and a uses: job. It leaves out the changes.outputs entry and the status.needs entry. The suggested grep (js-id / python-id / @ e2b/id / @ e2b/python-id) matches neither line. Fix: list all four sdk_tests.yml edits, including <name>: ${{ steps.filter.outputs.<name> }} under changes.outputs and the new job under status.needs, and point the grep at id-tests and outputs.id too.
Why this was flagged
In .github/workflows/sdk_tests.yml, a job reads a filter result only through the changes job's outputs: map (line 38: id: ${{ steps.filter.outputs.id }}). A filter added under filters: with no entry in that map leaves needs.changes.outputs.<name> empty. The new job's if: is then false on every pull_request event, so it only runs on workflow_dispatch. The status aggregator (its needs: list ends with - id-tests) fails only when a listed job failed or was cancelled. A skipped job, or one left out of needs, therefore still gives a green 'SDK Tests Status'. SKILL.md:86-87 asks only for 'a path filter under changes and a job that uses: the new test workflow'. The grep at SKILL.md:80-81 matches only the packages/js-id/** and packages/python-id/** filter lines (sdk_tests.yml:118-119), not id: in outputs or id-tests in status.needs. A package scaffolded this way can merge with broken tests that never run or never block. release.yml then runs its tests for the first time at release.
Verification: SKILL.md step 3 asks only for "a path filter under changes and a job that uses: the new test workflow." In .github/workflows/sdk_tests.yml, a filter with no changes.outputs entry leaves the job condition false on every pull_request. A job missing from status.needs can fail without turning "SDK Tests Status" red. The suggested grep matches neither line.
| changeset that turns the placeholder into the first real version: | ||
|
|
||
| ```md | ||
| --- | ||
| '@e2b/<name>': minor | ||
| --- | ||
|
|
||
| Initial release: <one line on what the package is>. | ||
| ``` |
There was a problem hiding this comment.
🟡 (optional) An agent following the skill writes no changeset for a new Python package, so e2b-<name> is never versioned or released past the 0.0.0 placeholder. The sample changeset only names '@ e2b/<name>'. The skill's own table gives the Python workspace the different name @ e2b/python-<name>. For a Python-only package, '@ e2b/<name>' matches no workspace at all. Fix: show one changeset entry per workspace being added, like the two repo files .changeset/js-id-package.md ('@ e2b/id': minor) and .changeset/python-id-package.md ('@ e2b/python-id': minor).
Why this was flagged
The skill's step 1 table (SKILL.md:26-29) names the Python workspace @ e2b/python-<name> (private), and changesets versions it through privatePackages.version: true in .changeset/config.json. Step 2 (SKILL.md:66-74) gives only one sample changeset, keyed '@ e2b/<name>', which is the npm package. An agent adding both packages bumps only the JS one. publish_selected.cjs:16-25 runs postPublish only for packages selected for release, so the Python package stays at 0.0.0 and never uploads. An agent adding only a Python package writes a changeset naming a package that does not exist, and changesets rejects it. The ID reference this skill copies used two separate files, .changeset/js-id-package.md and .changeset/python-id-package.md, each keyed by its own workspace name.
Verification: SKILL.md:30 names the Python workspace @ e2b/python-<name>, but the only sample changeset, at lines 68-71, is keyed '@ e2b/<name>': minor. .changeset/config.json has privatePackages.version: true, and .github/scripts/publish_selected.cjs runs postPublish only when selected.has(pkg.name) && pkg.scripts?.postPublish. A Python package with no changeset of its own therefore stays at 0.0.0.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
TASTE.md compliance: 0 violations. This PR only adds .claude/skills/new-package/SKILL.md, a contributor workflow doc. It doesn't change the public API of packages/js-sdk or packages/python-sdk, so the API-shape rules (T-1–T-74) don't apply. I spot-checked the scaffolding guidance that could shape a new package's surface: JS/Python parity (T-1), one flat entry point (T-54), no hardcoded version (T-56), and changeset use with no major bump. None of it conflicts.
Outside TASTE.md: the step 2 changeset template only keys '@e2b/<name>'. A Python package is versioned through its private workspace entry, so it needs '@e2b/python-<name>': minor (see .changeset/python-id-package.md). Without it the Python half never leaves 0.0.0.
There was a problem hiding this comment.
Devin Review found 3 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
|
|
||
| ```md | ||
| --- | ||
| '@e2b/<name>': minor |
There was a problem hiding this comment.
🔴 Python package never receives initial release
When adding both packages, the changeset omits @e2b/python-<name> from the release plan. publish_selected.cjs uploads only selected Python workspaces, leaving PyPI at its placeholder version.
Learn more
The Python workspace has its own Changesets identity. A changeset naming only the npm workspace cannot bump its version from 0.0.0 or select its postPublish script. publish_selected selects Python uploads from the release plan, while the reference Python changeset names @e2b/python-id separately.
Example: Adding @e2b/foo and e2b-foo with the shown changeset releases @e2b/foo@0.1.0; e2b-foo stays at 0.0.0 on PyPI.
Recommended fix: Show separate minor changeset entries for @e2b/<name> and @e2b/python-<name> when both packages are added, and specify the appropriate single entry for one-registry packages.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - `.github/workflows/sdk_tests.yml`: a path filter under `changes` and a job that | ||
| `uses:` the new test workflow. |
There was a problem hiding this comment.
🔴 New package failures escape required test status
When the new test job fails, status can still pass because the instructions omit that job from status.needs. The required SDK test check then reports success without the new package passing.
Learn more
The SDK test workflow has an aggregate status job that gates PRs. Its result check sees only jobs listed under status.needs, not every job in the workflow. Adding a new package test job without adding that dependency lets the aggregate check pass when that suite fails.
Example: The new foo-tests job runs for a PR and fails. Other suites pass or skip, and status sees no failure in its needs collection, so SDK Tests Status succeeds.
Recommended fix: Add the new test job to status.needs as well as creating its path filter and job. Verify a failing new-package test causes the aggregate check to fail.
Was this helpful? React with 👍 or 👎 to provide feedback.
| The Python workspace entry is a `"private": true` `package.json` whose only job is to | ||
| let changesets version and tag the package; `publish_selected.cjs` runs its | ||
| `postPublish` script for the upload, and `changeset publish` skips it on npm. The | ||
| Python import name is `e2b_<name>`. |
There was a problem hiding this comment.
🟡 Hyphenated names break Python imports
For a package named e2b-foo-bar, e2b_<name> becomes e2b_foo-bar. That is not a valid Python import name, so the scaffolded package cannot be imported as instructed.
Learn more
PyPI distribution names can contain hyphens, but Python import identifiers cannot. The skill also uses the raw suffix in its placeholder module directory and build configuration. That produces a wheel with a module name that users cannot write as a standard Python import.
Example: For e2b-foo-bar, the prescribed module name is e2b_foo-bar, rather than importable e2b_foo_bar.
Recommended fix: Specify that every hyphen in <name> is converted to an underscore for the Python import, module directory, and module-name setting, while keeping hyphens in the PyPI distribution name.
Was this helpful? React with 👍 or 👎 to provide feedback.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0fef604. Configure here.
|
|
||
| Registry settings are not visible from any CLI, so stop here and hand the author the | ||
| exact values. Do not open the PR as ready, and do not merge, until they confirm both | ||
| are registered. The two registries check different OIDC claims, so the workflow |
There was a problem hiding this comment.
Trusted publisher expires before first release
High Severity
The skill registers the npm trusted publisher during the package PR, but unused npm trusted-publisher configs expire after 48 hours. The first OIDC publish is the later 0.1.0 CI release, so the binding lapses and that release fails after the version bump is already on main.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0fef604. Configure here.
058e250 to
6749ed3
Compare
0fef604 to
fda4e70
Compare
…sher Adding a package here has a manual tail that only fails at the first release: the name must exist on npm or PyPI before a trusted publisher can be attached, and the trusted publisher must name a different workflow file per registry. npm validates the calling workflow (release.yml), PyPI validates job_workflow_ref, which is the called workflow (publish_packages.yml). @e2b/id and e2b-id went through this by hand; the skill writes the procedure down so the next package does not rediscover it. The skill scaffolds from the ID packages, lists every CI and release file that names packages explicitly, publishes a 0.0.0 placeholder from a temp directory, and then stops to hand the author the exact trusted publisher values, since registry settings cannot be read or written from any CLI. The PR description carries the same tables so the manual step is visible on the PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
6749ed3 to
20103e3
Compare
fda4e70 to
37b2de9
Compare
Summary
Adds a project skill at
.claude/skills/new-package/SKILL.mdfor adding a new publishable package to this monorepo. It covers the part that is easy to miss and only fails at the first release: the package name must exist on the registry before a trusted publisher can be attached, and the trusted publisher must name a different workflow file on npm than on PyPI.The skill:
packages/js-id/packages/python-idand lists every CI and release file that names packages explicitly (sdk_tests.yml,release.ymlpreflight +publish.needs+report-failure.needs,build_release_itinerary.cjs,pkg_artifacts.yml,lint.yml/typecheck.yml)0.0.0placeholder to npm and/or PyPI from a temp directory, matching the convention@e2b/idande2b-idalready use (description "Placeholder for …. The first real release follows shortly.", wheel only on PyPI)deploymentGitHub environment #1850 didStacked on #1628 (
python-id) because the skill points at both ID packages as the template.Usage
or ask for it in words: "add a new package
@e2b/foo/e2b-foo". The agent scaffolds, wires CI, publishes the placeholder, then stops with this prompt:Why the workflow filename differs
npm publishruns insidepublish_packages.yml, whichrelease.ymlcalls. npm's trusted publishers docs (Troubleshooting) say validation "checks the calling workflow's name instead of the workflow that actually contains the publish command". The provenance attestation one2b@2.63.0names.github/workflows/release.yml.job_workflow_ref, which names the called workflow. ci: publish Python packages to PyPI via trusted publishing #1828 registeredpublish_packages.ymlfore2b,e2b-code-interpreterande2b-desktop.deploymentGitHub environment #1850 left it empty on both registries, which matches any environment; the skill says so and namesdeploymentas the optional tightening.Notes
pnpm run format/lint/typecheckonly recurse intopackages/, so the file was formatted withprettierdirectly.Linear: SDK-441
🤖 Generated with Claude Code