diff --git a/CLAUDE.md b/CLAUDE.md index c823bd4..8178179 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,19 +52,20 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `cmd/root.go` — the cobra root: `--url`/`--username`/`--debug`/`--no-color` persistent flags, version from `internal/buildinfo`, and registration of every subcommand. `Execute()` prints cobra-generated errors (bad args/flags) but not `ui.ErrSilent`, which marks a failure a command already reported. - `cmd/{update,create,fix,check,info,read,export,children,find,search}/` — one package per command (each exports `Cmd`), orchestrating the `internal` packages and `internal/ui` output. `create` is two-phase and transactional (validate all, then create parents-first in topological order); `fix` is read-only on the server; `check` is read-only on both the server and disk and touches neither, since it never constructs one (see its own bullet below). `create` accepts a **page or a folder** as `parent`: `checkParentInSpace` asks the page route, then `/folders/{id}`, and finding nothing as a page proves nothing until both have been asked — that single missing fallback was #68, since Confluence itself accepts a folder `parentId` with no other accommodation. It also validates a frontmatter `page_id` **first** (before space/parent/title lookups: most specific error, three fewer API calls) and treats all three outcomes as failures — non-numeric, resolves to nothing, or already taken. The middle one is the bug from #18: publishing anyway would create a second page and overwrite the id, so an id that can't be explained is never "create a new page". Both "already exists" errors (page_id taken, title clash) link the page in the way; the `pageIDFailure` typed error carries `page_id`/`url` so `--json` reports them as fields, which is the only case where a failed result names a page. - `cmd/check/` — `check` (#42): validate one or more markdown FILEs against the converter and frontmatter rules with **no network access, no credentials, and no writes** — the first command whose `run()` never constructs a `client.ConfluenceClient` (`root.go`'s `PersistentPreRunE` doesn't force one into existence either, so nothing upstream requires it). It builds `root`/`index` per file exactly like `update`/`create` (`internal/project.Cache`/`internal/linkindex.Cache`), against hardcoded `baseURL`/`spaceKey` (the regression suite's own `https://wiki.example.net`/`ENG`) rather than flags — both are read only to build a rewritten doc-link's *text*, and nothing in `Broken`/`Warnings` reads either, so hardcoding them costs nothing and makes `check` byte-identical across machines. Frontmatter validation is deliberately narrow: an unparseable/unterminated block (`frontmatter.ErrUnterminatedFrontmatter`), an invalid `page_width` (`pagewidth.Declared`), a present-but-non-numeric `page_id` (`pageref.IsDigits`) — never whether `page_id`/`space`/`parent` are set at all, since `check` cannot know whether the caller is about to `create` or `update`, and a false positive there is worse than a miss. A `broken` result is `ok: false` with `error`/`code` both left `null`: unlike every other failure, `broken`/`warnings` already say everything there is to say, so `code: VALIDATION` is reserved for `status: failed` (a file that never reached the converter at all). `--show-html` surfaces `ConfluencePage.HTML`/`Attachments` — nothing else in the CLI ever prints either — as `debug: {html, attachments} | null`; `html` stays compact/unindented in `--json` (matching what `update`/`create` would literally publish) while human output indents it by nesting depth (`indentHTML`, a per-line indent based on tag-open/close counting, not a whitespace-normalizing reformat, since the renderer already breaks lines at every structural boundary and reformatting within a line could alter meaningful inline text). -- `cmd/export/` — `export`: `pagedoc` for the body, `attachfile` for the attachments. Markdown only, and attachments land at their recorded paths — there is deliberately no `--attachments-dir`, since collecting them would require rewriting image `src`s, which would make the next `update` publish under different attachment names and orphan the originals. Only referenced attachments are exported, found by scanning raw storage for `ri:filename` (not just `ac:image`, which is all the converter special-cases, so a link target or a macro-internal reference would otherwise be dropped). A reference with no attachment is a warning, not a failure. +- `cmd/export/` — `export`: `pagedoc` for the body, `attachfile` for the attachments. **`--depth` exports a subtree** (`0` default / a number / `all`), **`--space KEY` a whole space** (requiring an explicit `--depth`, since the default would export nothing and defaulting to `all` would make a typo walk a whole space), and a **folder** may be the target — a folder and a space have no file of their own, so their children become the top level, which is why `layout`'s `rootRef` carries the id children hang off *separately* from whether anything is written for it: the walk's top-level nodes report a folder as their parent but report nothing for a space, and conflating the two placed every page at the destination root. `layout` owns every path an export writes (mirrored hierarchy: `.md` plus a `/` for children and unrecorded attachments, a folder as a bare directory) and the `-` suffix for a group of siblings that slug the same — applied to *every* member so a filename never depends on walk order, and disambiguating rather than refusing because a space nobody can retitle would otherwise be unexportable over a punctuation variant (an exported filename is ergonomic; identity is `page_id`, per L8). `parent:` is a relative path to the parent's own `.md` so the tree publishes into fresh pages, except for the export root and a page whose parent is a folder, which keep an id. Two things the layout buys that are easy to miss: page directories are unique, which is what makes page-scoped attachment placement collision-free — so `pagedoc.Placement` must carry the *disambiguated* directory (`AttachmentDirFor`), or two colliding siblings silently share one attachment file, which no checksum catches because a native attachment has none. `destClaims` reserves every page's destination before any attachment is written, since a recorded `path=` is server data that can name a page's own file and a parent's attachments are written before its children exist — otherwise the attachment lands first and the page is reported `skipped (exists)`. A page already on disk skips its *render* but not its attachment pass, so a retry resumes a run that died mid-download. `markfluence.yaml` is planted at `dest` for a multi-page export **before the first page**, because a partial tree with no marker republishes every shared asset as `IMAGE BROKEN`. Markdown only. An attachment with a recorded `path=` lands there; one without is page-scoped. Still no `--attachments-dir`, but for a different reason than before the naming change: moving an asset no longer renames its attachment, so it is no longer unsafe — it would simply reintroduce the collision a base name has to refuse, since two pages' `diagram.png` cannot share a directory. Only referenced attachments are exported, found by scanning raw storage for `ri:filename` (not just `ac:image`, which is all the converter special-cases, so a link target or a macro-internal reference would otherwise be dropped). A reference with no attachment is a warning, not a failure. - `cmd/children/` — `children`: list the pages and folders under a page or folder, via `internal/pagetree`. `--depth` is a **string** vocabulary (a positive number or `all`, default `1`), not an int: `all` is not a number, and `0` is refused rather than read as "unlimited" the way it is elsewhere, because silently walking a whole space for someone who meant "none" is worse than an error that names `all`. Folder rows are emitted with a `type` column, which is what makes "a folder counts as a level" safe. Empty is a success: `No children.` and exit 0. **`--space KEY` lists a whole space instead of a page** (#98), which makes `PAGE` optional — exactly one of the two, checked before credentials. Depth 1 is then the space's **root pages**, not the homepage's children: a space can have several roots (`create` with a null parent makes one), so seeding the walk from `homepageId` would drop a root and its whole subtree, and there is no root-level *folder* to miss because a folder created with no parent lands under the homepage ([docs/confluence/spaces.md](docs/confluence/spaces.md)). A root row's `parent_id` is `null` — the one place `childrenResult` needs `stringOrNull` for it — since a space is not a node. The key is resolved through `ResolveSpaceID` before the walk even though the v1 route it feeds takes a key: an unknown key must fail as a typo (exit 2) the way it does for `find`/`search`, and the v1 route reports one as a 404, which is also what a rejected credential looks like. Because a space's top level is usually one row, human output adds a `--depth` reminder when `--depth` was left at its default — on **stderr**, via `ui.Hint`, so the table stays pipeable; `--json` never sees it. A failing space walk likewise reports an `errorObject` on stderr rather than a `results[0]` failure, since `SingleOpFailure.page_id` would otherwise carry a space key. - `cmd/find/` — `find`: resolve a title to the ids carrying it, via `client.FindByTitle`. A title is the one handle `internal/pageref` cannot resolve. It reports **current pages, archived pages, and folders**, which takes two requests because no single API sees all three — and the three-way split is the thing to keep straight before touching it ([docs/confluence/search.md](docs/confluence/search.md)). An **archived** page is reported, with a `status` column, because it is absent from the page tree yet still reserves its title; a **folder** is reported because a folder id is a legitimate `parent`, but a folder reserves nothing, so a folder row must never be treated as a naming conflict. `--space` is a space **key**, and an unknown one is a hard error rather than an empty result — CQL answers an unknown key with zero rows, which reads exactly like "no such page". Either half failing fails the whole command: a partial answer reads as "nothing found", and the caller's next move on that is to create a duplicate. Empty is a success: `No matches found.` and exit 0. Its operational failure is an `errorObject` on stderr rather than a `results[0]` entry — there is no page id to name — which it shares with `search` and with `children --space`, and nothing else. - `cmd/search/` — `search`: find pages by **full text**, via `client.SearchText` (or `SearchRawCQL` under `--cql`). The complement to `find`: `find` needs the exact title, `search` is for when it is unknown. The evidence for every choice here is in [docs/confluence/search.md](docs/confluence/search.md), and two things there must be understood before touching the query. First, **`text ~` — the field Atlassian documents — ranks uselessly**: for "deploy runbook" it returned six unrelated pages above every page titled with both words, where the undocumented `siteSearch ~` returned them in order. Second, **`siteSearch` is silently discarded when it is the middle clause of three**, which turns a `--space` search into a listing of the entire space with no error — so `buildTextCQL` puts it **first** and adds a redundant `text ~` clause as a floor, and both are pinned by tests. Do not reorder that query. There is no client-side recovery, because the API reports `score` as `0.0` on every row, so **the server's order is the only ranking and nothing may re-sort a result set** (this is the first command whose result order is not its own). Output is a block per hit rather than a table: the excerpt answers "why did this match?" and is too long for a column. Matched terms in that excerpt are **reverse-videoed from the server's own `@@@hl@@@` markers**, which `cleanExcerptSpans` keeps as `SearchMatch.Spans` instead of discarding — highlighting by matching the query text would have to reimplement Confluence's stemming and would have nothing to work from under `--cql`. `Excerpt` stays the canonical string the schema pins and the spans reassemble to it exactly, so the human and `--json` paths cannot disagree; the flags are built *during* cleaning, since unescaping and whitespace-collapse both change length and marker offsets taken beforehand do not survive them. `renderSpans` takes the highlighter as a parameter rather than calling `ui.Match`, because tests run with stdout not a terminal where lipgloss emits nothing at all — a test wired to the real style would pass against unhighlighted text. `--limit` is a **string** vocabulary (a positive number or `all`, default `10`) refusing `0` exactly as `children --depth` does — and the default is a bound rather than "all" because a full-text query matches thousands of pages where a title lookup matches a handful, kept as low as `10` because a hit is a 5-6 line block rather than a row; the pager fetches one extra row so "more exist" is reported without claiming a count `totalSize` cannot supply. `--type` (`page` default, `blogpost`, `all`) exists because an untyped query returns attachment, comment and database ids no verb accepts; `folder` is **refused** with a pointer to `find`, since full text cannot match a folder at all and always answering "no matches" is worse than an error. `--cql` passes the query through verbatim and **refuses `--space` and an explicitly-set `--type`** (via `Flags().Changed`, which only `children`'s `--depth` hint also uses), because ANDing a clause onto a query containing `or` regroups it and silently answers something else. Two things it cannot see, both `find`'s job: **archived pages and folders**. A blank query is refused locally, because the API answers one with a 500 rather than a 400. -- `cmd/attachment{list,upload,download}/` — the flat `attachment-list`/`attachment-upload`/`attachment-download` commands (noun-first so cobra's alphabetized help keeps them together and `attachment-` completes as a group). `upload` reuses the checksum skip/update logic, with `--force` (`client.ForceUploadAttachments`) and `--dry-run` (`PlanAttachments`); its `--name` takes a *path* and encodes it, and the recorded `path=` is always the decode of the stored name, so a later publish can't create a duplicate under a different name. `download` restores an attachment to its recorded `path=` (never a decode of the stored name — a hand-uploaded `a%2Fb.png` is indistinguishable from a published one), with `--flat` to opt out; `destPath` is the only place server data becomes a filesystem path and clamps to `--dest`, refusing rather than clipping an escape, since `..` is legitimate in a source path. +- `cmd/attachment{list,upload,download}/` — the flat `attachment-list`/`attachment-upload`/`attachment-download` commands (noun-first so cobra's alphabetized help keeps them together and `attachment-` completes as a group). `upload` reuses the checksum skip/update logic, with `--force` (`client.ForceUploadAttachments`) and `--dry-run` (`PlanAttachments`); its `--name` takes a *path* whose base name becomes the stored name, and the recorded `path=` is the path as given, so a later publish can't create a duplicate under a different name; a batch whose base names collide is refused, since `planAttachments` reads the page's attachments once before its loop and would otherwise plan two creates for one name. `download` restores an attachment to its recorded `path=`, using the stored name verbatim when there is none (never interpreting it — a file really called `a%2Fb.png` must not be scattered into `a/b.png`, and `convert.sourceFor` answers the same way on the markdown side, which is what keeps a downloaded file where the markdown says it is), with `--flat` to opt out; `destPath` is the only place server data becomes a filesystem path and clamps to `--dest`, refusing rather than clipping an escape, since `..` is legitimate in a source path. - `cmd/schema/` — `schema`: print the embedded `--json` schema to stdout verbatim (no args, no credentials, no Confluence call). `--json` is deliberately a no-op — the output is already the schema document, not an envelope — which is also why `schema` is absent from the schema's own `command` enum. - `schema/` — the published JSON Schema (`json-output/v1.json`) *and* the `schema` Go package that embeds it (`V1`). The Go file lives beside the schema because `go:embed` cannot reach outside its own directory, and the schema stays at a top-level path a non-Go consumer can browse, mirroring its own `$id`. `internal/schematest` validates against the embed rather than reading the file, which is what makes "what ships" and "what the tests checked" the same bytes — do not reintroduce a disk read or a second copy. The version number is **not** restated here: `jsonout.SchemaVersion` and the document's own `schema_version` const are the two copies, tied together by a test in `cmd/schema`. -- `internal/pagedoc` — a fetched page as a markdown document: `Render` (frontmatter + converted body), `Frontmatter`, and the two lookups the converter can't do for itself — `Sources`/`SourcesFrom` (attachment name → recorded source path) and `PageLinks` (the page an `` points at → its URL). Shared by `read` and `export`, which must emit byte-identical markdown, and **both build their `convert.StorageOptions` through `Options`** rather than assembling their own — options built in two places are options that can disagree. It needs a client (page width, attachment list, title lookups), which is why it isn't in `internal/convert` — that package is deliberately client-free, and it's why `StorageToMarkdown` takes those maps rather than fetching them. Every one of them is best-effort in the same shape: no references in the body means no request at all, and a lookup that fails is omitted rather than fatal (an omitted page link renders as raw storage, not as a link with no destination). `PageLinks` resolves a space id **once per space key**, not once per link, and refuses to search site-wide when it can't scope a title to a space — a same-titled page in the wrong space is a wrong answer, which is worse than the passthrough a miss produces. +- `internal/pageslug` — `Slug`/`For`/`Filename`: a title to a filename-safe slug. A package rather than a helper because `export`, `read` and `attachment-download` all place attachments under a page's own directory and must agree. It lowercases (so case-variant titles collide and can be caught) and drops `/` (so no title can inject a path separator); it is lossy, and no readable slug can avoid being, so the caller decides what a collision means. Known limit: NFD and NFC spellings of one title are different Go strings but one filename on APFS, so that pair is not disambiguated. +- `internal/pagedoc` — a fetched page as a markdown document: `Render` (frontmatter + converted body), `Frontmatter`, and the two lookups the converter can't do for itself — `Sources`/`SourcesFrom` (attachment name → recorded source path) and `PageLinks` (the page an `` points at → its URL). **One conversion, parameterized by a `Placement`**: where the page's file sits, where its unrecorded attachments go, what `parent:` says, and the attachment listing the caller already has. `read`, `export` and `attachment-download` all go through `Options`/`AttachmentDirFor` rather than assembling their own, so they cannot drift by accident — only by argument. For a page at the top level of what is being written, which is what `read` prints and what a single-page export writes, `read` and `export` are byte-identical; deeper in a tree they differ in exactly the position-dependent parts (a sourced attachment's `../` prefix, a `-` suffix a sibling forced, and `parent:`), because `read` has no tree to be positioned in. It needs a client (page width, attachment list, title lookups), which is why it isn't in `internal/convert` — that package is deliberately client-free, and it's why `StorageToMarkdown` takes those maps rather than fetching them. Every one of them is best-effort in the same shape: no references in the body means no request at all, and a lookup that fails is omitted rather than fatal (an omitted page link renders as raw storage, not as a link with no destination). `PageLinks` resolves a space id **once per space key**, not once per link, and refuses to search site-wide when it can't scope a title to a space — a same-titled page in the wrong space is a wrong answer, which is worse than the passthrough a miss produces. - `internal/attachfile` — `Resolve` (where an attachment goes under a destination root, **including the traversal clamp**) and `Write` (download it there, honoring force/dry-run). Shared by `attachment-download` and `export`; the clamp must never exist in two copies. - `internal/pagetree` — `Walk`, the traversal of pages *and folders* under a node, plus `WalkSpace` (the same traversal seeded from a space's root pages, via `client.ListSpaceRootPages`) and `AllDepths`. Both go through one `walker`, so the depth rule and the visited guard exist in a single copy. It is a package rather than command-local because listing a subtree and exporting one (#59) need the identical walk, and its rules must not exist in two copies: siblings arrive from two requests (`/child/page`, `/child/folder`) and are **merged by `extensions.position`**, or the output loses the order Confluence displays; a folder **counts as a level** like a page, which is only reasonable because folders are reported rather than silently traversed; and the walk descends folders even when only pages matter, since a folder may hold the only pages in a subtree. `nodeURL` uses `SiteURL()` — a v1 child row carries `webui` but no `base`. A visited set guards the unbounded case. - `internal/pageref` — `Resolve`, the single page-argument resolver: a numeric id, a Confluence page **or folder** URL (`pagePathRE` matches both `/pages/` and `/folder/`, since `children` takes a folder and a folder URL is what a browser hands you — the id is all it returns, so a command that can only use a page reports its own not-found), or a `.md` file whose frontmatter has a `page_id` (stat'd first, so `123.md` is a file). Every command taking a page uses it. `message.go` also owns the wording for the two ways a *frontmatter* `page_id` is wrong — `NotFoundMessage` (caller supplies the remedy, which differs per command) and `NotNumericMessage` — because `create`, `update`, and `fix` all report them and a reader should recognize the same problem across all three. They return strings, not errors: `create` wraps the text in its typed `pageIDFailure` (which also carries the `--json` fields), the others want a plain error. Anything checking a `page_id` before a request uses `IsDigits`, since the API answers a non-numeric id with a 400 whose body says nothing useful. - `internal/client` — `ConfluenceClient` over `net/http` with basic auth. Built from a `Config` (site URL, cloud ID, username, token) via `New`; it carries **two bases**: `BaseURL()` is where requests go (the gateway when a cloud ID is set) and `SiteURL()` is always the site. Anything a reader sees uses `SiteURL()` — printed page URLs and, critically, the `baseURL` handed to `convert.MdToConfluence`, since rewritten links are published *into* the page. Pages are Confluence **v2**; attachment writes and the user lookup are **v1** (`/wiki/rest/api/...`). A **folder** — the Cloud content type that can parent a page — has its own v2 route, `GetFolderOrNil` against `/wiki/api/v2/folders/{id}`, because every v2 *page* route answers a folder id with 404; enumerating children, if it is ever added, must be v1, since v2 cannot list inside a folder at all and its page-children route silently omits folders ([docs/confluence/folders.md](docs/confluence/folders.md)). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send`. `HTTPError.Error()` appends a **hint** for the three auth failures whose status misleads, matched on the response *body* rather than deduced from the status and always **appended** to it, never replacing it. The one that matters: **a rejected credential is a 404 on every v2 route**, so a revoked token used to make `read` answer `page ... not found` about a page that exists. `RejectedCredential` tells it apart by the fact that every genuine v2 404 *names* what it could not find and the auth one does not, `notFound` gates the three `…OrNil` helpers on it so they stop reading it as "absent", and `jsonout.CodeFor` checks it before the status switch so `--json` reports `AUTH` rather than `NOT_FOUND`. A 403 that is not one of the two measured credential phrasings gets no hint, because that is what a genuine permission denial looks like ([docs/confluence/api.md](docs/confluence/api.md#scopes)). **Retry rules**: 429 for any method; 502/503/504 for idempotent methods; **any other 5xx only when the response carries `Retry-After`** — that is how a 500 becomes retryable, and it is why `parseRetryAfter` reports the header's *presence* apart from its delay (`Retry-After: 0` means "retry now", not "no header"). The exponential delay is jittered, a server-supplied `Retry-After` never is. Decisions go to a package-level hook (`SetRetryLogger`, set once in `root.go` beside `ui.SetDebug`) and fire whichever way they went, because `internal/client` prints nothing and a silent twelve-minute retry storm is otherwise indistinguishable from a hang. **A versioned PUT is not as idempotent as its method**: `SetContentProperty` retry-once on top (recovers a lost create-POST response) and `UpdatePage`'s `updateLanded` both exist for the same reason — a write whose response was lost gets re-sent, and the re-sent version is refused. `updateLanded` requires version *and* title *and* body to match what was sent, since a concurrent edit could have produced the version alone and claiming success over someone else's content is worse than a false failure ([docs/confluence/api.md](docs/confluence/api.md)). `SyncAttachments` (skip/update by a SHA-256 recorded in the attachment's comment, alongside the source path so `read` recovers image paths exactly; only the current comment form is parsed — an attachment stamped by a markfluence predating a comment-format change reads as unmanaged and is re-uploaded once, the same as any hand-uploaded file — except that a *recorded path disagreeing with the local source* is an update even when the checksum matches, so a mangled path repairs itself instead of surviving every later publish; a comment with no source recorded at all is not a disagreement. Every text part of the upload form must go through `writeTextField`, never `multipart.Writer.WriteField`, which emits no charset and gets decoded as Latin-1), `_links.next` pagination. **Three pagination schemes, and picking the wrong one truncates silently.** v1 *child/attachment* collections page through the generic `listV1` helper by `start`/`limit` offset, never `_links.next` (absent when the results fit one page, so it cannot terminate a loop); `ListAttachments`, `ListChildPages`, and `ListChildFolders` all go through it. v2 collections page through `listV2` by the cursor in `_links.next`, which is a `/wiki`-prefixed absolute path `resolveNext` handles unchanged; `ListContentProperties` and `SearchPagesByTitle` share it. **`/wiki/rest/api/search` is neither**: it ignores `start` outright, its `next` is context-relative so it needs the `/wiki` prefix `resolveNext` does not add, a short page does *not* mean the end, and `totalSize` can be nonzero against an empty `results` — so `searchCQL` terminates only on a missing `next` and nothing may branch on `totalSize` ([docs/confluence/search.md](docs/confluence/search.md)). `searchCQLBounded` adds a row bound under it (`SearchCQL` is that call with no bound, which is why `find` is unaffected): it asks for `max+1` and reports the surplus as `more`, since `totalSize` cannot supply a count. Full text goes through `SearchText`/`SearchRawCQL`, which return the cleaned `SearchMatch` the way `FindByTitle` returns `TitleMatch` — and **every field of a match comes from the row's `content` object**, because the row-level `title` is HTML-escaped *and* wrapped in `@@@hl@@@` markers where `content.title` is neither. The `excerpt` exists only at row level, so `cleanExcerpt` strips those markers, unescapes once, and collapses to one line — in the client, so the human and `--json` paths cannot disagree about it. `excerpt=highlight` is passed explicitly and **re-attached when following the cursor** (the `next` link carries `cql` and `limit` but not `excerpt`, and `doJSON` appends params with a bare `?`); an unrecognized value there yields an empty excerpt with a 200, so a rename by Atlassian degrades to no excerpts rather than an error. A row with no `content` object is skipped and **counted** — `type = space` answers with hundreds of them, and a silent skip would report a successful empty result. A bare v1 child row already carries `webui`, `status`, and `extensions.position`, so child listing needs no `expand`. `DownloadAttachment` goes through `send` (inheriting retry/backoff) against `_links.download`; **never** add a `CheckRedirect` that forwards headers — it would leak site credentials to Atlassian's media host, which neither needs nor wants them. `config.go` holds `Resolve` and the `.env` reader. Why each of these is shaped this way, with the evidence: [docs/confluence/api.md](docs/confluence/api.md) and [attachments.md](docs/confluence/attachments.md). -- `internal/convert` — the converter (the crux). `MdToConfluence(md *frontmatter.MarkdownFile, root *project.Root, index *linkindex.Index, baseURL, spaceKey, version string) (*ConfluencePage, error)`. `root` bounds which images and parent references may be read (S1/S2) and is what an image's recorded `Source` is relative to; `index` is the tree-wide link/anchor index for `root` (`internal/linkindex.Build`), built once and shared across every file converted under it rather than rebuilt per conversion — both are discovered/built by the caller (`internal/project`/`internal/linkindex`), which is why this package stays client-free. It parses with goldmark (GFM) and renders through a custom `storageRenderer` registered at priority 100 (below the default HTML=1000 and table=500 renderers) that emits Confluence storage format. `shield.go` renames raw `ac:`/`ri:` tags to colon-free sentinels around the goldmark step so pasted storage passes through; `callouts.go` is an AST transformer + blockquote renderer for GitHub alerts; `aclink.go` is the *inverse* direction's one element with enough shape to need its own file — ``, which the editor writes for every internal link and `MdToConfluence` never emits, so nothing in the regression suite covers it. One rule decides its whole mapping: **convert when the markdown republishes to a link resolving to the same target, pass the storage through when it would not** — so a page link and a space link convert, while a mention (80% of all real usage), an attachment link (only images are uploaded, so a relative href would be dead) and an unresolvable target stay raw, which the shield republishes byte-identical. A page target is a **title, never an id**, so `PageLinkTargets` reports what needs resolving and `StorageOptions.PageLinks` carries the answers back. An `ac:anchor` is **percent-encoded** where `confluenceSlug` output is not: decode it before matching a heading, leave it encoded inside a URL. A same-page anchor recovers its heading from the document rather than inverting the slug, which is impossible — `confluenceSlug` turns both a space and a hyphen into `-`. The survey the mapping rests on, and the `xml.HTMLAutoClose` trap that made `` crash the parser outright (#88), are in [docs/confluence/links-and-anchors.md](docs/confluence/links-and-anchors.md); `attachname.go` owns the source-path↔attachment-name mapping (percent-encoding `%`→`%25` then `/`→`%2F`, which is **bijective** — that is what makes the dedupe collision-free and lets `read` recover an image's original path; decode refuses an absolute result, which markfluence never produces; what names Confluence accepts is in [docs/confluence/attachments.md](docs/confluence/attachments.md)); `destination.go` owns the **other** codec, destination↔path (`decodeDestination`/`encodeDestination`), shared by images *and* doc links — a markdown destination is a URL, so decode inbound (**before** `withinRoot`, or an encoded `..%2F` slips the clamp) and encode outbound in `storage_to_md.go` (or `export` emits markdown that no longer parses, and `sourceFor`'s absolute-path refusal is undone by the next read); an undecodable destination is a literal `%` in a filename, not an error; the reasoning is in [docs/confluence/links-and-anchors.md](docs/confluence/links-and-anchors.md); `images.go` (resolution stays page-relative like GitHub; the documentation root — cwd — bounds what may be published, and an image above it is `IMAGE BROKEN`), `links.go` (GitHub/Confluence slugs, doc-link + anchor rewriting against `internal/linkindex`'s tree-wide index; `resolveDocKey` resolves a destination to the index's root-relative key and reports `escapes` — a purely lexical check on the *query* side, since the index itself needs no clamp: an escaping key can never be in it, built by walking downward from root). A doc-link target is one of four severities, #42: missing entirely or escaping root is **Broken** (`LINK BROKEN: … (not found|outside the documentation root)`) and replaces the whole `` element — tags and visible text alike — with that literal message, matching `images.go`'s precedent for a missing image (`renderLink` needs a small per-node flag, `linkBrokenText`, since goldmark still invokes a container node's renderer on the matching leaving call regardless of `WalkSkipChildren` on entering, and there is no `` to write in the broken case); existing on disk with no `page_id` yet is unchanged — a **warning**, the normal state of an unpublished tree; a `#fragment` matching no heading on an otherwise-resolving target also **warns**, gated on `linkindex.Index.FileExists` so a missing/escaping target isn't double-reported. `tables.go` (the `` tag, stamped with `data-layout="align-start"` so tables auto-size and left-align — this must stay if column widths are ever emitted, or a `` silently induces a layout; plus cells: an AST transformer consumes a leading `` comment in a cell and `renderTableCell` emits it as `data-highlight-colour`; `storage_to_md.go`'s `cellTexts` reverses this, reading `data-highlight-colour` back into a `bg:` marker (`cellBGNames`, the reverse of `tables.go`'s swatch map — a hex outside the 21 swatches round-trips as the literal hex, and where two names share a hex the British spelling wins, matching Confluence's own `-colour`), and a column's GFM alignment becomes a `

