feat(cargo-anvil): manage JSON array entries by martin-kolinek · Pull Request #283 · microsoft/ox-tools

martin-kolinek · 2026-10-20T16:44:50Z

Summary

add generic JsonArrayRegionSpec / with_json_array_region support for repository-owned JSON object paths and arrays
preserve unrelated JSON/JSONC structure, comments, entries, and formatting while adopting each required string exactly once
fail closed on malformed JSON, wrong/duplicate property shapes, ownership drift, unsafe migration, ambiguous hosts, and structural dependencies
support schema-1-compatible regeneration, retirement, and selector-aware catalog checksums
document the shared TOML/JSON array-region contract and cover the CloudBuild EnlistmentRootPaths.SourceRootDirs scenario

Validation

just anvil-pr-fast
cargo test -p cargo-anvil --lib
cargo test -p cargo-anvil --doc
cargo package -p cargo-anvil --list --allow-dirty
full cargo test -p cargo-anvil reached one timing-sensitive pre-existing recipe test failure; the exact failed test passed on immediate rerun: cargo test -p cargo-anvil --test recipe_contracts examples_run_honors_default_exclusions_and_explicit_selection -- --exact --nocapture

The fast tier is partial; the pull-request checks should run the remaining complete Anvil groups.

Add structural JSON array regions with JSONC parsing, exact required-string adoption, fail-closed ownership checks, retirement support, tests, and design documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88b757e4-6a54-4e4f-8610-8513b39e6e41

Copilot

🟡 Changes recommended

Scaffolding can violate slice and ownership boundaries, and pre-indented templates do not reach a stable rendering.

3 open issues

What changed in this PR

Extends cargo-anvil’s shared-array ownership model to JSON/JSONC, including the CloudBuild source-root scenario.

Changes:

Adds JSON array catalog specifications and selector-aware checksums.
Implements entry adoption, scaffolding, regeneration, and retirement.
Documents the shared contract and adds lifecycle tests.

File	Description
crates/cargo-anvil/src/run.rs	Integrates JSON array planning and validation.
crates/cargo-anvil/src/region.rs	Adds JSON-aware marker scanning and removal.
crates/cargo-anvil/src/lib.rs	Exports the JSON array specification.
crates/cargo-anvil/src/emit/mod.rs	Registers the JSON array emitter.
crates/cargo-anvil/src/emit/managed_region.rs	Adds a generated-JSON refusal category.
crates/cargo-anvil/src/emit/json_array_region.rs	Implements JSON array management and tests.
crates/cargo-anvil/src/catalog/mod.rs	Re-exports the new specification.
crates/cargo-anvil/src/catalog/builder.rs	Adds selector registration, validation, and checksums.
crates/cargo-anvil/src/catalog/artifact.rs	Defines `JsonArrayRegionSpec`.
crates/cargo-anvil/docs/design/updates.md	Generalizes shared-array documentation links.
crates/cargo-anvil/docs/design/array-regions.md	Documents JSON support and ownership rules.
crates/cargo-anvil/Cargo.toml	Adds runtime JSON dependencies.
Cargo.toml	Declares the JSONC parser dependency.
Cargo.lock	Locks the JSONC parser dependency.
.spelling	Adds JSONC terminology.

_{Review effort: Balanced}

Codecov Comments Bot (codecov-commenter) · 2026-10-20T17:04:11Z

Codecov Report

❌ Patch coverage is 99.80861% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.4%. Comparing base (6912269) to head (5de06f6).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/cargo-anvil/src/emit/json_array_region.rs	99.7%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main    #283     +/-   ##
=======================================
- Coverage   99.5%   99.4%   -0.2%     
=======================================
  Files        313     340     +27     
  Lines      52196   60844   +8648     
=======================================
+ Hits       51985   60517   +8532     
- Misses       211     327    +116

Flag	Coverage Δ
linux	`99.4% <99.8%> (-0.1%)`	⬇️
linux-arm	`99.4% <99.8%> (-0.1%)`	⬇️
scheduled	`?`
windows	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Correct inline-object scaffolding, protect neighboring managed separators, stabilize pre-indented templates, and add complete behavioral coverage for JSON array ownership paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88b757e4-6a54-4e4f-8610-8513b39e6e41

