MOB-40: reversible plugin merges into host build files (managed blocks)

[GenericJam]

Follow-up from #32 review (F4). Makes plugin contributions to host-owned build files reversible on plugin removal.

Problem

Plugin <uses-permission>s, AndroidManifest <application> components (new in #32), and Gradle deps are spliced into AndroidManifest.xml / build.gradle — files the host also hand-edits. A plain "append if absent" merge can't be undone, so a removed plugin's lines linger: a dangling <service> whose class is gone, an orphan permission/dep. Unlike the copied artifacts (bridge_kt, res_files) there's no ledger to prune, because the target is a shared, hand-authored file.

Fix

New MobDev.Plugin.ManagedBlock: fence each contribution in begin/end marker comments and regenerate the whole region every build from the current plugin set. A removed plugin simply isn't in the fresh region, so its lines disappear; anything outside the fence is untouched.

• Idempotent by construction — the region is inserted as whole lines at a line boundary (insert_before/insert_before_index) and strip/2 removes exactly those lines, so strip(place(x)) == x and re-running with an unchanged set is a fixed point.
• All three merge sites (merge_android_permissions, merge_android_manifest_components, merge_gradle_deps) rewired to it — one consistent mechanism across permissions + components + deps, as the reviewer suggested rather than a component-only prune.
• Still de-dupes against host-declared entries (a hand-added permission/component isn't doubled).
• XML comment markers for the manifest, // for Gradle.

Tests

ManagedBlock unit tests (insert / idempotent / replace / remove / strip round-trip / malformed) + removal + no-duplicate tests at each of the three sites (e.g. "removing the plugin strips its managed region, keeping host perms"). 427 plugin tests + 198 native_build/managed_block pass; mix credo --strict clean.

Also

Relocates the MOB-39 + new_plugin-scaffold CHANGELOG entries from the already-released 0.6.15 (2026-06-23) to [Unreleased] — they were absorbed there by #32's squash-merge against a moved-on master.

🤖 Generated with Claude Code

[GenericJam]

Review — approve-with-fixes (design is right; one must-fix + a scope gap)

The managed-block / strip-and-regenerate approach is the correct shape, and one subtle thing is done well: each merge site dedupes against stripped (not the raw file), so a still-active plugin doesn't see its own prior managed line and self-exclude. All three sites (permissions, components, gradle) are consistently converted, the old append path is gone, dedupe-vs-host is preserved on removal, and CHANGELOG relocation is right. CI green, CLEAN.

Two real gaps, one of them a must-fix:

🔴 Must-fix — strip/2 can silently delete host content on a duplicate/orphan BEGIN marker

strip/2 matches the first BEGIN and the first END at/after it, and the else guard only handles missing end / end-before-begin. It does not handle an orphan BEGIN followed by a real region. Scenario:

<!-- BEGIN mob managed ... -->        # orphan (interrupted write / hand-edit / bad merge-conflict resolution)
<host line A>
<host line B>
<!-- BEGIN mob managed ... -->        # the real region this build wrote
  <uses-permission .../>
<!-- END mob managed ... -->

Next build, strip takes first-BEGIN … first-END-after-it = orphan-BEGIN through the real region's END, and removes everything in between — including host lines A and B. Silent loss of hand-authored AndroidManifest.xml / build.gradle content, which is the worst failure mode for a feature whose whole job is safely mutating host-owned files. The "malformed" test only covers end-before-begin, so this is untested. Fix: in strip, detect >1 BEGIN occurrence and either strip all regions or refuse with a loud error (treat "another BEGIN before the END" as malformed), plus a regression test. (Verified directly in strip/2; also reproducible by running the module — two builds over an orphan-BEGIN file.)

🟡 Should-fix (or document) — no migration for pre-existing UNFENCED lines

Apps generated before this PR already have plugin <uses-permission> / <service> / gradle-dep lines injected without markers. On the first build with #33, the dedupe predicates (permission_present?, manifest_component_present?, the gradle substring check) match those old lines and treat them as host-authored, so they're excluded from the fresh managed region — no duplicate, no build break (good), but they're never fenced. So when that plugin is later removed, the old unfenced line lingers — exactly the dangling-<service>/orphan-permission MOB-40 set out to eliminate. Net: the fix is forward-only — it governs only lines it newly writes, and does nothing for the installed base until each plugin is toggled off/on or the line is hand-deleted. That's a claim-vs-delivery gap vs the PR/CHANGELOG wording. Either backfill (strip matching unfenced lines during the transition) or state the forward-only scope explicitly and soften the claim.

Nits

• The permission EOF-append fallback (place_before_application/2, no-<application> branch) isn't a fixed point — it pads a trailing newline each build (+1 byte/build). Only triggers in a manifest lacking both <application and </manifest> (not a real Android manifest), so low — but it contradicts the "re-running is a fixed point" claim; mirror the gradle fallback which synthesizes a stable anchor.
• native_build_test.exs:565 "inserts after the LAST existing uses-permission line" no longer describes the behavior (region now precedes <application>); passes only incidentally. And no test covers the orphan-BEGIN clobber (F1) or migration-from-unfenced (F2) — the two highest-risk paths.

Verdict: approve once F1 is fixed (harden strip + regression test). F2 I'd at least document explicitly if not backfilling; nits are follow-ups. Solid mechanism otherwise.

(F1 verified against strip/2 in the diff; F2/F3 the reviewer reproduced by running the module — worth a maintainer re-check at the strip guard and the merge_android_permissions([...]) → ([]) removal path over an unfenced manifest.)

[GenericJam]

Pushed fe15abb addressing both.

🔴 F1 (blocker) — orphan-BEGIN data loss → FIXED. Nice catch. strip/2 no longer anchors on first-BEGIN→first-END; it anchors on the last BEGIN before the first END, so the removed span provably contains no other marker and can never reach across a stray BEGIN into host lines. It also loops until no well-formed region remains (clears duplicates). Regression tests reproduce your scenario:

• orphan BEGIN + host-line + real region → only the real region is removed, host-line-that-must-survive stays.
• duplicate well-formed regions → all cleared.
• trailing orphan BEGIN → left alone, no host loss.

🟡 F2 — forward-only → wording softened + test. You're right that already-generated apps carry the old unfenced lines, and there's no safe migration: an unfenced entry is byte-identical whether an older mob_dev appended it or the host hand-wrote it — the exact ambiguity that motivated fencing means we can't reclaim them without risking deleting a host line. So I've made the claim accurate rather than pretend otherwise: the CHANGELOG now says reversible going forward, and a new test documents that a pre-existing unfenced entry is treated as host-authored (not double-added, not auto-removed) — regenerate or hand-remove to fence it.

Nits: the EOF fallback (pathological manifest, no <application>/</manifest>) now appends whole lines so strip round-trips it; renamed the stale after the LAST uses-permission test to describe the managed-region placement.

617 plugin/native_build tests pass; mix credo --strict clean. CI should confirm.

[GenericJam]

Re-review (fe15abb) — fixes verified, approving ✅

F1 (blocker) — resolved, and the fix is provably correct. strip_one now anchors on the last BEGIN before the first END (not first→first), so the removed span can't contain another marker and therefore can't reach across a stray/orphan BEGIN into host lines — and strip loops until no well-formed region remains, clearing duplicates. Walking my scenario through the new logic: [orphan-BEGIN, host A, host B, real-BEGIN, …, real-END] → first END is real-END, last BEGIN before it is real-BEGIN, so only the real region is removed and host A/B + the orphan survive. Correct. The regression tests pin exactly this ("orphan BEGIN … does NOT eat the host lines between them", "clears duplicate well-formed regions", "trailing orphan BEGIN … left alone") — and the loop-termination test passing also rules out infinite recursion.

F2 — resolved correctly by scoping the claim. Agreed with the reasoning: a pre-existing unfenced entry is byte-identical whether an old mob_dev appended it or the host hand-wrote it, so auto-reclaiming it would risk deleting a host line — the very ambiguity fencing exists to resolve. Documenting it as reversible going forward (+ the test that a pre-existing unfenced entry is treated as host-authored: not doubled, not auto-removed) is the honest, safe call rather than an unsafe backfill.

Nits — EOF fallback now round-trips, stale test renamed. Good.

617 tests + credo clean, CI green, CLEAN. LGTM — approving for merge. Nice, thorough turnaround.