` wrapper **inside** the cell — never the `align` attribute the GFM renderer would emit, which is the one form Confluence discards. Only center and right are emitted: Confluence has no explicit left, so `:---` publishes bare and `read` recovers it as `---`. Since alignment is per-paragraph there and per-column in GFM, `columnSeparators` in `storage_to_md.go` takes each column's most common declared alignment (ties to the first seen) and drops the rest. Rows still fall through to the GFM renderer. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

` tag, stamped with `data-layout="align-start"` so tables auto-size and left-align — this must stay if column widths are ever emitted, or a `` silently induces a layout; plus cells: an AST transformer consumes a leading `` comment in a cell and `renderTableCell` emits it as `data-highlight-colour`; `storage_to_md.go`'s `cellTexts` reverses this, reading `data-highlight-colour` back into a `bg:` marker (`cellBGNames`, the reverse of `tables.go`'s swatch map — a hex outside the 21 swatches round-trips as the literal hex, and where two names share a hex the British spelling wins, matching Confluence's own `-colour`), and a column's GFM alignment becomes a `

` wrapper **inside** the cell — never the `align` attribute the GFM renderer would emit, which is the one form Confluence discards. Only center and right are emitted: Confluence has no explicit left, so `:---` publishes bare and `read` recovers it as `---`. Since alignment is per-paragraph there and per-column in GFM, `columnSeparators` in `storage_to_md.go` takes each column's most common declared alignment (ties to the first seen) and drops the rest. Rows still fall through to the GFM renderer. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
SettingDefault
retries3
timeout30s
\npython

