fix(mutate): modify --add-tag drops the WHOLE FILE when a tag needs quoting (REQ-287, P0 data loss) - #765
Merged
Merged
Conversation
…ag needs quoting (REQ-287, P0 data loss)
Mutation-path stress test (user report: "we very often get into failures"
modifying artifacts) found a P0 silent data-loss bug. yaml_edit.rs re-emitted
the tag flow list via `current_tags.join(", ")` WITHOUT quoting individual
tags — unlike the hardened setters (title/status/field, #687) and the `add`
path (mutate.rs). A tag carrying a YAML flow indicator breaks: a legitimately
quoted `"release: v1.0"`, read back from the store, was re-emitted bare
(`release: v1.0`), turning the flow list into a map so the WHOLE FILE failed to
parse — silently dropping EVERY artifact in it — while `modify` exited 0
reporting success.
Reproduced: 2 artifacts -> a benign, unrelated `--add-tag simpletag` -> 0
artifacts loadable. Same tag-mutation fragility class as #625, on the quoting
axis.
Fix: quote each tag via `yaml_quote_inline_scalar` before joining, mirroring
the hardened `add` path. Also closes a silent-split variant (`--add-tag "a,b"`
stored two tags). Regression test asserts both artifacts survive an unrelated
--add-tag and the pre-existing quoted tag stays quoted. All 38 yaml_edit unit
tests still pass.
Fixes: REQ-287
Refs: REQ-203
📐 Rivet artifact delta
Graphgraph LR
REQ_287["REQ-287"]:::added
classDef added fill:#d4edda,stroke:#28a745,color:#155724
classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Added
Posted by |
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: f643607 | Previous: e1a539f | Ratio |
|---|---|---|---|
store_insert/100 |
87080 ns/iter (± 1131) |
67561 ns/iter (± 2504) |
1.29 |
store_lookup/100 |
2139 ns/iter (± 7) |
1441 ns/iter (± 56) |
1.48 |
store_lookup/1000 |
26780 ns/iter (± 740) |
16101 ns/iter (± 513) |
1.66 |
store_lookup/10000 |
388257 ns/iter (± 1955) |
260752 ns/iter (± 5937) |
1.49 |
store_by_type/100 |
96 ns/iter (± 0) |
68 ns/iter (± 1) |
1.41 |
store_by_type/1000 |
96 ns/iter (± 0) |
66 ns/iter (± 1) |
1.45 |
store_by_type/10000 |
96 ns/iter (± 0) |
69 ns/iter (± 3) |
1.39 |
schema_load_and_merge |
1523862 ns/iter (± 25304) |
1193878 ns/iter (± 46244) |
1.28 |
validate/100 |
480866 ns/iter (± 2271) |
378790 ns/iter (± 17921) |
1.27 |
validate/1000 |
15076684 ns/iter (± 113051) |
12070154 ns/iter (± 340350) |
1.25 |
validate/10000 |
1247387288 ns/iter (± 11110283) |
831299935 ns/iter (± 15730153) |
1.50 |
traceability_matrix/100 |
4466 ns/iter (± 27) |
3149 ns/iter (± 112) |
1.42 |
traceability_matrix/1000 |
64051 ns/iter (± 204) |
33943 ns/iter (± 1312) |
1.89 |
diff/100 |
61516 ns/iter (± 401) |
48775 ns/iter (± 1179) |
1.26 |
diff/1000 |
699381 ns/iter (± 3519) |
501545 ns/iter (± 12268) |
1.39 |
diff/10000 |
7712715 ns/iter (± 338666) |
6139888 ns/iter (± 159935) |
1.26 |
query/100 |
1179 ns/iter (± 3) |
780 ns/iter (± 17) |
1.51 |
query/1000 |
15237 ns/iter (± 94) |
9884 ns/iter (± 463) |
1.54 |
query/10000 |
332404 ns/iter (± 1501) |
258837 ns/iter (± 4830) |
1.28 |
document_parse/10 |
24101 ns/iter (± 182) |
18495 ns/iter (± 554) |
1.30 |
document_parse/100 |
168472 ns/iter (± 1793) |
128852 ns/iter (± 4536) |
1.31 |
document_parse/1000 |
1581448 ns/iter (± 11809) |
1182878 ns/iter (± 32105) |
1.34 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
avrabe
added a commit
that referenced
this pull request
Aug 5, 2026
… + security (#766) Bumps workspace 0.31.0 -> 0.32.0 (rivet-core, rivet-cli, etch) + vscode-rivet. Scope (all implemented + merged): - REQ-287 P0 data-loss fix: modify --add-tag no longer drops the whole file (#765) - REQ-284 embedded-docs truth-drift: 8 snippet/claim fixes (#764) - Security: wasmtime 45->47, RUSTSEC-2026-0222 cleared (#762) Trace: skip
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
P0 silent data loss found by a mutation-path stress test (your report: modifying artifacts "very often" fails).
The bug
rivet-core/src/yaml_edit.rsre-emitted the tag flow list viacurrent_tags.join(", ")without quoting individual tags — unlike the hardened setters (title/status/field, #687) and theaddpath. Any tag with a YAML flow indicator (:,,,[…) breaks.Worst case — corruption from a benign operation: an artifact with a legitimately-quoted tag
"release: v1.0", hit by an unrelated--add-tag simpletag, gets the quotes stripped →[alpha, release: v1.0, …]→ the flow list becomes a map → the whole file fails to parse → every artifact in it vanishes — whilemodifyexits 0 reporting "modified".Reproduced: 2 artifacts → benign
--add-tag→ 0 loadable.The fix
Quote each tag via
yaml_quote_inline_scalarbefore joining (one-line change mirroring the already-hardenedaddpath). Also closes a silent-split variant (--add-tag "a,b"stored two tags).Verification
"release: v1.0"quoted.verifies REQ-287); all 38yaml_editunit tests still pass.--all-targets -D warnings0, fmt clean.Context
Same tag-mutation fragility class as #625, on the quoting axis. The broader mutation-path audit found only this live defect —
set-field/link/unlink/sql UPDATE/addall preserved rich content faithfully (the surgicalyaml_edit.rseditor was already hardened elsewhere). Part of v0.32.Fixes: REQ-287
🤖 Generated with Claude Code