From 566e541d864280411a4ddd3db80f75f41beefb83 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Sat, 12 Sep 2026 14:49:12 -0400 Subject: [PATCH] docs: bring CLAUDE.md up to date, and prune what --help now owns Six gaps, all from the same branch: the README restructure (#102) added a package, exported a function, added two make targets and introduced a generated artifact, and none of it reached this file. A docs-only branch felt like it needed no architecture note while changing all four of those. Added: make docs/docs-check in the Commands list and in the make check ordering; tools/gendocs in the layout list; a Documentation section covering the README-is-a-50'-view split, that docs/commands/ is *generated* and must never be hand-edited, why cmd.Root() is exported for exactly one caller, and what the drift guard is for; the three new docs/ files; and escapeLinkText's rule, which is the kind of thing that gets re-broken -- it applies to raw sources only, because an ac:link-body has already been rendered to markdown and escaping it yields a literal \*\*bold\*\*. Removed, on one criterion: keep what carries a because, a test name, an issue number or a guarantee id; drop bare behaviour statements that `--help` or docs/ now own. The frontmatter-driven-publishing section was the outlier, 3501 chars mostly restating field domains and per-verb flag behaviour -- written before --help became the reference. Now 2323, keeping only what neither --help nor docs/markdown_file.md records: parent being an opaque id whichever kind it names, the validate-before-write/apply-after ordering and #127's reasoning for it, L9's asymmetry and fix's tri-state nil, and the two output rules that look arbitrary and are not. Also 270 chars of text now verbatim in --help (search's archived-and-folders, children's folder-counts-as-a-level, the find/search complement). Deliberately *not* regenerated from scratch. /init would produce a competent structural summary and lose every "that missing fallback was #68", "pinned by TestErrorDoesNotDependOnTheIndex", "the clamp must never exist in two copies" -- none of which is recoverable by reading the repo, and which is the entire reason the file is worth having. An audit found no broken path or package reference in it, so nothing was wrong; it was only incomplete. --- CLAUDE.md | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3c5c126..6e937d8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -11,12 +11,23 @@ Tasks run through a `Makefile` (run `make` with no target for the annotated rule - `make build` — build `./bin/markfluence` (version stamped via ldflags) - `make test` — `go test ./...` (also runs the `go vet` subset) - `make lint` — golangci-lint (installs the pinned v2.6.0 into `./bin`; enables the default set plus `lll` at 120) -- `make check` — the pre-flight: vet, fmt-check, test, build, lint, in CI's order (CI runs this exact target) +- `make check` — the pre-flight: vet, fmt-check, docs-check, test, build, lint, in CI's order (CI runs this exact target) - `make vet` / `make fmt` / `make fmt-check` (`fmt-check` reports without modifying anything) - `make regen-regressions` — regenerate the converter's golden outputs (`go test ./internal/convert -run TestRegression -update`) +- `make docs` — regenerate `docs/commands/` from each command's `--help`; `make docs-check` fails when the two disagree - Run a single package/test: `go test ./internal/frontmatter -run TestReadValues` -Run **`make check`** before considering work done: vet, fmt-check, test, build, lint, in that order. CI runs the same target and nothing else, so the two cannot drift. Do not run the pieces individually as a substitute — `fmt-check` is the one that gets forgotten, and `golangci-lint` does not enable `gofmt`, so `lint` passes on a file CI rejects. +Run **`make check`** before considering work done: vet, fmt-check, docs-check, test, build, lint, in that order. CI runs the same target and nothing else, so the two cannot drift. Do not run the pieces individually as a substitute — `fmt-check` is the one that gets forgotten, and `golangci-lint` does not enable `gofmt`, so `lint` passes on a file CI rejects. + +## Documentation + +The README is a 50,000-foot view — what markfluence does, which command to reach for, how to configure it — and **not** a command reference (#102). `markfluence CMD --help` is the reference: every command carries its reasoning in `Long` and a worked invocation in `Example`, and `cmd`'s `TestSubcommandsDocumentThemselves` fails a command that ships without both. + +**`docs/commands/` is generated, not written.** `tools/gendocs` renders every command's help to markdown via cobra's doc generator (using `cmd.Root()`, exported for that one caller — a `main()` inside `cmd` would land in the binary, and a `_test.go` that writes into the repo is not a test). It is a second copy of the help text, and only safe because it cannot drift: `make docs-check` regenerates into a temp dir and diffs, inside `make check`. `DisableAutoGenTag` is set, or a timestamp would make every regeneration a diff and the guard noise. **Never hand-edit a file under `docs/commands/`.** + +The rest of `docs/` is written: `markdown_file.md` (the page format — every frontmatter field and every body construct, the two halves that used to be split between the README and a separate file), `json-output.md` (per-command `--json` detail the schema cannot express as reasoning), `github-actions.md` (CI setup), `root-model.md`, `guarantees.md`, and `confluence/`. + +A change that adds a package, exports a function, adds a make target, or introduces a generated artifact belongs in **this file** as part of the same change. The only time it has drifted was a docs-only branch that felt like it needed no architecture note while adding all four. ## Configuration @@ -53,9 +64,9 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `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 three-phase and transactional (preflight all, reserve parents-first in topological order, then publish); `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. Preflight also **converts every file and throws the page away**, keeping only the error (#127/S7): a defect the converter refuses is a property of the file on disk, so asking before the reserve phase is what keeps it from leaving a content-less page and a `page_id` the author has to undo by hand. The result cannot be reused by `publishOne` — reserve seeds the batch's ids into the shared link index in between, so an in-set link renders unresolved in preflight and resolves in publish, and phase 1's `Broken`/`Warnings` are discarded for that reason. The *error* is identical across the two, for a narrower reason than "the converter ignores the index": whether `renderImage` runs at all does depend on it (`renderLink` skips a broken link's children, and `Broken` is decided by `FileExists`), but reserve only calls `SetPage`, which writes `idx.pages` alone — nothing there can raise an error or change one's text, and `FileExists`/`Anchor` read `idx.anchors`, fixed at `Build` time. Making `SetPage` also mark a file as existing would break it; pinned by `TestErrorDoesNotDependOnTheIndex`. It is called **last**, after every server check, so `page_id`-first precedence is untouched, and its failure carries `CodeConvert` on the `failure` struct rather than `abort()`'s old hardcoded `VALIDATION`. - `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`, plus everything real YAML now refuses — a nested value, a `|` block, a duplicate key, a tab indent, a reserved indicator), an invalid `page_width` (`pagewidth.Declared`), a present-but-non-numeric `page_id` (`pageref.IsDigits`), and a **present-but-empty `title`** — 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. `title` is the one exception to that reasoning and only in its present-but-empty form: `create` and `update` both reject it, so no verb makes it valid and there is no false positive to have. An *absent* title stays unreported, since `update` accepts one and keeps the live page's title. 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. **`--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/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`. 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/search/` — `search`: find pages by **full text**, via `client.SearchText` (or `SearchRawCQL` under `--cql`). 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. 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* 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`. The envelope's and the error object's top-level **`warnings`** are the one field no command fills: `jsonout.NewEnvelope`/`EmitError` drain a package-level collector (`AddWarning`), because the only thing in it is raised during credential resolution — below any command, before either document exists. @@ -65,12 +76,13 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `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`. **Two error types on the request path, and one predicate for them**: an `*HTTPError` once a response has a status, an unexported `requestError` when there is none (a transport failure, a request that would not build, a body that would not decode), and `FromRequest` answers whether an error is either. That is what lets a caller tell a server failure from a local one — `jsonout.CodeOr(err, fallback)` is the whole point of it, since `CodeFor` alone reports every non-`HTTPError` as `NETWORK` and so turns `no title given` into a network problem (#133). The rule is deliberately scoped to the request: `DownloadAttachment` writing to the caller's writer, `uploadAttachment` opening the caller's file, and `Resolve` reading the environment stay untyped, because tagging them would misreport an unreadable file as a network failure. The wrapper carries no message of its own, so `Error()` is the inner text verbatim and nothing a reader sees changed. 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, plus the **`.env` permission warning** (#136): a `.env` reachable by anyone but its owner (`mode.Perm()&0o077`) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a `.env` holding only the URL and username leaks nothing, and a warning that fires on a file with no secret in it is how one becomes something people scroll past. It stats rather than lstats (a link's own `0777` would cry wolf over a `0600` target), lives in `loadDotenv` because that is the one function both the discovered `.env` and `--env-file` pass through, and reaches the reader through `SetSecurityWarner` for the same reason `SetRetryLogger` exists — wired to `cmd/root.go`'s `reportSecurityWarning`, which prints it (human mode) *and* records it via `jsonout.AddWarning`, since stderr under `--json` is a schema-validated document with no room for a stray line. A group/world-*writable* `.env` with no token in it is knowingly **not** covered, though `CONFLUENCE_URL` resolves from there too and rewriting it would redirect the token: see #136. 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; `mention.go` owns the user-mention mapping in both directions (#91): a mention is 80% of all `` usage, and it converts to `[@Display Name](https://home.atlassian.com/people/{accountId})`. Three things decide its shape, each measured rather than reasoned. **The URL is Atlassian Home, not the site** — Confluence's own renderer still emits `{site}/wiki/people/{id}`, which no longer resolves usefully in a browser, so a mention in markdown names *no site*, needs nothing from configuration, and is therefore recognisable by `check` with no client at all. **Matching is on the path, ignoring host and query**, because several spellings of one target circulate (the Home URL, the modal's `?cloudId=` copy, the `/o/{orgId}` redirect, both Confluence forms, root-relative) and none of `cloudId`/`ref`/the org segment identifies the person — only the id does, and `ri:user` stores nothing else. **The `@` on the link text is the marker**, load-bearing rather than decoration: the URL cannot tell "mention this person" from "link to their profile", so without it anyone writing the second would silently get the first. The account id is **not** pattern-validated (two shapes are live on one instance, so a pattern tight enough for one rejects the other) and `ri:local-id` is never emitted (a mention carrying only the id resolves to the same person, verified via ADF). `ConfluencePage.Mentions` reports the ids the *forward* direction emitted so the caller can warn about one that names nobody — the `Attachments` arrangement, and necessary because Confluence accepts any id and renders `@Unlicensed user` rather than failing, and the profile URL 200s either way. An unresolvable mention still renders as a link, `[@Unlicensed user](…)` — that wording mirrors Confluence because the only ids reaching it are the ones the page labels that way: a **deactivated account resolves normally** and keeps its name (measured across every mention on a real page — 18 of them, six departed, all 200, returning e.g. `Mark Reid (Deactivated)`), so a departed colleague never takes that branch. Name resolution is `pagedoc.UserCache`, a per-run cross-page cache, and the tri-state is the part to preserve: `client.LookupUser` separates a name from `ErrNoSuchUser` from an unaskable question, `StorageOptions.UserNames` carries that as name / `""` / absent, and only a **confirmed** absence renders the placeholder. Flattening those would write a fabricated name over a real one the moment a VPN dropped mid-export, across a whole tree, into a file that then looks authoritative — which is also why the cache remembers a 404 but not a timeout (one is an answer, the other is not) and why `MentionWarnings` warns only about a confirmed absence; `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 and a space link convert, and 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, which is now the path's **base name** and nothing else (#59/`_plans/029`): the name is the attachment's identity, so an encoded path moved the name every time the file moved and orphaned the old attachment, and the path is recorded in the comment anyway. The mapping is therefore lossy, and what the bijection used to buy is an explicit refusal — two assets in one document whose base names agree return a typed `NameCollisionError` from `MdToConfluence`, which is a *failure* and not a `Broken` entry, since nothing blocks a publish on `Broken`. `check` catches that error and reports it as `Broken` anyway, because there it is a document defect like a dead link rather than a converter failure. A stored name is never interpreted in the other direction either: `sourceFor` reads the recorded path or uses the name verbatim. 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 `