Skip to content

client.updateLanded can never fire for a page holding a macro #163

Description

@willkg

client.updateLanded — the recovery for a versioned PUT whose response was lost — compares what Confluence stored against what markfluence sent, and Confluence never stores what it was sent for a page holding a macro. A code block is a macro. So for most real pages the recovery can never fire, and when it is needed it is guaranteed to report a failure for a publish that actually succeeded.

Split out of #149 / _plans/041_page-moved-detection.md, which ran into the same measurement from the other direction and had to rule a sent-vs-stored body comparison out of its own design. Pre-existing, not caused by that work.

What updateLanded is for

A versioned PUT is not as idempotent as its method. UpdatePage sends version.number = N+1; if the response is lost in transit, send's retry re-sends a version the page has already reached and Confluence refuses it. The write landed, and markfluence reports a failure — which is the shape client.SetContentProperty's retry-once also exists to handle.

updateLanded (internal/client/client.go:945) is the recovery: after any error from the PUT, re-read the page and ask whether it is now exactly what was sent. Version, title and body.storage must all match. It insists on all three deliberately, and that must survive any fix: the version alone proves nothing, since a concurrent human edit could have produced the same number, and claiming success over somebody else's content is worse than a false failure.

The measurement

Re-measured 2026-09-13 against a scratch page in the personal space, specifically because docs/confluence/storage-format.md's HTML-comment finding had made author-written comments look like the only obstacle:

- <ac:structured-macro ac:name="code" ac:schema-version="1">
+ <ac:structured-macro ac:name="code" ac:schema-version="1" ac:macro-id="36f410f4-…">

A fresh uuid, per macro, per write. storage-format.md already recorded this alongside ac:schema-version injection and the space before /> on a self-closing tag, verified 2026-08-07 — what was not recorded is the consequence for updateLanded, and the comment section reads as an edge case when the macro-id injection is the general one.

A code block is a macro, and so is the TOC (convert.tocMacro) and every GitHub alert callout (callouts.go → ac:structured-macro). So live.Body.Storage.Value != body holds unconditionally for a runbook with a shell snippet in it, which is most of what anybody publishes.

The failure is narrow but total: the branch only runs when a PUT errors, which is rare — but when it does run against a page carrying any macro, the comparison is guaranteed to fail, so the recovery degrades to precisely the behaviour it was written to fix.

What a fix needs

A normalizer applied to both sides before comparing. From the measurements on hand, at minimum: strip ac:macro-id and ac:local-id (storage_to_md.go already strips both on the read path, for the adjacent reason that reading and republishing a page would otherwise churn ids forever), reconcile self-closing-tag spacing, drop HTML comments, and collapse the whitespace their removal leaves behind — an inline comment leaves a double space.

Establishing the full surface of what Confluence rewrites on write is most of the work and belongs with the fix rather than before it, per docs/confluence/README.md's method. The four above are what has been measured, not what exists.

Two things to carry in:

  • Over-normalizing is the dangerous direction. Too aggressive and updateLanded reports that my write landed when the page actually holds somebody else's content that normalized to look like mine — the exact outcome the three-way match exists to prevent. A normalizer that erases a real difference is not acceptable here at any strength.
  • Erring the other way is free, which is what makes this incrementally fixable. Too-weak normalization degrades to today's behaviour: a spurious failure on a write that landed. Nothing gets worse. (This is not true of a body-comparison idempotence check, where too-weak normalization costs a real republish — a version bump and a notification to every watcher. update silently overwrites a page that moved on: nothing records what a local copy was derived from #149's sha check sidesteps the whole question by hashing what was sent rather than comparing against what was stored.)

So: partial coverage is a real improvement, and stripping ac:macro-id alone would rescue the common case.

Verifying it

The reproduction is awkward in the way the original bug is: it needs a PUT whose response is lost. A test can call updateLanded directly against a fake returning a stored body with an injected ac:macro-id, which pins the comparison without staging a lost response. docs/confluence/storage-format.md should gain the updateLanded consequence either way, so the next reader does not have to re-derive it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions