feat(export): recursive export, and the attachment name it depends on - #128
Merged
Conversation
Exports a page's subtree, or a whole space, as a directory tree mirroring the Confluence hierarchy. The plan's first half is not about export at all: 025 asserted that nothing depends on single-page export's flat-in-the-root attachment placement, and the attachment name does. Page-scoping an attachment changes the path its markdown points at, which changes the percent-encoded name, which makes republishing create a second attachment and orphan the first. So an attachment is named by its basename instead, relying on the path already recorded in its comment since 026 commit 7. That makes page-scoped placement free, stops an asset that moves from orphaning itself, and retires the disagreement where attachfile.Resolve reads a stored name verbatim while sourceFor decodes it. It costs a publish-time refusal when two assets in one file share a basename, and one round of re-uploads for names already published with a directory component. The same rule applies to read and attachment-download, not to export alone, so the three agree on where an attachment lives and byte-identity survives for a top-level page. Slug collisions among siblings are disambiguated with a -<id> suffix rather than refused, which also makes every page directory unique and so makes page-scoped placement collision-free by construction. L5 and L6 flip to Holds, gated on the property test they have never had. Refs #59.
The name is the attachment's identity -- Confluence matches ri:filename literally, and planAttachments decides create-vs-update by name -- so an encoded name moves whenever the path moves, and a moved name is a new attachment with the old one orphaned. Exporting a page positions its images, which moves paths; so does reorganising a repository. The path is not lost, because it is recorded in the attachment's comment since 026 commit 7, which is authoritative where a decoded name was only ever a guess about a name markfluence might not have written. What the encoding did buy is collision-freedom, and that becomes an explicit refusal in the next commit. localAttachments stops round-tripping the source through the name for the same reason: decoding a base name back into a source would record "x.png" for an asset at "docs/assets/x.png". The invariant inverts rather than disappearing -- the stored name is the base name of the recorded source -- and its test says so. decodeName went with it, having no callers left. The five affected regression goldens are regenerated here rather than in a later sweep: a commit that leaves the suite red is a commit nobody can bisect through. Refs #59.
029 says what to build and why; it does not say what lands in what order, where the work is reviewed, or what has already turned out differently while building it. Splitting those apart keeps 029 a design document rather than a log that has to be rewritten every time an ordering assumption fails. The standing rule is that every commit passes make check on its own, which has already moved work earlier three times in commit 1 alone. Six reviews across twenty-four commits: four checkpoints where a complete piece of behaviour first exists, one solo review of the pagedoc position commit because its failure mode is silent, and one security review once page titles have become directory names -- an untrusted input the clamp's reasoning was never written against. 029's last two commits are swapped here: a property test that gates a docs commit has to land before it, or the gate can only be enforced by editing history. Refs #59.
An attachment name is unique per page, so two images whose base names agree cannot both be published: one upload would overwrite the other, the page would show a single image in both places, and only one of the two paths would be recorded. There is no correct output, so there is no output -- MdToConfluence returns an error and the file fails. Deliberately not a Broken entry, which is what 029 called for. Nothing blocks a publish on Broken (cmd/update appends it to the report and carries on), so reporting it that way would put the page up wrong and say so afterwards. The message names both paths and both lines, since "diagram.png is ambiguous" leaves the author hunting for the second one. seen now records the source path each name was claimed by, rather than the bare fact of a claim. That is what keeps the same image referenced twice a dedupe -- one upload, several references -- while two different assets are a collision. A pasted ri:filename gets a warning instead. The shield renames raw storage tags before goldmark, so renderImage never sees such a reference and cannot refuse it in the same breath; and unlike two images, a pasted reference may legitimately mean the very attachment being published. But it is not silent: publishing rebinds it, so a part of the page the author never edited starts displaying something else. Refs #59.
A collision fails the conversion, and check's existing handling of a conversion failure is to report the file as failed with code CONVERT -- the bucket meaning the converter could not run, alongside unparseable frontmatter. That is the wrong shelf. Two images wanting one attachment name is a defect in the document, fixed by renaming a file, exactly like the dead link two lines above it. So the collision gets a typed error and check reports it as Broken: ok false, error and code null, the message in the broken list where an author looking for what to fix will find it. Publishing commands need no such distinction and get none -- either way the file does not go up. The test asserts the bucket through --json rather than the human output, which prints a broken file and a failed one identically. Refs #59.
The converter refuses two images in one document that want one attachment name. The same collision is reachable through a batch of FILEs, where nothing would catch it: planAttachments reads what is already on the page once, before its loop, so two files claiming one name both plan "created", the second upload lands on top of the first, and both are reported as successes. Naming the same file twice is still one name claimed once, not a collision. Refs #59.
AttachmentSource is gone, along with the escape constants it shared with the encoder. A stored name is no longer decoded anywhere: it is the base name, so there is nothing in it to decode, and a name containing "%2F" is a filename with a "%2F" in it. That retires a disagreement rather than only a function. attachfile.Resolve read a.Title verbatim while sourceFor decoded it, so an attachment whose name was encoded but whose comment had been lost landed at dest/assets%2Fbrand.png with markdown pointing at assets/brand.png -- a broken image, in single-page export, today. Both sides now read the recorded path and fall back to the same name. storage2md's images-encoded-src case is renamed to images-uninterpreted-names and rebuilt around what is worth pinning now: exotic filenames (a space, an accent, a literal percent, parentheses) are URL-encoded into a destination and never decoded, so a file called "100%25.png" comes back as "100%2525.png". The regression case of the same name keeps its name -- it is about encoded image *destinations*, which is unaffected. TestRoundTripEncodedImageSources keeps its property and loses its premise: it asserted a codec inverted itself, and now asserts that a destination decodes to the path recorded on the attachment. That matters more than it did, since a single recorded field is what stands between an export and a flattened tree. Refs #59.
The suite was full of attachments named "assets%2Fx.png" carrying a recorded path of "assets/x.png" -- a pairing markfluence can no longer produce, since it would name that file "x.png". The tests passed either way; they were describing a scheme that no longer exists, which is how a reader ends up believing it does. attachfile's literal-name case keeps its encoded name, because that is the case: a file really called "a%2Fb.png" must not be scattered into a/b.png. What changed is why it holds. It used to be a judgement call about a name that might have been one markfluence wrote; now no name is ever interpreted, on either the placement or the markdown side, and the comment says so. Refs #59.
The file opened by stating that markfluence percent-encodes a path into a name and that the mapping is bijective. It does not, since the naming change. The measurements stay. That a `%2F` name resolves and re-escapes to `%252F` in the image URL, that a space and an `é` survive byte-identically -- these are facts about Confluence, not about markfluence, and they remain useful: someone will meet such a name on a page published before the change. Separating the two is the whole edit here; nothing measured was deleted for having stopped being load-bearing. Two consequences were restated rather than dropped. The name-length limit now binds against a single path component rather than a path with its slashes expanded, so it binds even later than it did. And the recorded-path-disagrees branch turns out to do more than repair a mangled comment: it is how an asset that moved keeps its attachment, which under the old names was impossible because the name moved with the path. Refs #59.
L3's note said two things that the base-name scheme falsifies: that moving an asset still changes its identity, and that fixing that would cost the ability to reconstruct a tree on export. Moving an asset within the tree now keeps its identity and restamps its recorded path, and reconstruction was never the name's job -- the comment carries the path. What identity follows now is the asset's file name, so renaming the file is what mints a new attachment; the note says which claims died rather than quietly dropping them. R2 grows a step at the front. An attachment that cannot be *named* -- two assets in one document whose base names agree -- is as unusable as one that cannot be placed, and it is reported in three places: the converter refuses the file, attachment-upload refuses the batch, check reports it as Broken. The label does not change. This file says labels are as permanent as ids because a renamed label makes an old citation silently wrong, so the widening lands in the note and report-unplaceable-attachments stays. root-model.md's attachment-identity paragraph described Source as what the name encodes. It is now the only record of the path, which is a stronger claim than the one it replaces. Refs #59.
Four places in the README described the encoding as current behaviour: the attachment-naming paragraph under Body, --name "which markfluence encodes for you", attachment-list's example row, and export's refusal of --attachments-dir. That last one had to be re-reasoned rather than reworded. It argued the flag was unsafe because rewriting an image's src would change its attachment name and orphan the original -- which is no longer true, since moving assets/x.png to attachments/x.png keeps the name x.png. The flag stays unimplemented for a different reason: collecting everything into one directory reintroduces exactly the collision a base name has to refuse, because two pages' diagram.png cannot share a directory. The naming paragraph and the CLI's own --name help now document the refusal as part of the scheme rather than leaving it to be discovered on a failed publish. attachment-list's note about "attachments left behind by the encoding change" was already there from a previous naming change and now covers this one too. Refs #59.
Two claims in 025 turned out to be wrong, and both are load-bearing enough that a reader finding them unmarked would re-derive the same conclusions. "Nothing depends on today's flat-in-the-root behaviour" -- the attachment name did, which is the whole reason 029 changes the naming scheme before it changes anything about export. "Slug collisions are refused" -- 029 suffixes with the page id instead. Refusing leaves a space unexportable over a punctuation variant, and --space exists precisely for spaces the caller has no permission to retitle. Annotated in place rather than rewritten: the plan is a record of what was decided when, and the reasoning it gives for both is still worth reading. Refs #59.
… the source The guard compared recorded sources, and with no markfluence.yaml above them each file's root is its own directory -- so arch/diagram.png and deploy/diagram.png both record the source "diagram.png", the guard saw one asset claiming its name twice, and waved the collision through. That is the default case, not a corner of one, and the test only passed because it planted a markfluence.yaml. It now runs both spellings. Found in review of the naming half. R2 and the README both assert this refusal holds, so it was asserted in three documents and true in none. Also from that review: - pastedAttachmentNames walks the parsed document for raw-HTML nodes instead of regex-scanning the body, so a fenced code block *documenting* storage format is an example rather than a reference. markfluence's own docs are that shape. The scan parser deliberately omits the AST transformers, which carry the renderer and would report every table-cell warning twice. - The scan itself is one function now (convert.ReferencedAttachmentNames), used by export over stored storage and by the converter over the raw-HTML nodes it just parsed, with the tests moved to sit beside it. - localAttachments' failures are typed, so --json reports VALIDATION for a collision or a directory rather than IO for everything. - Three doc comments still described the deleted codec: localAttachments' own (which contradicted an inline comment forty lines below it), attachref's LocalAttachment (the one place both convert and client read for what these fields mean), and renderImage's "deduped by filename". - check's collision branch notes what it cannot do: a collision aborts the conversion, so it is the only entry that file gets. Refs #59.
It stops being export's private helper the moment read and attachment-download need the same answer: both are about to place an attachment under the page's own directory, and a second copy of the slug rule would mean an attachment landing somewhere the markdown does not point. The package documents the two properties callers lean on rather than leaving them to be rediscovered -- it lowercases, so titles differing only in case collide and can be caught; and it drops "/", so no title can inject a path separator -- along with the fact that it is lossy and that no readable slug can avoid being, which is why the caller decides what a collision means. Also names what a page occupies: For (slug, or the id when a title slugs to nothing) and Filename. The id fallback was buried in pageFilename and is needed by the directory name too. Refs #59.
A recorded attachment path is relative to the root; a markdown destination is relative to the file that carries it. Those coincide only for a page written at the top level, which is the only case single-page export ever produced -- so sourceFor wrote a recorded path verbatim and got away with it. Writing a tree ends the coincidence: dest/home/child.md carrying assets/brand.png resolves it to dest/home/assets/brand.png, finds nothing, and republishes IMAGE BROKEN for every shared asset below the root. StorageOptions therefore carries two positions, not one. PageDir is where the page's file sits, which is what a recorded path is made relative to. AttachmentDir is where an attachment with *no* recorded path is placed -- the directory named after the page -- and it is a separate field because deriving it would mean the converter knowing how a title becomes a directory name. pagedoc computes it, once, so that every command placing such an attachment agrees with the markdown pointing at it. That makes read's output change here rather than in its own commit: read and export share Options, which is the point of Options. An attachment with no recorded path now reads as runbook/diagram.png, which is where attachment-download is about to write it. Nothing in the suite noticed, so the tests that should have are here too -- the four rows of the layout table at the converter, and both provenances through read. Refs #59.
…ectory An attachment name is unique per page, not per space, so fifty Confluence-native pages can each carry a diagram.png. Resolve fell back to the stored name, so all fifty resolved to one file -- self-consistent for a single page, and a collision the moment a second page is written beside it. Options.Dir is where such an attachment goes: the directory named after its page. It must match the AttachmentDir the markdown was rendered with, which is why pagedoc computes both from one place. A recorded path is unaffected -- it is relative to the root and authoritative -- and --flat still means "bare name, straight in --dest", beating both. Resolve takes Options rather than a root and a bool. Two of its four inputs were already in Options, and adding Dir as a third positional argument to a function whose signature ends in a bare bool is how a caller ends up passing them in the wrong order. The clamp is tested against the new join specifically. Dir comes from a page title through internal/pageslug, which drops path separators and so cannot traverse -- but the containment check is what guarantees that, not pageslug's good behaviour. Refs #59.
The last of the three commands to adopt one placement rule. An attachment with no recorded path is written under a directory named after its page, which is where `read` and `export` point at it -- so downloading what read printed resolves, rather than read saying runbook/diagram.png while download writes diagram.png. It costs a request the command did not make: a slug needs the page's title, and this only ever fetched the attachment list. Fetched before anything is written and fatal if it fails, because half the attachments scoped and half not is worse than none of them. A folder id is accepted, as pageref.Resolve already accepts a folder URL, with the page route's 404 for a folder as the signal to look in the other family rather than as a missing id. --flat is unchanged and is now the documented way back to the old behaviour: everything directly under --dest, under stored names. Refs #59.
…t it Positioning the markdown (16328bd) and page-scoping the write (18c7163) were two commits, and export got only the first: it rendered  and then wrote the file to dest/diagram.png. The exported tree previewed broken, and republishing it wrote IMAGE BROKEN over a live image, since update reports Broken and publishes anyway. attachment-download got the write half; export did not, which is exactly the drift both commits' doc comments warned about. pagedoc.AttachmentDir is now exported and both sides call it, so the destination and the path are one decision rather than two that agree by inspection. The test that catches it reads both halves: it exports a Confluence-native page and asserts the file exists at the path the markdown names, for a recorded path and an unrecorded one. Nothing in the suite did that -- cmd/export had no unsourced-attachment case at all, which is why two commits could disagree and stay green. Confirmed failing before the fix. Also from the two reviews: - AttachmentDir's doc claimed an empty value leaves an attachment beside the page; with PageDir set it means the root, a directory too high. - relativeTo's doc understated when filepath.Rel fails. A recorded path containing ".." reaches it, and the fallback leaves a destination pointing at a file attachfile refuses to write -- stated now, with why neither half invents a plausible path instead. - Resolve's doc still described the decode it no longer does and the unscoped fallback it no longer takes. - pageDirFor had been inserted between report's doc comment and report. - README still said an unrecorded attachment is written under its stored name, contradicting the help text updated in the same commit. Refs #59.
Render and Options took a bare pageDir, and the tree export is about to need a second thing that depends on the same decision: what parent: says. A page below the export root points at its parent's own .md file, so the tree publishes into fresh pages rather than only back into the ids it came from. Placement carries both, with a zero value meaning "on its own" -- no directory, parent from the page -- which is exactly what read prints and what a single-page export writes. Passing them separately would let a caller position the body one way and the frontmatter another. No behaviour change: every caller passes the zero value. Refs #59.
Where every page and folder lands, as a function of the walk and nothing else, so the rules can be tested without a server and exist in one place: a page is <slug>.md with a <slug>/ beside it for its children and its own unrecorded attachments, and a folder is that directory with no file. Two rules carry the weight. parent: points at the parent's own .md, relative to the referring file, which is how create resolves it -- that is what makes an exported tree publishable into fresh pages rather than only back into the ids it came from. And a group of siblings that slug the same all take their page ids as suffixes, every member rather than the later arrivals, so a filename does not depend on walk order. An empty root id means the thing named has no file of its own -- a folder, or a whole space -- and its children are the top level. pagetree gains TypePage/TypeFolder so the comparison is not a bare string in another package. Refs #59.
export walks the subtree under the named page and writes it as a mirror of the Confluence hierarchy. --depth is a string vocabulary like children's -- a non-negative number or "all" -- with one deliberate difference: 0 is legal and is the default, where children refuses it. There it would be a request for no rows at all; here it is the named page by itself, which is a real answer and the one this command gave before it could walk. What the tree buys is in the child files: each carries parent: ../home.md rather than an id, so the whole thing can be published into fresh pages, and each page's Confluence-native attachments land under its own directory rather than colliding with its siblings'. Both were already true of the layout; this is the commit that walks, fetches, and writes. Failure is per page. A body that will not fetch fails that page and skips its subtree with create's wording, so nothing is written carrying a parent: path to a file that does not exist. A failure of the *walk* is different and fails the command: pagetree aborts on the first listing error, before any page has been exported, so there is no partial result to report against. --file is refused with --depth. It names one file, while a page's directory comes from its slug regardless, so honouring it in a tree would let a page's file and its own subdirectory disagree. fix's help gains a sentence: reconciling an exported tree rewrites those parent: paths back into ids, which is what fix is for but is a surprise if you have not thought about it. Refs #59.
A space's root pages become the top level of the export, which is what pagetree.WalkSpace already reports; there is no file for the space itself, so the layout takes an empty root and starts at dest. --space requires an explicit --depth. The default of 0 would export nothing at all, since the space is not a page, and defaulting it to "all" instead would make a bare typo fire one pair of requests per page and folder in the space at a shared instance. Asking is the only option that is neither useless nor dangerous. The key is resolved before the walk even though the route the walk uses takes a key: an unknown key is a typo and deserves to be named as one, and the v1 route answers it with a 404 -- which is also what a rejected credential looks like. A failing space walk reports the stderr errorObject shape children --space established, since there is no page id for a results entry to name. exportTree splits into the walk and exportNodes, which is what lets a space share every rule with a subtree: the layout, the per-page failure, the skipped subtree, and the collision warnings. Refs #59.
Without it an exported tree is not republishable, which makes this the least optional file the command writes. The documentation root falls back to a markdown file's own directory, so dest/home/child.md takes dest/home/ as its root, and a shared asset reconstructed at dest/assets/brand.png sits above that root and republishes as IMAGE BROKEN. 025's worked example missed this because a single-page export puts its file at dest, where the fallback root happens to be right -- so a single page still gets no marker, deliberately. Written before the first page rather than after the last: a run that dies partway is exactly what the resume behaviour is for, and a partial tree with no marker is a tree whose every shared asset republishes broken. Never over an existing file (S3), reported on its own line, honoured by --dry-run, and it sets the envelope's roots -- dest is the root every path in the export is relative to, and this file is what makes it one. Refs #59.
One shared asset referenced from many pages is the model's success case: each page carries its own attachment recording the same path, all of them resolve to one file, the bytes match, and every write after the first skips under S3. A differing checksum under one path is two pages disagreeing about what that path holds, and skipping that leaves whichever page was walked first deciding the contents. The rule is narrow on purpose: it compares the checksums in two attachment comments, and a recorded path implies a managed attachment, so both sides of a shared-path collision always have one. Everything else takes the S3 skip, which is a narrowing rather than a hole -- two unsourced attachments cannot collide at all, since each is scoped into its own page's directory and the slug pass makes those unique, and an unsourced one meeting a recorded path needs that path to equal another page's slug directory, which is the same shape as an attachment landing on a page file. Reported as the attachment failing, so the page's other attachments still write and the run's exit status tells the truth. Not overridable by --force, which is about local files rather than about picking a winner between two pages. Refs #59.
A file that exists is not written again (S3), but until now it was rendered first and the result thrown away -- so a retry cost as much as the first run, which for a tree is a few thousand requests. Skipping the render skips the page-width read and every <ac:link> title lookup with it. The attachment pass still runs, deliberately. That is what makes a retry resume a run that died partway through downloading attachments rather than partway through pages; skipping the page entirely would leave those missing forever. --force still redoes everything, and is how a tree whose pages changed upstream is refreshed. The test asserts the saving rather than the wording: it counts the page-property reads and the downloads across two runs, with the attachment deleted in between to stand in for a run that died mid-download. It also fixes a route the fake servers never matched -- the download link is served under /wiki, so the handler's /download/ prefix test always fell through to the page JSON, and the earlier tests were asserting on attachment files that held a JSON page body. Refs #59.
Three changes to the --json surface, where the plan budgeted one. exportResult gains parent_file: the parent: value actually written into the file, null when the parent stayed an id. Without it a consumer cannot tell what hierarchy the tree on disk describes, only what the live pages say. create already uses that name for the concept. The summary becomes a typed struct rather than the map every other batch uses, because project_file is not a count -- it says whether the markfluence.yaml a tree needs was written, found, or not needed. And it carries skipped, because skip-and-resume is how a retry works here: a run that exports nothing new is all skipped and has still succeeded, which basicSummary cannot express. The conformance test now builds its document with the command's own builder. It was assembling an envelope from a hand-written map, which is the thing internal/schematest's rules exist to prevent -- and is why the summary change did not fail it until the shape was validated. envelope() is that builder, shared by report and the test. Refs #59.
Both found at checkpoint C, one by the security review and one by both reviews. An attachment could be written over an exported page's file. A recorded path= is a server-side string that can name any file under dest, and a parent's attachments are written before its children are exported -- so an attachment recording path=handbook/onboarding.md lands there first, and the child page is then reported "skipped (exists)" and counted as a success. The reader gets attacker-chosen bytes in a file they believe is a page they exported, and publishing it back sends those bytes to whatever page_id its frontmatter names. Every page's destination is now reserved before any attachment is written, and an attachment resolving onto one fails rather than landing. And two siblings whose titles slug the same shared an attachment directory. The layout disambiguates them -- deploy-prod-2/ and deploy-prod-3/ -- but the attachment directory was recomputed from the raw title, so both pages' native diagram.png resolved to deploy-prod/diagram.png; the second was skipped as already there, and its markdown pointed at the first page's image. Nothing else would have caught it: a native attachment has no checksum, so the conflict rule sees no disagreement. That falsified the premise the conflict rule is documented to rest on, which is why the rule could be as narrow as it is. Placement now carries the directory the layout named, and pagedoc derives one only when the caller names none. Five smaller findings from the same review: - --space --depth 0 passed the target check, walked nothing, planted a project file, and exited 0. checkTarget only asks whether --depth was given. - Every page's attachments were listed twice, once by the command and once by pagedoc.Sources -- one extra request per page in a tree, on top of the walk. Placement carries the listing the caller already has, which is what SourcesFrom was written for. - Collision warnings were folded into the first result, whose warnings are not printed when that result failed. They belong to the run and are printed as such. - roots and the marker line reported the raw --dest, which is "." by default, beside dest_paths that are absolute. - exists()'s comment claimed a fail-closed guard the body does not implement. Usage errors now precede the credential check, as children's do: `export` with no arguments reported a missing token rather than saying what to pass. Refs #59.
The property is that markdown is a fixed point: export a page, publish that markdown back, export again, and get the same markdown. Once a page has been through markfluence it stops moving. That is deliberately weaker than L5's wording, and the live instance is why. Measured against a real page today: Confluence's editor writes <li><p>text</p></li> where the converter emits <li>text</li>, and a TOC macro carries ac:local-id/ac:macro-id/data-layout attributes the canonical form omits. Both render identically and neither loses content, but the stored bytes differ -- so "publishing an export back changes nothing on the page" is false for reasons that predate this branch and have nothing to do with it. What a reader depends on, and what this asserts, is that the export is stable. The corpus is every storage2md case rather than a hand-kept list, because a hardcoded list is exactly how #125 stayed invisible: an ac:adf-extension existed and simply was not in it. It found a bug on its first run. Storage is XHTML, so "<br />\nSecond" is the ordinary spelling, and that newline normalizes to a space landing right after markdown's two-space hard break -- indenting the next line one character further on every cycle. Trimmed where the sibling order is known rather than with a pass over the assembled string, which cannot tell that whitespace from the two spaces that *are* the following hard break; the first attempt did exactly that and turned two consecutive breaks in a table cell into a broken row. Refs #59.
029 called for this and the implementation never did it: every v2 page route answers a folder id with 404, so `export <folder-id> --depth all` reported "page not found". Found by running the command against a real space, where a folder is an ordinary thing to point at -- pageref accepts a folder URL precisely because that is what a browser hands you. A folder has no body, so it is a target only with --depth: what is inside it becomes the top level, exactly as a space's roots do. At --depth 0 it is refused with a message saying so rather than a bare not-found. That exposed a second bug in the layout. The walk's top-level nodes report the *folder* as their parent, while a space's root pages report no parent at all -- so grouping by parent found nothing for a folder and every page resolved to the destination directory itself, which then reported four pages "skipped (exists)" against one path. rootRef carries the three facts the layout actually needs: the id children hang off, the title, and whether anything is written for it. Passing a title and an id and inferring the rest is what let those disagree. Verified against the live instance: a four-page folder exports to a flat top level with each page's Confluence-native attachments under its own directory, the markdown resolves to them, a re-run skips everything, and `check` reports all four files clean with the root resolving to the export directory. Refs #59.
README gains the --depth/--space/folder surface, the layout, the parent: paths, the marker file and why it is not optional, and the skip-and-resume behaviour. CLAUDE.md gains internal/pageslug and rewrites the export and pagedoc bullets around what a reader would otherwise have to rediscover: that rootRef carries the id children hang off separately from whether anything is written for it, that the layout's disambiguated directory is what makes page-scoped placement collision-free, and that destClaims exists because a recorded path= is server data that can name a page's own file. pagedoc's byte-identity claim is narrowed rather than deleted: read and export are identical for a page at the top level of what is being written, and differ deeper in exactly the position-dependent parts, because read has no tree. guarantees.md is the substantive change. #59 was cited as the thing that would settle L5 and L6, and it settled them the other way: measured against a live page, export-then-republish rewrites <li><p>text</p></li> as <li>text</li> and drops a TOC macro's local-id/macro-id/data-layout. Both render identically and neither loses content -- semantic rather than byte equivalence is the converter's stated design target -- so the guarantee as worded asks for something markfluence deliberately does not do. They stay Partial with that recorded, and with the fixed-point property that *is* verified stated beside it. Rewording them is its own decision and is not taken here. The export section's --attachments-dir rationale is re-reasoned in CLAUDE.md the way it already was in the README: no longer unsafe, still not wanted. Refs #59.
True for a page at the top of the export, which is every single-page export. Deeper in a tree the paths are relative to where the file sits, and read has no tree to be positioned in. Refs #59.
Collision warnings were dropped entirely under --json. layout returned them as a run-level slice, and the envelope has nowhere to put one -- so a space export wrote deploy-prod-123.md and deploy-prod-456.md and said nothing anywhere about why those two names differ from every other page's. They are per-page now, carried on the placement and into results[].warnings, which the schema already has; every member of a colliding group gets it, and human output prints it before the failure branch so a page that went on to fail still says it. --file bypassed the reservation that keeps an attachment off a page's own file. The override was applied after the reservation was built, so the reserved path was the slug this run does not use: the real file was unprotected, and an attachment recorded at the unused slug was refused for colliding with nothing. A failed walk still planted markfluence.yaml. That file re-roots the documentation root for every later run in that directory, so a command that exported nothing was changing how an unrelated publish resolves its images. It now waits until the walk has succeeded, which is still before the first page -- the property the ordering exists for. --name stopped normalizing the path it records. The round-trip through the attachment name used to do it as a side effect; without it, --name /assets/x.png recorded an absolute path that Resolve refuses outright while sourceFor falls back to the page directory, so the file could never be restored where the markdown says it is. convert.NormalizeSource is that step, exported. results[].ok disagreed with the summary: a page whose file wrote but whose attachment download failed reported ok true inside a summary counting it as failed. Same condition both places now. And two comments still described the name-decoding fallback that no longer exists -- in the two places that explain where a file lands versus where the markdown points, which is the worst place to be wrong. Refs #59.
The previous commit moved collision warnings onto the placement and appended them in exportOne -- and then assigned over them twenty lines later, where missingReferences' result lands. So the warning survived only the three early failure paths, and was lost on exactly the case the fix was written for: two siblings written as deploy-prod-123.md and deploy-prod-456.md, exported cleanly, with nothing said anywhere about why. The test that was supposed to cover it built its result by hand, setting both place.warning and result.warnings, so it validated a shape the code never produces. Both tests here go through exportOne/exportNodes instead: TestFileFlagIsReservedNotTheSlug also re-implemented the override-then-reserve sequence it claimed to be exercising, and would have passed against the ordering it was written to catch. Also: human output prints a page's warnings after its own lines again, rather than ahead of them, with the failure branch keeping its own copy since there is nothing for them to follow there; --name reports the path as typed rather than after normalization, where "/" has already become ""; and the schema says what warnings now holds. Refs #59.
run, exportFolder and exportSpace each ended with the same four steps -- dry-run banner, walk, project file, report -- differing only in what they walk, what they hang the layout off, and how a walk failure is reported. Three copies of an ordering the marker's own comment calls load-bearing, and they had already drifted once in the way that matters: the marker was written before the walk in all three, so a failed walk planted a file that re-roots every later run in that directory. target names the four differences and runExport is the order, once. Both of target's scalar fields turned out to be invisible to the suite -- a review inverted multiPage for the page path and swapped the space's failure reporter, and every test still passed -- so runexport_test.go covers the wiring: which targets plant a project file, that a failed walk plants none, and that the target's own reporter is the one used. Two more gaps closed while here. A folder that collided with a sibling was renamed with no warning anywhere, because a folder produces no result for one to ride on -- the page half of that was fixed in the previous commit and the folder half was not. And reportOne's two warning loops had no test; only --json was asserted. Refs #59.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #59.
Exports a page's subtree, a folder's contents, or a whole space as a directory tree that mirrors the Confluence hierarchy and can be published back. The design is
_plans/029; the commit sequence, the checkpoints, and everything that turned out differently while building it are_plans/031.Half of this is not about export
_plans/025asserted that nothing depended on single-page export's flat attachment placement. The attachment name did. A name was the percent-encoded root-relative path, and the name is the attachment's identity — Confluence matchesri:filenameliterally, andplanAttachmentsdecides create-vs-update by name — so page-scoping an attachment moved its markdown path, moved its name, and made republishing create a second attachment while orphaning the first. Exporting a page positions its images; so does reorganising a repository.So an attachment is named by its base name, and the path lives only in the comment, where
_plans/026already put it. That makes page-scoped placement free, stops an asset that moves from orphaning itself, and retires a disagreement that was already producing broken single-page exports —attachfile.Resolveread a stored name verbatim whileconvert.sourceFordecoded it, so an attachment with an encoded name and a lost comment landed atdest/assets%2Fbrand.pngwhile its markdown saidassets/brand.png.What the encoding bought — collision-freedom — becomes an explicit refusal: two assets in one document whose base names agree cannot both be published, since a name is unique per page. The converter refuses the file,
attachment-uploadrefuses the batch, andcheckreports it offline as a document defect rather than a converter failure.Costs, all stated in
_plans/029§"So the name changes instead": a new publish-time refusal, path recovery resting entirely on the comment, one round of re-uploads for names already published with a directory component, and a wider surface for the pre-existing hand-uploaded-attachment clobber.The tree
A page becomes
<slug>.mdwith a<slug>/beside it for its children and its own Confluence-native attachments; a folder becomes a directory with no file. Each child'sparent:points at its parent's file, so the tree publishes into fresh pages rather than only back into the ids it came from.--depth(0default / a number /all),--space KEY(which needs an explicit--depth), and a folder as the target.-<id>suffix — every member, so a filename never depends on walk order. Refusing was the earlier design and would leave a space nobody can retitle unexportable over a punctuation variant; an exported filename is ergonomic, where identity ispage_id(L8).markfluence.yamlis planted at--destafter the walk and before the first page. Without it each exported file's root is its own directory, so a shared asset above a page republishes asIMAGE BROKEN; after the walk because a command that exported nothing must not re-root every later run in that directory.readandattachment-downloadadopt the same placement rule, so all three agree on where an attachment lives.--flatis the way back.L5 and L6 stay Partial, and this is what proved it
docs/guarantees.mdnamed #59 as the thing that would settle them. Measured against a live page, export-then-republish rewrites<li><p>text</p></li>as<li>text</li>and drops a TOC macro'slocal-id/macro-id/data-layout. Both render identically and neither loses content — semantic rather than byte equivalence is the converter's stated design target — so the guarantee as worded asks for something markfluence deliberately does not do. They stay Partial with that recorded and dated. Rewording them is its own decision and is not taken here.What is verified instead is the property that holds: markdown is a fixed point. Export, republish, export again, identical — over every
storage2mdcase rather than a hand-kept list, since a hardcoded list is how #125 stayed invisible. It found real drift on its first run: a hard break gained a leading space on every cycle, because storage spells<br />\nSecondand that newline normalizes to a space.Review
Eight review passes at four checkpoints, plus a security pass and two closing passes, per
_plans/031. Every one found something, and the pattern was consistent: a test existed and passed over the bug it was written for. Among them:exportrendering attachments page-scoped while writing them flat: two commits that disagreed, with no test covering an unsourced attachment at all.path=names an exported page's file (written first, page then reportedskipped (exists), exit 0), and two slug-colliding siblings sharing an attachment directory because it was recomputed from the raw title. The second falsified the premise the conflict rule is documented to rest on.--json, then dropped again on the success path by an assignment twenty lines from the fix.targetstruct inverted with the whole suite still green.The security pass verified what holds, too: the
--destclamp andos.Rootconfinement are not circumventable via a recordedpath=, an attachment name, or a page title, andpageslugprovably cannot emit a traversal segment.Live verification against a real space found two more that no fake server would: a folder as the export target was specified in
_plans/029and never implemented, and fixing it exposed that the layout conflated "the id children hang off" with "the root has a file".Notes
createshape this made reachable: a document defect surfaces in phase 3, after the stub page exists and itspage_idis persisted.make checkon its own.