Merge pull request #336 from quarto-dev/bugfix/bd-3zp3z4jx-link-url-corrupted-write

fix(reconcile): don't inherit an adjacent link's URL on write-back (bd-3zp3z4jx)

Author: cscheid

claude-notes/plans/2026-06-23-link-url-write-back-corruption.md

@@ -0,0 +1,61 @@
+# bd-3zp3z4jx — link URL corrupted on write-back
+
+**Date:** 2026-06-23
+**Branch:** `braid/bd-3zp3z4jx-link-url-corrupted-write` (off `origin/main`)
+
+## Symptom
+
+Editing a paragraph so it contains a link whose URL differs from the original
+(adding a new link, or changing an existing link's URL) persisted the **wrong**
+URL: a new link inherited an *adjacent* link's URL, and a changed URL silently
+reverted to the old one.
+
+Discovered while testing the rich-text editor (bd-sjb4pzx8), but the bug is in
+the **shared write-back path** (`apply_node_edit` → reconcile → incremental
+writer), so it affects the monospaced textarea editor's link edits too.
+
+## Root cause
+
+The incremental writer's **inline splice** (`assemble_recursed_container` in
+`crates/pampa/src/writers/incremental.rs`) preserves a container inline's
+*delimiters* verbatim from the original source while re-writing only its
+children. For a `Link`, the closing delimiter is `](url "title")` — i.e. the URL
+is part of the verbatim-copied delimiter, **not** a recursable child.
+
+The reconciler (`compute_inline_alignments` in
+`crates/quarto-ast-reconcile/src/compute.rs`) decided two container inlines
+"correspond" (→ `RecurseIntoContainer`) using **only the type discriminant**.
+So any new `Link` matched any old `Link` regardless of target, and the splice
+then copied the old link's `](url)` over the new one.
+
+## Fix
+
+In the reconciler's type-match step, require a container's **non-child identity**
+to be unchanged before treating two containers as the same (and recursing):
+
+- `Link` / `Image`: `target` (url, title) **and** `attr` equal
+- `Span`: `attr` equal
+- (`Custom`: `type_name` equal — pre-existing)
+
+When that identity differs, the new inline falls through to `UseAfter`, which
+re-serializes it via the qmd writer (which emits the URL straight from the AST —
+`link.target.0`), producing the correct output. Single-line, value-only
+comparison; no type or API changes.
+
+## Tests (TDD — failing first)
+
+`crates/pampa/tests/integration/node_edit_tests.rs`:
+
+- `apply_node_edit_preserves_distinct_link_urls` — adding a 2nd link keeps both
+  distinct URLs.
+- `apply_node_edit_changes_existing_link_url` — changing a link's URL persists.
+
+Both failed before the fix (new link got the old URL), pass after.
+
+## Verification
+
+- `quarto-ast-reconcile` + `pampa`: 4248 pass.
+- Full workspace `cargo nextest run --workspace`: **10337 pass**, 0 regressions.
+- `cargo xtask verify --skip-hub-build`: Rust build + clippy (`-D warnings`) +
+  fmt clean. (ts-packages/hub legs need `npm install`; not affected by this
+  Rust-only change — CI covers them.)

crates/pampa/tests/integration/node_edit_tests.rs

@@ -1424,3 +1424,46 @@ fn preserve_leaf_variant_does_not_fire_on_multi_block_replacement() {
         bl.content[0][0]
     );
 }
+
+// =============================================================================
+// bd-3zp3z4jx — link URL corruption on write-back
+// =============================================================================
+
+/// Editing a paragraph so it gains a SECOND link must keep both links' distinct
+/// URLs. Repro for bd-3zp3z4jx: a new link in a multi-link paragraph was getting
+/// an adjacent link's URL on write-back (found via the rich-text editor, but the
+/// bug is in apply_node_edit / the incremental writer, shared by both editors).
+#[test]
+fn apply_node_edit_preserves_distinct_link_urls() {
+    // Original paragraph has ONE link.
+    let content = "Text with a [link](https://existing.example) here.\n";
+    // Edit it to add a NEW link (before the existing one) with a DIFFERENT url.
+    let replacement = "Text with a [newlink](https://added.example) and a [link](https://existing.example) here.\n";
+    let result = edit_block(content, 0, replacement);
+
+    assert!(
+        result.contains("https://added.example"),
+        "the NEW link's URL must survive write-back; got: {result:?}"
+    );
+    assert!(
+        result.contains("https://existing.example"),
+        "the original link's URL must survive write-back; got: {result:?}"
+    );
+}
+
+/// Changing an EXISTING link's URL must persist the new URL (bd-3zp3z4jx).
+/// Previously the inline splice preserved the old `](url)` delimiter verbatim.
+#[test]
+fn apply_node_edit_changes_existing_link_url() {
+    let content = "Go to [the site](https://old.example) now.\n";
+    let replacement = "Go to [the site](https://new.example) now.\n";
+    let result = edit_block(content, 0, replacement);
+    assert!(
+        result.contains("https://new.example"),
+        "changed URL must persist; got: {result:?}"
+    );
+    assert!(
+        !result.contains("https://old.example"),
+        "old URL must be gone; got: {result:?}"
+    );
+}

crates/quarto-ast-reconcile/src/compute.rs

@@ -700,9 +700,26 @@ fn compute_inline_alignments<'a>(
                 if !is_container_inline(orig_inline) {
                     return false;
                 }
-                // For Custom inlines, also check type_name matches
+                // A container's non-child "identity" is preserved VERBATIM from the
+                // original source by the inline splice (assemble_recursed_container
+                // copies the delimiters around the children) — it is NOT a
+                // recursable child. So two containers may only be treated as "the
+                // same" (and recursed) when that identity is unchanged; otherwise
+                // we must fall through to UseAfter, which re-serializes the new
+                // inline with its correct target/attr.
+                //
+                // bd-3zp3z4jx: without this, a NEW link in a multi-link paragraph
+                // matched an OLD link by type alone and inherited the old link's
+                // URL (the `](url)` is part of the verbatim-copied closing
+                // delimiter). The non-child identity is: Link/Image `target`+`attr`,
+                // Span `attr`, Custom `type_name`.
                 match (*orig_inline, exec_inline) {
                     (Inline::Custom(o), Inline::Custom(e)) => o.type_name == e.type_name,
+                    (Inline::Link(o), Inline::Link(e)) => o.target == e.target && o.attr == e.attr,
+                    (Inline::Image(o), Inline::Image(e)) => {
+                        o.target == e.target && o.attr == e.attr
+                    }
+                    (Inline::Span(o), Inline::Span(e)) => o.attr == e.attr,
                     _ => true,
                 }
             });