diff --git a/CLAUDE.md b/CLAUDE.md index ae11603..eea05b8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,7 +67,7 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `cmd/userfind/` — `user-find` (#143): resolve a person's name to the account id a mention needs, via `client.SearchUsers`. The on-ramp #91 lacked — a mention round-tripped, but writing one from scratch meant leaving the editor for Confluence's person modal. **The Markdown line is the deliverable, not the id**: the id is what the tool needs, and the line is what the author needs, so it is the second line of every hit and a `--json` field rather than something a consumer assembles. It comes from `convert.MentionMarkdown`, exported for this, which is what makes the line an author pastes byte-identical *by construction* to the one `read`/`export` writes for the same mention — the alternative was a second copy of the `@` convention and of name escaping, and a name holding `[` produces a line that does not parse as a link without it. Output is a block per hit for `search`'s reason (the line is far too long for a column), and the `--limit` notice goes through `ui.Hint` to **stderr** rather than `search`'s `ui.Info` — `children --space`'s rule, with a stronger case: these lines exist to be redirected into a file or a clipboard, so stdout holds nothing but hits (`TestStdoutIsNothingButHits`). Four things about the route decide the rest, all in [docs/confluence/users.md](docs/confluence/users.md). It is **deliberately not `LookupUser`'s neighbour**: that route resolves *any* account and answers `Mark Reid (Deactivated)`, while this index cannot see a deactivated account at all and no `sitePermissionTypeFilter` value brings one back, so sharing code would hand the second's blind spot to the first. It **cannot go through `listV1`** — see `internal/client` below, the one finding that shaped the whole implementation. `totalSize` counts the current page rather than the result set, so `summary.truncated` is a flag derived from fetching one row more than asked, and nothing reads the field. And the match is **word-prefix and ordered** (`kahn` and `william kahn` find William Kahn-Greene; `ahn` and `kahn william` find nobody), which the `Long` states rather than papering over with wildcards, because the failure mode is a confident empty result and guessing at the author's intent would change which people a name finds. `--limit` is `search`'s string vocabulary and is not optional: a single-letter query matched 304 accounts on the instance it was built against. An empty NAME is refused locally, since the route answers one with a 500. No `type` field in `--json` though the row carries one — every account answers `known`, an automation account and a page-template account included, so it discriminates nothing. - `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. +- `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 published schema is open** (#200, `_plans/054`): no `additionalProperties: false` anywhere, because v0.1.0 has consumers (`markfluence-action`'s `results-json`) and a key the file closes is a key no later release can add without breaking whoever validates against the copy they hold. [docs/json-output.md](docs/json-output.md#compatibility) holds the rule — a new key, a new command, or a new `diff` `field` value is compatible (`field` is a plain string for that reason); a removed/renamed/retyped key, a newly nullable key, or a new value in any other enum bumps `schema_version`. The schema's own `description` only points there, so the rule has one copy. Opening the objects is also why every result-or-failure union is `anyOf`, not `oneOf`: `exportResult` declares every key `singleOpFailure` requires, so an open failed export row matches both, and `oneOf` refuses a row matching two. 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. - `internal/project` — the documentation root: `Discover` (walk up from a directory looking for `markfluence.yaml`), `FromPath` (`--root`), `Resolve`, and `Cache`, which consults itself at every level of the walk so a batch spanning a subtree pays for the walk — and `os.OpenRoot` — once rather than per directory (the quadratic cost `_plans/025` measured). A `Root` carries `Dir`, `File`, `Config` and an `os.Root` that refuses an escape even through a symlink partway down. `Discover` starts from a Markdown file's own directory, to bound its reads and name its attachments, which is why it returns a type rather than a string; nothing discovers a root from the working directory any more (that walk located `.env` until #188). Files in different projects have different roots, so a multi-root batch is allowed and nothing refuses it ([docs/root-model.md](docs/root-model.md)). `config.go` reads the project file's **settings** (#100): `space` and `page_width`, resolving **flag > frontmatter > project file**, which is *not* the credentials chain and must never be conflated with it. Three things about it are load-bearing. It is read through `frontmatter.Dialect.ReadMapping` rather than a second parser, since every rule there was found by probing goccy and a second copy would be a second set of the same bugs. An **unknown top-level key is fatal**, and that is the point rather than a cost — a silently ignored `spce: ENG` is wrong for every file at once, and a file written for a newer markfluence holds keys this binary would ignore, so there is no schema version and this must not be loosened; an empty or comment-only file stays valid, being what ships and what `export` plants. And loading happens in `open()`, the single place a `Root` is built from a marker hit, so `Discover`/`Cache`/`FromPath` cannot disagree that a file which cannot be understood **is not a valid marker**: the walk does not continue upward and does not fall back to the starting directory, because the root decides every attachment name and guessing at it is worse than stopping. `ConfigError`/`IsConfigError`/`RootError` exist so a caller reports that as a local defect (`VALIDATION`) rather than under `resolving the documentation root` as I/O. `pages.go` holds the **`pages:`** key (#139) and `SetPageEntry`, which records one file's metadata there: read-modify-write **once per page, not once per run**, because `create` writes each file's frontmatter as that page is published and a run that dies partway has to leave every already-created page recorded (D10). It **retries once** on a concurrent write, because the read-modify-write is not serialized and this file is shared by every page in the project: before the manifest each page's metadata went into its own file, so two concurrent `create`s could not collide, and they now can — A reads, B reads, A writes, B writes, and A's entry is gone while A's page exists. Optimistic rather than locked, matching `client.SetContentProperty`'s retry-once, since a lock file brings stale-lock handling for a verb a person invokes by hand. `beforeReplace` is a test hook for the give-up path, the `SetRetryLogger` arrangement. It also verifies twice — `frontmatter.SetNested` re-reads its own output, then `parseConfig` (split out of `loadConfig` for this) re-runs the *loader's* rules — since a tool that corrupts the file it is recording success in is the worst version of the feature; a test pins that a file which would not load afterwards is left byte-identical. A root with no project file refuses rather than creating one (#5's question, not a `create`'s to answer silently). An `Entry` is `{Fields, Lists}`, the same two maps `frontmatter.MarkdownFile` carries, which is the design rather than a convenience — `pagewidth` and `labels` both reach `client`, which holds a `*Cache`, so a typed validated entry would need a broken cycle or a second copy of every field's rules, and with two maps `labels.Declared(e.Lists, e.Fields)` works unchanged. `entryFields` is the manifest's schema and the only place it is written down; it mirrors frontmatter's fields deliberately, since adding one there and not here would make a field expressible in a file and not in an entry. Load checks **structure** — a mapping of mappings, legal paths, known field *names*, right shapes — and never a field's **value**, because #139 requires a semantically bad entry to be reported only when its file is one of the arguments, and this package has no idea which files the command was given. An unknown field *name* is the exception and is fatal at load, being the same typo class as an unknown setting. `NormalizePageKey` is lexical (L2 forbids a key whose meaning depends on the checkout's layout), and an escaping key or two keys normalizing to one are load-time errors naming both spellings. `Config.Pages` is **nil when there is no `pages:` key and empty-non-nil for `pages: {}`**, which is how a command tells "has not chosen the manifest" from "has, and has registered nothing". What it validates is **structure only**: `internal/pagewidth` cannot be imported here (`pagewidth` → `client` → `project`), so a width's vocabulary is checked by `pagewidth.Declared` where it already runs and by `check`'s offline lint. `Config` deliberately holds **no `url` or token**, and the reason is sharper than "those are credentials": basic auth goes to whatever host the resolved URL names, so a committed, walked-up file naming one would decide where `CONFLUENCE_TOKEN` is sent — a worse version of the walked-up `.env` hole #188 closed. - `internal/pagemeta` — `Resolve`, the one merge of a file's page metadata from the two places it may live: its own frontmatter and a `pages:` entry in the project file (#139). A package because `update`, `create`, `check` **and `internal/linkindex`** all need it, and a per-command copy is how two commands come to publish one file to two different pages. It imports `frontmatter` and `project` and nothing else, which is also why it validates no *value*: `pagewidth` and `labels` are unreachable from here, and the commands that need them already call them on the maps it returns. **Frontmatter and an entry are two spellings of one level, not two levels of a precedence chain** — when both speak the rule is not "the higher wins" but a grading: `page_id`/`space`/`parent` are coordinates and a disagreement fails the file (for `create`, the batch, since it preflights everything), while `title`/`page_width`/`labels` are visible and recoverable, so they warn and frontmatter wins. Agreement is silent, which is what makes migration incremental. Three things took a second pass and should not be flattened: `Source` (what `--json` reports as `metadata_source`) and `Managed` are computed from **different predicates** — the first answers "who contributed metadata", the second "should `update` act on this file", which is true when an entry exists or a `page_id` is named, so `a.md: {}` is a claim that must fail for want of an id rather than be skipped; contributed metadata is counted only over fields `project.IsPageField` knows, or a file carrying only keys markfluence *preserves but does not understand* (`reviewers:`, pinned by a frontmatter test) reads as claimed and a whole tree of them fails; and `labels` is compared as a **set**, since Confluence has no label order and a reordering cannot reach the page. A blank value is not a disagreement — every null spelling already reads as `""`. `KeyFor` is the one place a file's path becomes a manifest key, used on both sides because a mismatch is a silent skip rather than an error. `Origin` is the same question per field — which location supplied *this* value — recorded as `Resolve` grades them, because `diff` reports it beside every frontmatter difference and "the title differs" is otherwise ambiguous about which file to edit; unlike `MetadataSource()` it does **not** collapse `FromBoth`, which is the most useful of the three there (correcting a field two locations supply means editing two files). Computed here rather than by the caller for the reason the package exists: a second copy of the precedence rules is a second copy whatever it is used for. - `internal/actionlog` — the append-only record of what markfluence published, one log per project root at `/.markfluence/log.jsonl` (#149). `create`, `update` and `export` each append a line as a page completes, carrying the page version they left behind and `Sum`'s hash of what the body `PUT` sent; the last successful line for a file is that copy's **merge base**. It exists because nothing else can tell "the page differs because I have edits" from "the page differs because somebody published first" — that needs what *this copy* was derived from, which is a per-copy fact no page-side state can hold. Three things about it are load-bearing. **It is not committed**, and the reason is structural: a shared repository is itself a declaration that the repository is the source of truth, which is the arrangement where `update --force` is the answer and no base is consulted — so the log serves a local copy with the source of truth in Confluence, where per-checkout state is the right shape. The directory ignores itself (a planted `.gitignore` holding `*`, never overwritten) rather than editing a `.gitignore` markfluence does not own. **Nothing in it may fail a command**: a missing, unreadable, corrupt or half-written log degrades the check that reads it and never the run, which is why `read` skips a line it cannot parse instead of erroring and why a failed `Append` is a warning at the call site — the page is published by then, so failing would report that it was not. And **`Sum` covers exactly what the body `PUT` sends**, the resolved title and the rendered body: the title because `convert.ConfluencePage` carries none (a render-only hash would skip the publish of a file whose only change was its title), and *not* page width, labels or attachments, each of which has its own pass that runs whether or not the body is republished — folding them in would bump the page version for a change that never touched the body. Its parts are length-prefixed, which is part of the persisted format: changing the framing invalidates every recorded base. A root with `File == ""` gets no log at all (`For` returns nil, and every method is nil-safe), matching #139's rule that a root with no project file refuses rather than creating one. `Cache` hands out one `Log` per root so a batch reads each log once, and a batch spanning roots writes to several. `update` reads it for **two orthogonal checks, both only when `--force` is absent** (`--force` means always PUT, and no logic may suppress the request): the logged **`page_version`** against the live one decides **divergence** and *refuses* the file with `CodeConflict` — deliberately not qualified by the sha, since the case where you have no local edits is the worse one, publishing their work away with the bytes they started from — and the logged **`publish_sha256`** against this run's decides **idempotence** and skips the *body* `PUT` alone, leaving the attachment, width and label passes to run. An entry naming a different `page_id` is discarded rather than compared (a retarget would otherwise read as "the page moved 40 versions"), and the two fields degrade independently, so an `export` line written before its sha pass still refuses a moved page. A body-unchanged skip **records a line too**, which is load-bearing: once the sha does the skipping most runs skip, and a publish-only log would never keep a base current in a tree that is already published. Principles: **S8** (`no-overwrite-of-a-moved-page`), decided by the logged page version, and **L4** (`publish-is-idempotent`), decided by the publish sha rather than by mtime; both accept that a file with no base, or a root with no project file, gets neither. @@ -84,7 +84,7 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `internal/pagestatus` — the `page_status` frontmatter field (#168): the coloured lozenge beside a page title, which Atlassian's API calls a content *state*. Named for the field rather than the route because the UI calls it a status and frontmatter already speaks of `page_id`/`page_width` — and because **three things wear the word**: a page's content *status* is `current`/`archived`/`trashed` (`page-info`'s `content_status` row, renamed from `status`/`page_status` by #168), and `markfluence status` is #148. `Declared` is pure and offline, which is what lets `check` use it; `Resolve` turns a name into the status to write; `Apply`/`Read` orchestrate the client. There is deliberately **no cache** — see below. Three findings shape everything, all measured ([docs/confluence/page-status.md](docs/confluence/page-status.md)). **A name is never sent to Confluence** — the wire format is an id, because the body's `id` is *optional* and a `PUT` naming a status the server does not recognise **creates** one, 200, that no API route can delete (`DELETE /rest/api/content-states/{id}` 404s, the collection 405s). Writing only the id makes that unreachable rather than something validation must remember; a 400 demanding a `color` is the tell for a request that lost its id. **The vocabulary is per `(caller, page)`, which is the finding everything else bends around** — not per space, though it took a second account to see it: one account was offered four statuses on a page it had created and three on a page in the same space that it had not, and the write enforces the difference (`User is not permitted to use this ContentState on this content.`). So the page to ask is *the page the status is going on*, **nothing caches one page's answer for another** (there is no vocabulary cache, and adding one keyed by space is the mistake to avoid), and every message says "this page can be given …" rather than "this space offers …", which would send an author to space settings for a difference that is not there. `update` asks the page it is publishing to. **`create` cannot validate up front at all** and resolves the name *after* the page exists, since the only authoritative page is the one it has not made yet — knowingly paying #127's cost for this one field (a misspelled status is a warning on a created page) because the alternative refuses files whose status is fine: probing the parent or the space homepage was measured refusing `Verified` for a page that then accepted it, and `state/available` needs **edit** permission on whatever page it is asked of, which a collaborator who can create pages often lacks on the homepage. The space-scoped routes stay the broken ones: `GET /space/{key}/state` returns the four product defaults whatever the caller may use, and `state/settings`, which is correct, 403s for anyone who is not a space admin. Only `spaceContentStates` are valid; `customContentStates` follow the **account** (a status created in a personal space showed up as available on `AGILE` pages), so accepting them would make a committed file publish for its author and fail for a colleague — they are carried only so the refusal can say why, which is otherwise an unresolvable "it works for me". **A state write bumps the page version** (`minorEdit: false`, empty message), unlike a label write, so `Apply` reads and compares before writing and every request is gated on the field being *declared* — an absent `page_status` makes no state request at all, read included. The match is **case-insensitive with nothing rewritten**, which departs from `labels`' warn-and-lowercase for a precise reason: labels sends the author's string, this sends an id, so a case variant cannot reach the page and is not worth a warning (two of the space's own statuses differing only in case are refused as ambiguous). There is **no spelling that clears a status** — every null form of a scalar reads as `""`, indistinguishable from an unfinished edit, so an empty value is a validation failure; `labels: []` can mean "remove them all" only because a sequence has an empty spelling a scalar lacks. And no project-level default, unlike `page_width`: a width is house style, a status is a claim about one page. Principle: **L9**, whose Accepts records that clear-asymmetry. - `internal/pagewidth` — the `page_width` `Width` enum (`narrow`/`wide`/`max`, default `max`), `Declared`, the vocab↔content-property maps, `WidthFromProperties`, and `Apply`/`Read` against the client. Width lives in two content properties and **both** must be written or the reader and the editor disagree: [docs/confluence/page-width.md](docs/confluence/page-width.md). - `internal/testenv` — `RunIsolated`, which every package whose code calls `client.Resolve` (and `internal/client` itself) calls from `TestMain`: it points `HOME`/`XDG_CONFIG_HOME` at an empty temp directory and unsets `CONFLUENCE_*`, so a developer's real credentials file or an exported cloud ID cannot reach a test. A package of its own, importing nothing from this module, because `internal/client`'s tests are `package client` and `internal/clienttest` imports `client`; it repeats the four variable names for the same reason. Called for packages whose tests do not reach `Resolve` yet, so a later test cannot forget it. -- `internal/schematest` — the `--json` drift guard, and the reason the schema can't fall behind the code. `ValidateEnvelope`/`ValidateError` validate an emitted document against the embedded schema; `document.go` checks the schema *document* instead (`Commands`, plus tests that every name in the `command` enum has an `if/then` branch that constrains `results.items` **and** `summary`). That last one matters because outside a branch the schema says only "results is an array": a command added to the enum without a branch is completely unvalidated, and adding just the enum entry is exactly how a new command's conformance test goes green. `cmd`'s `TestCommandEnumMatchesRegisteredCommands` closes the loop from the other side — every registered subcommand is in the enum or in that test's `noJSONEnvelope` list. Two rules keep all this working: **every result field lives on a typed struct and nothing uses `omitempty`** (so every field always marshals and `additionalProperties:false`/`required` catch an added, renamed, or removed one no matter what a fixture sets — never build a result as a `map[string]any`), and **a conformance test builds its document with the command's own builder** (`failEnvelope`, `jsonResult`) rather than a hand-copied literal, or it validates a copy while the real output drifts. +- `internal/schematest` — the `--json` drift guard, and the reason the schema can't fall behind the code. `ValidateEnvelope`/`ValidateError` validate an emitted document against the embedded schema **closed in memory** (`closeObjects`), which sets `additionalProperties: false` on every node declaring `"type": "object"` with `properties` — the published file is open for consumers, and the guard needs the opposite. The `type` test is what keeps it off the envelope's `if`/`then` branches, which declare none: a closed `then` would forbid every envelope key but `results`/`summary`. Three tests keep this honest: `TestPublishedSchemaIsOpen` keeps the file open, `TestEveryPropertiesNodeIsTyped` keeps every other node listing properties typed (an untyped one would silently escape the guard), and the walker skips instance data (`const`, `enum`...) and steps *through* name-keyed maps (`properties`, `$defs`) — `infoResult` has a property literally named `properties`; `document.go` checks the schema *document* instead (`Commands`, plus tests that every name in the `command` enum has an `if/then` branch that constrains `results.items` **and** `summary`). That last one matters because outside a branch the schema says only "results is an array": a command added to the enum without a branch is completely unvalidated, and adding just the enum entry is exactly how a new command's conformance test goes green. `cmd`'s `TestCommandEnumMatchesRegisteredCommands` closes the loop from the other side — every registered subcommand is in the enum or in that test's `noJSONEnvelope` list. Two rules keep all this working: **every result field lives on a typed struct and nothing uses `omitempty`** (so every field always marshals and the closed schema/`required` catch an added, renamed, or removed one no matter what a fixture sets — never build a result as a `map[string]any`), and **a conformance test builds its document with the command's own builder** (`failEnvelope`, `jsonResult`) rather than a hand-copied literal, or it validates a copy while the real output drifts. - `internal/completion` — the shell-completion functions the commands share (`MarkdownFiles` for a FILE/PAGE argument, `PageThenFiles`/`PageThenNames`, `Directories`, `Values`, and `RegisterFlag`). Cobra's built-in `completion` command generates the scripts; this only decides what they offer. Nothing here may call Confluence — completion runs on every keystroke — so a server-side value (an attachment name) completes to nothing instead of stalling the shell. A subcommand with no `ValidArgsFunction` fails `TestSubcommandsCompleteArgs` in `cmd`. - `tools/gendocs` — the only thing outside `cmd`/`internal`/`schema`: a `main` that renders `docs/commands/`. Not built into the binary and not run by it; see **Documentation** above. - `internal/buildinfo` — `Version` (set via ldflags), `CommitDate` (from the `vcs.time` build setting), and `Stamp`. diff --git a/_plans/054_open-json-schema.md b/_plans/054_open-json-schema.md new file mode 100644 index 0000000..3056225 --- /dev/null +++ b/_plans/054_open-json-schema.md @@ -0,0 +1,109 @@ +# 054: adding a key is a compatible schema change + +Answers #200. v0.1.0 is released and `--json` has consumers: +`mozilla/markfluence-action` exposes the envelope to later workflow steps as +its `results-json` output. `docs/json-output.md` says a change that breaks +compatibility increases `schema_version`, but not what breaks it. And the +published schema sets `additionalProperties: false` on every object, so a +consumer validating against a copy of the v0.1.0 schema rejects any new key: +adding a field is breaking in practice. #10 needs one (`update`'s `moved`), so +this comes first. + +## What the survey established + +- `additionalProperties: false` appears 48 times, always on a node with + `"type": "object"` and `properties`, and every such node has it. So "close + every typed object that lists properties" reproduces the current schema + exactly. The `if`/`then` branches carry no `type`, and neither does the + envelope's per-command `then`, so neither is closed; they constrain + `results` and `summary`, which the envelope's own `properties` declare. +- **Opening the objects breaks three unions.** Eight `results.items` are a + `oneOf` of a command's result and `singleOpFailure`. Five tell them apart by + `ok: const true` against `ok: const false`. The other three + (`exportResult`, `attachmentUploadResult`, `attachmentDownloadResult`) + carry `ok: boolean` and rely on closed objects to be exclusive. + `exportResult` declares every key `singleOpFailure` requires, so with open + objects a failed export row matches both, and `oneOf` fails. The two + attachment results lack `page_id` only, so they would become ambiguous the + day one gains it. + +## Decisions + +**D1. The compatibility rule, in `docs/json-output.md`.** + +- *Compatible*, and `schema_version` stays: a new key on the envelope, a + result, a summary, or the error object; a new value in an enum the schema + documents as open (none today); loosening a constraint. +- *Breaking*, and `schema_version` goes up: removing or renaming a key; + changing a key's type, or what it means; making a key nullable that was + not; a new value in a closed enum (`code`, `status`, `command`...). +- Consumers must ignore keys they do not know, and should validate against + the schema that `markfluence schema` prints, which is the running binary's. + +A new `command` enum value counts as breaking for the rule's sake, since a +consumer switching on `command` meets a value it has no case for. In practice +a new command is new output nobody consumed before; that is judged when it +happens, not here. + +**D2. The published schema is open.** Remove every `additionalProperties: +false` from `schema/json-output/v1.json`. Every document valid before stays +valid, so this is itself compatible. + +**D3. The unions become `anyOf`.** All eight result-or-failure unions change +from `oneOf` to `anyOf`. `anyOf` is looser than `oneOf`, so this is +compatible too, and it is what an open schema needs: "matches at least one +shape" is the claim a consumer can rely on. The scalar unions +(`stringOrNull`, `codeOrNull`) stay `oneOf`; their branches cannot overlap. + +**D4. The tests close it again.** `internal/schematest` compiles a *closed* +copy: it walks the embedded schema and sets `additionalProperties: false` on +every node with `"type": "object"` and `properties` that does not already say +otherwise. `ValidateEnvelope`/`ValidateError` validate against that, so the +drift guard is unchanged: a key the code emits and the schema does not list +still fails. The closing lives in one exported function (`Closed`) so a test +can check it directly. + +**D5. The embedded schema is still what ships.** `markfluence schema` prints +the open document, byte for byte the file. The closing happens in memory, in +test code only. + +## Implementation + +- Before editing the schema, a one-off check: strip every + `additionalProperties: false` from the current file, run `Closed` on the + result, and compare with the original as parsed JSON. They must be equal. + That is the proof that D4 restores exactly what D2 removes; the + permanent tests below keep it true. +- Then the schema edit, done by a script (strip the key; `oneOf` → `anyOf` + in the eight unions), with the formatting kept. + +## Files + +| file | change | +|---|---| +| `schema/json-output/v1.json` | D2, D3; the top-level `description` says the schema is open and why | +| `internal/schematest/schematest.go` | D4: `Closed`, compile the closed copy; package doc | +| `internal/schematest/document_test.go` (or a new test file) | tests below | +| `docs/json-output.md` | D1 | +| `CLAUDE.md` | the `schematest` and `schema/` bullets | +| code comments naming `additionalProperties:false` (`cmd/diff`, `cmd/search`, `cmd/userfind`, `cmd/spaceinfo`, `internal/jsonout`) | say "the closed schema the tests validate against" | + +## Tests + +- The published schema contains no `additionalProperties: false`. +- `Closed` closes every typed object that lists properties, and nothing else + (the `if`/`then` nodes stay open). +- A document with an extra key on a result fails `ValidateEnvelope` and passes + the published schema. That pins both halves: the drift guard, and the + consumer promise. +- A failed export row validates against the published schema (the union + ambiguity D3 fixes). It fails today with objects opened and `oneOf` kept. +- Every existing conformance test passes unchanged. + +## Not in scope + +- **A test comparing against the released schema** (fetching v0.1.0's file + and checking no key was removed or retyped). `required` already catches a + removed key from our side; a check across releases waits until a breaking + change slips through. +- **A `schema_version` bump.** Nothing here breaks a consumer. diff --git a/cmd/diff/json.go b/cmd/diff/json.go index 92e0a59..c77ac50 100644 --- a/cmd/diff/json.go +++ b/cmd/diff/json.go @@ -11,8 +11,8 @@ import ( // diffResult is the --json result for one file. // // Every field is on this struct and nothing uses omitempty, so every one always -// marshals and the schema's additionalProperties:false / required catch an -// added, renamed or removed field whatever a fixture sets. +// marshals and the closed schema the tests validate against (schematest.Closed) +// and required catch an added, renamed or removed field whatever a fixture sets. type diffResult struct { OK bool `json:"ok"` File string `json:"file"` diff --git a/cmd/schema/schema.go b/cmd/schema/schema.go index 87a290a..fd99d0c 100644 --- a/cmd/schema/schema.go +++ b/cmd/schema/schema.go @@ -18,8 +18,13 @@ var Cmd = &cobra.Command{ "Thus a script, a CI job, or an agent can get the contract from the binary, and\n"+ "not from the repository.\n\n"+ "The build puts the schema into the binary. It describes schema_version %d,\n"+ - "which is the version that this binary writes. The schema command, and the tests\n"+ - "that check real --json output, read the same copy.\n\n"+ + "which is the version that this binary writes. Validate against this copy, and\n"+ + "not against a copy from another release.\n\n"+ + "The schema is open: an object can have keys that the schema does not list. A\n"+ + "later release can add a key and keep the same schema_version, so a consumer must\n"+ + "ignore a key that it does not know. A change that can break a consumer, such as\n"+ + "a key that is removed or renamed, increases schema_version. docs/json-output.md\n"+ + "has the whole rule.\n\n"+ "The output is the schema document itself, so --json has no effect here.", jsonout.SchemaVersion), Example: " # Save the schema\n" + diff --git a/cmd/search/json.go b/cmd/search/json.go index 1a1d54e..73e2ed4 100644 --- a/cmd/search/json.go +++ b/cmd/search/json.go @@ -28,8 +28,9 @@ type jsonSearchResult struct { // jsonSearchSummary is search's summary. // -// basicSummary cannot be reused: it is additionalProperties:false, and both -// fields below are load-bearing. +// basicSummary cannot be reused: it lists neither field, the closed schema the +// tests validate against refuses an unlisted key, and both fields below are +// load-bearing. // // truncated is a flag rather than a count of what was dropped, because there is // no trustworthy count to report -- totalSize drifted 294/292/291 against 289 diff --git a/cmd/spaceinfo/spaceinfo_test.go b/cmd/spaceinfo/spaceinfo_test.go index 32a23af..b69587f 100644 --- a/cmd/spaceinfo/spaceinfo_test.go +++ b/cmd/spaceinfo/spaceinfo_test.go @@ -548,7 +548,7 @@ func TestUnknownSpaceIsNil(t *testing.T) { } // Every field always marshals, per internal/schematest's rule: no omitempty, -// so additionalProperties:false and required catch an added or renamed one. +// so the tests' closed schema and required catch an added or renamed one. func TestJSONResultAlwaysCarriesEveryField(t *testing.T) { r := stub{pages: [][]string{{row("1", "current", "", 30, 30)}}}.build(t, 7) blob, err := json.Marshal(r.jsonResult()) diff --git a/cmd/userfind/json.go b/cmd/userfind/json.go index cdb1750..a4d1091 100644 --- a/cmd/userfind/json.go +++ b/cmd/userfind/json.go @@ -31,8 +31,9 @@ type jsonUserResult struct { // jsonUserSummary is user-find's summary. // -// basicSummary cannot be reused: it is additionalProperties:false, and -// truncated is load-bearing. It is a flag rather than a count for a sharper +// basicSummary cannot be reused: it does not list truncated, the closed schema +// the tests validate against refuses an unlisted key, and truncated is +// load-bearing. It is a flag rather than a count for a sharper // reason than search's drift -- this route's totalSize reports the rows on the // current page, so it cannot describe the result set at all. type jsonUserSummary struct { diff --git a/docs/commands/markfluence_schema.md b/docs/commands/markfluence_schema.md index b3452cb..d2ab217 100644 --- a/docs/commands/markfluence_schema.md +++ b/docs/commands/markfluence_schema.md @@ -9,8 +9,14 @@ Thus a script, a CI job, or an agent can get the contract from the binary, and not from the repository. The build puts the schema into the binary. It describes schema_version 1, -which is the version that this binary writes. The schema command, and the tests -that check real --json output, read the same copy. +which is the version that this binary writes. Validate against this copy, and +not against a copy from another release. + +The schema is open: an object can have keys that the schema does not list. A +later release can add a key and keep the same schema_version, so a consumer must +ignore a key that it does not know. A change that can break a consumer, such as +a key that is removed or renamed, increases schema_version. docs/json-output.md +has the whole rule. The output is the schema document itself, so --json has no effect here. diff --git a/docs/json-output.md b/docs/json-output.md index 1d3aec8..e484561 100644 --- a/docs/json-output.md +++ b/docs/json-output.md @@ -10,12 +10,51 @@ The JSON Schema at [`schema/json-output/v1.json`](../schema/json-output/v1.json) is the authoritative contract for each field. The `markfluence schema` command also prints it. +## Compatibility + +`schema_version` is `1`. It increases only for a change that can break a +consumer. + +These changes are compatible, and `schema_version` stays the same: + +- a new key on the envelope, on a result, on a summary, or on the error + object; +- a constraint that is made looser; + +- a new command, which adds a value to the `command` enum. Only the new + command's own output carries that value, so no consumer of an existing + command sees it; +- a new value of `field` in a `diff` result. It is a string rather than an + enum for this reason: a frontmatter field that markfluence learns later is + reported there too. + +These changes break compatibility, and `schema_version` increases: + +- a key that is removed or renamed; +- a key whose type or meaning changes; +- a key that can be `null` and could not before; +- a new value in any other enum, such as `code` or a result's `status`. A + consumer can reasonably handle each value of those, and a new one would + reach a consumer of an existing command. + +Thus a consumer must ignore a key that it does not know. The published schema +is open for this reason: an object can have keys that the schema does not list. +If you validate the output, validate against the schema that +`markfluence schema` prints, which is the schema of the binary that you run. +A copy from an older release does not list the newer keys, but it still +accepts them. + +markfluence's own tests validate against a closed copy of the schema, which +refuses any key that the schema does not list. Thus a key cannot get into the +output before it gets into the schema. + ## Notes on the schema - **Stable for each command.** Each command always writes the same keys in the same shapes. An empty value is `null` or `[]`. The *set* of keys is different for each command. Any change that breaks compatibility increases - `schema_version`. + `schema_version`. See [Compatibility](#compatibility) for which changes + those are. - **`roots`** lists each different [documentation root](../README.md#the-documentation-root) that the command resolved, in sorted order. It is `[]` for a command that has no per-file root, diff --git a/internal/jsonout/types.go b/internal/jsonout/types.go index 66481b0..83abaf5 100644 --- a/internal/jsonout/types.go +++ b/internal/jsonout/types.go @@ -84,8 +84,8 @@ type AttachmentActionResult struct { // #/$defs/singleOpFailure in the schema. // // It is a struct rather than the map each command used to build inline because -// every field then marshals unconditionally, which is what lets the schema's -// additionalProperties:false and required catch a renamed or added key. A map +// every field then marshals unconditionally, which is what lets the closed +// schema the tests validate against, and required, catch a renamed or added key. A map // only carries the keys the caller remembered to set, so drift in one showed up // nowhere. type SingleOpFailure struct { diff --git a/internal/schematest/closed_test.go b/internal/schematest/closed_test.go new file mode 100644 index 0000000..42ae146 --- /dev/null +++ b/internal/schematest/closed_test.go @@ -0,0 +1,125 @@ +package schematest + +import ( + "strings" + "testing" + + "github.com/mozilla/markfluence/schema" + "github.com/santhosh-tekuri/jsonschema/v6" +) + +func decoded(t *testing.T) any { + t.Helper() + doc, err := jsonschema.UnmarshalJSON(strings.NewReader(schema.V1)) + if err != nil { + t.Fatal(err) + } + return doc +} + +// TestPublishedSchemaIsOpen pins the consumer half of #200: an object the +// published file closes is one no later release may add a key to without +// breaking whoever validates against the copy they have. Any other +// additionalProperties (a schema for a map's values, say) is fine. +func TestPublishedSchemaIsOpen(t *testing.T) { + eachSchema(decoded(t), "#", func(path string, node map[string]any) { + if node["additionalProperties"] == false { + t.Errorf("%s sets additionalProperties: false; the published schema must stay open "+ + "(the tests close it: see closeObjects)", path) + } + }) +} + +// TestEveryPropertiesNodeIsTyped keeps the closing complete. closeObjects +// closes only a node declaring "type": "object", so a nested object written +// without a type would be left open, and a stray key on it would pass the +// drift guard with every test green. The one legitimate untyped node listing +// properties is an envelope if/then branch. +func TestEveryPropertiesNodeIsTyped(t *testing.T) { + eachSchema(decoded(t), "#", func(path string, node map[string]any) { + if _, ok := node["properties"]; !ok || isObjectSchema(node) { + return + } + if strings.HasPrefix(path, "#/allOf/") && + (strings.HasSuffix(path, "/if") || strings.HasSuffix(path, "/then")) { + return + } + t.Errorf(`%s lists properties without "type": "object", so the drift guard leaves it open`, path) + }) +} + +// TestCloseObjectsClosesObjectSchemasOnly: every object schema is closed, and +// nothing else is -- the if/then branches least of all. +func TestCloseObjectsClosesObjectSchemasOnly(t *testing.T) { + doc := decoded(t) + closeObjects(doc) + closed := 0 + eachSchema(doc, "#", func(path string, node map[string]any) { + ap, said := node["additionalProperties"] + switch { + case isObjectSchema(node): + if ap != false { + t.Errorf("%s: object schema not closed", path) + } + closed++ + case said: + t.Errorf("%s: closed, but it is not an object schema", path) + } + }) + if closed == 0 { + t.Fatal("closed nothing") + } +} + +// TestCloseObjectsLeavesInstanceDataAlone: a const is a value, and closing an +// object-shaped one would change what it requires. +func TestCloseObjectsLeavesInstanceDataAlone(t *testing.T) { + doc := map[string]any{ + "type": "object", + "properties": map[string]any{"x": map[string]any{}}, + "const": map[string]any{"type": "object", "properties": map[string]any{}}, + } + closeObjects(doc) + if doc["additionalProperties"] != false { + t.Error("the schema itself was not closed") + } + if _, said := doc["const"].(map[string]any)["additionalProperties"]; said { + t.Error("closeObjects wrote into a const") + } +} + +// exportEnvelope is a valid export envelope whose one row is a failed export, +// with extra added to that row. +func exportEnvelope(extra string) []byte { + return []byte(`{"schema_version": 1, "markfluence_version": "dev", "command": "export", + "roots": [], "warnings": [], + "results": [{"ok": false, "page_id": "123", "title": "", "space": "", "parent": null, + "parent_type": null, "parent_file": null, "dry_run": false, "status": "", + "dest_path": null, "attachments": [], "warnings": [], + "error": "boom", "code": "NETWORK"` + extra + `}], + "summary": {"total": 1, "succeeded": 0, "failed": 1, "skipped": 0, "project_file": null}}`) +} + +// TestAnUnlistedKeyIsCompatible pins both halves at once: the drift guard +// refuses a key the schema does not list, and the published schema accepts +// it, which is what makes adding a key a compatible change. +func TestAnUnlistedKeyIsCompatible(t *testing.T) { + doc := exportEnvelope(`, "brand_new": 1`) + if err := check(t, compile(t, true).envelope, doc); err == nil { + t.Error("the closed schema accepted an unlisted key; the drift guard is off") + } + if err := check(t, compile(t, false).envelope, doc); err != nil { + t.Errorf("the published schema refused an unlisted key: %v", err) + } +} + +// TestFailedExportRowIsValid is the union #200 found. A failed export row has +// every key singleOpFailure requires, so once objects are open it matches +// both shapes, and a oneOf refuses a row that matches two. +func TestFailedExportRowIsValid(t *testing.T) { + doc := exportEnvelope("") + if err := check(t, compile(t, false).envelope, doc); err != nil { + t.Errorf("published schema: %v", err) + } + ValidateEnvelope(t, doc) +} diff --git a/internal/schematest/schematest.go b/internal/schematest/schematest.go index 20f5133..1004db2 100644 --- a/internal/schematest/schematest.go +++ b/internal/schematest/schematest.go @@ -1,15 +1,20 @@ // Package schematest is a test-only helper that validates markfluence's --json // output against the published JSON Schema (schema/json-output/v1.json). It is -// the drift guard: because the schema uses additionalProperties:false -// throughout, any new field on a result struct fails validation until the schema -// is updated to match. +// the drift guard: any new field on a result struct fails validation until the +// schema is updated to match. // // It validates against the copy embedded in package schema -- the same bytes -// `markfluence schema` prints -- so what ships is what these tests checked. +// `markfluence schema` prints -- so what ships is what these tests checked. But +// not as published: the published schema is open, because a new key is a +// compatible change and a consumer holding an older copy must not reject one +// (#200, docs/json-output.md). The drift guard needs the opposite, so the +// tests validate against a closed copy (closeObjects), which forbids every key +// the schema does not list. package schematest import ( "bytes" + "strconv" "strings" "sync" "testing" @@ -20,63 +25,140 @@ import ( const schemaID = "https://github.com/mozilla/markfluence/schema/json-output/v1.json" -var ( - once sync.Once - envelope *jsonschema.Schema - errObject *jsonschema.Schema - compileErr error -) - -func compile() { - doc, err := jsonschema.UnmarshalJSON(strings.NewReader(schema.V1)) - if err != nil { - compileErr = err - return - } - c := jsonschema.NewCompiler() - if err := c.AddResource(schemaID, doc); err != nil { - compileErr = err - return - } - if envelope, err = c.Compile(schemaID); err != nil { - compileErr = err - return - } - errObject, err = c.Compile(schemaID + "#/$defs/errorObject") - compileErr = err +// compiled is the schema compiled one way: open (as published) or closed (as +// the drift guard validates). Each is compiled once per test binary. +type compiled struct { + once sync.Once + envelope *jsonschema.Schema + errObject *jsonschema.Schema + err error } -func load(t *testing.T) { +var openSchema, closedSchema compiled + +// compile builds the envelope and error-object schemas from the embedded +// document, closed first when closed is set. The one path both variants take, +// so the test that checks the published schema and the drift guard cannot come +// to disagree about anything but the closing. +func compile(t *testing.T, closed bool) *compiled { t.Helper() - once.Do(compile) - if compileErr != nil { - t.Fatalf("compiling JSON Schema: %v", compileErr) + c := &openSchema + if closed { + c = &closedSchema + } + c.once.Do(func() { + doc, err := jsonschema.UnmarshalJSON(strings.NewReader(schema.V1)) + if err != nil { + c.err = err + return + } + if closed { + closeObjects(doc) + } + comp := jsonschema.NewCompiler() + if err := comp.AddResource(schemaID, doc); err != nil { + c.err = err + return + } + if c.envelope, err = comp.Compile(schemaID); err != nil { + c.err = err + return + } + c.errObject, c.err = comp.Compile(schemaID + "#/$defs/errorObject") + }) + if c.err != nil { + t.Fatalf("compiling JSON Schema: %v", c.err) } + return c } // ValidateEnvelope fails the test if instance (a marshaled --json stdout -// document) does not conform to the envelope schema. +// document) does not conform to the envelope schema, closed. func ValidateEnvelope(t *testing.T, instance []byte) { t.Helper() - load(t) - validate(t, envelope, instance) + validate(t, compile(t, true).envelope, instance) } // ValidateError fails the test if instance (a marshaled stderr error object) -// does not conform to #/$defs/errorObject. +// does not conform to #/$defs/errorObject, closed. func ValidateError(t *testing.T, instance []byte) { t.Helper() - load(t) - validate(t, errObject, instance) + validate(t, compile(t, true).errObject, instance) } func validate(t *testing.T, sch *jsonschema.Schema, instance []byte) { + t.Helper() + if err := check(t, sch, instance); err != nil { + t.Errorf("instance does not conform to schema:\n%v\n--- instance ---\n%s", err, instance) + } +} + +// check validates instance against sch and returns the result. +func check(t *testing.T, sch *jsonschema.Schema, instance []byte) error { t.Helper() v, err := jsonschema.UnmarshalJSON(bytes.NewReader(instance)) if err != nil { t.Fatalf("instance is not valid JSON: %v\n%s", err, instance) } - if err := sch.Validate(v); err != nil { - t.Errorf("instance does not conform to schema:\n%v\n--- instance ---\n%s", err, instance) + return sch.Validate(v) +} + +// instanceKeywords hold instance data rather than subschemas. A walk looking +// for schemas must not descend into them: a const that happened to be an +// object with "type" and "properties" keys is data, and closing it would +// change the value it requires. +var instanceKeywords = map[string]bool{"const": true, "enum": true, "default": true, "examples": true} + +// namedSchemas hold a map from a name to a schema rather than a schema. The +// map itself is not visited as one: infoResult has a property called +// "properties", and read as a schema its properties map lists properties. +var namedSchemas = map[string]bool{"properties": true, "patternProperties": true, "$defs": true} + +// eachSchema calls f on every subschema of doc -- a schema decoded by +// jsonschema.UnmarshalJSON -- with its JSON pointer, skipping instance data. +func eachSchema(doc any, path string, f func(path string, node map[string]any)) { + switch n := doc.(type) { + case map[string]any: + f(path, n) + for k, v := range n { + switch { + case instanceKeywords[k]: + case namedSchemas[k]: + if m, ok := v.(map[string]any); ok { + for name, sub := range m { + eachSchema(sub, path+"/"+k+"/"+name, f) + } + } + default: + eachSchema(v, path+"/"+k, f) + } + } + case []any: + for i, v := range n { + eachSchema(v, path+"/"+strconv.Itoa(i), f) + } } } + +// isObjectSchema reports whether node is a schema the drift guard closes: one +// declaring "type": "object" and listing its properties. +// +// The type is what keeps the closing off the envelope's if/then branches: an +// if closed this way would stop matching any real envelope, and a then closed +// this way would forbid every envelope key but results and summary. Neither +// declares a type. TestEveryPropertiesNodeIsTyped keeps every other node that +// lists properties typed, so nothing is left open by omission. +func isObjectSchema(node map[string]any) bool { + _, hasProps := node["properties"] + return hasProps && node["type"] == "object" +} + +// closeObjects sets additionalProperties to false, in place, on every object +// schema in doc that does not already say something about it. +func closeObjects(doc any) { + eachSchema(doc, "#", func(_ string, node map[string]any) { + if _, said := node["additionalProperties"]; !said && isObjectSchema(node) { + node["additionalProperties"] = false + } + }) +} diff --git a/schema/json-output/v1.json b/schema/json-output/v1.json index 24cf134..8f0fc01 100644 --- a/schema/json-output/v1.json +++ b/schema/json-output/v1.json @@ -2,14 +2,13 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://github.com/mozilla/markfluence/schema/json-output/v1.json", "title": "markfluence --json output (schema_version 1)", - "description": "The single JSON document markfluence writes to stdout under --json. results holds one object per target (a single element for page-info/read; one per attachment for attachment-list); its item shape and the summary shape depend on command. The typed error object written to stderr on a fatal/pre-flight failure conforms to #/$defs/errorObject.", + "description": "The single JSON document markfluence writes to stdout under --json. results holds one object per target (a single element for page-info/read; one per attachment for attachment-list); its item shape and the summary shape depend on command. The typed error object written to stderr on a fatal/pre-flight failure conforms to #/$defs/errorObject. The schema is open: an object may carry keys it does not list, and a consumer must ignore keys it does not know. Which changes keep schema_version and which increase it: https://github.com/mozilla/markfluence/blob/main/docs/json-output.md#compatibility", "type": "object", - "additionalProperties": false, "required": ["schema_version", "markfluence_version", "command", "roots", "warnings", "results", "summary"], "properties": { "schema_version": { "const": 1 }, "markfluence_version": { "type": "string" }, - "command": { "enum": ["page-info", "space-info", "user-info", "read", "update", "create", "check", "diff", "children", "find", "search", "user-find", "attachment-list", "attachment-upload", "attachment-download", "export"] }, + "command": { "description": "A new command adds a value here. That is a compatible change: only the new command's own output carries it.", "enum": ["page-info", "space-info", "user-info", "read", "update", "create", "check", "diff", "children", "find", "search", "user-find", "attachment-list", "attachment-upload", "attachment-download", "export"] }, "roots": { "type": "array", "items": { "type": "string" }, "description": "Every distinct documentation root the command resolved, sorted. Empty for a command with no per-file root concept, and for a pre-flight failure that never reached root resolution." }, "warnings": { "type": "array", "items": { "type": "string" }, "description": "Warnings about the invocation itself rather than about any page or file -- currently the two warnings raised while reading credentials: a file holding the API token (the credentials file, or one named by --env-file) that is reachable by anyone but its owner, and a CONFLUENCE_CLOUD_ID that was ignored because it came from a different place than the URL. Always present, [] when there is nothing to report, the same convention roots and results follow. Not per-result warnings: those live on the result." }, "results": { "type": "array" }, @@ -20,7 +19,7 @@ "if": { "properties": { "command": { "const": "page-info" } }, "required": ["command"] }, "then": { "properties": { - "results": { "items": { "oneOf": [{ "$ref": "#/$defs/infoResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, + "results": { "items": { "anyOf": [{ "$ref": "#/$defs/infoResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, "summary": { "$ref": "#/$defs/basicSummary" } } } @@ -47,7 +46,7 @@ "if": { "properties": { "command": { "const": "read" } }, "required": ["command"] }, "then": { "properties": { - "results": { "items": { "oneOf": [{ "$ref": "#/$defs/readResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, + "results": { "items": { "anyOf": [{ "$ref": "#/$defs/readResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, "summary": { "$ref": "#/$defs/basicSummary" } } } @@ -85,7 +84,7 @@ "properties": { "results": { "items": { - "oneOf": [{ "$ref": "#/$defs/diffResult" }, { "$ref": "#/$defs/singleOpFailure" }] + "anyOf": [{ "$ref": "#/$defs/diffResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, "summary": { "$ref": "#/$defs/basicSummary" } @@ -98,7 +97,7 @@ "properties": { "results": { "items": { - "oneOf": [{ "$ref": "#/$defs/attachmentUploadResult" }, { "$ref": "#/$defs/singleOpFailure" }] + "anyOf": [{ "$ref": "#/$defs/attachmentUploadResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, "summary": { "$ref": "#/$defs/attachmentSummary" } @@ -110,7 +109,7 @@ "then": { "properties": { "results": { - "items": { "oneOf": [{ "$ref": "#/$defs/exportResult" }, { "$ref": "#/$defs/singleOpFailure" }] } + "items": { "anyOf": [{ "$ref": "#/$defs/exportResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, "summary": { "$ref": "#/$defs/exportSummary" } } @@ -122,7 +121,7 @@ "properties": { "results": { "items": { - "oneOf": [{ "$ref": "#/$defs/attachmentDownloadResult" }, { "$ref": "#/$defs/singleOpFailure" }] + "anyOf": [{ "$ref": "#/$defs/attachmentDownloadResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, "summary": { "$ref": "#/$defs/attachmentSummary" } @@ -135,7 +134,7 @@ "properties": { "results": { "items": { - "oneOf": [ + "anyOf": [ { "$ref": "#/$defs/childrenResult" }, { "$ref": "#/$defs/singleOpFailure" } ] @@ -178,7 +177,7 @@ "properties": { "results": { "items": { - "oneOf": [{ "$ref": "#/$defs/attachmentListResult" }, { "$ref": "#/$defs/singleOpFailure" }] + "anyOf": [{ "$ref": "#/$defs/attachmentListResult" }, { "$ref": "#/$defs/singleOpFailure" }] } }, "summary": { "$ref": "#/$defs/basicSummary" } @@ -196,7 +195,6 @@ "childrenResult": { "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"], "properties": { "ok": { "const": true }, @@ -213,7 +211,6 @@ "findResult": { "description": "One page or folder whose title matches exactly. There is no failure variant: find names no page to fail about, so a failed search is an error object on stderr with no envelope. status may be \"archived\" -- such a page is absent from the page tree but still reserves its title, so it blocks creating a page with that title in the same space. A folder reserves nothing, so a folder match is discovery only and never explains a creation conflict. space and url are both derived from the match's link, so a row missing one is missing both.", "type": "object", - "additionalProperties": false, "required": ["ok", "id", "type", "title", "space", "status", "url"], "properties": { "ok": { "const": true }, @@ -228,7 +225,6 @@ "searchResult": { "description": "One full-text match. The array is in Confluence's relevance order, best first; that order is the only ranking available (the API reports score as 0.0 on every row), so re-sorting results discards it. The ordering is stable in membership but not in position: two identical searches return the same set of hits, but equally-ranked neighbours can trade places, so do not diff two runs expecting the same sequence or treat a position as an identifier. There is no failure variant and no status field: search names no page to fail about, so a failed search is an error object on stderr with no envelope, and the search index cannot see archived content, so every hit is current -- use find to discover an archived page. type is an open string rather than an enum because --type all and --cql can return whiteboard, database, or any content type Atlassian adds. space and url are both derived from the hit's link, so a result missing one is missing both; excerpt is null when the match was in the title alone, and is always a single line with Confluence's highlight markers removed and entities unescaped.", "type": "object", - "additionalProperties": false, "required": ["ok", "id", "type", "title", "space", "url", "excerpt"], "properties": { "ok": { "const": true }, @@ -243,7 +239,6 @@ "searchSummary": { "description": "search's summary. failed is always 0, since a failed search is an error object with no envelope rather than a failed result. truncated says --limit was reached with matches left over; it is a flag and not a count because the API's totalSize is an estimate that has been observed both to drift between pages and to be nonzero against an empty results array. skipped counts index rows that carried no addressable content object -- a `type = space` hit, for example -- which is reachable only via --cql or --type all; it is reported rather than dropped quietly, because otherwise such a query yields total 0 and reads as a genuine miss.", "type": "object", - "additionalProperties": false, "required": ["total", "succeeded", "failed", "truncated", "skipped"], "properties": { "total": { "type": "integer" }, @@ -256,7 +251,6 @@ "userFindResult": { "description": "One matched Confluence user. mention is the whole Markdown line that mentions this person, built by the same converter function that renders a mention back out of storage, so pasting it into a body and publishing round-trips exactly. It is carried rather than left to the consumer because assembling it requires knowing two non-obvious things: the profile host is Atlassian Home rather than the site, and the leading \"@\" on the link text is what distinguishes a mention from an ordinary link to a profile. The account id appears in two live shapes on a single instance -- a 24-character hex string and a \"712020:\"-prefixed UUID -- so treat it as opaque and never pattern-match it. There is no failure variant: user-find names no page to fail about, so a failed lookup is an error object on stderr with no envelope. There is deliberately no type field even though the API returns one, because every account answers \"known\", automation and page-template accounts included, so it distinguishes nothing; and no separate url, since the profile URL is already inside mention. Deactivated accounts never appear: Confluence omits them from the user directory and no filter restores them.", "type": "object", - "additionalProperties": false, "required": ["ok", "account_id", "display_name", "mention"], "properties": { "ok": { "const": true }, @@ -268,7 +262,6 @@ "userFindSummary": { "description": "user-find's summary. failed is always 0, since a failed lookup is an error object with no envelope rather than a failed result. truncated says --limit was reached with matches left over, and is a flag rather than a count for a stronger reason than search's: this route's totalSize reports the number of rows on the page just fetched, not the size of the result set, so no count of the remainder exists to report.", "type": "object", - "additionalProperties": false, "required": ["total", "succeeded", "failed", "truncated"], "properties": { "total": { "type": "integer" }, @@ -286,7 +279,6 @@ }, "pageWidth": { "type": "object", - "additionalProperties": false, "required": ["value", "default"], "properties": { "value": { "type": "string" }, @@ -299,7 +291,6 @@ "pageStatus": { "description": "What a publishing verb did with the page's status -- the coloured lozenge beside its title. action is \"unchanged\" when the page already carried it, which is reported rather than implied because writing a status bumps the page version.", "type": "object", - "additionalProperties": false, "required": ["name", "action"], "properties": { "name": { "type": "string" }, @@ -316,7 +307,6 @@ }, "author": { "type": "object", - "additionalProperties": false, "required": ["account_id", "name"], "properties": { "account_id": { "type": "string" }, @@ -325,7 +315,6 @@ }, "stamp": { "type": "object", - "additionalProperties": false, "required": ["at", "by"], "properties": { "at": { "type": "string" }, @@ -337,7 +326,6 @@ }, "attachment": { "type": "object", - "additionalProperties": false, "required": ["action", "filename"], "properties": { "action": { "type": "string" }, @@ -347,7 +335,6 @@ "labelAction": { "description": "What happened to one label in an asserted set.", "type": "object", - "additionalProperties": false, "required": ["action", "name"], "properties": { "action": { @@ -367,7 +354,6 @@ "labelInfo": { "description": "One label a page carries. managed is true for the global prefix, the only one markfluence writes or removes.", "type": "object", - "additionalProperties": false, "required": ["name", "prefix", "managed"], "properties": { "name": { "type": "string" }, @@ -385,7 +371,6 @@ "singleOpFailure": { "description": "An operational failure for a single-target command (page-info/read): page not found, fetch error, etc.", "type": "object", - "additionalProperties": false, "required": ["ok", "page_id", "error", "code"], "properties": { "ok": { "const": false }, @@ -397,7 +382,6 @@ "userInfoResult": { "description": "One account: who the configured credentials belong to (self true) or who an account id names (self false). user-info reports exactly one of these; an operational failure is an errorObject on stderr instead, since there is no page id to name in a result.", "type": "object", - "additionalProperties": false, "required": [ "ok", "self", "account_id", "display_name", "public_name", "email", "account_type", "account_status", "external_collaborator", "guest", @@ -425,7 +409,6 @@ "oneOf": [ { "type": "object", - "additionalProperties": false, "required": ["id", "key", "name", "url"], "properties": { "id": { "type": "string" }, @@ -445,7 +428,6 @@ "oneOf": [ { "type": "object", - "additionalProperties": false, "required": ["visible", "write", "admin"], "properties": { "visible": { "type": "integer" }, @@ -461,7 +443,6 @@ "spaceInfoResult": { "description": "One space's metadata, what the calling account may do in it, the page statuses a page_status: line here may use, and exact page counts. space-info reports exactly one of these; an operational failure is an errorObject on stderr instead, since there is no page id to name in a result.", "type": "object", - "additionalProperties": false, "required": [ "ok", "key", "name", "id", "type", "status", "description", "labels", "homepage_id", "homepage_title", "access", "page_statuses", "pages", "recent" @@ -488,7 +469,6 @@ "oneOf": [ { "type": "object", - "additionalProperties": false, "required": ["read", "create_pages", "admin"], "properties": { "read": { "type": "boolean" }, @@ -502,7 +482,6 @@ "page_statuses": { "description": "What a page_status: line for a page in this space may say, and where the answer came from. source \"space\" is the space's own configured list, readable only by a space admin. source \"page\" is what THIS account may set on probe_page_id, which is NOT the space's list: Confluence decides the list per (caller, page), so another page in the same space may allow more or fewer, and a write of a status absent from it is refused. source null with names null means neither could be read.", "type": "object", - "additionalProperties": false, "required": ["source", "probe_page_id", "names"], "properties": { "source": { "enum": ["space", "page", null] }, @@ -517,7 +496,6 @@ "oneOf": [ { "type": "object", - "additionalProperties": false, "required": ["current", "archived", "roots"], "properties": { "current": { "type": "integer" }, @@ -533,7 +511,6 @@ "oneOf": [ { "type": "object", - "additionalProperties": false, "required": ["days", "pages_created", "pages_touched", "pages_created_and_touched", "last_activity"], "properties": { "days": { "type": "integer" }, @@ -550,7 +527,6 @@ }, "infoResult": { "type": "object", - "additionalProperties": false, "required": [ "ok", "page_id", "title", "content_status", "space", "parent", "parent_type", "version", "page_width", "page_status", "page_status_available", "labels", @@ -569,7 +545,6 @@ "parent_type": { "$ref": "#/$defs/parentTypeOrNull" }, "version": { "type": "object", - "additionalProperties": false, "required": ["number"], "properties": { "number": { "type": "integer" } } }, @@ -591,7 +566,6 @@ "type": "array", "items": { "type": "object", - "additionalProperties": false, "required": ["key", "value"], "properties": { "key": { "type": "string" }, "value": {} } } @@ -602,7 +576,6 @@ }, "readResult": { "type": "object", - "additionalProperties": false, "required": [ "ok", "page_id", "title", "space", "parent", "parent_type", "page_width", "page_status", "labels", "format", "body" @@ -630,10 +603,9 @@ "frontmatterDifference": { "description": "One frontmatter field the file and the page disagree about. Only differing fields appear, and only fields the file declares: an absent labels or page_width leaves the page's own alone, so a field the file is silent about has no difference to have.", "type": "object", - "additionalProperties": false, "required": ["field", "confluence", "local", "source", "comparable", "note"], "properties": { - "field": { "enum": ["title", "space", "parent", "page_status", "page_width", "labels"] }, + "field": { "type": "string", "description": "The frontmatter field: currently title, space, parent, page_status, page_width, or labels. A string rather than an enum, because a frontmatter field markfluence learns later is reported here too, and a consumer must accept a field name it does not know." }, "confluence": { "description": "The page's value; null when it could not be read, which is the one case comparable is false. A list is spelled in frontmatter's flow form, [a, b].", "$ref": "#/$defs/stringOrNull" @@ -653,7 +625,6 @@ "diffResult": { "description": "What differs between one file and its page. diff exits 1 when differs is true and 2 for any trouble, which is diff(1)'s contract rather than this schema's usual 1/2 -- see docs/json-output.md.", "type": "object", - "additionalProperties": false, "required": [ "ok", "file", "page_id", "title", "url", "metadata_source", "differs", "body_differs", "added", "removed", "diff", "frontmatter", "warnings" @@ -689,7 +660,6 @@ }, "updateResult": { "type": "object", - "additionalProperties": false, "required": [ "ok", "status", "dry_run", "file", "page_id", "title", "space", "url", "version", "page_width", "labels", "page_status", "attachments", "warnings", "broken", @@ -709,7 +679,6 @@ { "type": "null" }, { "type": "object", - "additionalProperties": false, "required": ["previous", "new"], "properties": { "previous": { "type": "integer" }, "new": { "type": "integer" } } } @@ -728,7 +697,6 @@ { "type": "null" }, { "type": "object", - "additionalProperties": false, "required": ["page_version", "publish_sha256"], "properties": { "page_version": { "type": "integer" }, @@ -748,7 +716,6 @@ "createResult": { "description": "One input file's create outcome. page_id and url describe the created page; on a failure they are null, except in two cases where a real page exists and its id must stay visible: the file's frontmatter page_id blocked creation (page_id is that id, url is the page already at it, null when the id resolves to nothing), or a page was created on the server but recording its metadata afterward failed -- in the file's frontmatter or in its markfluence.yaml entry -- in which case page_id/url name that orphaned page so it can be recovered.", "type": "object", - "additionalProperties": false, "required": [ "ok", "status", "dry_run", "file", "page_id", "title", "space", "parent", "parent_type", "parent_file", "url", "page_width", "labels", "page_status", "persisted", "attachments", "warnings", "broken", @@ -781,7 +748,6 @@ "checkResult": { "description": "One checked file. broken/warnings are always [] (never null), matching ConfluencePage's own convention. status=broken means broken is non-empty (frontmatter or converter); status=failed means the file never reached a clean answer at all (unreadable, unterminated frontmatter, frontmatter that is not valid YAML or not a flat mapping of single-line scalars, bad page_width, non-numeric page_id) -- code is VALIDATION in that case. debug is non-null only when --show-html was passed and the file reached the converter (never on a failed file).", "type": "object", - "additionalProperties": false, "required": ["ok", "status", "file", "broken", "warnings", "debug", "error", "code"], "properties": { "ok": { "type": "boolean" }, @@ -794,7 +760,6 @@ { "type": "null" }, { "type": "object", - "additionalProperties": false, "required": ["html", "attachments"], "properties": { "html": { "type": "string" }, @@ -810,7 +775,6 @@ "checkAttachment": { "description": "ConfluencePage.Attachments verbatim: a local image check's --show-html surfaces, not an upload outcome (contrast update/create's attachments array, which reports an action).", "type": "object", - "additionalProperties": false, "required": ["filename", "path", "source"], "properties": { "filename": { "type": "string" }, @@ -821,7 +785,6 @@ "attachmentUploadResult": { "description": "One uploaded file. status uses the same verbs as the attachments array on update/create. dest_path is always null: upload has no local destination to report, and only exists here so upload and download share one result shape.", "type": "object", - "additionalProperties": false, "required": ["ok", "status", "dry_run", "filename", "dest_path", "error", "code"], "properties": { "ok": { "type": "boolean" }, @@ -836,7 +799,6 @@ "exportSummary": { "description": "export batch summary. skipped counts pages whose file was already on disk; a run that exports nothing new is all skipped and still succeeded. project_file says what happened to the markfluence.yaml a multi-page export needs to be republishable: null for a single-page export, which needs none.", "type": "object", - "additionalProperties": false, "required": ["total", "succeeded", "failed", "skipped", "project_file"], "properties": { "total": { "type": "integer" }, @@ -849,7 +811,6 @@ "exportResult": { "description": "One exported page. status is whether the page file was written or skipped; the attachments array holds every attachment considered, including unreferenced ones (dest_path null, status skipped_unreferenced). warnings lists anything worth saying about this page besides its outcome, and is present whether or not it exported: references it makes to attachments that are not attached, and a name it did not choose because a sibling slugged the same.", "type": "object", - "additionalProperties": false, "required": [ "ok", "page_id", "title", "space", "parent", "parent_type", "parent_file", "dry_run", "status", "dest_path", "attachments", "warnings", "error", "code" @@ -872,7 +833,6 @@ "type": "array", "items": { "type": "object", - "additionalProperties": false, "required": ["status", "filename", "dest_path", "error", "code"], "properties": { "status": { "enum": ["downloaded", "skipped", "skipped_unreferenced", "failed"] }, @@ -891,7 +851,6 @@ "attachmentDownloadResult": { "description": "One attachment written to disk. filename is the stored attachment name; dest_path is the local path written, which depends on the recorded source path, --flat, and --dest. dest_path is null only when resolving it is what failed.", "type": "object", - "additionalProperties": false, "required": ["ok", "status", "dry_run", "filename", "dest_path", "error", "code"], "properties": { "ok": { "type": "boolean" }, @@ -906,7 +865,6 @@ "attachmentListResult": { "description": "One attachment on the page. filename is the name Confluence stores; for an attachment markfluence published that is the encoded source path, and source is the Markdown image path it came from. managed is false for a hand-uploaded attachment (sha256 and source both null). source may also be null on a managed attachment published before markfluence recorded source paths, in which case sha256 is still set.", "type": "object", - "additionalProperties": false, "required": ["ok", "id", "filename", "size", "media_type", "version", "comment", "managed", "sha256", "source"], "properties": { "ok": { "const": true }, @@ -924,7 +882,6 @@ "attachmentSummary": { "description": "attachment-upload/attachment-download batch summary.", "type": "object", - "additionalProperties": false, "required": ["total", "succeeded", "failed", "skipped"], "properties": { "total": { "type": "integer" }, @@ -936,7 +893,6 @@ "basicSummary": { "description": "page-info/read batch summary (total:1), and attachment-list (total: the attachment count).", "type": "object", - "additionalProperties": false, "required": ["total", "succeeded", "failed"], "properties": { "total": { "type": "integer" }, @@ -946,7 +902,6 @@ }, "updateSummary": { "type": "object", - "additionalProperties": false, "required": ["total", "succeeded", "failed", "skipped"], "properties": { "total": { "type": "integer" }, @@ -957,7 +912,6 @@ }, "createSummary": { "type": "object", - "additionalProperties": false, "required": ["total", "succeeded", "failed", "aborted"], "properties": { "total": { "type": "integer" }, @@ -969,7 +923,6 @@ "checkSummary": { "description": "clean/warnings count files on the ok:true side (clean has neither broken nor warnings; warnings has only warnings); failed already covers both the broken and failed statuses on the ok:false side rather than splitting them the way status does.", "type": "object", - "additionalProperties": false, "required": ["total", "succeeded", "failed", "clean", "warnings"], "properties": { "total": { "type": "integer" }, @@ -982,7 +935,6 @@ "errorObject": { "description": "The typed error object written to stderr on a fatal/pre-flight failure. command may be empty for a pre-parse (bad-flag) error.", "type": "object", - "additionalProperties": false, "required": ["schema_version", "command", "error", "code", "warnings"], "properties": { "schema_version": { "const": 1 },