🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.
Split out of #140, where it was found. This is a pre-existing defect on main, not one #140 introduces — #140 fixes the neighbouring [lints] case and leaves this one untouched.
The defect
Introducing a managed region into a TOML host appends it. When the host already declares the same ordinary table by hand, and that hand-written table carries configuration the managed body does not, the result is two identical table headers in one file. TOML rejects that outright.
The repository's own migration_preserves_user_content fixture produces such a file. Instrumenting it to dump the deny.toml it writes and feeding that output to a TOML parser gives:
TOML parse error at line 8, column 2
|
8 | [advisories]
| ^^^^^^^^^^
duplicate key
from:
[advisories]
ignore = ["RUSTSEC-9999-0001"]
# >>> anvil-managed: anvil-deny-advisories
[advisories]
yanked = "deny"
The fixture passes because it asserts only on substrings, never on parseability.
Why table adoption does not cover it
#140 removes a hand-written table when the managed body already covers every one of its configuration lines. Here the user's ignore entry is not covered, so adoption correctly declines rather than deleting a real setting — and splice then inserts the region anyway. Declining is right; inserting regardless is what produces the broken file.
Impact
Any repository onboarding with a customised deny.toml gets an unparsable one. A non-dry run rewrites the file on disk and records the region in the manifest, so cargo deny fails after the generator has already modified the repository.
Options considered
- Refuse. Return a structured conflict from the adoption helper, propagate it to
push_region_at, and emit a refusal diagnostic plus a no-op instead of writing. Correct and visible, but converts silent corruption into blocked onboarding, and the remedy it can suggest ("move those settings into the region") is impossible — the region is regenerated. Implemented experimentally: all unit tests passed, then migration_preserves_user_content failed because run_update(...).unwrap() turns the refusal into a hard error.
- Emit the
deny.toml section regions without their table header, so user keys and managed keys share one table, in the same way the [workspace.lints] region already uses dotted keys. Fixes the class rather than reporting it. Changes the region bodies in the catalog.
- Merge user-only keys into the managed body. Contradicts the rule that managed content is generated.
Option 2 looks correct, but it is a design change to the deny.toml catalog rather than a review fix, which is why it was not made in #140.
Acceptance
Whichever option is taken, the fixtures that write TOML hosts should assert the output parses, not merely that it contains expected substrings — that is what let this survive.
🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.
Split out of #140, where it was found. This is a pre-existing defect on
main, not one #140 introduces — #140 fixes the neighbouring[lints]case and leaves this one untouched.The defect
Introducing a managed region into a TOML host appends it. When the host already declares the same ordinary table by hand, and that hand-written table carries configuration the managed body does not, the result is two identical table headers in one file. TOML rejects that outright.
The repository's own
migration_preserves_user_contentfixture produces such a file. Instrumenting it to dump thedeny.tomlit writes and feeding that output to a TOML parser gives:from:
The fixture passes because it asserts only on substrings, never on parseability.
Why table adoption does not cover it
#140 removes a hand-written table when the managed body already covers every one of its configuration lines. Here the user's
ignoreentry is not covered, so adoption correctly declines rather than deleting a real setting — andsplicethen inserts the region anyway. Declining is right; inserting regardless is what produces the broken file.Impact
Any repository onboarding with a customised
deny.tomlgets an unparsable one. A non-dry run rewrites the file on disk and records the region in the manifest, socargo denyfails after the generator has already modified the repository.Options considered
push_region_at, and emit a refusal diagnostic plus a no-op instead of writing. Correct and visible, but converts silent corruption into blocked onboarding, and the remedy it can suggest ("move those settings into the region") is impossible — the region is regenerated. Implemented experimentally: all unit tests passed, thenmigration_preserves_user_contentfailed becauserun_update(...).unwrap()turns the refusal into a hard error.deny.tomlsection regions without their table header, so user keys and managed keys share one table, in the same way the[workspace.lints]region already uses dotted keys. Fixes the class rather than reporting it. Changes the region bodies in the catalog.Option 2 looks correct, but it is a design change to the
deny.tomlcatalog rather than a review fix, which is why it was not made in #140.Acceptance
Whichever option is taken, the fixtures that write TOML hosts should assert the output parses, not merely that it contains expected substrings — that is what let this survive.