From de1f44730471ccbdf9b99d8d5bf79a56208a33cf Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 31 Aug 2026 10:22:47 -0400 Subject: [PATCH 1/7] docs(plans): plan children --space --- _plans/028_children-space.md | 247 +++++++++++++++++++++++++++++++++++ 1 file changed, 247 insertions(+) create mode 100644 _plans/028_children-space.md diff --git a/_plans/028_children-space.md b/_plans/028_children-space.md new file mode 100644 index 0000000..c729e30 --- /dev/null +++ b/_plans/028_children-space.md @@ -0,0 +1,247 @@ +# Plan: `children --space` + +Let `children` list a whole space, given its key. Closes #98. + +## Current state of the codebase + +#98 reports that there is no first-class way to enumerate what a space +contains. Every existing path is blocked or indirect: + +- `children PAGE` goes through `internal/pageref.Resolve`, which accepts a + numeric id, a page/folder URL, or a `.md` file with a `page_id`. A space key + is none of those, so `children ENG` fails with pageref's own "not a numeric + id, a Confluence page or folder URL, or a markdown file with a page_id". +- `search --space ENG --limit all ""` is refused locally: an empty query is a + validation error, because the API answers one with a 500 + (`docs/confluence/search.md`). +- `find` needs an exact title, so it cannot enumerate anything. + +The only working invocation is the raw-CQL escape hatch: + +``` +markfluence search --cql 'space = "ENG" and type = page' --limit all +``` + +which is flat, folder-blind, and requires knowing CQL — for the single most +common browse task a Confluence wrapper has. + +What already exists and is reusable: + +- `internal/pagetree.Walk(c, rootID, maxDepth)` walks pages *and* folders + under one node, depth-first, siblings merged by `extensions.position`, + guarded by a visited set. It is already a package rather than command-local + because listing and exporting a subtree must share it. +- `internal/pagetree.siblings` gets a node's children from the two v1 routes + (`/child/page`, `/child/folder`) and sorts the merged slice by position. +- `client.ChildNode` is the row shape both routes return, needing no `expand`: + `id`, `type`, `title`, `status`, `extensions.position`, `_links.webui`. +- `client.listV1` pages through a v1 collection by `start`/`limit` offset. +- `client.ResolveSpaceID` maps a space key to a space id, or `""` for unknown; + `client.ErrSpaceNotFound` is the sentinel `find` raises and `search` maps to + `space %q not found`. +- `cmd/children` renders the walk as a `TYPE`/`ID`/`TITLE` table indented by + depth, and emits one `jsonChildResult` per node under `--json`. + +## What was verified live (2026-08-31) + +Against `mozilla-hub.atlassian.net`, basic auth on the site domain. The +evidence goes into `docs/confluence/spaces.md`; the summary here is what the +design rests on. + +1. **`GET /wiki/rest/api/space/{key}/content/page?depth=root`** returns the + space's root pages, and a bare row carries exactly the `ChildNode` fields — + `id`, `type`, `title`, `status`, `extensions.position`, `_links.webui`. So + `listV1[ChildNode]` works on it unchanged. +2. **A space can have more than one root page.** `AIM` has two: the homepage + (`Africa Innovation Mradi Home`) and `What is Africa Mradi?`. This is the + finding that decides the design: "the space root is its homepage, so walk + from `homepageId`" would have silently dropped a whole subtree, which is the + same class of wrong answer as v2 `/children` omitting folders. +3. **Archived root pages are excluded** from that route. The personal space has + an archived parentless page (`Some Archived Page`); `depth=root` reports + only the homepage. That matches how v1 child listings already behave, so no + status filter is needed. +4. **An unknown key answers 404**, body + `No space found with key : NOSUCHSPACEXYZ`. +5. **There is no route for a root-level folder** — `/space/{key}/content/folder` + answers 500 (`NullPointerException: PageRequest should not be null`), there + is no `/api/v2/spaces/{id}/folders`, and `/api/v2/folders?space-id=` is a + 500. And it does not matter: **`POST /wiki/api/v2/folders` with a `spaceId` + and no `parentId` puts the folder under the space homepage**, reporting + `parentId: `, `parentType: "page"`. A folder appears not to be + able to sit at a space root at all. (Probe folder created and trashed.) + This resolves the "a folder at a space root" bullet under *Unverified* in + `docs/confluence/folders.md`. +6. **`GET /wiki/api/v2/spaces/{id}/pages`** is a flat cursor-paginated list of + every page in a space carrying `parentId`/`parentType` — one request for the + 34-page personal space, where a walk needs a request pair per node. It is + *not* used, for two reasons: it lists no folders (so a folder's title is + unrecoverable and a subtree under a folder cannot be placed), and it + includes archived pages by default. +7. `/wiki/api/v2/spaces/{id}/direct-children` answers 400 with + `Provided value {spaces} for 'hierarchical-content-type' is not the correct + type`, which says the route is `/{hierarchical-content-type}/{id}/direct-children` + and there is no spaces variant. Recorded so it is not tried again. + +## Decisions + +**Surface: `children --space KEY`, with `PAGE` becoming optional.** Not a new +command: the output shape, the `--depth` vocabulary, the `--json` result type +and the schema branch are all identical, so a second command would buy a +duplicate. Not a fourth `pageref` spelling either: `pageref.Resolve` returns a +*page id* and holds no client, so it cannot resolve a key at all, and a bare +word is exactly what it currently rejects cleanly. + +`Args` becomes `cobra.MaximumNArgs(1)`, and exactly one of `PAGE` / `--space` +is required. Both, or neither, is a validation-fatal error (exit 2) raised +before credentials are resolved. + +**Depth 1 is the space's root pages.** The homepage is normally the only row +at depth 1 and its children are depth 2. The alternative — starting one level +down so the default is immediately useful — was rejected: it never lists the +homepage at all, it cannot represent AIM's second root, and its depth numbers +disagree with `children `. + +**A hint line when `--depth` was left alone.** Since most spaces have a single +root page, the default invocation prints one row, which reads like the whole +answer. When `--space` was given and `--depth` was not (`Flags().Changed`), +human output gets a trailing `ui.Info` line naming `--depth 2` and +`--depth all`. It costs no extra request and is absent from `--json`, where a +consumer is not reading prose. + +**An unknown key is a hard error, via `ResolveSpaceID`.** The space id is then +unused — the v1 root route takes the key — so this is one deliberately wasted +request. It buys the same wording a mistyped key already gets from `find` and +`search`, and it keeps the auth-404 trap where it is already handled: a +rejected credential answers 404 on every v2 route, and translating the v1 +404's Spring exception text into "no such space" would reintroduce exactly the +misreport `client.RejectedCredential` exists to prevent. + +**`--space` is a key, not a URL.** `--space` means a space key on `find` and +`search`; making it mean two things on one command out of three is worse than +rejecting a pasted URL with `space "…" not found`. + +**`parent_id` is `null` for a root page.** `childrenResult.parent_id` widens +from `string` to the existing `stringOrNull` `$def`. `pagetree.WalkSpace` +passes `""` as the parent of a root page and `jsonChildResult`'s existing +`nullable()` turns it into JSON `null`. The space id was rejected: that field +has only ever held a content id, and nothing in the row would say which id +namespace it came from. + +**Nothing new about walk cost.** `--space KEY --depth all` is a request pair +per node, which `children ID --depth all` already is; retry/backoff and the +retry logger already cover the failure modes, and any cap would be a number +invented here. The help text says walking a whole space is one request pair +per node. + +**No new guarantee id.** "A listing never silently omits a node kind" is +already doctrine in `docs/confluence/folders.md` and `CLAUDE.md` and is +enforced by the walk. `guarantees.md` ids are permanent, so minting one is its +own decision, not a rider on this feature. + +## Implementation + +### `internal/client` + +```go +// ListSpaceRootPages lists the pages at the root of a space, by key. +func (c *ConfluenceClient) ListSpaceRootPages(spaceKey string) ([]ChildNode, error) +``` + +`listV1[ChildNode](c, "/wiki/rest/api/space/"+url.PathEscape(spaceKey)+"/content/page", url.Values{"depth": {"root"}})`. + +Notes to carry as comments: the `/content/page` path form rather than +`/content`, because the latter also returns blogposts and `children` is about +the page tree; a space can have several root pages, so this is not a +homepage lookup by another name; and there is deliberately no folder companion +call, since a folder cannot sit at a space root (`docs/confluence/spaces.md`). + +### `internal/pagetree` + +Extract `Walk`'s inner recursion so both entry points share it, then: + +```go +// WalkSpace walks a space's tree from its root pages, by space key. +func WalkSpace(c *client.ConfluenceClient, spaceKey string, maxDepth int) ([]Node, error) +``` + +Root pages become depth-1 `Node`s with `ParentID: ""`, sorted by +`extensions.position` with the same stable sort `siblings` uses (so two roots +come back in the order Confluence shows them), then each is descended exactly +as `Walk` descends a child. The visited set starts empty rather than seeded +with a root id, and root pages are added to it. + +### `cmd/children` + +- `Use: command + " [PAGE]"`, `Args: cobra.MaximumNArgs(1)`. +- `--space` flag, completion `cobra.NoFileCompletions` (a space key lives on + the server, and completion may never call Confluence). +- Validation order, all before `client.Resolve`: exactly-one-of PAGE/`--space`, + then `--depth`. + - neither: `no page given: pass a PAGE or --space KEY` + - both: `PAGE and --space cannot be combined: --space lists a whole space` +- On the `--space` path: `ResolveSpaceID` → `""` means + `space %q not found` (VALIDATION, exit 2); then `pagetree.WalkSpace`. +- The hint line, human output only, when `--space` is set and + `!cmd.Flags().Changed("depth")` and at least one row was printed. +- `Long` gains the space paragraph: what depth 1 means, that `--depth all` + walks the whole space at one request pair per node, and that `--space` takes + a key. + +### `schema/json-output/v1.json` + +`childrenResult.parent_id` → `{"$ref": "#/$defs/stringOrNull"}`, description +noting `null` for a page at a space root. `jsonChildResult.ParentID` becomes +`*string` through the existing `nullable()`. + +## Tests + +- `internal/client`: `ListSpaceRootPages` against a `clienttest` server — + route and `depth=root` asserted, a `~personal` key path-escaped, offset + pagination followed. +- `internal/pagetree`: `WalkSpace` with **two** root pages (the AIM shape), + a folder under a root page (so the descend-into-folders rule is exercised + from a space seed), `ParentID == ""` on roots, depth numbering, and + `maxDepth` honoured. +- `cmd/children`: `--space` happy path; neither-arg and both-arg validation + errors (message and exit 2); unknown key → `space "X" not found`; the hint + line present at default depth and absent with `--depth all` and under + `--json`; `--json` root row carries `parent_id: null`. +- `internal/schematest` conformance: the `children` envelope with a null + `parent_id` validates. +- `make check`. +- One live run pasted into the PR body: `--space AIM` (multi-root) and + `--space '~60c36d0718e9f60071326951' --depth all`. + +## Docs + +- `docs/confluence/spaces.md` — new, holding the seven findings above with the + requests and responses that produced them. +- `docs/confluence/folders.md` — the *Unverified* "a folder at a space root" + bullet becomes a verified statement, pointing at `spaces.md`. +- `README.md` — the `children` section: usage line, `--space` flag, examples + (`--space ENG`, `--space ENG --depth all`, the `--json`+`jq` one-liner), and + a note that a space's top level is usually just its homepage. +- `CLAUDE.md` — the `cmd/children/` bullet gains `--space`; the `cmd/search/` + bullet's claim that `--cql`'s `Flags().Changed` is "the only use of it in + `cmd/`" stops being true and must be corrected in the same commit. + +## Commits + +1. `docs(plans): plan children --space` — this file. +2. `docs(confluence): how a space root enumerates, and folders at a root` — + `spaces.md` + the `folders.md` bullet. +3. `feat(pagetree): walk a space's tree from its root pages` — client + + pagetree + their tests. +4. `feat(children): add --space to list a space by key` — command + schema + + tests. +5. `docs: document children --space` — README + CLAUDE.md. + +## Out of scope + +- `export --space` / a multi-page export, which is #59. `WalkSpace` lands in + `pagetree` rather than in the command so that work can use it. +- Teaching `find`/`search`/`children` to accept a space URL wherever a key is + taken. Worth its own issue if the paste is a real annoyance. +- Any filter on what is listed (type, label, updated-since). `--json` plus + `jq` covers it, and `search --cql` covers the rest. From 067d0c4b42aaae00b936fa10584b55bef9a8b84b Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 31 Aug 2026 10:23:58 -0400 Subject: [PATCH 2/7] docs(confluence): how a space root enumerates, and folders at a root Records what was measured for #98: the v1 depth=root route and the fact that its rows are already ChildNode-shaped, that a space can have more than one root page (AIM has two, so walking from homepageId would silently drop a subtree), that an unknown key must be detected by resolving it rather than by reading a 404 body, and that a folder created with no parentId lands under the homepage rather than at the root -- which resolves an Unverified bullet in folders.md. --- docs/confluence/README.md | 1 + docs/confluence/folders.md | 18 ++++- docs/confluence/spaces.md | 136 +++++++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 docs/confluence/spaces.md diff --git a/docs/confluence/README.md b/docs/confluence/README.md index 779a551..083078f 100644 --- a/docs/confluence/README.md +++ b/docs/confluence/README.md @@ -11,6 +11,7 @@ person who ran the experiment. - [links-and-anchors.md](links-and-anchors.md) — heading anchors and page links - [page-width.md](page-width.md) — the content properties behind `page_width` - [folders.md](folders.md) — the Cloud folder type, and why child listing is v1 +- [spaces.md](spaces.md) — what sits at the top of a space, and how to enumerate it - [search.md](search.md) — finding content by title and by full text, and `/search`'s paging traps ## How to read an entry diff --git a/docs/confluence/folders.md b/docs/confluence/folders.md index d56f048..26aa745 100644 --- a/docs/confluence/folders.md +++ b/docs/confluence/folders.md @@ -144,11 +144,23 @@ absence cannot terminate a loop. [ccli]: https://github.com/pchuri/confluence-cli +## Verified 2026-08-31 + +### A folder cannot be created at a space root + +What a folder created directly at the top of a space reports was an open +question here until it turned out there is no such folder. +`POST /wiki/api/v2/folders` with a `spaceId` and **no `parentId`** returns 200 +having created the folder **under the space homepage**: `parentId` is the +homepage's id and `parentType` is `"page"`. There is also no route that would +list a root-level folder if one existed; both halves are recorded, with the +requests, in [spaces.md](spaces.md). + +So enumerating what is at the top of a space means enumerating its root +*pages*. Folders turn up as soon as the walk descends into them. + ## Unverified -- **A folder at a space root.** Every folder observed had a parent — a page in one - case, a folder in another. What `parentType` reports for a folder created - directly at the top of a space was not observed. - **How deep nesting may go**, and whether Confluence enforces a limit. Two levels were verified; nothing suggests two is special. - **Data Center.** Asserted to have no folder content type. Not tested — no DC diff --git a/docs/confluence/spaces.md b/docs/confluence/spaces.md new file mode 100644 index 0000000..511760a --- /dev/null +++ b/docs/confluence/spaces.md @@ -0,0 +1,136 @@ +# Spaces + +What sits at the top of a space, and how to enumerate it. This is the evidence +behind `children --space` (#98). + +Everything below was established against `mozilla-hub.atlassian.net`, basic auth +on the site domain, using two spaces: + +- a **personal** space (`~60c36d0718e9f60071326951`, id `76646426`, homepage + `76646878`) holding 34 pages and 3 folders, all of them under the homepage; +- **`AIM`** (id `2097152`, homepage `2097154`), a global space picked because + it turned out to have *two* pages at its root. + +## Verified 2026-08-31 + +### A space's root pages come from a v1 route, and its rows are already `ChildNode` + +`GET /wiki/rest/api/space/{key}/content/page?depth=root` — 200, and a bare row +(no `expand`) carries every field child listing already relies on: + +```json +{ "id": "2097154", "type": "page", "status": "current", + "title": "Africa Innovation Mradi Home", + "extensions": { "position": 117908152 }, + "_links": { "webui": "/spaces/AIM/overview", "tinyui": "/x/AgAg", ... } } +``` + +So `listV1[ChildNode]` reads it unchanged, and a root row renders with a URL and +a space key with no follow-up request. Note the homepage's `webui` is +`/spaces/{key}/overview` rather than `/spaces/{key}/pages/...` — +`SpaceKeyFromWebUI` already handles both, as [search.md](search.md) records. + +`.../content` without the `/page` suffix answers with both a `page` and a +`blogpost` collection. `children` wants the page tree, so the type belongs in +the path. + +### A space can have more than one root page + +| space | `depth=root` pages | +|---|---| +| personal | 1 — `Things` (the homepage) | +| `AGILE` | 1 — `Becoming Agile` (the homepage) | +| `AT` | 1 — `Away Team Home` (the homepage) | +| `AMZ` | 1 — `Amazon` (the homepage) | +| **`AIM`** | **2 — `Africa Innovation Mradi Home` (the homepage) and `What is Africa Mradi?`** | + +This is the finding that decides how a space is walked. "The space root is the +homepage, so start from `homepageId`" is true of four spaces out of five and +**wrong** on the fifth, where it would drop a root page and everything under it +— the same class of wrong answer as v2 `/children` omitting folders +([folders.md](folders.md)), and just as silent. + +`markfluence create` with `parent: null` produces exactly this shape, so a +multi-root space is not an exotic case a user has to go out of their way to +build. + +### Archived root pages are not listed + +The personal space has an archived page with no parent +(`Some Archived Page`, `2973663237` — it appears in the v2 flat listing below +with `parentId: null`). `depth=root` reports only `Things`. So the route +already behaves like a v1 child listing: current content, no status filter +needed. + +### An unknown key answers 404, and that is not the way to detect one + +``` +GET /wiki/rest/api/space/NOSUCHSPACEXYZ/content/page?depth=root +404 {"statusCode":404,"message":"org.springframework.web.server.ResponseStatusException: + 404 NOT_FOUND \"No space found with key : NOSUCHSPACEXYZ\""} +``` + +The message names the key, but reading it is the wrong move: **a rejected +credential is also a 404** ([api.md](api.md)), the text is a Spring exception +string rather than a documented body, and misreading an auth failure as "no such +space" is the exact defect `RejectedCredential` exists to prevent. Resolve the +key with `GET /wiki/api/v2/spaces?keys={key}` first, the way `find` and `search` +already do. + +### A folder cannot sit at a space root + +There is no route that would list one: + +| request | result | +|---|---| +| `GET /wiki/rest/api/space/{key}/content/folder?depth=root` | **500** `java.lang.NullPointerException: PageRequest should not be null` | +| `GET /wiki/api/v2/spaces/{id}/folders` | 404, and an HTML error page — not an API route | +| `GET /wiki/api/v2/folders?space-id={id}` | **500** `INTERNAL_SERVER_ERROR` | +| `GET /wiki/api/v2/spaces/{id}/direct-children` | **400** `Provided value {spaces} for 'hierarchical-content-type' is not the correct type` | + +That last one is informative rather than merely a failure: the route is +`/wiki/api/v2/{hierarchical-content-type}/{id}/direct-children`, and there is no +`spaces` member of that type. Do not go looking for one again. + +None of it matters, because a folder appears to be unable to sit at a root in +the first place: + +``` +POST /wiki/api/v2/folders {"spaceId":"76646426","title":"mf-98 root folder probe"} +200 {"id":"3026485251","type":"folder","parentId":"76646878","parentType":"page", ...} +``` + +**With no `parentId`, the folder was created under the space homepage.** Asking +for a root-level folder does not produce one; it produces a child of the +homepage. (The probe folder was trashed immediately; +`DELETE /wiki/api/v2/folders/{id}` → 204, and the homepage's folder children +went back to 3.) + +So enumerating a space root means enumerating its root **pages**. Folders are +found the moment the walk descends into them, which is what +`pagetree.siblings` already does. + +### The v2 flat listing exists, and is not usable here + +`GET /wiki/api/v2/spaces/{id}/pages` returns every page in the space as a flat +cursor-paginated collection, each row carrying `parentId` and `parentType` — the +whole 34-page personal space in one request, where a walk costs a request pair +per node. Tempting, and rejected twice over: + +- **It lists no folders.** A folder id appears as some page's `parentId`, with no + title and no position, so a subtree hanging off a folder cannot be placed and + the folder itself cannot be shown. Reconstructing the tree from it would + reproduce exactly the "wrong answer, not a partial one" that v2 + `/pages/{id}/children` produces. +- **It includes archived pages by default**, as the archived root page above + demonstrates. + +## Unverified + +- **Data Center.** All of the above is Cloud. The v1 space content route exists + there too, but nothing here was tested against a DC instance. +- **Whether the UI can place a folder at a space root** even though the API's + own create route will not. Only the API path was exercised. +- **A space with no homepage at all** (possible for a space created by an + import, allegedly). Not observed; `depth=root` would presumably just report + whatever pages are there, which is what the walk already handles. From 0b918489634d72bb93e18dce6982da8c0d7262f0 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 31 Aug 2026 10:26:12 -0400 Subject: [PATCH 3/7] feat(pagetree): walk a space's tree from its root pages client.ListSpaceRootPages reads the v1 depth=root collection, whose rows are already ChildNode-shaped, and pagetree.WalkSpace seeds the existing traversal with them. The seed is the space's root *pages*, not its homepage: a space can have several roots, so starting from homepageId would drop one and its whole subtree. A root page reports ParentID "", since the space it sits in is not a node. Walk and WalkSpace now share one walker, so the depth rule and the visited guard exist in one copy. --- internal/client/client.go | 19 ++++ internal/client/client_test.go | 38 ++++++++ internal/pagetree/pagetree.go | 136 ++++++++++++++++++--------- internal/pagetree/pagetree_test.go | 142 +++++++++++++++++++++++++---- 4 files changed, 275 insertions(+), 60 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index 01f9929..9492cd0 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -744,6 +744,25 @@ func (c *ConfluenceClient) ListChildFolders(id string) ([]ChildNode, error) { return listV1[ChildNode](c, "/wiki/rest/api/content/"+id+"/child/folder", nil) } +// ListSpaceRootPages lists the pages at the root of a space, named by key. +// +// This is not a homepage lookup by another way round: a space can hold several +// root pages -- one space in the survey behind docs/confluence/spaces.md has the +// homepage plus a second root, and `create` with a null parent produces exactly +// that -- so starting a walk from the space's homepageId would silently drop a +// root page and its whole subtree. +// +// The type is in the path (/content/page) because /content also answers with a +// blogpost collection, and the page tree is what a child listing is about. +// Archived pages are absent without a status filter, matching the v1 child +// routes. There is deliberately no root-folder companion to this call: a folder +// created with no parent lands under the homepage rather than at the root, so +// there is nothing for one to find (docs/confluence/spaces.md). +func (c *ConfluenceClient) ListSpaceRootPages(spaceKey string) ([]ChildNode, error) { + return listV1[ChildNode](c, "/wiki/rest/api/space/"+url.PathEscape(spaceKey)+"/content/page", + url.Values{"depth": {"root"}}) +} + // GetPageBodyOrNil fetches a page including its storage-format body, returning // nil (no error) on HTTP 404. The plain GetPage/GetPageOrNil stay bodyless so // metadata-only callers don't pay to transfer the body. diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 96c090b..100769e 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -606,6 +606,44 @@ func TestListChildFoldersHitsTheFolderPath(t *testing.T) { } } +// TestListSpaceRootPagesAsksForRootsOnly pins the three things the route needs +// to be right: the page-typed path (plain /content also answers with blogposts), +// depth=root (without it the route lists every page in the space), and a key +// path-escaped, since a personal space key starts with a "~". +func TestListSpaceRootPagesAsksForRootsOnly(t *testing.T) { + var gotPath, gotDepth string + c := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + gotPath, gotDepth = r.URL.EscapedPath(), r.URL.Query().Get("depth") + _, _ = w.Write([]byte(`{"results":[{"id":"11","type":"page","title":"Things",` + + `"status":"current","extensions":{"position":117908152},` + + `"_links":{"webui":"/spaces/~abc/overview"}}]}`)) + }) + + got, err := c.ListSpaceRootPages("~abc") + if err != nil { + t.Fatalf("ListSpaceRootPages: %v", err) + } + if want := "/wiki/rest/api/space/~abc/content/page"; gotPath != want { + t.Errorf("path = %q, want %q", gotPath, want) + } + if gotDepth != "root" { + t.Errorf("depth = %q, want root", gotDepth) + } + if len(got) != 1 { + t.Fatalf("len = %d, want 1", len(got)) + } + // The rows are ChildNode-shaped, which is what lets a space seed the same + // walk a page does. A homepage's webui is /spaces/{key}/overview rather than + // /pages/..., and the space key still has to come out of it. + n := got[0] + if n.Status != "current" || n.Extensions.Position != 117908152 { + t.Errorf("status/position = %q/%d, want current/117908152", n.Status, n.Extensions.Position) + } + if SpaceKeyFromWebUI(n.Links.WebUI) != "~abc" { + t.Errorf("space from webui = %q, want ~abc", SpaceKeyFromWebUI(n.Links.WebUI)) + } +} + func TestResolveSpaceID(t *testing.T) { c, _ := newServer(t, resp{200, `{"results":[{"id":"123"}]}`}) if id, err := c.ResolveSpaceID("ENG"); err != nil || id != "123" { diff --git a/internal/pagetree/pagetree.go b/internal/pagetree/pagetree.go index 62277bb..f61be0d 100644 --- a/internal/pagetree/pagetree.go +++ b/internal/pagetree/pagetree.go @@ -22,7 +22,8 @@ type Node struct { Title string Status string // ParentID is the node this one hangs off, which for a top-level result is - // the id the walk started from. + // the id the walk started from — or "" for a page at the root of a space, + // which hangs off no node at all. ParentID string // Depth is 1 for a direct child, 2 for its child, and so on. Depth int @@ -39,48 +40,92 @@ type Node struct { // folders are reported rather than silently traversed — the caller can see there // is more below and ask for it. func Walk(c *client.ConfluenceClient, rootID string, maxDepth int) ([]Node, error) { - // Confluence trees should not contain cycles, but an unbounded walk has no - // other backstop if one ever appears, and the set costs nothing. - visited := map[string]bool{rootID: true} - var out []Node + w := &walker{c: c, maxDepth: maxDepth, visited: map[string]bool{rootID: true}} + if err := w.descend(rootID, 1); err != nil { + return nil, err + } + return w.out, nil +} - var walk func(parentID string, depth int) error - walk = func(parentID string, depth int) error { - if maxDepth != AllDepths && depth > maxDepth { - return nil - } - children, err := siblings(c, parentID) - if err != nil { - return err - } - for _, ch := range children { - if visited[ch.ID] { - continue - } - visited[ch.ID] = true - out = append(out, Node{ - ID: ch.ID, - Type: ch.Type, - Title: ch.Title, - Status: ch.Status, - ParentID: parentID, - Depth: depth, - Space: client.SpaceKeyFromWebUI(ch.Links.WebUI), - URL: nodeURL(c, ch.Links.WebUI), - }) - // Depth-first, so a node's subtree is printed under it rather than - // after all of its siblings. - if err := walk(ch.ID, depth+1); err != nil { - return err - } - } - return nil +// WalkSpace returns every page and folder in a space, named by key, in the same +// order and shape Walk returns them. +// +// Depth 1 is the space's *root pages* — normally just the homepage, sometimes +// more (docs/confluence/spaces.md) — so their children are depth 2. A root page +// has no parent node, and reports ParentID "" to say so. +// +// The space is the level above them rather than a node of its own: a space is not +// a page, so it cannot be a row, and every root page really does sit at the top +// of the tree a reader sees in Confluence. +func WalkSpace(c *client.ConfluenceClient, spaceKey string, maxDepth int) ([]Node, error) { + roots, err := c.ListSpaceRootPages(spaceKey) + if err != nil { + return nil, err } + // Sorted for the same reason siblings are: two root pages come back in + // whatever order the collection route chose, and position is the order + // Confluence shows them in. + byPosition(roots) - if err := walk(rootID, 1); err != nil { + w := &walker{c: c, maxDepth: maxDepth, visited: map[string]bool{}} + if err := w.emit(roots, "", 1); err != nil { return nil, err } - return out, nil + return w.out, nil +} + +// walker carries the state one traversal accumulates, so Walk and WalkSpace can +// differ only in what they seed it with. Both go through emit, which is what +// keeps "a folder counts as a level" and the visited guard in one copy. +type walker struct { + c *client.ConfluenceClient + maxDepth int + visited map[string]bool + out []Node +} + +// descend lists what is directly under parentID and emits it at depth. +// +// The bound is checked here as well as in emit, and not redundantly: this one +// saves the request pair a level nobody asked for would have cost, where emit's +// decides what is reported. +func (w *walker) descend(parentID string, depth int) error { + if w.maxDepth != AllDepths && depth > w.maxDepth { + return nil + } + children, err := siblings(w.c, parentID) + if err != nil { + return err + } + return w.emit(children, parentID, depth) +} + +// emit records each node at depth and recurses into it, depth-first, so a node's +// subtree is printed under it rather than after all of its siblings. +func (w *walker) emit(nodes []client.ChildNode, parentID string, depth int) error { + if w.maxDepth != AllDepths && depth > w.maxDepth { + return nil + } + for _, ch := range nodes { + if w.visited[ch.ID] { + continue + } + w.visited[ch.ID] = true + w.out = append(w.out, Node{ + ID: ch.ID, + Type: ch.Type, + Title: ch.Title, + Status: ch.Status, + ParentID: parentID, + Depth: depth, + Space: client.SpaceKeyFromWebUI(ch.Links.WebUI), + URL: nodeURL(w.c, ch.Links.WebUI), + }) + if err := w.descend(ch.ID, depth+1); err != nil { + return err + } + } + return nil } // nodeURL builds the link a reader follows. A v1 child row carries webui but no @@ -110,10 +155,15 @@ func siblings(c *client.ConfluenceClient, id string) ([]client.ChildNode, error) all := make([]client.ChildNode, 0, len(pages)+len(folders)) all = append(all, pages...) all = append(all, folders...) - // Stable, so two rows sharing a position keep pages-before-folders rather - // than reordering between runs. - sort.SliceStable(all, func(i, j int) bool { - return all[i].Extensions.Position < all[j].Extensions.Position - }) + byPosition(all) return all, nil } + +// byPosition orders nodes the way Confluence displays them. Stable, so two rows +// sharing a position keep the order they arrived in — pages before folders for a +// merged sibling listing — rather than reordering between runs. +func byPosition(nodes []client.ChildNode) { + sort.SliceStable(nodes, func(i, j int) bool { + return nodes[i].Extensions.Position < nodes[j].Extensions.Position + }) +} diff --git a/internal/pagetree/pagetree_test.go b/internal/pagetree/pagetree_test.go index 799ba7e..6738081 100644 --- a/internal/pagetree/pagetree_test.go +++ b/internal/pagetree/pagetree_test.go @@ -26,6 +26,16 @@ func treeServer(t *testing.T, tree map[string][]node) (*client.ConfluenceClient, c := clienttest.New(t, func(w http.ResponseWriter, r *http.Request) { calls++ parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + // .../space/{key}/content/page?depth=root -- the WalkSpace seed. Keyed + // in the fixture as "space:{key}", so one fixture describes both the + // roots and everything under them. + if len(parts) >= 4 && parts[len(parts)-4] == "space" { + if got := r.URL.Query().Get("depth"); got != "root" { + t.Errorf("space content depth = %q, want root", got) + } + writeRows(w, tree["space:"+parts[len(parts)-3]], "page") + return + } // .../content/{id}/child/{kind} if len(parts) < 6 { t.Errorf("unexpected path: %s", r.URL.Path) @@ -33,27 +43,31 @@ func treeServer(t *testing.T, tree map[string][]node) (*client.ConfluenceClient, return } parentID, want := parts[len(parts)-3], parts[len(parts)-1] - - rows := make([]map[string]any, 0) - for _, n := range tree[parentID] { - if n.kind != want { - continue - } - slug := "pages" - if n.kind == "folder" { - slug = "folder" - } - rows = append(rows, map[string]any{ - "id": n.id, "type": n.kind, "title": n.title, "status": "current", - "extensions": map[string]any{"position": n.position}, - "_links": map[string]any{"webui": fmt.Sprintf("/spaces/ENG/%s/%s", slug, n.id)}, - }) - } - _ = json.NewEncoder(w).Encode(map[string]any{"results": rows}) + writeRows(w, tree[parentID], want) }) return c, &calls } +// writeRows answers a v1 collection with the fixture nodes of one kind. +func writeRows(w http.ResponseWriter, nodes []node, kind string) { + rows := make([]map[string]any, 0) + for _, n := range nodes { + if n.kind != kind { + continue + } + slug := "pages" + if n.kind == "folder" { + slug = "folder" + } + rows = append(rows, map[string]any{ + "id": n.id, "type": n.kind, "title": n.title, "status": "current", + "extensions": map[string]any{"position": n.position}, + "_links": map[string]any{"webui": fmt.Sprintf("/spaces/ENG/%s/%s", slug, n.id)}, + }) + } + _ = json.NewEncoder(w).Encode(map[string]any{"results": rows}) +} + // fixture: root holds a folder and two pages, interleaved by position; the // folder holds a page; that page holds a page three levels down. func fixture() map[string][]node { @@ -193,3 +207,97 @@ func TestWalkSurvivesACycle(t *testing.T) { t.Errorf("got %v, want just A", titles(got)) } } + +// spaceFixture is the AIM shape: two root pages, the second of them out of +// position order, with a folder under the first holding the only page in its +// subtree. +func spaceFixture() map[string][]node { + return map[string][]node{ + "space:ENG": { + {"r2", "page", "Second root", 900}, + {"r1", "page", "Home", 100}, + }, + "r1": {{"f1", "folder", "Articles", 10}}, + "f1": {{"p1", "page", "Inside Articles", 10}}, + } +} + +// TestWalkSpaceListsEveryRoot is the finding the whole feature rests on: a space +// can have more than one root page, so a walk seeded from its homepage alone +// would silently drop a root and everything under it. +func TestWalkSpaceListsEveryRoot(t *testing.T) { + c, _ := treeServer(t, spaceFixture()) + got, err := WalkSpace(c, "ENG", 1) + if err != nil { + t.Fatalf("WalkSpace: %v", err) + } + want := []string{"p:Home@1", "p:Second root@1"} + if fmt.Sprint(titles(got)) != fmt.Sprint(want) { + t.Errorf("got %v, want %v (both roots, in position order)", titles(got), want) + } +} + +// TestWalkSpaceRootHasNoParent is what --json reports as parent_id: null. A root +// page hangs off no node, and the space is not one. +func TestWalkSpaceRootHasNoParent(t *testing.T) { + c, _ := treeServer(t, spaceFixture()) + got, err := WalkSpace(c, "ENG", 2) + if err != nil { + t.Fatalf("WalkSpace: %v", err) + } + for _, n := range got { + wantParent := "" + if n.Depth > 1 { + wantParent = "r1" + } + if n.ParentID != wantParent { + t.Errorf("%s (depth %d) ParentID = %q, want %q", n.Title, n.Depth, n.ParentID, wantParent) + } + } +} + +// TestWalkSpaceDescendsFolders pins that a space seed reaches the same places a +// page seed does: the folder counts as a level, and the walk goes into it rather +// than stopping at the row. +func TestWalkSpaceDescendsFolders(t *testing.T) { + c, _ := treeServer(t, spaceFixture()) + got, err := WalkSpace(c, "ENG", AllDepths) + if err != nil { + t.Fatalf("WalkSpace: %v", err) + } + want := []string{"p:Home@1", "f:Articles@2", "p:Inside Articles@3", "p:Second root@1"} + if fmt.Sprint(titles(got)) != fmt.Sprint(want) { + t.Errorf("got %v, want %v", titles(got), want) + } +} + +// TestWalkSpaceHonoursDepth checks the bound is applied from the roots, and that +// a level nobody asked for costs no requests. +func TestWalkSpaceHonoursDepth(t *testing.T) { + c, calls := treeServer(t, spaceFixture()) + got, err := WalkSpace(c, "ENG", 1) + if err != nil { + t.Fatalf("WalkSpace: %v", err) + } + if len(got) != 2 { + t.Fatalf("got %v, want the two roots only", titles(got)) + } + // One request for the roots, and nothing below them. + if *calls != 1 { + t.Errorf("calls = %d, want 1 (no requests below the depth limit)", *calls) + } +} + +// TestWalkSpaceEmptyIsNotAnError: a space with no root pages is empty, not +// broken. It should not happen -- every space has a homepage -- but a walk that +// errored on it would turn a permissions oddity into a failure. +func TestWalkSpaceEmptyIsNotAnError(t *testing.T) { + c, _ := treeServer(t, map[string][]node{}) + got, err := WalkSpace(c, "ENG", AllDepths) + if err != nil { + t.Fatalf("an empty space must not be an error: %v", err) + } + if len(got) != 0 { + t.Errorf("got %d nodes, want none", len(got)) + } +} From 3998875d99b4724ff235419ac5eb57ec4a00dc61 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 31 Aug 2026 10:29:16 -0400 Subject: [PATCH 4/7] feat(children): add --space to list a space by key PAGE becomes optional and exactly one of PAGE / --space is required, checked before credentials are resolved. Depth 1 under --space is the space's root pages -- usually just the homepage, which is why human output adds a line pointing at --depth when the caller left it alone. An unknown key is reported as a typo (exit 2, VALIDATION) after resolving it with the v2 spaces route, rather than by reading the v1 404's body: a rejected credential answers 404 too. childrenResult.parent_id widens to stringOrNull, since a page at a space root hangs off no node and the space is not one. --- cmd/children/children.go | 87 +++++++++++++++---- cmd/children/children_test.go | 154 ++++++++++++++++++++++++++++++++++ cmd/children/json.go | 7 +- cmd/children/json_test.go | 13 +++ schema/json-output/v1.json | 4 +- 5 files changed, 246 insertions(+), 19 deletions(-) diff --git a/cmd/children/children.go b/cmd/children/children.go index efa3a19..4363c9e 100644 --- a/cmd/children/children.go +++ b/cmd/children/children.go @@ -23,21 +23,28 @@ const command = "children" // depthAll is the --depth value meaning "however deep it goes". const depthAll = "all" -var depthOpt string +var ( + depthOpt string + spaceOpt string +) // Cmd is the children command. var Cmd = &cobra.Command{ - Use: command + " PAGE", - Short: "List the pages and folders under a Confluence page or folder", + Use: command + " [PAGE]", + Short: "List the pages and folders under a Confluence page, folder, or space", Long: "List the pages and folders under a Confluence page or folder.\n\n" + "PAGE is a numeric id, a Confluence page or folder URL, or a markdown\n" + "file whose frontmatter has a page_id.\n\n" + + "Pass --space KEY instead of a PAGE to list a whole space. Depth 1 is\n" + + "then the space's top level, which is usually just its homepage, so\n" + + "--depth 2 or --depth all is what shows the tree. Walking a space costs\n" + + "one pair of requests per page and folder in it.\n\n" + "Folders are listed alongside pages, with a TYPE column, because a\n" + "folder can hold the only pages in a subtree -- listing pages alone\n" + "would show nothing for a folder that contains folders.\n\n" + "A folder counts as a level: at the default --depth 1 a child folder\n" + "appears as a row, and --depth 2 shows what is inside it.", - Args: cobra.ExactArgs(1), + Args: cobra.MaximumNArgs(1), ValidArgsFunction: completion.MarkdownFiles, RunE: run, } @@ -45,7 +52,12 @@ var Cmd = &cobra.Command{ func init() { Cmd.Flags().StringVar(&depthOpt, "depth", "1", `How deep to recurse: a positive number, or "all".`) + Cmd.Flags().StringVar(&spaceOpt, "space", "", + "List a whole space, by key, instead of a PAGE.") completion.RegisterFlag(Cmd, "depth", completion.Values("1", "2", "3", depthAll)) + // A space key lives on the server, and completion runs on every keystroke, + // so it completes to nothing rather than stalling the shell. + completion.RegisterFlag(Cmd, "space", cobra.NoFileCompletions) } func run(cmd *cobra.Command, args []string) error { @@ -54,8 +66,11 @@ func run(cmd *cobra.Command, args []string) error { cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - // Before the credential check: a bad --depth is a usage error and does not - // need a server to be recognized. + // Before the credential check: neither of these needs a server to be + // recognized as a usage error. + if err := checkTarget(args, spaceOpt); err != nil { + return fatalFail(err.Error(), jsonout.CodeValidation) + } depth, err := parseDepth(depthOpt) if err != nil { return fatalFail(err.Error(), jsonout.CodeValidation) @@ -68,14 +83,36 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail(err.Error(), jsonout.CodeConfig) } - id, err := pageref.Resolve(args[0]) - if err != nil { - return fatalFail(err.Error(), jsonout.CodeValidation) - } - - nodes, err := pagetree.Walk(c, id, depth) - if err != nil { - return operationalFail(id, err, jsonout.CodeFor(err)) + var ( + id string + nodes []pagetree.Node + ) + if spaceOpt != "" { + // The key is resolved rather than handed straight to the walk, even + // though the route it feeds takes a key: an unknown key is the user's + // typo and deserves to be named as one, and the v1 route reports it as a + // 404 -- which is also what a rejected credential looks like. + spaceID, err := c.ResolveSpaceID(spaceOpt) + if err != nil { + return operationalFail(spaceOpt, err, jsonout.CodeFor(err)) + } + if spaceID == "" { + return fatalFail(fmt.Sprintf("space %q not found", spaceOpt), jsonout.CodeValidation) + } + id = spaceOpt + nodes, err = pagetree.WalkSpace(c, spaceOpt, depth) + if err != nil { + return operationalFail(id, err, jsonout.CodeFor(err)) + } + } else { + id, err = pageref.Resolve(args[0]) + if err != nil { + return fatalFail(err.Error(), jsonout.CodeValidation) + } + nodes, err = pagetree.Walk(c, id, depth) + if err != nil { + return operationalFail(id, err, jsonout.CodeFor(err)) + } } if ui.IsJSON() { @@ -93,6 +130,28 @@ func run(cmd *cobra.Command, args []string) error { return nil } fmt.Println(tree(nodes)) + if spaceOpt != "" && !cmd.Flags().Changed("depth") { + // A space's top level is usually one row -- its homepage -- which reads + // like the whole answer. Human output only: a --json consumer is not + // reading prose, and the row it would explain is already in the array. + fmt.Println() + ui.Info("Showing the space's top level. Use --depth 2, or --depth all for the whole tree.") + } + return nil +} + +// checkTarget requires exactly one of PAGE and --space. +// +// They are alternatives rather than a filter and a target: --space names the +// root, so combining them would mean two roots, and neither of them means +// nothing to walk. +func checkTarget(args []string, space string) error { + switch { + case len(args) == 0 && space == "": + return fmt.Errorf("no page given: pass a PAGE, or --space KEY to list a whole space") + case len(args) > 0 && space != "": + return fmt.Errorf("PAGE and --space cannot be combined: --space lists a whole space") + } return nil } diff --git a/cmd/children/children_test.go b/cmd/children/children_test.go index c618553..a21d65b 100644 --- a/cmd/children/children_test.go +++ b/cmd/children/children_test.go @@ -26,6 +26,9 @@ func testCmd(t *testing.T, url string) *cobra.Command { c.Flags().String("username", "u", "") c.Flags().String("cloud-id", "", "") c.Flags().String("env-file", "", "") + // run reads --depth's *value* from the package-level flag var, but asks the + // command whether it was set at all, so the flag has to exist here too. + c.Flags().String("depth", "1", "") return c } @@ -68,6 +71,42 @@ func childServer(t *testing.T) string { return c.SiteURL() } +// spaceServer answers the space-id resolve, the space root-page collection, and +// the child routes under the one root it reports. spaceID "" makes the key +// unknown. +func spaceServer(t *testing.T, spaceID string) string { + t.Helper() + c := clienttest.New(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/wiki/api/v2/spaces": + if spaceID == "" { + _, _ = w.Write([]byte(`{"results":[]}`)) + return + } + _, _ = w.Write([]byte(`{"results":[{"id":"` + spaceID + `"}]}`)) + case r.URL.Path == "/wiki/rest/api/space/ENG/content/page": + _, _ = w.Write([]byte(`{"results":[{"id":"1","type":"page","title":"Home",` + + `"status":"current","extensions":{"position":0},` + + `"_links":{"webui":"/spaces/ENG/overview"}}]}`)) + case strings.HasPrefix(r.URL.Path, "/wiki/rest/api/content/1/child/page"): + _, _ = w.Write([]byte(`{"results":[{"id":"2","type":"page","title":"Child",` + + `"status":"current","extensions":{"position":0},"_links":{"webui":"/spaces/ENG/pages/2/Child"}}]}`)) + case strings.HasPrefix(r.URL.Path, "/wiki/rest/api/content/"): + _, _ = w.Write([]byte(`{"results":[]}`)) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + }) + return c.SiteURL() +} + +// withSpace sets --space for one test, restoring the package-level flag after. +func withSpace(t *testing.T, key string) { + t.Helper() + spaceOpt = key + t.Cleanup(func() { spaceOpt = "" }) +} + // TestParseDepth covers the flag's whole vocabulary. 0 is the interesting case: // it is a common spelling of "unlimited" elsewhere, so accepting it would launch // an unbounded walk for someone who may have meant the opposite. @@ -197,3 +236,118 @@ func TestRunJSONOutput(t *testing.T) { t.Errorf("envelope = %+v, want command=children with one result id=2", env) } } + +// TestCheckTarget is the exactly-one-of rule. Both spellings name the root of the +// walk, so neither "both" nor "neither" has an answer. +func TestCheckTarget(t *testing.T) { + if err := checkTarget([]string{"1"}, ""); err != nil { + t.Errorf("PAGE alone: %v", err) + } + if err := checkTarget(nil, "ENG"); err != nil { + t.Errorf("--space alone: %v", err) + } + err := checkTarget(nil, "") + if err == nil || !strings.Contains(err.Error(), "--space") { + t.Errorf("neither = %v, want an error naming --space", err) + } + err = checkTarget([]string{"1"}, "ENG") + if err == nil || !strings.Contains(err.Error(), "cannot be combined") { + t.Errorf("both = %v, want a refusal", err) + } +} + +// TestRunNoTargetIsAUsageError: the check happens before credentials are +// resolved, so it fails the same way with no server to talk to. +func TestRunNoTargetIsAUsageError(t *testing.T) { + _, err := captureStdout(t, func() error { + return run(testCmd(t, "https://wiki.example.net"), nil) + }) + if !ui.IsSilent(err) || ui.ExitCode(err) != 2 { + t.Fatalf("run: %v, want a silent exit-2 usage error for no PAGE and no --space", err) + } +} + +func TestRunListsASpace(t *testing.T) { + withSpace(t, "ENG") + url := spaceServer(t, "77") + out, err := captureStdout(t, func() error { return run(testCmd(t, url), nil) }) + if err != nil { + t.Fatalf("run: %v", err) + } + // Depth 1 is the space's root pages, so the homepage is a row and its child + // is not. + if !strings.Contains(out, "Home") { + t.Errorf("output = %q, want the space's root page listed", out) + } + if strings.Contains(out, "Child") { + t.Errorf("output = %q, want nothing below the root at --depth 1", out) + } +} + +// TestRunSpaceHintsAtDepth: one row is what a space's top level usually is, and +// reading it as the whole space is the trap the hint exists for. +func TestRunSpaceHintsAtDepth(t *testing.T) { + withSpace(t, "ENG") + url := spaceServer(t, "77") + + out, err := captureStdout(t, func() error { return run(testCmd(t, url), nil) }) + if err != nil { + t.Fatalf("run: %v", err) + } + if !strings.Contains(out, "--depth all") { + t.Errorf("output = %q, want a hint naming --depth all", out) + } + + // Not when the caller already said how deep to go: they know the flag. + cmd := testCmd(t, url) + if err := cmd.Flags().Set("depth", "1"); err != nil { + t.Fatal(err) + } + out, err = captureStdout(t, func() error { return run(cmd, nil) }) + if err != nil { + t.Fatalf("run: %v", err) + } + if strings.Contains(out, "--depth all") { + t.Errorf("output = %q, want no hint once --depth was given", out) + } +} + +// TestRunSpaceJSONHasNoHint: the hint is prose for a human, and a stray line in +// stdout would make the envelope unparseable. +func TestRunSpaceJSONHasNoHint(t *testing.T) { + ui.SetJSON(true) + t.Cleanup(func() { ui.SetJSON(false) }) + withSpace(t, "ENG") + + out, err := captureStdout(t, func() error { return run(testCmd(t, spaceServer(t, "77")), nil) }) + if err != nil { + t.Fatalf("run: %v", err) + } + var env struct { + Results []struct { + ID string `json:"id"` + ParentID *string `json:"parent_id"` + Depth int `json:"depth"` + } `json:"results"` + } + if err := json.Unmarshal([]byte(out), &env); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, out) + } + if len(env.Results) != 1 || env.Results[0].ID != "1" { + t.Fatalf("results = %+v, want the one root page", env.Results) + } + // A root page hangs off no node, and the space is not one. + if env.Results[0].ParentID != nil { + t.Errorf("parent_id = %q, want null", *env.Results[0].ParentID) + } +} + +// TestRunUnknownSpaceIsAUsageError: an unknown key is a typo, not a failed walk, +// and it must not be confused with the 404 a rejected credential produces. +func TestRunUnknownSpaceIsAUsageError(t *testing.T) { + withSpace(t, "ENG") + _, err := captureStdout(t, func() error { return run(testCmd(t, spaceServer(t, "")), nil) }) + if !ui.IsSilent(err) || ui.ExitCode(err) != 2 { + t.Fatalf("run: %v, want a silent exit-2 usage error for an unknown space key", err) + } +} diff --git a/cmd/children/json.go b/cmd/children/json.go index 5c10c0a..003b833 100644 --- a/cmd/children/json.go +++ b/cmd/children/json.go @@ -20,7 +20,7 @@ type jsonChildResult struct { Type string `json:"type"` Title string `json:"title"` Status string `json:"status"` - ParentID string `json:"parent_id"` + ParentID *string `json:"parent_id"` Depth int `json:"depth"` Space *string `json:"space"` URL *string `json:"url"` @@ -33,7 +33,7 @@ func buildResult(n pagetree.Node) jsonChildResult { Type: n.Type, Title: n.Title, Status: n.Status, - ParentID: n.ParentID, + ParentID: nullable(n.ParentID), Depth: n.Depth, Space: nullable(n.Space), URL: nullable(n.URL), @@ -41,7 +41,8 @@ func buildResult(n pagetree.Node) jsonChildResult { } // nullable maps an empty string to a JSON null, else a pointer to the value. -// space and url are both derived from webui, so a row without one has neither. +// space and url are both derived from webui, so a row without one has neither; +// parent_id is null for a page at the root of a space, which hangs off no node. func nullable(s string) *string { if s == "" { return nil diff --git a/cmd/children/json_test.go b/cmd/children/json_test.go index ff97270..094919d 100644 --- a/cmd/children/json_test.go +++ b/cmd/children/json_test.go @@ -20,6 +20,9 @@ func TestSchemaConformance(t *testing.T) { // A row with no webui: space and url are both null, which the schema has // to allow. {ID: "33", Type: "page", Title: "Linkless", Status: "current", ParentID: "22", Depth: 2}, + // A space's root page: no parent node at all, so parent_id is null too. + {ID: "44", Type: "page", Title: "Home", Status: "current", Depth: 1, + Space: "ENG", URL: "https://wiki.example.net/wiki/spaces/ENG/overview"}, } results := make([]any, 0, len(nodes)) for _, n := range nodes { @@ -89,3 +92,13 @@ func TestBuildResultNullsWithoutWebUI(t *testing.T) { t.Errorf("url = %v, want null", *res.URL) } } + +// TestBuildResultNullsParentAtASpaceRoot: WalkSpace reports "" for a root page's +// parent, and "" must reach --json as null rather than as an empty string that +// reads like an id. +func TestBuildResultNullsParentAtASpaceRoot(t *testing.T) { + res := buildResult(pagetree.Node{ID: "11", Type: "page", Title: "Home", Depth: 1}) + if res.ParentID != nil { + t.Errorf("parent_id = %q, want null", *res.ParentID) + } +} diff --git a/schema/json-output/v1.json b/schema/json-output/v1.json index 1e5e63b..5a48c73 100644 --- a/schema/json-output/v1.json +++ b/schema/json-output/v1.json @@ -162,7 +162,7 @@ "oneOf": [{ "$ref": "#/$defs/code" }, { "type": "null" }] }, "childrenResult": { - "description": "One page or folder under the requested node. The results array is flat and in walk order (depth-first, siblings in the order Confluence displays them); parent_id and depth carry the hierarchy. space and url are both derived from the row's webui link, so a row missing one is missing both.", + "description": "One page or folder under the requested node, or under the requested space's root. The results array is flat and in walk order (depth-first, siblings in the order Confluence displays them); parent_id and depth carry the hierarchy. parent_id is null for a page at the root of a space, which hangs off no node -- under --space, that is every depth-1 row. space and url are both derived from the row's webui link, so a row missing one is missing both.", "type": "object", "additionalProperties": false, "required": ["ok", "id", "type", "title", "status", "parent_id", "depth", "space", "url"], @@ -172,7 +172,7 @@ "type": { "enum": ["page", "folder"] }, "title": { "type": "string" }, "status": { "type": "string" }, - "parent_id": { "type": "string" }, + "parent_id": { "$ref": "#/$defs/stringOrNull" }, "depth": { "type": "integer", "minimum": 1 }, "space": { "$ref": "#/$defs/stringOrNull" }, "url": { "$ref": "#/$defs/stringOrNull" } From 287356ea4f786e9b1d2915199c18ce9580b34883 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 31 Aug 2026 10:30:17 -0400 Subject: [PATCH 5/7] docs: document children --space README gains a --space subsection under children (what depth 1 means, the null parent_id, the unknown-key error) and the space-key resolve row in the scope table now names children. CLAUDE.md records the design in the children and pagetree bullets, and corrects search's claim to being the only Flags().Changed caller in cmd/. --- CLAUDE.md | 6 +++--- README.md | 39 ++++++++++++++++++++++++++++++++++++--- docs/confluence/spaces.md | 6 ++++++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a4c7af7..d9277e0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,15 +53,15 @@ 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 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/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. +- `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, which `--json` never sees. - `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 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`, the only use of it in `cmd/`), 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 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/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/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 `AllDepths`. 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/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 `