client.updateLanded can never fire for a page holding a macro
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2
- Forks
- 0
- Avg merge
- 2h 8m
- Merged PRs (30d)
- 49
Description
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
updateLandedreports 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. #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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at internal/client/client.go:945 and trace updateLanded and its page reread path. Review storage_to_md.go plus docs/confluence/storage-format.md and docs/confluence/README.md, then build a focused fake-client test with an injected macro ID and other measured rewrites. Done means a conservative comparison test passes without treating differing content as landed, and the measured consequence is documented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, cli
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100