Skip to content

fix(cli): --set-release "" clears the field so an artifact goes to backlog (#1115) - #1120

Open
avrabe wants to merge 2 commits into
mainfrom
fix/issue-1115-clear-release-to-backlog
Open

avrabe wants to merge 2 commits into
mainfrom
fix/issue-1115-clear-release-to-backlog

Conversation

@avrabe

@avrabe avrabe commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Closes #1115.

What changed

rivet modify <ID> --set-release "" now removes the release: key from the artifact's mapping instead of writing release: "". The artifact becomes unassigned (backlog) in the sense rivet modify --help documents.

Why release: "" wasn't backlog

The old code path was yaml_quote_inline_scalar(new_release) → set_field(id, "release", &quoted). yaml_plain_scalar_needs_quoting returns true for an empty string by design, so an empty value quoted to "" and was written verbatim as release: "". That left a quoted-empty value in the mapping, which rivet query '(= release "")' still matches as a value, not as absence. The other apparent escape hatch — rivet release move <ID> <VERSION> — requires a version, so it had no backlog target either. The only working path was hand-editing the YAML, which is the escape hatch rivet's own guidance tells callers to avoid.

Scope — Fix 1 only

Issue #1115 sketched two suggested fixes:

  1. --set-release "" (or --clear-release) removes the key. This PR lands that.
  2. rivet release move <ID> --backlog, so the scope decision is a logged transition. Remains an open follow-up on friction: no way to move an artifact back to backlog — --set-release "" writes release: "" and release move needs a version #1115.

Keeping the diff focused on Fix 1 because it unblocks the reporter's immediate case (clearing six draft site artifacts out of a release they had blocked three times running on pulseengine.eu) with the smallest surface.

How each ask in #1115 is addressed

  • "--set-release "" (or --clear-release) removes the key." — Done. When params.set_release is Some(""), modify_artifact_yaml now calls editor.remove_field(id, "release") instead of set_field. The release: line is gone from the output; the artifact becomes an unassigned mapping entry.
  • CLI help documents the clear-to-backlog behaviour. — Done at rivet-cli/src/main.rs:1317-1321: the --set-release doc now reads "pass an empty value (--set-release "") to clear the release and move the artifact back to backlog."
  • rivet release move <ID> --backlog. — Deferred; remains open on friction: no way to move an artifact back to backlog — --set-release "" writes release: "" and release move needs a version #1115 (Fix 2).

How the fix doesn't drag neighbours

remove_field reuses the same field_block_end extent helper the write path (set_field) uses, so a hypothetical multi-line release value (block scalar, nested block mapping) would be removed as a whole and would not orphan deeper-indented children into the next field's scope. This is the mirror of the set_field_inserts_after_trailing_provenance_block and set_field_inserts_after_folded_scalar_with_blank_line regression guards on the write path (#613 / #618).

Test plan

  • cargo test -p rivet-core --lib yaml_edit:: — 55 tests pass, including the three new ones (set_release_to_empty_clears_the_field, set_release_to_empty_on_backlog_artifact_is_a_noop, set_release_to_empty_preserves_following_provenance_block).
  • cargo test -p rivet-cli --test cli_commands — 247 tests pass (no regression in the end-to-end rivet modify surface).
  • cargo fmt --all -- --check clean.
  • cargo clippy -p rivet-core --lib -- -D warnings clean.
  • cargo clippy -p rivet-cli --bins -- -D warnings clean.
  • CI green on this branch.

Negative controls baked into the new tests

  • set_release_to_empty_clears_the_field asserts !out.contains("release:") and that the key is absent from the parsed mapping. If a future refactor regressed to writing release: with an empty value (either quoted or bare), both halves would fail — matching the (= release "") matching behaviour cited in the issue.
  • set_release_to_empty_preserves_following_provenance_block plants a provenance: block right after the release: line; if remove_field mis-calculated the field extent and drained the next field too, the provenance.created-by assertion would fail.

Generated by Claude Code

…to backlog (#1115)

The documented backlog state (`rivet modify --help`: "An unassigned
artifact is backlog") was unreachable from the CLI. The only path to it
was `--set-release ""`, which wrote `release: ""` verbatim through
`yaml_quote_inline_scalar` (empty strings quote to the double-quoted form
by design, cf. `yaml_plain_scalar_needs_quoting`), leaving a quoted empty
value that `rivet query '(= release "")'` still matches as a value. The
other candidate — `rivet release move <ID> --backlog` — requires a
version, so it was no help either. Hand-editing the YAML was the
workaround, which is the hand-edit-then-validate escape hatch rivet
tells agents not to use.

Fix 1 of the three the issue sketched: when `--set-release` is passed
with an empty value, remove the `release:` key from the artifact's
mapping entirely instead of writing it. A no-op clear on an artifact
that is already backlog still succeeds. Fix 2 (a `rivet release move
<ID> --backlog` verb) and Fix 3 (a `--clear-release` flag spelt out as
its own option) remain open follow-ups on the same issue.

The `remove_field` helper reuses `field_block_end`, the same extent
helper the write path uses — so a hypothetical multi-line release value
(block scalar, nested block mapping) would be removed as a whole and
would not orphan deeper-indented children into the next field's scope,
mirroring the regression guards for #613/#618 on the write path.

Tests:
- set_release_to_empty_clears_the_field — the field is gone (not
  "release: """ and not "release:" with a blank value, both of which
  `(= release "")` still matches).
- set_release_to_empty_on_backlog_artifact_is_a_noop — clearing a
  release that was never set is not an error.
- set_release_to_empty_preserves_following_provenance_block — the
  mirror of `set_field_inserts_after_trailing_provenance_block`:
  removing `release` must not drain the nested `provenance:` block
  that follows it.

Fixes: REQ-034

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gifkRw7MTUK8ayW4Bighz
@temper-pulseengine

temper-pulseengine Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Outdated — this review was for an earlier revision. See the latest review below.

Automated review for PR #1120

pulseengine/rivet:fix/issue-1115-clear-release-to-backlog → pulseengine/rivet:main

Verdict: 💬 Comment

Summary: The provided code changes the behavior of setting a release field in an artifact YAML file. The old behavior wrote release: "" as a string, which caused issues with querying for artifacts that were unassigned but had an empty release value. The new behavior removes the release: field entirely when the new value is the empty string, ensuring that the artifact is correctly marked as unassigned.

Findings: 0 mechanical (rivet) · 2 from local AI model.

Findings (2):

  1. src/mutate/mod.rs:964

                let quoted = yaml_quote_inline_scalar(new_release);
    

    The old behavior wrote release: "" as a string, which caused issues with querying for artifacts that were unassigned but had an empty release value.

  2. src/mutate/mod.rs:964

                let quoted = yaml_quote_inline_scalar(new_release);
    

    The new behavior removes the release: field entirely when the new value is the empty string, ensuring that the artifact is correctly marked as unassigned.


Generated by a local AI model and post-validated against a strict JSON contract. Each finding includes the verbatim line being criticised — verify by reading the file at the cited location.

Reviewed at 773fd8e

@github-actions

Copy link
Copy Markdown

📐 Rivet artifact delta

No artifact changes in this PR. Code-only changes (renderer, CLI wiring, tests) don't touch the artifact graph.

…rom #1114)

Playwright E2E failed on this PR's first CI run with the same install
error as #1109: `ubuntu-latest` now resolves to Ubuntu 26.04 and the
pinned Playwright release fails with
"Playwright does not support chromium on ubuntu26.04-x64". The diff is
`.github/workflows/ci.yml:558` only — Rust is untouched.

Ported verbatim from PR #1114 so the fix lands here now rather than
waiting on that PR to merge: it no-ops once main carries it, and until
then it keeps this PR's Playwright job honest (same approach the posture
calls for on a failure whose fix already exists). The pin stays until
Playwright publishes a release that supports 26.04.

Refs: FEAT-001

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gifkRw7MTUK8ayW4Bighz

avrabe commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Playwright E2E failing on 773fd8e — standing down on this one as it is not this PR's.

The install step dies before any test body runs with:

Error: ERROR: Playwright does not support chromium on ubuntu26.04-x64

Same shape as #1109: GitHub's ubuntu-latest now resolves to Ubuntu 26.04 and the pinned Playwright release does not support it. The diff on this PR touches no tests/playwright/** and no workflow file, so this is not a regression from the --set-release "" change.

PR #1114 already has the fix (pin runs-on: ubuntu-24.04 on the Playwright job). Rather than wait on it to merge, I've ported the one-line change into this PR as 9c64d05 so CI can clear the install step here now — it no-ops once main carries #1114.

Not spending my one re-run on the pre-port commit (773fd8e); the new push supersedes it.


Generated by Claude Code

@temper-pulseengine

Copy link
Copy Markdown
Contributor

Automated review for PR #1120

pulseengine/rivet:fix/issue-1115-clear-release-to-backlog → pulseengine/rivet:main

Verdict: 💬 Comment

Summary: The pull request is approved and has passed all the required checks.

Findings: 0 mechanical (rivet) · 1 from local AI model.

Findings (1):

  1. .github/workflows/ci.yml:552
    # Stays on hosted ubuntu: `npx playwright install --with-deps` runs
    
    The workflow specifies that it should run on a hosted Ubuntu environment, which is appropriate for installing Playwright dependencies.

Generated by a local AI model and post-validated against a strict JSON contract. Each finding includes the verbatim line being criticised — verify by reading the file at the cited location.

Reviewed at 9c64d05

@codecov

codecov Bot commented Oct 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

avrabe commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

CI green on 9c64d05. All 25 non-skipped check runs completed success:

  • Aggregate: CI Gate ✅
  • Core: Clippy ✅, Format ✅, Test ✅, Proptest ✅, Miri (safety) ✅, MSRV ✅, Semver ✅, WASM ✅
  • E2E: Playwright ✅ (install step clears on the ported ubuntu-24.04 pin)
  • Supply chain: Cargo Deny ✅, Supply Chain (cargo-vet) ✅, Security Audit (RustSec) ✅
  • Proofs: Rocq ✅, Verus ✅
  • Other: Docs Check ✅, Zola export smoke ✅, Code Coverage ✅, Criterion Benchmarks ✅, Mutation Testing (rivet-core PR-diff) ✅, Mutation Testing (rivet-cli) ✅, VS Code Extension ✅, codecov/patch ✅ (all modified lines covered), Schema version bump ✅, Detect changed areas ✅, Artifact delta ✅

Skipped as expected (non-triggers for this diff): Mutation Testing matrix, Traceability hosted fallback, Fuzz Testing, Miri (full, nightly), Publish Test Results on Release.

The PR is waiting on reviewers.


Generated by Claude Code

This branch has not been deployed

No deployments
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.

friction: no way to move an artifact back to backlog — --set-release "" writes release: "" and release move needs a version

2 participants