镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Publish the ESP-IDF component to the registry - #24712

Draft
rascani wants to merge 1 commit into
esp-idf-nightlyfrom
esp-idf-publish
Draft

rascani wants to merge 1 commit into
esp-idf-nightlyfrom
esp-idf-publish

Conversation

@rascani

@rascani rascani commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

publish-esp-idf.yml publishes pytorch/executorch to the ESP Component
Registry. An RC tag vX.Y.Z-rcN publishes X.Y.Z-rc.N to the staging
registry, a release tag vX.Y.Z publishes X.Y.Z to staging and then
production, and a manual run on release/X.Y publishes a respin, X.Y.Z~N.
Other v* tags publish nothing.

The workflow packs the component once, under the generator's release-stamp
checks, and asserts the version matches the tag. Before anything is
uploaded, it builds and runs that archive in every consumer configuration
on ESP-IDF 6.0.1 and 6.1 and validates it with both registries. Each upload
waits for a reviewer of the esp-component-registry environment. After each
upload, the new registry mode of test_espressif_idf_nightly.sh polls until
the registry lists the version, builds the consumer with pytorch/executorch
resolved from that registry, checks that the registry served the tested
archive's files (apart from the examples' manifests, which the registry
rewrites to drop override_path), and runs it on the emulator. Production
waits for that check on staging. The nightly's build mode runs the same
file check against the mirror, so it is exercised before the first upload.
The consumer takes the registry as -DET_COMPONENT_REGISTRY_URL.

Uploads go through .ci/scripts/upload_espressif_idf_component.sh, which the
nightly's dry-run job now shares. It installs the component manager from a
hash-locked requirements file, and the two upload jobs pin their actions by
commit, since anything they run could mint a registry token. It
authenticates with the job's GitHub OIDC token for the registry, the way
espressif/upload-components-ci-action does, and uploads the archive that
was tested, with --allow-existing so a failed run can be rerun. A version
already on the registry is kept, so a rerun that packed different bytes
fails the registry check instead of shipping them. prepare now records the
commit and generator path in that archive at pack time, because compote
applies them only when it packs. Manual runs validate without uploading
unless dry_run is cleared. Pull requests that touch the workflow run it as a
dry run that stamps a release version at a throwaway local tag, because
workflow_dispatch cannot run before the workflow is on main, and the
nightly runs the same release-stamp checks every day, so a change that
would break them shows up before a real tag. Runs share a concurrency group
per tag or branch, so a run waiting for an approval never cancels another
tag's.

The manifest names production as esp-nn's registry, because the staging
registry records a dependency without one as its own, and staging has no
esp-nn; the component manager treats that as the same source as an
unnamed dependency on the default registry.

The README's runbook covers the one-time setup, in order: the protected
environment first, since GitHub creates an unprotected one as soon as a
workflow names it, then the staging namespace, then each registry's
trusted uploader, which names pytorch/executorch as the repository and
esp-component-registry as the environment, with no branch, because the
registry cannot pin a workflow and tag pushes carry no branch.

Authored with Claude Code.

@rascani
rascani added this pull request to stack #24713 November 5, 2026 23:48
@pytorch-bot

pytorch-bot Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24712

Note: Links to docs will display an error until the docs builds have been completed.

❌ 3 New Failures, 1 Unrelated Failure, 12 Unclassified Failures

As of commit 80d6309 with merge base 60a8b5f (image):

NEW FAILURES - The following jobs have failed:

UNCLASSIFIED FAILURES - DrCI could not classify the following jobs because the workflow did not run on the merge base. The failures may be pre-existing on trunk or introduced by this PR:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 5, 2026
@rascani

rascani commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@claude review effort=max

@claude

claude Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Claude finished @rascani's task in 39m 6s —— View job


Claude finished @rascani's task

This PR adds a tag-driven workflow that packs, tests, and uploads pytorch/executorch to the ESP Component Registry, and it needs changes before merge: one concurrency group, shared by every tag push and dispatch, can silently cancel a pending release run, and the jobs that hold the registry token install unpinned dependencies.

PR Review: #24712

Summary

publish-esp-idf.yml packs the component once for each RC tag, release tag, or dispatched respin, tests that archive on ESP-IDF v6.0.1 and v6.1, uploads it with a GitHub OIDC token behind the esp-component-registry environment (RCs go to staging; releases go to staging, then production), and checks that each registry serves the tested files.

