Skip to content

ci: fix panic reading rust-step side effects in source release job - #4239

Merged
Ben Hillis (benhillis) merged 1 commit into
microsoft:mainfrom
benhillis:user/benhill/openvmm-source-release-sideeffect
Aug 13, 2026
Merged

ci: fix panic reading rust-step side effects in source release job#4239
Ben Hillis (benhillis) merged 1 commit into
microsoft:mainfrom
benhillis:user/benhill/openvmm-source-release-sideeffect

Conversation

@benhillis

@benhillis Ben Hillis (benhillis) commented Aug 13, 2026

Copy link
Copy Markdown
Member

The first real dispatch of the OpenVMM source release workflow (run 31733701937) aborted in the pin source release tag step:

thread 'main' panicked at flowey/flowey_core/src/node.rs:1800:32: db is missing var auto_se:flowey_lib_hvlite::_jobs::publish_openvmm_gh_release:13

Root cause

The SideEffect that ctx.emit_rust_step hands back is created via new_prefixed_var("auto_se") and claimed for write on behalf of the step, but nothing ever writes it to the runtime var db. It is the claim in a downstream step that creates the dependency edge in the DAG; the value is never meant to be read. Calling rt.read() on one therefore panics in RuntimeVarDb::get_var.

The established idiom elsewhere in the tree (e.g. download_openvmm_vmm_tests_artifacts) is to claim the side effect and never read it.

Fix

  • publish_openvmm_gh_release: claim no_existing_release without reading it.
  • publish_gh_release: drop the equivalent rt.read(prerequisite) loop. prerequisites is already claimed in GhReleaseParams::claim, so ordering is unaffected. This path had never executed — the only prior caller passes Vec::new() — and would have hit the identical panic one step later, since tag_is_pinned is also a rust-step side effect.

Comments added at both sites so this doesn't get reintroduced.

Validation

cargo clippy -p flowey_lib_hvlite -p flowey_lib_common --all-targets and cargo xtask fmt both clean; no generated workflow YAML changes.

The first dispatch of the OpenVMM source release workflow aborted in
`pin source release tag` with::

    db is missing var auto_se:flowey_lib_hvlite::_jobs::publish_openvmm_gh_release:13

The side effect that `emit_rust_step` hands back is only ever claimed,
never written to the runtime var db, so `rt.read()` on one panics. It
is the claim that creates the dependency edge, so ordering still holds
without the read.

Drop the read in the pin step, and drop the equivalent read of
`prerequisites` in the shared publisher, which would have hit the same
panic one step later once a non-empty prerequisite list was passed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2f283c1b-ae06-4449-9ac9-897cd279f65e
Copilot AI lite review requested due to automatic review settings August 13, 2026 19:34
@benhillis
Ben Hillis (benhillis) requested a review from a team as a code owner August 13, 2026 19:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a runtime panic in Flowey-based CI/release pipelines by ensuring rust-step SideEffect vars are used strictly for dependency ordering (claimed) and never read at runtime.

Changes:

  • Stop reading a rust-step SideEffect in the OpenVMM source release job; claim it only to preserve DAG ordering.
  • Remove runtime rt.read() of prerequisites in GitHub release publishing, relying on GhReleaseParams::claim for ordering.
  • Add clarifying comments documenting the “claim-only, never read” contract for rust-step side effects.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
flowey/flowey_lib_hvlite/src/_jobs/publish_openvmm_gh_release.rs Avoids rt.read() on a rust-step side effect that is never written to the var DB, preventing the observed panic.
flowey/flowey_lib_common/src/publish_gh_release.rs Documents and enforces claim-only prerequisites ordering by removing runtime reads that could panic when prerequisites are rust-step side effects.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@benhillis
Ben Hillis (benhillis) enabled auto-merge (squash) August 13, 2026 19:50
@benhillis
Ben Hillis (benhillis) merged commit e01a057 into microsoft:main Aug 13, 2026
68 of 70 checks passed
@github-actions

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants