Repository navigation
feat(cargo-anvil): support managed TOML array entries - #235
Evgenii (Vaiz) wants to merge 17 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Adoption can modify content behind malformed sentinels, and one refusal provides misleading recovery guidance.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Adds managed TOML array-entry regions while preserving repository-owned array structure.
Changes:
- Adds catalog APIs and selector checksum metadata.
- Implements scaffolding, adoption, validation, updates, and retirement.
- Documents and tests the new behavior.
| File | Description |
|---|---|
crates/cargo-anvil/src/run.rs |
Dispatches array-region planning. |
crates/cargo-anvil/src/region.rs |
Exposes canonical TOML value rendering. |
crates/cargo-anvil/src/lib.rs |
Exports the new specification type. |
crates/cargo-anvil/src/emit/toml_array_region.rs |
Implements and tests array-entry management. |
crates/cargo-anvil/src/emit/mod.rs |
Registers the new emitter module. |
crates/cargo-anvil/src/emit/managed_region.rs |
Generalizes region planning with custom splicing. |
crates/cargo-anvil/src/catalog/mod.rs |
Re-exports array-region specifications. |
crates/cargo-anvil/src/catalog/builder.rs |
Stores, validates, and checksums selectors. |
crates/cargo-anvil/src/catalog/artifact.rs |
Defines TomlArrayRegionSpec. |
crates/cargo-anvil/docs/design/updates.md |
Documents update and ownership semantics. |
crates/cargo-anvil/docs/design/extensibility.md |
Documents the public API and behavior. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #235 +/- ##
======================================
Coverage 98.9% 98.9%
======================================
Files 304 305 +1
Lines 44436 45162 +726
======================================
+ Hits 43975 44698 +723
- Misses 461 464 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
martintmk
added
the
human-review-required
|
Posted by an AI agent Human review is required at 132cb4d: this new TOML array-entry engine and public catalog API change ownership, adoption, and retirement behavior beyond the bounded auto-approval fast path. An existing review finding also flags adoption through an unpaired managed sentinel. Please review the feature and resolve that finding before proceeding. No automated approval was issued. |
132cb4d to
c74c120
Compare
|
Posted by an AI agent Review preparation has started for #235 at pinned head The submitted review will contain the result. |
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Warning: Incomplete review
I reviewed the public contract, behavior, tests, performance, naming, telemetry, resilience, and documentation. I could not check:
- Public API surface: matching base and head cargo public-api outputs were unavailable, and generating them was not permitted.
No overall verdict is given. The comments below come from the reviewed areas. Tests, builds, and benchmarks were not run; the performance observations are source-based, not measured. CI was pending during analysis. Earlier discussions and the newer Copilot partial-overlap finding are not repeated here.
c74c120 to
8af6ed7
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The extensive source-span and ownership logic warrants final human validation despite strong test coverage.
0 open findings
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🟡 Changes recommended
The container design documentation references a recipe that is not generated.
1 open finding
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
The container design documents a nonexistent anvil-container-shell command.
1 open finding
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🟡 Changes recommended
Partial-apply recovery can permanently block a pristine region retirement when a live array selector is present.
1 open finding
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Add supported array-region selector metadata to CatalogBuilder without changing the public Artifact enum. Create missing scaffolding and adopt or insert generated entries while preserving repository-owned array content. Reuse region checksum, edited-body, dry-run and retirement policy. Cover array placement, ownership overlap, lifecycle and source preservation with deterministic in-memory regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validate every host sentinel before adoption, protect owned separators, and refuse compound entries whose interior comments would be lost. Scaffold missing arrays outside managed parent bodies while preserving original line endings and string bytes. Provide actionable ownership and selector migration advice, document editable catalog inputs, and cover production planning alongside exact-output emitter regressions. Reuse the documented checksum separator and reserve body capacity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Guard neighboring updates and retirements against delimiter loss and selector rebinding, including retirement-aware migrations and region relocation. Preserve granular refusals and projected lock provenance. Add deterministic production-plan regressions, streamline adoption, and complete catalog docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Correct implementation-plan relative paths and replace stale section references with current heading links. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document the existing no-argument anvil-container recipe instead of a nonexistent shell recipe. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Parse and scaffold against byte-offset-preserving retirement projections while keeping raw marker, checksum and ownership checks. Move complete new scaffold deltas outside existing managed regions, including newly inserted parent headers. Reuse resolved cached hosts throughout region planning so deterministic in-memory fixtures never consult the filesystem under Miri isolation. Cover both catalog orders for initial, update and in-sync arrays; preserve edited/untracked orphans and LF/CRLF Unicode host bytes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…memory Extract the existing enumeration-to-selection boundary so entry failures are covered without filesystem fixtures, including failure after an exact match. Keep syscall handling, full enumeration, and path diagnostics unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pin the exact ArrayDependency advice and execute its entry-retirement remedy. Exercise real unsafe dotted-key scaffold reordering, pending-retirement array splice rejection, and selector removal with independent registrations intact. Assert no rejected write enters the host cache or projected ownership state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…file Exercise the non-region branch of without_artifact identified by measured coverage, preserving the live array selector and its checksum contribution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assert the typed enumeration error kind and exact message in both in-memory error tests. Remove obsolete numbered design citations identified by review, following the repository rule that source documentation must not depend on design sections. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rediscover independently safe retirements for array and in-sync ordinary regions, and include each candidate in its validation projection. Validate a reconstructed pre-write view too, so an unparsable duplicate table cannot hide live array delimiters or bindings supplied by the orphan. Add in-memory pipeline regressions for both artifact orders, array introduction and established entries, tracking states, repeated reruns, and unsafe orphan preservation. Document the recovery protocol. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sander Saares (sandersaares)
left a comment
Sander Saares (sandersaares)
left a comment
There was a problem hiding this comment.
[Copilot speaking]
Published 8 findings. No finding follows up on an existing discussion thread.
See diagnostics
| Diagnostic | Value |
|---|---|
| Cache | Miss |
Only roll back synchronized ordinary regions after the retirement candidate. Keep the preceding source context intact so an orphan-owned array cannot disappear from validation when its live parent table matches the current template. Candidate-projected validation separately protects suffix selectors with live regions present. Add an in-memory regression covering both artifact orders, tracked and untracked synchronized neighbors, new and established array ownership, complete host-byte preservation, manifest ownership, and repeated runs. Document the conservative source-order boundary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fc386ac to
9445d67
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Catalog validation currently accepts sentinel-bearing array bodies that are guaranteed to be refused during emission.
2 open findings
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
| body_array(&spec.region.body)?; | ||
| Ok(()) |
| assert!(spliced.contains(&old)); | ||
| assert!(spliced.contains(&new), "{spliced:?}"); | ||
| assert!(spliced.starts_with(&prefix.split('=').next().unwrap().replace('\n', newline))); |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Recognize ownership sentinels only at TOML comment tokens, including raw partial-apply intermediates. Reject every non-insertion scaffold before constructing it, and give actionable shape and scaffold refusals. Scan real entry tokens for interior comments instead of implicit dotted-key proxy spans. Move validated template arrays instead of cloning them. Add in-memory production and span regressions and clarify helper contracts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Exercise actual comment markers splitting nested values and a live selector introduced by an earlier ordinary write while retirement is pending. Assert exact dotted inline-table adoption output as well as its semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve marker-looking string data through replacement, relocation, residue insertion, adoption projections and legacy migration lookups. Keep non-TOML region operations lexical and reuse located byte spans. Add in-memory production regressions for introduction, update and reruns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Array regions use inconsistent marker scanners for valid TOML hosts without a .toml suffix.
3 open findings
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
| Some(text) => crate::region::find_host_region(text, region_id, syntax, host_relpath) | ||
| .map_err(|error| ManagedRegionRefusal::new(error, RefusalRemedy::MalformedMarkers))?, |
🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.
What this changes
CatalogBuilder::with_toml_array_regionadds managed entries inside a selected TOML array. The engine creates missing scaffolding and adopts matching unmanaged entries.Effects
Artifactvariants remain unchanged.