Request Changes. The blocking problems:

  • The shared concurrency group can drop a pending release run.
  • The jobs that can publish install unpinned dependencies.
  • The release artifact carries an unpacked component that nothing reads.

Several release-only paths also run for the first time at a real tag. I could not check compote 3.1.2, the registry API, or linux_job_v3's defaults offline, so findings that depend on them are marked suspected.

Code Quality

  • Minor. examples/espressif/README.md:72-76 promises more than the version stamp allows:

    • "A release whose publishing failed in CI is published as a respin once the CI is fixed" holds only if the fix stays inside the respin allowlist (build_idf_component.sh:163-166). The publish path also depends on .ci/scripts/utils.sh (install_executorch), .github/workflows/_docker-image.yml, and the .ci/docker/ image. A respin that fixes any of those fails with "a respin may change only examples/espressif/ and its CI". Describe the allowlist here, or widen it.
    • "A failed run can be rerun" is true only for Re-run failed jobs, which reuses esp-idf-release. Re-run all jobs packs and tests a new archive, --allow-existing won't upload it, and the run then passes only if its files match the ones the registry already serves.

    Fix this →

  • Minor. examples/espressif/README.md:81-82 gates the staging approval on "the nightly matrix is green on the release commit". nightly-espressif-idf.yml has no push trigger, and its schedule runs only on the default branch, so it never runs on a tag or a release/X.Y commit unless someone dispatches it there. Say to dispatch it on the tag. It adds coverage the publish run lacks: the all-ops build, ESP-IDF master, and the macOS and Windows configure jobs. Fix this →

  • Minor. .ci/scripts/test_espressif_idf_nightly.sh:288: registry mode greps for a hard-coded smoke: PASS, while build mode reads the model from the build's sdkconfig (line 161). If sdkconfig.ci.default or the Kconfig default ever selects another model, verification fails after the upload even though the firmware passed. Fix this →

Infrastructure

  • Confirmed storage waste (small). .github/workflows/publish-esp-idf.yml:129 (upload-artifact: esp-idf-release) stores everything prepare writes, including component/executorch, the unpacked component. Its vendored sources alone are about 6 MB before compression.

    • Nothing that consumes esp-idf-release reads component/:
      • dry-run and both upload jobs read dist/ and version.
      • build reads mirror/, models/, and version.
      • Registry mode reads dist/, models/, and version.
    • The only reader of component/executorch is the nightly's test_espressif_idf_configure.py:97.
    • Footprint: one upload per RC, release, respin, dispatch, and push to a PR that touches the two paths, kept for the repository's default retention. That is small, but unused.
    • S3 doesn't fit here: the upload jobs run on ubuntu-22.04 and read the artifact with actions/download-artifact.
    • Fix: rm -r "${RUNNER_ARTIFACT_DIR}/component" in the package script after the version check. Leave prepare unchanged for the nightly.

    Fix this →

  • Minor. The release stamp's checks run only in publish-esp-idf.yml: the release-tag check (build_idf_component.sh:127-130) and the clean-tree and untracked-source checks (181-193). The nightly calls prepare with the default --allow-dev-stamp, which skips them, and the publish workflow's pull_request paths (lines 31-34) list only itself and the upload script.

    • The first time those checks see these changes is at a real tag, where they fail that tag's package job:
      • a change to build_idf_component.sh, idf_component_template/, test_espressif_idf_nightly.sh, esp_idf_mirror.py, or utils.sh
      • any repo change that leaves a tracked file modified after install_executorch or the model exports
    • Suggested fix: have the nightly's prepare job also force a local v$(cat version.txt) tag, as publish-esp-idf.yml:138 does, and then run build_idf_component.sh --version --version-suffix "". Main would then exercise the release checks every day. The alternative is widening paths.
    • Separately, a PR that changes only the upload script triggers both workflows (nightly-espressif-idf.yml:12, publish-esp-idf.yml:34), and both run the same production --dry-run.

    Fix this →

  • Minor, suspected. publish-esp-idf.yml:222-239 and 265-282: verify-staging and verify-production are the only linux_job_v3 calls in either workflow without ref:. If linux_job_v3 falls back to github.ref, a respin dispatched on release/X.Y is verified at whatever the branch tip is when the job starts. Verification waits for an approval, so that can be days later, and the registry-mode script and consumer project may then differ from the ones the archive was tested with. Pass the same ref: as the other jobs. Fix this →

  • Minor, suspected. publish-esp-idf.yml:169-170: dry-run validates only against production, so an RC is never validated against staging, its only target. Staging is already known to behave differently (see the comment at idf_component.yml.in:24). A rejection specific to staging, such as a missing pytorch namespace, would show up only after a reviewer approves upload-staging. Fix this →