A pasted status macro: Stable

\n", + "html": "

Release Notes

\n\n

See the upgrade guide before starting.

\n

Back up your data before upgrading.

\n

What's New

\n

A soft-wrapped paragraph describing the release across multiple source lines that collapse into one.

\n

\n

Configuration

\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
SettingDefault
retries3
timeout30s
\npython

A pasted status macro: Stable

\n", "warnings": [] } diff --git a/internal/convert/testdata/storage2md/images-encoded-src/input.storage b/internal/convert/testdata/storage2md/images-encoded-src/input.storage deleted file mode 100644 index a7e4edc..0000000 --- a/internal/convert/testdata/storage2md/images-encoded-src/input.storage +++ /dev/null @@ -1,6 +0,0 @@ -

-

-

-

-

-

diff --git a/internal/convert/testdata/storage2md/images-encoded-src/output.md b/internal/convert/testdata/storage2md/images-encoded-src/output.md deleted file mode 100644 index 67110c1..0000000 --- a/internal/convert/testdata/storage2md/images-encoded-src/output.md +++ /dev/null @@ -1,11 +0,0 @@ -![spaced](assets/my%20image.png) - -![accented](assets/caf%C3%A9.png) - -![literal percent](assets/100%25.png) - -![parenthesized](shot%20%281%29.png) - -![plain](assets/plain.png) - -![remote](https://cdn.example.net/my%20image.png) diff --git a/internal/convert/testdata/storage2md/images-uninterpreted-names/input.storage b/internal/convert/testdata/storage2md/images-uninterpreted-names/input.storage new file mode 100644 index 0000000..c9153f7 --- /dev/null +++ b/internal/convert/testdata/storage2md/images-uninterpreted-names/input.storage @@ -0,0 +1,6 @@ +

+

+

+

+

+

diff --git a/internal/convert/testdata/storage2md/images-uninterpreted-names/output.md b/internal/convert/testdata/storage2md/images-uninterpreted-names/output.md new file mode 100644 index 0000000..2d19b0d --- /dev/null +++ b/internal/convert/testdata/storage2md/images-uninterpreted-names/output.md @@ -0,0 +1,11 @@ +![spaced](my%20image.png) + +![accented](caf%C3%A9.png) + +![literal percent](100%2525.png) + +![parenthesized](shot%20%281%29.png) + +![looks encoded](assets%252Fplain.png) + +![remote](https://cdn.example.net/my%20image.png) diff --git a/internal/convert/testdata/storage2md/images/input.storage b/internal/convert/testdata/storage2md/images/input.storage index f3f5e7b..c72c7b7 100644 --- a/internal/convert/testdata/storage2md/images/input.storage +++ b/internal/convert/testdata/storage2md/images/input.storage @@ -1,5 +1,5 @@ -

+

-

+

diff --git a/internal/convert/testdata/storage2md/images/output.md b/internal/convert/testdata/storage2md/images/output.md index c3006f9..106b8a4 100644 --- a/internal/convert/testdata/storage2md/images/output.md +++ b/internal/convert/testdata/storage2md/images/output.md @@ -1,4 +1,4 @@ -![an architecture diagram](assets/diagram.png) +![an architecture diagram](diagram.png) ![a bare block-level image](bare.png) @@ -6,4 +6,4 @@ ![tooltipped](shot.png "A Tooltip") -![sized shot](assets/shot.png '{"title":"A Tooltip","width":300,"height":150,"align":"center"}') +![sized shot](shot.png '{"title":"A Tooltip","width":300,"height":150,"align":"center"}') diff --git a/internal/pagedoc/pagedoc.go b/internal/pagedoc/pagedoc.go index 4fc5d65..d2f51b3 100644 --- a/internal/pagedoc/pagedoc.go +++ b/internal/pagedoc/pagedoc.go @@ -1,24 +1,66 @@ // Package pagedoc renders a fetched Confluence page as a markdown document: // frontmatter plus the converted body. // -// It exists so `read` and `export` emit byte-identical markdown -- that they -// agree is the property export rests on, since an exported tree is meant to be -// the same thing `read` prints. It needs a client (page width, attachment list) -// and so cannot live in internal/convert, which is deliberately client-free; -// that is why StorageToMarkdown takes a sources map rather than fetching one. +// One conversion, parameterized. Every command that renders a page goes through +// Options, so `read` and `export` cannot drift apart by accident -- only by +// argument, and the argument is where the page sits in whatever is being +// written. For a page at the top level of that tree, which is what `read` +// prints and what a single-page export writes, the two are byte-identical. +// +// It needs a client (page width, attachment list) and so cannot live in +// internal/convert, which is deliberately client-free; that is why +// StorageToMarkdown takes a sources map rather than fetching one. package pagedoc import ( "fmt" + "path" "strings" "github.com/mozilla/markfluence/internal/client" "github.com/mozilla/markfluence/internal/convert" "github.com/mozilla/markfluence/internal/frontmatter" + "github.com/mozilla/markfluence/internal/pageslug" "github.com/mozilla/markfluence/internal/pagewidth" "github.com/mozilla/markfluence/internal/ui" ) +// Placement is where a page is being written and what that implies for its +// frontmatter. The zero value is "on its own": no tree, no directory, parent +// taken from the page itself -- which is what `read` prints and what a +// single-page export writes. +type Placement struct { + // Dir is where the page's file sits relative to the root of what is being + // written, in slash form. "" is that root. + Dir string + + // AttachmentDir overrides where an attachment with no recorded path is + // placed. Empty derives it from the page's own title, which is right for a + // page standing alone. + // + // A tree sets it, because a title is not enough there: two siblings whose + // titles slug the same are written to directories disambiguated by page id, + // and deriving the directory here would put both pages' attachments back in + // one place -- the very collision page-scoping exists to prevent. The caller + // that named the directories is the only one that knows. + AttachmentDir string + + // Attachments is the page's attachment list, when the caller has already + // fetched it. Options otherwise fetches its own, which for a tree export is + // a second listing of every page on top of the walk's own requests. + Attachments []client.Attachment + + // Parent overrides the parent: frontmatter field. Empty derives it from the + // page, which yields its parent id or "null". + // + // A tree export sets it to a relative path to the parent's own .md file, so + // the tree can be published into fresh pages; create resolves such a path + // against the referring file's directory. It stays an id for the export + // root, whose parent is outside the tree, and for a page whose parent is a + // folder, which has no file to point at. + Parent string +} + // Doc is a page rendered as markdown. Frontmatter and Body are separate because // read prints them together while export writes them to a file, and because a // caller may want the body alone. @@ -32,30 +74,84 @@ func (d Doc) String() string { return d.Frontmatter + "\n" + d.Body } // Render converts a page's storage body to markdown and builds its frontmatter. // +// See Placement for what pl carries. +// // The page must have been fetched with its body (GetPageBodyOrNil). -func Render(c *client.ConfluenceClient, page *client.Page) (Doc, error) { - body, err := convert.StorageToMarkdown(page.Body.Storage.Value, Options(c, page)) +func Render(c *client.ConfluenceClient, page *client.Page, pl Placement) (Doc, error) { + body, err := convert.StorageToMarkdown(page.Body.Storage.Value, Options(c, page, pl)) if err != nil { return Doc{}, err } - return Doc{Frontmatter: Frontmatter(c, page), Body: body}, nil + return Doc{Frontmatter: Frontmatter(c, page, pl.Parent), Body: body}, nil } // Options assembles what the converter cannot fetch for itself: the attachment -// source paths, the resolved page URLs, and the site base. +// source paths, the resolved page URLs, the page's position, and the +// site base. +// +// Every command that renders a page goes through it rather than assembling its +// own, which is the same reason Render exists: options built in two places are +// options that can disagree, and here a disagreement means an attachment +// written to a path the markdown does not point at. // -// Both read and export go through it rather than assembling their own, which is -// the same reason Render exists -- the two must emit byte-identical markdown, -// and options built in two places are options that can disagree. -func Options(c *client.ConfluenceClient, page *client.Page) convert.StorageOptions { +// One conversion, parameterized by where the page sits. read and export produce +// identical markdown for the same position; read has no tree, so it passes the +// empty one. +func Options(c *client.ConfluenceClient, page *client.Page, pl Placement) convert.StorageOptions { return convert.StorageOptions{ - Sources: Sources(c, page), + Sources: pl.sources(c, page), PageLinks: PageLinks(c, page), + PageDir: pl.Dir, + // Where an attachment with no recorded path is placed: the directory + // named after the page, beside the page's own file. Computed here rather + // than in the converter, which has no business knowing how a title + // becomes a directory name, and computed once so that every command + // placing such an attachment agrees with the markdown that points at it. + AttachmentDir: AttachmentDirFor(page, pl), // The site, never the gateway: these URLs are published into a page. SiteURL: c.SiteURL(), } } +// sources is the attachment name-to-path map, from the caller's own listing +// when it has one. +func (pl Placement) sources(c *client.ConfluenceClient, page *client.Page) map[string]string { + if pl.Attachments != nil { + return SourcesFrom(pl.Attachments) + } + return Sources(c, page) +} + +// AttachmentDirFor is where this placement puts an attachment with no recorded +// path: the directory the placement names, or one derived from the page when it +// names none. +// +// Exported because the write side needs the identical answer -- the markdown +// destination and attachfile.Options.Dir are the same decision, and computing +// it twice is how they drift. +func AttachmentDirFor(page *client.Page, pl Placement) string { + return pl.attachmentDir(page) +} + +func (pl Placement) attachmentDir(page *client.Page) string { + if pl.AttachmentDir != "" { + return pl.AttachmentDir + } + return AttachmentDir(page, pl.Dir) +} + +// AttachmentDir is where an attachment with no recorded path belongs when +// nothing else has decided: the directory named after the page, beside the +// page's own file at pageDir. +// +// Exported because both sides of the same decision need it and must not compute +// it twice: the markdown that points at the attachment (through Options) and +// the write that puts it there (attachfile.Options.Dir). A caller that renders +// a page and writes its attachments passes this to both. +func AttachmentDir(page *client.Page, pageDir string) string { + return path.Join(pageDir, pageslug.For(page.Title, page.ID)) +} + // PageLinks maps each page an in this body points at to its absolute // URL. Confluence names a link target by title and never by id, so every one of // them costs a lookup -- which is why the converter asks for the list rather @@ -130,9 +226,11 @@ func pageURLByTitle(c *client.ConfluenceClient, title, spaceID string) string { // location exactly rather than inferring it from the attachment name. // // It is an optimization, not a requirement: a page with no attachment -// references skips the lookup entirely, and a failed lookup returns nil so the -// converter falls back to decoding names -- a read is worth completing without -// it, the same way a failed page-width read is tolerated in Frontmatter. +// references skips the lookup entirely, and a failed lookup returns nil, which +// leaves every attachment looking unrecorded -- so the markdown points into the +// page's own directory, which is where an unrecorded one is placed anyway. A +// read is worth completing without it, the same way a failed page-width read is +// tolerated in Frontmatter. func Sources(c *client.ConfluenceClient, page *client.Page) map[string]string { if !strings.Contains(page.Body.Storage.Value, " Max { + s = strings.Trim(string([]rune(s)[:Max]), "-") + } + return s +} + +// For is the slug to use for a page or folder, falling back to its id when the +// title slugs to nothing at all -- a title of "?!" or "..." leaves no +// characters, and an id is always usable. +func For(title, id string) string { + if s := Slug(title); s != "" { + return s + } + return id +} + +// Filename is the markdown file a page is written as. +func Filename(title, id string) string { return For(title, id) + ".md" } diff --git a/internal/pageslug/pageslug_test.go b/internal/pageslug/pageslug_test.go new file mode 100644 index 0000000..2799a73 --- /dev/null +++ b/internal/pageslug/pageslug_test.go @@ -0,0 +1,63 @@ +package pageslug + +import ( + "strings" + "testing" +) + +// TestSlug pins the mapping, including the distinction that decides which +// titles collide: punctuation is dropped rather than separated, so equivalence +// runs through whitespace alone. +func TestSlug(t *testing.T) { + for _, c := range []struct{ title, want string }{ + {"Title 1", "title-1"}, + {"Title: 1", "title-1"}, + {"Title:1", "title1"}, + {"Title-1", "title-1"}, + {"Deploy: Prod", "deploy-prod"}, + {"Deploy Prod", "deploy-prod"}, + {" spaced out ", "spaced-out"}, + {"Über Café", "über-café"}, + {"Q3 (2026)", "q3-2026"}, + {"?!", ""}, + } { + if got := Slug(c.title); got != c.want { + t.Errorf("Slug(%q) = %q, want %q", c.title, got, c.want) + } + } +} + +// TestForFallsBackToTheID covers a title with nothing sluggable in it: an id is +// always usable, and is what keeps such a page exportable at all. +func TestForFallsBackToTheID(t *testing.T) { + if got := For("………", "2848423944"); got != "2848423944" { + t.Errorf("For = %q, want the id", got) + } + if got := Filename("", "123"); got != "123.md" { + t.Errorf("Filename = %q, want 123.md", got) + } +} + +// TestSlugCaps keeps a very long title from producing a filename the +// filesystem rejects, and must not leave a trailing hyphen when the cut lands +// on a word boundary. +func TestSlugCaps(t *testing.T) { + got := Slug(strings.Repeat("long title ", 40)) + if len([]rune(got)) > Max { + t.Errorf("slug is %d runes, want <= %d", len([]rune(got)), Max) + } + if strings.HasSuffix(got, "-") { + t.Errorf("slug %q ends in a hyphen", got) + } +} + +// TestSlugNeverProducesAPath is the safety property: --file is the only way +// to write outside the destination directory's top level. +func TestSlugNeverProducesAPath(t *testing.T) { + for _, title := range []string{"../escape", "/etc/passwd", "a/b/c", `a\b`} { + got := Slug(title) + if strings.ContainsAny(got, `/\`) { + t.Errorf("Slug(%q) = %q, which contains a path separator", title, got) + } + } +} diff --git a/internal/pagetree/pagetree.go b/internal/pagetree/pagetree.go index f61be0d..6337a25 100644 --- a/internal/pagetree/pagetree.go +++ b/internal/pagetree/pagetree.go @@ -15,10 +15,17 @@ import ( // AllDepths walks the whole subtree, however deep it goes. const AllDepths = -1 +// The two node types a walk reports. A folder is the Cloud content type that +// can hold pages without being one. +const ( + TypePage = "page" + TypeFolder = "folder" +) + // Node is one page or folder in a walked subtree. type Node struct { ID string - Type string // "page" or "folder" + Type string // TypePage or TypeFolder Title string Status string // ParentID is the node this one hangs off, which for a top-level result is diff --git a/schema/json-output/v1.json b/schema/json-output/v1.json index 5a48c73..5493e39 100644 --- a/schema/json-output/v1.json +++ b/schema/json-output/v1.json @@ -89,7 +89,7 @@ "results": { "items": { "oneOf": [{ "$ref": "#/$defs/exportResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, - "summary": { "$ref": "#/$defs/basicSummary" } + "summary": { "$ref": "#/$defs/exportSummary" } } } }, @@ -487,13 +487,26 @@ "code": { "$ref": "#/$defs/codeOrNull" } } }, + "exportSummary": { + "description": "export batch summary. skipped counts pages whose file was already on disk; a run that exports nothing new is all skipped and still succeeded. project_file says what happened to the markfluence.yaml a multi-page export needs to be republishable: null for a single-page export, which needs none.", + "type": "object", + "additionalProperties": false, + "required": ["total", "succeeded", "failed", "skipped", "project_file"], + "properties": { + "total": { "type": "integer" }, + "succeeded": { "type": "integer" }, + "failed": { "type": "integer" }, + "skipped": { "type": "integer" }, + "project_file": { "enum": ["wrote", "exists", null] } + } + }, "exportResult": { - "description": "One exported page. status is whether the page file was written or skipped; the attachments array holds every attachment considered, including unreferenced ones (dest_path null, status skipped_unreferenced). warnings lists references the page makes to attachments that are not attached.", + "description": "One exported page. status is whether the page file was written or skipped; the attachments array holds every attachment considered, including unreferenced ones (dest_path null, status skipped_unreferenced). warnings lists anything worth saying about this page besides its outcome, and is present whether or not it exported: references it makes to attachments that are not attached, and a name it did not choose because a sibling slugged the same.", "type": "object", "additionalProperties": false, "required": [ - "ok", "page_id", "title", "space", "parent", "parent_type", "dry_run", "status", - "dest_path", "attachments", "warnings", "error", "code" + "ok", "page_id", "title", "space", "parent", "parent_type", "parent_file", "dry_run", + "status", "dest_path", "attachments", "warnings", "error", "code" ], "properties": { "ok": { "type": "boolean" }, @@ -502,6 +515,10 @@ "space": { "type": "string" }, "parent": { "$ref": "#/$defs/stringOrNull" }, "parent_type": { "$ref": "#/$defs/parentTypeOrNull" }, + "parent_file": { + "$ref": "#/$defs/stringOrNull", + "description": "The parent: value written into the exported file, when it is a path to the parent's own .md. Null when the parent stayed an id -- the export root, or a page whose parent is a folder." + }, "dry_run": { "type": "boolean" }, "status": { "enum": ["wrote", "skipped", ""] }, "dest_path": { "$ref": "#/$defs/stringOrNull" },