Copilot

🟡 Changes recommended

Malformed markers can panic, and proposed hosts can acquire duplicate properties that block subsequent runs.

2 open issues

3 resolved since last review

_{Review effort: Balanced}

Return structured refusals for noncanonical markers, reject duplicate properties introduced by neighboring region writes, and refresh vulnerable WebPKI lockfile dependencies flagged by cargo-deny. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88b757e4-6a54-4e4f-8610-8513b39e6e41

Copilot

🔵 Needs a closer look

Source-preserving edits and schema-1 retirement need human review, with unresolved selector and ownership-marker validation issues.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Reject manifest selectors during catalog validation

crates/cargo-anvil/src/emit/json_array_region.rs:91

Non-Path selectors bypass this check, but region_host_paths (run.rs:632–649) expands them to Cargo manifests, not JSON hosts. The catalog therefore accepts a specification that either selects no files or sends Cargo.toml through the JSON planner. Reject manifest selectors during catalog validation, add regression cases for all three variants, and clarify that the design doc's manifest-selector support applies only to TOML.

Validate marker topology for array and neighboring regions

crates/cargo-anvil/src/emit/json_array_region.rs:452

An ordinary region planned after the array can introduce nested or unpaired sentinel comments in its generated body. The generic planner returns the splice without checking its marker topology (managed_region.rs:252–259), and this validator accepts it if JSON properties and array bounds remain unchanged. For example, a settings body containing a separate nested marker pair can be written, but the next run refuses it in validated_regions. Validate the proposed marker topology here and add an array-first, neighbor-second composition regression.

2 resolved since last review

_{Review effort: Balanced}

…on-array-regions

Add independent ownership-boundary, indentation, scaffolding, strict-dialect, removal, and refusal-guidance coverage. Exclude byte-level lexical and source-offset state machines from mutation substitution with explicit rationale. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88b757e4-6a54-4e4f-8610-8513b39e6e41

Copilot

🟡 Changes recommended

Proposed writes can introduce malformed ownership markers, and JSON specifications accept incompatible TOML-only selectors.

2 open issues

_{Review effort: Balanced}

Sander Saares (sandersaares) · 2026-10-21T12:37:02Z

[Copilot speaking] ss-review-proactor is reserving this pull request for review with ss-review.

Sander Saares (sandersaares)

[Copilot speaking]

Published 9 findings. No finding follows up on an existing discussion thread.

See diagnostics

Diagnostic	Value
Cache	Hit

Validate marker topology after every composed write, restrict JSON regions to literal paths, preserve typed validation causes, stream JSON comment discovery, and document internal ownership invariants and public trait contracts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88b757e4-6a54-4e4f-8610-8513b39e6e41

…on-array-regions # Conflicts: # Cargo.lock

Copilot

🟡 Changes recommended

Retirement validation can be bypassed by normalized paths, and composed updates can violate required-entry uniqueness.

4 open issues

2 resolved since last review

_{Review effort: Balanced}

Disambiguate allocator-api2 edges introduced by the latest rallocator update so full locked Cargo metadata and Anvil impact analysis succeed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88b757e4-6a54-4e4f-8610-8513b39e6e41

Keep host-level JSON scanner provenance for edited ownership records that survive empty-catalog retirement, and normalize JSON array templates before indentation so emitted bytes and checksums agree across mixed line endings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8376a896-87c7-4843-9294-13c381602580

Copilot

🔵 Needs a closer look

The unresolved scanner-compatibility issue and interacting ownership and schema-migration paths require correction and final human review.

1 open issue

Retired JSON hosts bypass syntax checks for hash-comment regions · New

2 resolved since last review

_{Review effort: Balanced}

Reject non-slash-slash ordinary regions on hosts retaining JSON-aware scanning, preventing hash-marker duplication after structural array retirement. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8376a896-87c7-4843-9294-13c381602580

Copilot

🔵 Needs a closer look

An unresolved incomplete-scan defect and cross-cutting ownership transitions require correction and final human review of data-preservation guarantees.

1 open issue

Report incomplete scans to prevent hidden regions and tracking loss · New

1 resolved since last review

Retired JSON hosts bypass syntax checks for hash-comment regions

_{Review effort: Balanced}

Detect unterminated JSON-like strings and block comments before marker discovery so scanner-only updates and retirements preserve hidden ownership and tracking. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8376a896-87c7-4843-9294-13c381602580

Copilot

🔵 Needs a closer look

Source-span editing and persistent ownership changes need human review, with preservation and retirement defects still unresolved.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Handle bare CR separators in shared region helpers

crates/cargo-anvil/src/region.rs:1434

Bare-CR detection needs matching support in the shared separator helpers. separate_region (line 822) still checks only LF, while leading_newline_len and trailing_blank_line_len recognize only LF/CRLF. Appending an ordinary region to user\r therefore adds two CR separators, and retirement leaves user\r\r\r because neither separator is recognized. The equivalent LF and CRLF lifecycles restore the input. Update these helpers together and add an append/retire regression; the new bare-CR test starts with existing markers and does not exercise insertion separators.

1 resolved since last review

Report incomplete scans to prevent hidden regions and tracking loss

_{Review effort: Balanced}

Add focused behavioral coverage for recorded placement, absent-host scaffolding, scanner provenance filtering, lexical failures, and XML root finalization so the cargo-anvil 100% line-coverage gate passes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8376a896-87c7-4843-9294-13c381602580

Copilot

🔵 Needs a closer look

Source-span editing and provenance migration warrant human review, with an unresolved retirement-validation regression and test-isolation issue.

2 open issues

_{Review effort: Balanced}

Use suffix-based TOML history classification, make the missing JSON host test explicitly absent, and add structural dotfile retirement coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8376a896-87c7-4843-9294-13c381602580

Copilot

🟡 Changes recommended

Unresolved selector-provenance loss during host renames weakens retirement safeguards, and bare-CR separator handling leaves residual blank lines.

2 open issues

2 resolved since last review

_{Review effort: Balanced}

martintmk

Posted by an AI agent

Warning: Incomplete review

The public API surface could not be assessed because the available tool has no lock-preserving mode. No overall verdict is given.

The full review, evidence, mandatory reports, and rolling findings are in the rolling review.

Restore a fallible XML stack-pop path so unmatched closing tags produce a repository diagnostic instead of panicking, with direct regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8376a896-87c7-4843-9294-13c381602580

Copilot

🔵 Needs a closer look

Ownership-sensitive composition and the lock-schema migration warrant human review, with unresolved correctness and diagnostic issues.

3 open issues

1 previously missed issue

In code that hasn't changed since last review

Classify splice conflicts separately from invalid templates

crates/cargo-anvil/src/emit/json_array_region.rs:513

A splice failure here does not establish that the template is invalid on its own. For example, a tracked ordinary region can contain "old" with a repository-owned comma immediately after its closing sentinel. Opting it into the structural API with the valid body "new", leaves two adjacent commas. Refusing the write is correct, but InvalidGeneratedJson makes run.rs:1231–1233 tell the user that the template is invalid independently of repository content, even though it passed validate_spec. Classify this as a composition/ownership conflict, retain the parser detail, and direct the user to reconcile separators at the ownership boundary rather than repair the standalone template.

_{Review effort: Balanced}

+        .map_err(|error| refusal(error, RefusalRemedy::MalformedMarkers))?
+        .is_none()
+    {
+        return Ok(());


Transfer selectors across case aliases, handle bare-CR separators, reject multiline JSON-like strings, validate manifest host provenance, preserve scanner guidance, and cover schema migration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8376a896-87c7-4843-9294-13c381602580

Copilot

🔵 Needs a closer look

Source-preserving edits, region retirement, and persistent ownership-schema migration warrant final human review despite extensive added coverage.

1 open issue

Reject missing active or final regions

2 resolved since last review

_{Review effort: Balanced}

Copilot AI balanced review requested due to automatic review settings October 20, 2026 16:44

Copilot started reviewing on behalf of martin-kolinek October 20, 2026 16:45

View session

Copilot AI reviewed Oct 20, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs Outdated

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs Outdated

Copilot AI balanced review requested due to automatic review settings October 20, 2026 19:06

Copilot started reviewing on behalf of martin-kolinek October 20, 2026 19:07

View session

Copilot AI reviewed Oct 20, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs Outdated

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs

Copilot AI balanced review requested due to automatic review settings October 20, 2026 19:37

Copilot started reviewing on behalf of martin-kolinek October 20, 2026 19:38

View session

Copilot AI reviewed Oct 20, 2026

View reviewed changes

Evgenii (Vaiz) approved these changes Oct 21, 2026

View reviewed changes

martin-kolinek added 2 commits October 21, 2026 11:49

Merge remote-tracking branch 'origin/main' into makolnek-microsoft-js…

de07d7d

…on-array-regions

Copilot AI balanced review requested due to automatic review settings October 21, 2026 11:37

Copilot started reviewing on behalf of martin-kolinek October 21, 2026 11:37

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs Outdated

Sander Saares (sandersaares) self-requested a review October 21, 2026 12:38

Sander Saares (sandersaares) approved these changes Oct 21, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 21, 2026 15:18

Copilot started reviewing on behalf of martin-kolinek October 21, 2026 15:19

View session

Merge remote-tracking branch 'origin/main' into makolnek-microsoft-js…

cfc8729

…on-array-regions # Conflicts: # Cargo.lock

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/run.rs Outdated

Comment thread crates/cargo-anvil/src/run.rs

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs Outdated

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs

Copilot AI balanced review requested due to automatic review settings October 21, 2026 15:30

Copilot started reviewing on behalf of martin-kolinek October 21, 2026 15:31

View session

Copilot AI balanced review requested due to automatic review settings October 22, 2026 21:52

Copilot started reviewing on behalf of martin-kolinek October 22, 2026 21:53

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/run.rs

Copilot AI balanced review requested due to automatic review settings October 22, 2026 22:13

Copilot started reviewing on behalf of martin-kolinek October 22, 2026 22:14

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/region.rs

Copilot AI balanced review requested due to automatic review settings October 22, 2026 22:35

Copilot started reviewing on behalf of martin-kolinek October 22, 2026 22:36

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 22, 2026 23:52

Copilot started reviewing on behalf of martin-kolinek October 22, 2026 23:53

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/run.rs Outdated

Comment thread crates/cargo-anvil/src/run.rs

Pato Sandaña (psandana) reviewed Oct 23, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/region.rs Outdated

Copilot AI balanced review requested due to automatic review settings October 23, 2026 00:38

Copilot started reviewing on behalf of martin-kolinek October 23, 2026 00:38

View session

Copilot AI reviewed Oct 23, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/run.rs

Comment thread crates/cargo-anvil/src/region.rs

martintmk reviewed Oct 23, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/region.rs

Comment thread crates/cargo-anvil/src/manifest.rs

Comment thread crates/cargo-anvil/src/manifest.rs

Comment thread crates/cargo-anvil/src/run.rs Outdated

Comment thread crates/cargo-anvil/src/run.rs

Copilot AI balanced review requested due to automatic review settings October 23, 2026 01:29

Copilot started reviewing on behalf of martin-kolinek October 23, 2026 01:29

View session

Copilot AI reviewed Oct 23, 2026

View reviewed changes

Comment thread crates/cargo-anvil/src/emit/json_array_region.rs

.map_err(|error| refusal(error, RefusalRemedy::MalformedMarkers))?

.is_none()

{

return Ok(());

Copilot AI balanced review requested due to automatic review settings October 23, 2026 02:10

Copilot started reviewing on behalf of martin-kolinek October 23, 2026 02:11

View session

Copilot AI reviewed Oct 23, 2026

View reviewed changes

Conversation

martin-kolinek commented Oct 20, 2026

Summary

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Codecov Comments Bot (codecov-commenter) commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Sander Saares (sandersaares) commented Oct 21, 2026

Uh oh!

Sander Saares (sandersaares) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Codecov Comments Bot (codecov-commenter) commented Oct 20, 2026 •

edited

Loading