Testing

  • Minor. Two paths have never run: registry-mode verification (test_espressif_idf_nightly.sh:213-294) and the upload script's token branch (upload_espressif_idf_component.sh:32-38).

    • Run 42295239738 skipped all four upload and verify jobs, and the nightly only calls --dry-run.
    • Their first run is therefore the first RC tag after merge. A bug in the served-vs-tested comparison (the managed_components/pytorch__executorch layout, or the excluded files) would fail that run after the RC is already on staging.
    • For a release, the fix can't land at the tag, so production would get X.Y.Z~1 instead.
    • Suggestion: also run the comparison in build mode, against the mirror-resolved managed_components/pytorch__executorch.

    Fix this →

  • Minor. examples/espressif/test_version_stamp.sh:103-109 covers two of the four new exclusions, .ci/scripts/*espressif* and publish-esp-idf.yml. It does not cover esp_idf_mirror.py or .github/workflows/*espressif*. No negative case commits a CI file outside the allowlist, such as .ci/scripts/utils.sh, so the test still passes if a pattern loses an exclusion or widens to all of .ci/scripts. Fix this →

API Design

  • Question, low confidence. examples/espressif/idf_component_template/idf_component.yml.in:24-27 pins espressif/esp-nn's registry_url to production in every published manifest, to work around staging. Two questions for the author:

    • Does a per-dependency registry_url bypass a user's IDF_COMPONENT_STORAGE_URL mirror or a non-default IDF_COMPONENT_REGISTRY_URL, as in offline or regional setups?
    • Does it conflict when the project or another component also requires espressif/esp-nn without a registry_url?

    The pre-upload matrix can't answer either, because esp_idf_mirror.py:64 writes "registry_url": None for every dependency. That also means its docstring's "exactly as it would from the registry" (lines 12-13) no longer holds. If either answer is yes, consider leaving the shipped manifest unchanged and adding the pin only where staging needs it, if the component manager honors it there. For example, add it to the consumer's generated manifest (idf_component_consumer/CMakeLists.txt:39) when ET_COMPONENT_REGISTRY_URL is set.

Security

  • Major. upload-staging and upload-production (publish-esp-idf.yml:198-220, 241-263) hold id-token: write in the esp-component-registry environment. The repository and that environment are all the registries' trusted uploader checks.

    • upload_espressif_idf_component.sh:30 installs idf-component-manager==3.1.2, but its transitive dependencies are neither pinned nor hashed.
    • The jobs use actions/checkout@v4, actions/download-artifact@v4, and actions/setup-python@v5 by mutable tag.
    • ACTIONS_ID_TOKEN_REQUEST_URL and ACTIONS_ID_TOKEN_REQUEST_TOKEN are set in every step. A compromised dependency release or a retagged action could therefore mint a registry token and publish a malicious pytorch/executorch version that users' version ranges resolve to.
    • The approval does not stop this, because that code runs inside the approved job.

    Install from a hash-locked requirements file with --require-hashes, and pin the actions in these two jobs by commit SHA, as docathon-sync-label.yml already does. Fix this →

  • Minor. The one-time setup that keeps every upload reviewed appears only in the PR description: the protected esp-component-registry environment (required reviewers, prevent self-review, deployment refs limited to v* and release/*), the staging namespace, and the trusted uploaders. README.md:78-83 describes the result but not the steps. If you reference an environment that doesn't exist, GitHub creates it with no protection rules. So if the trusted uploaders are registered before the environment is configured, the first tag push uploads with no review. Move the steps into the README, with the environment first. Fix this →

Thread Safety

  • Major. .github/workflows/publish-esp-idf.yml:36-39: every tag push and dispatch shares one concurrency group, …-publish. GitHub keeps at most one running and one pending run per group, and a newly queued run cancels the pending one regardless of cancel-in-progress.

    • The group applies before plan runs, so runs that publish nothing also take a slot: any v* tag the plan ignores, and every default dry_run dispatch.
    • A run waiting for an environment approval keeps the running slot for up to 30 days.
    • Example: v1.6.0-rc3 is waiting for its staging approval when v1.6.0 is pushed and goes pending. A later dry_run dispatch or v* tag then cancels the v1.6.0 run, and the only sign is a cancelled run.
    • The comment's guarantee, "two runs can't upload the same version", is already provided by --allow-existing plus registry mode's served-vs-tested check.

    Key the group by ref, as the nightly does: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}. Fix this →

Specific Comments

  • .ci/scripts/upload_espressif_idf_component.sh:27: the array named upload holds the optional --dry-run, so an empty upload (line 32) means a real upload. dry_run would read correctly.
  • .ci/scripts/test_espressif_idf_nightly.sh:24-27: the registry usage omits that the mode installs ESP-IDF v6.1, waits up to 15 minutes for the registry to list the version, and fails if the served files differ from <dir>'s archive.
  • .ci/scripts/test_espressif_idf_nightly.sh:181-182: "release" in the argument list reads as an ExecuTorch release, but the argument is the ESP-IDF version.
  • .github/workflows/publish-esp-idf.yml:140: "a dispatch names the suffix to version.txt's" is hard to parse. Perhaps "a dispatch appends its suffix to version.txt".
  • examples/espressif/idf_component_consumer/CMakeLists.txt:14: "from either" should say "from -D or the environment".
  • examples/espressif/idf_component_template/idf_component.yml.in:24: this comment ships in every published manifest, because the generator only substitutes the version. "Resolves unnamed registries to itself" is also unclear; perhaps "resolves dependencies without a registry_url from itself".
  • examples/espressif/README.md:60-61: says "builds and runs that archive in every idf_component_consumer configuration", but .build-test-rules.yml:10-12 disables the run for p4_rev_less_v3, which is only built.

Recommendation

Request Changes

Three findings block approval:

  • the shared concurrency group (Thread Safety)
  • the unpinned dependencies and actions in the two jobs that can publish (Security)
  • the confirmed component/ storage waste, which is small and a one-line fix

The rest are minor. The Infrastructure and Testing items are still worth fixing before merge, because the first RC tag is the first time those paths run.


@rascani

rascani commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@claude review effort=max

@claude

claude Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Claude finished @rascani's task in 37m 54s —— View job


This PR adds a tag-driven workflow that publishes pytorch/executorch to the ESP Component Registry, and it needs changes before merge because a shared concurrency group lets a later run cancel a pending release run, the two jobs that can mint the registry token install dependencies without hash checks, and the release artifact stores an unpacked component that no publish job reads.

PR Review: #24712

Summary

For each RC tag, release tag, or respin dispatch, publish-esp-idf.yml:

  • packs the component once;
  • builds and runs that archive on ESP-IDF v6.0.1 and v6.1;
  • uploads it with a GitHub OIDC token from the esp-component-registry environment (RCs go to staging; releases go to staging, then production);
  • checks that each registry serves the files that were tested.

Request Changes. Three findings block approval:

  • the shared concurrency group (Thread Safety);
  • the upload jobs installing dependencies without hash checks (Security);
  • the confirmed component/ storage waste (Infrastructure; small, and one line to fix).

Several release-only paths also run for the first time at a real tag.

I reviewed all nine changed files. Some things can't be checked offline:

  • how compote 3.1.2 treats a dependency's registry_url;
  • what --dry-run validates;
  • the registry's /api/ response;
  • the OIDC audience the trusted uploader expects.

Findings that depend on these are marked suspected or low confidence.

Code Quality

  • Minor. examples/espressif/README.md:81-83 doesn't say how to meet the nightly gate or who checks staging:

    • "Approve the staging upload once the nightly matrix is green on the release commit": nightly-espressif-idf.yml has no push trigger, and its schedule runs only on the default branch. It never runs on a tag or a release/X.Y commit unless someone dispatches it there. Say to dispatch it on the tag. That run also covers things the publish run doesn't: ESP-IDF master, the all-ops build, and the macOS and Windows configure jobs.
    • "Once the version from staging has been checked" doesn't say who checks it. The PR description makes Muse's sign-off the gate.

    Fix this →

  • Minor. .ci/scripts/test_espressif_idf_nightly.sh:288: registry mode greps for a hard-coded smoke: PASS, while build mode reads the model from the build's sdkconfig (line 161). The two agree today because the Kconfig default is smoke. If sdkconfig.ci.default or that default changes, the check after the upload fails even though the firmware passed. Fix this →

Infrastructure

  • Confirmed storage waste (small). .github/workflows/publish-esp-idf.yml:129 (upload-artifact: esp-idf-release) stores everything prepare writes under RUNNER_ARTIFACT_DIR. That includes component/executorch, the unpacked component; its vendored sources alone are about 5.6 MB uncompressed.

    • No job that downloads esp-idf-release reads component/:
      • dry-run and both uploads read dist/ and version;
      • build reads mirror/, models/, and version;
      • registry mode reads dist/, models/, and version.
    • The only reader of component/executorch is the nightly's test_espressif_idf_configure.py, and it reads the nightly's own artifact.
    • Footprint: an estimated 1–2 MB compressed per upload, based on the size of the vendored sources. There is one upload per RC, release, respin, manual dispatch, and push to a PR that touches the two paths, kept for the repository's default retention.
    • S3 doesn't fit, because the upload jobs run on ubuntu-22.04 and read the artifact with actions/download-artifact. Narrow the payload instead: add rm -r "${RUNNER_ARTIFACT_DIR}/component" at the end of the package script, and leave prepare unchanged for the nightly.

    Fix this →

  • Minor. publish-esp-idf.yml:31-34: the PR description says the rehearsal exists because "the release stamp's clean-tree check would otherwise first run on a real tag". Because paths lists only the workflow and the upload script, that is still true for most changes that can break the check:

    • build_idf_component.sh, idf_component_template/, test_espressif_idf_nightly.sh, setup.sh, the export scripts, or utils.sh;
    • anything that leaves a tracked file modified after install_executorch or the model exports.

    The nightly doesn't catch these either: prepare defaults to --allow-dev-stamp, which skips the release-stamp checks (build_idf_component.sh:126-131 and 181-193), and the other CI callers of the generator pass it too. The failure then appears in the tag's package job and needs a new RC or a respin. A respin works only if the fix falls inside the allowlist (see below).

    Suggested fix: after prepare in the nightly (nightly-espressif-idf.yml:47), force a local v$(cat version.txt) tag as publish-esp-idf.yml:138 does, then run build_idf_component.sh --version --version-suffix "". Main would then run the checks every day. The alternative is to widen paths. Adding schedule: to this workflow instead would put the daily run in the shared concurrency group (see Thread Safety).

    Fix this →

  • Minor. README.md:69-73 says "A release whose publishing failed in CI is published as a respin once the CI is fixed". The respin allowlist (build_idf_component.sh:163-166) is narrower:

    • It covers examples/espressif/, the *espressif* files under .ci/scripts/ and .github/workflows/, esp_idf_mirror.py, and this workflow.
    • Publishing also runs .ci/scripts/utils.sh (install_executorch, publish-esp-idf.yml:134-135), _docker-image.yml (line 111), and the .ci/docker/ image. A respin that fixes any of these is refused.

    Widening the allowlist to .ci/docker/ would let toolchain and dependency changes into a "packaging-only" respin. So either list the exact allowlist in the README and in the error message at line 168 (where "its CI" is vague), or widen it deliberately, with a test case for each added path. Fix this →

  • Minor, suspected. publish-esp-idf.yml:169-170: dry-run validates only against production, but an RC uploads only to staging, and staging behaves differently from production (see the comment at idf_component.yml.in:24). A rejection specific to staging would show up only after a reviewer approves upload-staging. Validate against staging as well. Fix this →

Testing

  • Minor. Registry mode (test_espressif_idf_nightly.sh:213-294) and the upload script's token branch (upload_espressif_idf_component.sh:32-38) first run after the first real upload. The rehearsal skips the upload and verify jobs, and the nightly runs only --dry-run.

    • A defect in either one fails that RC after it is already on staging. Examples: the OIDC audience, the managed_components/pytorch__executorch layout the comparison assumes, or the files it excludes.
    • For a release tag, the fixed script can't run at the tag, so production gets X.Y.Z~1 and never X.Y.Z.

    Also run the comparison in build mode, after each shard's builds, against the managed_components/pytorch__executorch that the mirror resolves. Its assumptions are then checked on every PR and every nightly. Fix this →

  • Minor. examples/espressif/test_version_stamp.sh:104-109 exercises two of the four new exclusions: .ci/scripts/*espressif* and publish-esp-idf.yml. Nothing commits esp_idf_mirror.py or a .github/workflows/*espressif* file. No case commits a CI file outside the allowlist, such as .ci/scripts/utils.sh. The test therefore still passes if an exclusion is dropped, or if one is widened to all of .ci/. Fix this →

API Design

  • Question, low confidence. examples/espressif/idf_component_template/idf_component.yml.in:24-27 pins the registry_url of espressif/esp-nn to production in every published manifest, to work around staging. Two questions:

    • Does an explicit registry_url bypass a user's IDF_COMPONENT_STORAGE_URL mirror or a non-default IDF_COMPONENT_REGISTRY_URL, as in offline or regional setups?
    • Does it conflict with a project or another component that requires espressif/esp-nn without a registry_url?

    The pre-upload matrix can't answer either question: esp_idf_mirror.py:64 writes "registry_url": None for every dependency. As a result, its docstring's claim to resolve "exactly as it would from the registry" (lines 12-13) no longer holds.

    If either answer is yes, keep the shipped manifest unpinned and add the pin only where staging needs it, provided the component manager honors it there. One candidate is the consumer manifest that idf_component_consumer/CMakeLists.txt:39 generates when ET_COMPONENT_REGISTRY_URL is set.

Security

  • Major. upload-staging and upload-production (publish-esp-idf.yml:198-220, 241-263) can mint a token the registries accept. The trusted uploader checks only this repository and the esp-component-registry environment, and ACTIONS_ID_TOKEN_REQUEST_* is available to every step.

    • upload_espressif_idf_component.sh:30 installs idf-component-manager==3.1.2, but its transitive dependencies are neither pinned nor hash-checked.
    • A malicious release of any of those dependencies would run in that job. It could publish a pytorch/executorch version that users' version ranges resolve to.
    • The approval doesn't help, because that code runs inside the approved job.

    Install from a hash-locked requirements file with --require-hashes, named so the respin allowlist covers it. Fix this →

  • Minor. The one-time setup that makes every upload reviewed appears only in the PR description:

    • the protected esp-component-registry environment: required reviewers, self-review prevented, no admin bypass, and deployments limited to v* tags and release/* branches;
    • the staging namespace;
    • the trusted uploaders.

    README.md:78-83 describes the end state but not the steps or their order. The order matters: GitHub creates a missing environment, with no protection rules, as soon as a workflow references it. If a trusted uploader is registered before the environment is configured, the next tag push uploads without review. Move the steps into the README, with the environment first. Fix this →

Thread Safety

  • Major. publish-esp-idf.yml:36-39: every tag push and dispatch shares the group …-publish. GitHub keeps one running and one pending run per group, and a newly queued run cancels the pending one, whatever cancel-in-progress says.

    • The group applies before plan runs, so runs that publish nothing also take a slot: v* tags the plan ignores, and every default dry_run dispatch.
    • A run waiting for an environment approval holds the running slot for up to 30 days. The runbook makes approvals wait for the nightly and for Muse's sign-off.
    • Example: v1.6.0-rc3 is waiting for its staging approval when v1.6.0 is pushed and goes pending. A later dry-run dispatch then cancels the release run, and the only trace is a cancelled run.
    • The PR description's reason, "two runs never upload the same version at once", is already covered elsewhere: the registry keeps one archive per version, and registry mode fails any run whose archive differs from the one served.

    Key the group by ref instead: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}. Runs for the same tag or branch still run one at a time. Fix this →

Specific Comments

  • .ci/scripts/upload_espressif_idf_component.sh:27: the array upload holds the optional --dry-run, so an empty upload (line 32) means a real upload. dry_run would read correctly.
  • .ci/scripts/test_espressif_idf_nightly.sh:24-27: the registry usage text leaves out two behaviors. The mode waits up to 15 minutes for the registry to list the version, and it fails if the served files differ from the archive in <dir>.
  • .ci/scripts/test_espressif_idf_nightly.sh:232: ${storage} keeps any trailing slash from components_base_url, while line 224 strips one from registry_url.
  • .github/workflows/publish-esp-idf.yml:140: "a dispatch names the suffix to version.txt's" doesn't parse. Perhaps "a dispatch appends its suffix to version.txt".
  • .github/workflows/publish-esp-idf.yml: naming it publish-espressif-idf.yml would match nightly-espressif-idf.yml and the allowlist pattern .github/workflows/*espressif*. That would remove the special case at build_idf_component.sh:166.
  • examples/espressif/idf_component_consumer/CMakeLists.txt:14: "from either" should say "from -D or the environment".
  • examples/espressif/idf_component_consumer/CMakeLists.txt:28: registry is set in the project scope, which every component inherits. The file's other variables use the ET_ prefix.
  • examples/espressif/idf_component_template/idf_component.yml.in:24: "resolves unnamed registries to itself" is unclear. The PR description's wording, "records dependencies without one as its own", is clearer. This comment also uses #, while the file's other comments use ##.

Recommendation

Request Changes

These findings block approval:

  • the shared concurrency group (Thread Safety);
  • the dependencies installed without hash checks in the two jobs that can publish (Security);
  • the confirmed component/ storage waste, which takes one line to fix.

The rest are minor. The Infrastructure and Testing items are still worth fixing before merge, because the first RC tag is where those paths first run.

publish-esp-idf.yml publishes pytorch/executorch to the ESP Component
Registry. An RC tag vX.Y.Z-rcN publishes X.Y.Z-rc.N to the staging
registry, a release tag vX.Y.Z publishes X.Y.Z to staging and then
production, and a manual run on release/X.Y publishes a respin, X.Y.Z~N.
Other v* tags publish nothing.

The workflow packs the component once, under the generator's release-stamp
checks, and asserts the version matches the tag. Before anything is
uploaded, it builds and runs that archive in every consumer configuration
on ESP-IDF 6.0.1 and 6.1 and validates it with both registries. Each upload
waits for a reviewer of the esp-component-registry environment. After each
upload, the new registry mode of test_espressif_idf_nightly.sh polls until
the registry lists the version, builds the consumer with pytorch/executorch
resolved from that registry, checks that the registry served the tested
archive's files (apart from the examples' manifests, which the registry
rewrites to drop override_path), and runs it on the emulator. Production
waits for that check on staging. The nightly's build mode runs the same
file check against the mirror, so it is exercised before the first upload.
The consumer takes the registry as -DET_COMPONENT_REGISTRY_URL.

Uploads go through .ci/scripts/upload_espressif_idf_component.sh, which the
nightly's dry-run job now shares. It installs the component manager from a
hash-locked requirements file, and the two upload jobs pin their actions by
commit, since anything they run could mint a registry token. It
authenticates with the job's GitHub OIDC token for the registry, the way
espressif/upload-components-ci-action does, and uploads the archive that
was tested, with --allow-existing so a failed run can be rerun. A version
already on the registry is kept, so a rerun that packed different bytes
fails the registry check instead of shipping them. prepare now records the
commit and generator path in that archive at pack time, because compote
applies them only when it packs. Manual runs validate without uploading
unless dry_run is cleared. Pull requests that touch the workflow run it as a
dry run that stamps a release version at a throwaway local tag, because
workflow_dispatch cannot run before the workflow is on main, and the
nightly runs the same release-stamp checks every day, so a change that
would break them shows up before a real tag. Runs share a concurrency group
per tag or branch, so a run waiting for an approval never cancels another
tag's.

The manifest names production as esp-nn's registry, because the staging
registry records a dependency without one as its own, and staging has no
esp-nn; the component manager treats that as the same source as an
unnamed dependency on the default registry.

The README's runbook covers the one-time setup, in order: the protected
environment first, since GitHub creates an unprotected one as soon as a
workflow names it, then the staging namespace, then each registry's
trusted uploader, which names pytorch/executorch as the repository and
esp-component-registry as the environment, with no branch, because the
registry cannot pin a workflow and tag pushes carry no branch.

Authored with Claude Code.

This branch was previously deployed

1 inactive deployment
cadence — 80d63096 Deployed Nov 7, 2026 by rascani via hifi-op-test / hifi4 #37203
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant