From ce7d329fc79bc2167206b9c849bf26c4d6487502 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 5 Aug 2026 19:29:55 -0400 Subject: [PATCH 1/8] feat(client): expose attachment metadata and downloads Widen the attachment expand to metadata.comment,version,extensions and decode the fields the attachment subcommands need: fileSize and mediaType (under extensions), version.number, and the download link. ListAttachments now pages by start/limit offset instead of a hardcoded limit=250 that silently truncated. A v1 collection omits _links.next when the results fit one page, and its next is relative to the /wiki context rather than the v2 paths resolveNext handles, so offsets are the reliable form. DownloadAttachment goes through send, inheriting retry/backoff and typed errors. The endpoint 302s to Atlassian's media host with its own token, and Go drops Authorization on a cross-host redirect, so the site credentials never reach it; the test pins that, addressing the media server as localhost against a 127.0.0.1 origin so the hop is genuinely cross-host. Refs #9 --- _plans/019_attachment-subcommands.md | 271 +++++++++++++++++++++++++++ internal/client/client.go | 85 ++++++++- internal/client/client_test.go | 161 ++++++++++++++++ 3 files changed, 508 insertions(+), 9 deletions(-) create mode 100644 _plans/019_attachment-subcommands.md diff --git a/_plans/019_attachment-subcommands.md b/_plans/019_attachment-subcommands.md new file mode 100644 index 0000000..069c14b --- /dev/null +++ b/_plans/019_attachment-subcommands.md @@ -0,0 +1,271 @@ +# Plan: attachment subcommands + +Expose standalone attachment management — `attachment-list`, `attachment-upload`, +`attachment-download` — complementing the automatic sync that `update`/`create` +already perform. Closes #9. + +018 paused this work on one unsettled question: what remote name a standalone +upload should use. Bijective percent-encoding plus the `path=` comment answers +it, so the naming rule below is a consequence of 018 rather than a new invention. + +018 also deferred "clamping `..` when writing files" to #37, on the reasoning that +`read` only prints text. That deferral expires here: `attachment-download` +restores directory layout by default, so it is the first code that writes +attachment bytes to paths derived from server data. **The clamp lands in this +plan.** + +## Out of scope (deliberately) + +- **`attachment-delete`.** #9 does not ask for it and markfluence holds no delete + scopes. Removing an orphan is still a Confluence-UI operation; `attachment-list` + exists so users can *see* orphans, which is what 018 promised. +- **`export` (#37).** This plan builds the two pieces export needs — a download + client method and a layout-restoring writer — but ships no multi-file, image-src + reconciling command. +- **Filtering flags on `list`** (`--managed`, `--pattern`). `--pattern` belongs to + #37, which specifies it; filtering a table is `jq`'s or `grep`'s job in v1. +- **`--comment` on upload.** The comment slot is markfluence bookkeeping. A + user-supplied comment would either destroy the checksum (breaking skip) or need + a sub-field, and nothing asks for it yet. + +## Decisions locked + +### Flat, noun-first command names + +`attachment-list`, `attachment-upload`, `attachment-download`. + +Cobra alphabetizes `--help`, so a noun-first prefix keeps the three adjacent in +the listing and makes `attachment-` a completion group for #14. The JSON +`command` field is the command name verbatim, so it stays derivable from what the +user typed. + +**Rejected: an `attachment` parent command with `list`/`upload`/`download` +children.** The issue's own phrasing, and what confluence-cli does. It would make +these the only nested commands in the CLI and force a two-word JSON +discriminator. + +**Rejected: verb-first** (`list-attachments`, `upload-attachment`). Reads better +in a sentence, but scatters the three across the help listing under l/u/d. + +### One page-argument resolver for the whole CLI + +Two resolvers exist today and neither is a superset: + +| | numeric id | page URL | `.md` with `page_id` | +|---|---|---|---| +| `info.resolvePageID` | ✅ | ❌ | ✅ | +| `read.parsePageID` | ✅ | ✅ | ❌ | + +New `internal/pageref` accepts all three; `info` and `read` are retrofitted onto +it. Both changes are purely additive — each command gains the form it lacked — so +no existing invocation changes meaning. #37 and #44 inherit it. + +### `list` shows what a publish will and will not touch + +Columns `NAME`, `SIZE`, `VER`, `TYPE`, `SOURCE`, with human-readable sizes. +`NAME` is the **stored** name (`assets%2Fx.png`), because that is what identifies +the attachment to `attachment-download` and what appears in Confluence's own UI. +`SOURCE` is `Meta().Source`; an unmanaged attachment shows `—`. + +That em-dash is the point of the command: it distinguishes attachments a publish +will overwrite from hand-uploaded ones it will leave alone, and it surfaces the +orphans 018 chose not to warn about. + +**Rejected: decoding `NAME` for display.** Prettier, and makes `SOURCE` +redundant, but it lies about server state and hides the string you need in order +to download the file. + +**Rejected: emitting `download_url`.** Built on `SiteURL()` it 401s under a +scoped token; built on `BaseURL()` it leaks the gateway host into reader-facing +output, which the two-bases rule exists to prevent. A URL that works or does not +depending on the reader's token type is a footgun, and `attachment-download` is +the supported way to fetch bytes. + +### Upload: basename by default, `--name` takes a path + +``` +attachment-upload 123 docs/assets/x.png → x.png (path=x.png) +attachment-upload 123 docs/assets/x.png --name assets/x.png → assets%2Fx.png (path=assets/x.png) +``` + +`--name` accepts a **path** and markfluence encodes it, so percent-escapes never +leak into the UI. It is single-file only, like `--title` on `update`/`create`. + +**The recorded `path=` is always the decode of the stored name.** If the two were +allowed to drift — name `x.png`, path `docs/assets/x.png` — then a later publish +from `docs/assets/x.png` would encode to `docs%2Fassets%2Fx.png` and create a +*second* attachment, while `download` restored the first to a location the +markdown never references. One truth, enforced by construction: both derive from +the same input. + +**Rejected: encoding the given path by default.** Makes a hand-upload agree with +a publish for free, but 018's `normalizeSrc` strips a leading `/`, so +`~/Downloads/report.pdf` becomes `Users%2Fyou%2FDownloads%2Freport.pdf`. Upload +takes arbitrary filesystem paths; image `src`s are page-relative by construction. +The two are not the same input space. + +**Rejected: relative-when-under-cwd, basename otherwise.** Does the right thing +in both common cases with no flag, at the cost of a name that depends on where +the user is standing — the instability 018 rejected model A to avoid. + +Sync semantics are reused wholesale: created/updated/skipped by checksum, so a +hand upload and a publish agree on state. `--force` uploads regardless (bumping +the version), recovering an attachment whose bytes drifted server-side while its +comment still matches. `--dry-run` is nearly free via the existing +`PlanAttachments`. + +### Download: restore layout by default, from `path=` and never from the name + +A **managed** attachment is written to its recorded `path=`. An **unmanaged** one +is written under its literal stored name. `--flat` writes everything under +literal stored names. + +Restoring from the comment rather than by decoding the name sidesteps the +ambiguity `attachname.go` warns about: there is no way to tell a hand-uploaded +`a%2Fb.png` from one we published, so a decode-by-default would scatter a +literally-named file into `a/b.png`. The comment is truth; the name is inference. + +Restore is the default because round-trip is the product thesis. A download +yielding `docs%2Fassets%2Fx.png` leaks Confluence's no-slash restriction into the +user's filesystem and breaks local preview in GitHub or VSCode until every file +is renamed by hand. + +**The traversal clamp.** 018 model B legitimately produces `..%2Fassets%2Flogo.png`, +so `..` cannot be refused outright at decode. But a destination path resolving +outside `--dest` is a **hard error for that file**, not a silent clip: the +attachment comment is server-controlled data, and `path=../../../.ssh/authorized_keys` +must not write there. Absolute paths are already refused by +`attachmentSource`. Failing the single file rather than the run keeps one hostile +attachment from blocking a legitimate download. + +`--dest` defaults to `.` and is created if missing. An existing file is skipped +with a warning unless `--force`. Zero names means all attachments; a named +attachment absent from the page is a per-item failure. + +### `DownloadAttachment` goes through `send` + +```go +func (c *ConfluenceClient) DownloadAttachment(att Attachment, w io.Writer) error +``` + +Reusing `send` inherits retry/backoff, per-attempt timeouts, and typed +`HTTPError` for roughly twenty lines. It buffers the whole attachment in memory — +acceptable for docs and images, revisited in #37 if large exports appear. + +A new `timeoutDownload` (120s, matching upload) replaces `timeoutRead`'s 30s, +which is thin for a large file. + +**Redirect handling is load-bearing and correct by default.** The endpoint 302s +to `api.media.atlassian.com` with its own short-lived `token=`. `send` sets basic +auth via `req.SetBasicAuth` on a stock `&http.Client{}`, and Go strips +`Authorization` on a cross-host redirect — so Confluence credentials are never +sent to the media host, which does not want them anyway. This wants a comment; +adding a custom `CheckRedirect` that forwarded headers would be a credential +leak. + +**Rejected: streaming with its own retry loop.** Constant memory at any size, at +the cost of a second retry/backoff implementation parallel to `send` — the kind +of duplication that drifts. + +### `ListAttachments` paginates by offset + +The collection carries `size`/`limit`/`start` and **omits `_links.next` when +results fit one page** (018, confirmed again below). `resolveNext` is also written +for v2's `/wiki/api/v2/...` links, while v1's `next` is relative to the `/wiki` +context, so reusing it would drop `/wiki`. Loop on `start += limit` until a short +page. Today's hardcoded `limit=250` with no pagination silently truncates a page +with more attachments. + +### The name codec is exported from `internal/convert` + +`AttachmentFilename` and `AttachmentSource` become exported. The commands need to +encode a `--name` path and decode for `--flat`. One owner, and `cmd/read` already +imports `convert`. + +**Rejected: a new `internal/attachname` package.** The codec is intimately tied +to the converter's image handling, and 018 deliberately put it there. + +### JSON contract + +`command` is the command name verbatim. Status verbs follow the existing +per-command pattern: + +| command | verbs | summary | +|---|---|---| +| `attachment-list` | *(none — data only, like `info`/`read`)* | `basicSummary` | +| `attachment-upload` | `created` / `updated` / `skipped` / `failed` | `attachmentSummary` | +| `attachment-download` | `downloaded` / `skipped` / `failed` | `attachmentSummary` | + +`attachmentSummary` is `total`/`succeeded`/`failed`/`skipped`, shared by the two +write commands. `list` emits one result per attachment — target is the +attachment, so `summary.total` is the attachment count and `.results[] | +.filename` works directly. A page-level failure (not found, fetch error) uses the +existing `singleOpFailure` shape. + +Download results carry the local `dest_path` actually written. Per-item failures +are `{ok:false, error, code}` with exit 1 if any item failed; fatal/pre-flight +failures keep the stderr error object and exit 2. + +**Each command commit carries its own `schema/json-output/v1.json` update**, +because `schematest`'s `additionalProperties:false` makes code and schema fail +together otherwise. + +## Verified against the live API + +018 recorded incidental findings for #9; they were re-probed against page +`2848423944` before writing this plan, and one assumption was wrong. + +- **`extensions.fileSize` (171) and `extensions.mediaType` (`image/png`)** + confirmed; `metadata.mediaType` duplicates the latter. +- **`version.number`** confirmed, alongside `version.when` and + `version.by.displayName`. `version.message` duplicates the comment. +- **`_links.download` is `/rest/api/content/{pageId}/child/attachment/{attId}/download`** — + *not* the classic `/download/attachments/…` UI path. Absolute URL is + `baseURL + "/wiki" + link`. Because it is an API path, it works through the + `api.atlassian.com` gateway, which the UI path would not. +- **It 302s to `api.media.atlassian.com/file/{fileId}/binary?token=…`**, a + different host with a short-lived token — the finding that drove the redirect + decision above. Following it yields 200 and 171 bytes of valid PNG. +- **The collection returns `size`/`limit`/`start` with no `_links.next`**, + confirming offset pagination. +- **018's scheme is live in production**: stored title + `assets%2Fmarkfluence-test.png`, comment + `markfluence: sha256=e733ac00… path=assets/markfluence-test.png`. + +## Testing + +- `internal/client/client_test.go`: decoding the expanded `Attachment` + (`fileSize`/`mediaType`/`version.number`/`_links.download`) from a recorded + payload shaped like the live one; offset pagination across a short final page; + `DownloadAttachment` writing bytes, including across a redirect to a second + test server, asserting the `Authorization` header does **not** reach it. +- `internal/pageref/pageref_test.go`: all three argument forms, plus the + rejections (empty, non-numeric, a directory, a `.md` with no `page_id`). +- `cmd/attachment-*/`: per-command `json_test.go` validating each new result and + summary shape against `v1.json` via `schematest`; unit tests for the + size formatter, the `--name` encode path, and the destination resolver. +- The destination resolver gets adversarial cases: `path=../../escape`, + `path=/etc/passwd`, a legitimate `..%2Fassets%2Flogo.png` under model B landing + inside `--dest`, and an unmanaged attachment falling back to its literal name. +- `make test && make lint && make vet`. + +## Docs + +- `README.md`: a usage section per subcommand alongside `info`/`read`, the + naming rule for `--name`, the restore-vs-`--flat` behavior, and the `--json` + notes for the three new `command` values. +- `CLAUDE.md`: the three commands in the layout list (and the stale "four + subcommands" phrasing in the `cmd/root.go` bullet), `internal/pageref`, and the + download method plus offset pagination on the `internal/client` bullet. + +## Commits + +1. `feat(client): expose attachment metadata and downloads` — expanded + `Attachment`, offset pagination, `DownloadAttachment`, `timeoutDownload`. +2. `refactor(convert): export the attachment name codec`. +3. `refactor(pageref): unify page argument resolution` — new package, `info` and + `read` retrofitted. +4. `feat(cmd): add attachment-list` (+ schema). +5. `feat(cmd): add attachment-upload` (+ schema). +6. `feat(cmd): add attachment-download` (+ schema). +7. `docs: document the attachment subcommands`. diff --git a/internal/client/client.go b/internal/client/client.go index 326d15c..a72415e 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -55,6 +55,8 @@ const ( timeoutRead = 30 * time.Second timeoutWrite = 60 * time.Second timeoutUpload = 120 * time.Second + // timeoutDownload matches timeoutUpload: 30s is thin for a large attachment. + timeoutDownload = 120 * time.Second ) const ( @@ -179,13 +181,30 @@ type Links struct { Next string `json:"next"` } -// Attachment is a page attachment (v1), with the checksum comment expanded. +// Attachment is a page attachment (v1), with the comment, version, and +// extensions expanded. Title is the name Confluence stores, which for an +// attachment markfluence published is the encoded source path (see +// convert.AttachmentFilename). type Attachment struct { ID string `json:"id"` Title string `json:"title"` Metadata struct { Comment string `json:"comment"` } `json:"metadata"` + Version struct { + Number int `json:"number"` + When string `json:"when"` + } `json:"version"` + Extensions struct { + MediaType string `json:"mediaType"` + FileSize int64 `json:"fileSize"` + } `json:"extensions"` + Links struct { + // Download is context-relative ("/rest/api/content/{page}/child/ + // attachment/{id}/download"), not the /download/attachments/... UI path. + // Being an API path, it also works through the gateway. + Download string `json:"download"` + } `json:"_links"` } // Property is a page content property (v2). Value is decoded as-is (page @@ -550,18 +569,66 @@ func (c *ConfluenceClient) GetUser(accountID string) string { // --- attachments (v1) -------------------------------------------------------- -// ListAttachments lists a page's attachments with the checksum comment expanded. +// attachmentPageSize is the per-request page size for ListAttachments. +const attachmentPageSize = 250 + +// ListAttachments lists all of a page's attachments, with the comment, version, +// and extensions expanded. +// +// Pagination is by start/limit offset rather than _links.next: a v1 collection +// omits next when the results fit one page, and its next is relative to the +// /wiki context rather than the v2 paths resolveNext is written for. func (c *ConfluenceClient) ListAttachments(pageID string) ([]Attachment, error) { - var out struct { - Results []Attachment `json:"results"` + var all []Attachment + for start := 0; ; start += attachmentPageSize { + var out struct { + Results []Attachment `json:"results"` + } + err := c.doJSON(http.MethodGet, + c.baseURL+"/wiki/rest/api/content/"+pageID+"/child/attachment", + url.Values{ + "expand": {"metadata.comment,version,extensions"}, + "limit": {strconv.Itoa(attachmentPageSize)}, + "start": {strconv.Itoa(start)}, + }, nil, &out, timeoutRead) + if err != nil { + return nil, err + } + all = append(all, out.Results...) + if len(out.Results) < attachmentPageSize { + return all, nil + } } - err := c.doJSON(http.MethodGet, - c.baseURL+"/wiki/rest/api/content/"+pageID+"/child/attachment", - url.Values{"expand": {"metadata.comment"}, "limit": {"250"}}, nil, &out, timeoutRead) +} + +// DownloadAttachment fetches an attachment's bytes and writes them to w. +// +// The download endpoint 302s to Atlassian's media host with its own short-lived +// token in the query string. Go's default redirect policy drops the +// Authorization header on a cross-host hop, so the site credentials are never +// sent to that host -- which does not want them. Do not install a CheckRedirect +// that forwards headers: it would leak the API token to a third-party host. +// +// The body is buffered in memory, like every other response send handles, in +// exchange for its retry/backoff and typed errors. +func (c *ConfluenceClient) DownloadAttachment(att Attachment, w io.Writer) error { + if att.Links.Download == "" { + return fmt.Errorf("attachment %s (%s) has no download link", att.Title, att.ID) + } + rawURL := c.baseURL + "/wiki" + att.Links.Download + req, err := http.NewRequest(http.MethodGet, rawURL, nil) if err != nil { - return nil, err + return err } - return out.Results, nil + status, body, err := c.send(req, timeoutDownload) + if err != nil { + return err + } + if status >= 400 { + return &HTTPError{StatusCode: status, Method: http.MethodGet, URL: rawURL, Body: string(body)} + } + _, err = w.Write(body) + return err } // attachmentPlan is the decision for one local attachment: the action to take diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 39d8f76..84efbd3 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -1,6 +1,9 @@ package client import ( + "bytes" + "errors" + "fmt" "io" "net/http" "net/http/httptest" @@ -426,6 +429,164 @@ func TestPlanAttachmentsClassifiesWithoutUploading(t *testing.T) { } } +// TestListAttachmentsDecodesMetadata pins the fields against a payload shaped +// like a real one (verified against Cloud): fileSize and mediaType live under +// extensions, and the download link is an API path, not /download/attachments. +func TestListAttachmentsDecodesMetadata(t *testing.T) { + list := `{"results":[{` + + `"id":"att99","title":"assets%2Fx.png",` + + `"metadata":{"comment":"` + attachmentCommentPrefix + `sha256=abc path=assets/x.png"},` + + `"version":{"number":3,"when":"2026-08-05T22:17:28.040Z"},` + + `"extensions":{"mediaType":"image/png","fileSize":171},` + + `"_links":{"download":"/rest/api/content/1/child/attachment/att99/download"}` + + `}]}` + c, _ := newServer(t, resp{200, list}) + got, err := c.ListAttachments("1") + if err != nil || len(got) != 1 { + t.Fatalf("ListAttachments = %v, %v", got, err) + } + a := got[0] + if a.Extensions.FileSize != 171 || a.Extensions.MediaType != "image/png" { + t.Errorf("extensions = %+v, want 171/image/png", a.Extensions) + } + if a.Version.Number != 3 { + t.Errorf("version.number = %d, want 3", a.Version.Number) + } + if a.Links.Download != "/rest/api/content/1/child/attachment/att99/download" { + t.Errorf("download = %q", a.Links.Download) + } + if m := a.Meta(); !m.Managed || m.Source != "assets/x.png" || m.SHA256 != "abc" { + t.Errorf("meta = %+v", m) + } +} + +// TestListAttachmentsPaginates covers the offset loop: a full first page means +// there may be more, and a short page ends it. Without this, a page with more +// than attachmentPageSize attachments is silently truncated. +func TestListAttachmentsPaginates(t *testing.T) { + var full strings.Builder + full.WriteString(`{"results":[`) + for i := range attachmentPageSize { + if i > 0 { + full.WriteByte(',') + } + fmt.Fprintf(&full, `{"id":"a%d","title":"f%d.png"}`, i, i) + } + full.WriteString(`]}`) + + c, s := newServer(t, + resp{200, full.String()}, + resp{200, `{"results":[{"id":"last","title":"last.png"}]}`}, + ) + got, err := c.ListAttachments("1") + if err != nil { + t.Fatal(err) + } + if len(got) != attachmentPageSize+1 { + t.Fatalf("len = %d, want %d", len(got), attachmentPageSize+1) + } + if got[len(got)-1].Title != "last.png" { + t.Errorf("last title = %q, want last.png", got[len(got)-1].Title) + } + if len(s.calls) != 2 { + t.Errorf("calls = %v, want 2 requests", s.calls) + } +} + +// TestListAttachmentsStopsOnShortFirstPage guards the common case: one request, +// not a speculative second. +func TestListAttachmentsStopsOnShortFirstPage(t *testing.T) { + c, s := newServer(t, resp{200, `{"results":[{"id":"a1","title":"x.png"}]}`}) + if _, err := c.ListAttachments("1"); err != nil { + t.Fatal(err) + } + if len(s.calls) != 1 { + t.Errorf("calls = %v, want a single request", s.calls) + } +} + +func TestDownloadAttachmentWritesBytes(t *testing.T) { + c, _ := newServer(t, resp{200, "PNGBYTES"}) + var att Attachment + att.Links.Download = "/rest/api/content/1/child/attachment/att1/download" + var buf bytes.Buffer + if err := c.DownloadAttachment(att, &buf); err != nil { + t.Fatal(err) + } + if buf.String() != "PNGBYTES" { + t.Errorf("body = %q, want PNGBYTES", buf.String()) + } +} + +func TestDownloadAttachmentRequiresLink(t *testing.T) { + c, _ := newServer(t) + if err := c.DownloadAttachment(Attachment{Title: "x.png"}, io.Discard); err == nil { + t.Fatal("want an error when the attachment has no download link") + } +} + +func TestDownloadAttachmentPropagatesHTTPError(t *testing.T) { + c, _ := newServer(t, resp{404, "gone"}) + var att Attachment + att.Links.Download = "/rest/api/content/1/child/attachment/att1/download" + err := c.DownloadAttachment(att, io.Discard) + var he *HTTPError + if !errors.As(err, &he) || he.StatusCode != 404 { + t.Fatalf("err = %v, want *HTTPError 404", err) + } +} + +// TestDownloadAttachmentDoesNotLeakCredentialsOnRedirect is the security +// assertion behind reusing send: the real endpoint 302s to Atlassian's media +// host, which carries its own token in the query string and must never receive +// the site credentials. Go's default policy drops Authorization on a cross-host +// hop -- this fails if anyone adds a CheckRedirect that forwards headers. +// +// The media server is addressed as "localhost" while the origin is +// "127.0.0.1". Both resolve to the same listener, but Go compares hostnames +// with the port stripped, so two httptest servers would otherwise look like the +// same host and the header would be forwarded -- making the test pass +// vacuously. +func TestDownloadAttachmentDoesNotLeakCredentialsOnRedirect(t *testing.T) { + var mediaAuth string + var reached bool + media := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = true + mediaAuth = r.Header.Get("Authorization") + _, _ = w.Write([]byte("MEDIABYTES")) + })) + t.Cleanup(media.Close) + mediaURL := strings.Replace(media.URL, "127.0.0.1", "localhost", 1) + if mediaURL == media.URL { + t.Fatalf("expected a 127.0.0.1 test URL to rewrite, got %q", media.URL) + } + + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") == "" { + t.Error("origin request lost its Authorization header") + } + http.Redirect(w, r, mediaURL+"/file/abc/binary?token=xyz", http.StatusFound) + })) + t.Cleanup(origin.Close) + + c := New(Config{SiteURL: origin.URL, Username: "u", Token: "secret"}) + var att Attachment + att.Links.Download = "/rest/api/content/1/child/attachment/att1/download" + var buf bytes.Buffer + if err := c.DownloadAttachment(att, &buf); err != nil { + t.Fatal(err) + } + if !reached { + t.Fatal("redirect was not followed") + } + if buf.String() != "MEDIABYTES" { + t.Errorf("body = %q, want the redirected bytes", buf.String()) + } + if mediaAuth != "" { + t.Errorf("Authorization leaked to the media host: %q", mediaAuth) + } +} + // --- misc -------------------------------------------------------------------- func TestLoadDotenv(t *testing.T) { From d353a80dce13a36ff6375dfa1bda42d78a050189 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 5 Aug 2026 19:31:02 -0400 Subject: [PATCH 2/8] refactor(convert): export the attachment name codec The attachment subcommands need both directions: upload encodes a --name path so percent-escapes never leak into the UI, and download decodes a stored name for --flat. No behavior change. Refs #9 --- internal/convert/attachname.go | 17 +++++++++++------ internal/convert/attachname_test.go | 24 ++++++++++++------------ internal/convert/images.go | 2 +- internal/convert/storage_to_md.go | 2 +- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/internal/convert/attachname.go b/internal/convert/attachname.go index ec398ff..e190a87 100644 --- a/internal/convert/attachname.go +++ b/internal/convert/attachname.go @@ -13,6 +13,11 @@ package convert // Confluence stores these names verbatim and matches ri:filename literally -- // verified against Cloud, where "a%2Fb.png" resolves and renders with the name // re-escaped as "a%252Fb.png" in the image URL. +// +// The codec is exported because the attachment subcommands share it: upload +// encodes a --name path, and download decodes a stored name. It lives here +// rather than in its own package because the converter's image handling is what +// defines the mapping. import ( "path" @@ -24,25 +29,25 @@ const ( pctSlash = "%2F" // a "/" path separator ) -// attachmentFilename derives the Confluence attachment name for a markdown image +// AttachmentFilename derives the Confluence attachment name for a markdown image // src. The src is normalized first so a name can never decode to an absolute // path, then "%" is encoded before "/" -- in that order, so the escapes // introduced for "/" are not themselves escaped. -func attachmentFilename(src string) string { +func AttachmentFilename(src string) string { rel := normalizeSrc(src) rel = strings.ReplaceAll(rel, "%", pctEscape) return strings.ReplaceAll(rel, "/", pctSlash) } -// attachmentSource inverts attachmentFilename, recovering the source path an +// AttachmentSource inverts AttachmentFilename, recovering the source path an // attachment was published from. It reports false when the name could not have // come from markfluence -- currently when it decodes to an empty or absolute -// path, which attachmentFilename never produces -- so callers fall back to +// path, which AttachmentFilename never produces -- so callers fall back to // treating the attachment name as the path. // // A name markfluence did not create is decoded on a best-effort basis: there is // no way to tell a hand-uploaded "a%2Fb.png" from one we published. -func attachmentSource(filename string) (string, bool) { +func AttachmentSource(filename string) (string, bool) { // Decode "%2F" first and "%25" last. Replacing "%2F" only ever removes text // and cannot spell a new "%25", while "%25" must be replaced last precisely // so its output is not rescanned -- that is how a literal "%2F" in the source @@ -58,7 +63,7 @@ func attachmentSource(filename string) (string, bool) { // normalizeSrc reduces a markdown image src to a clean relative path: "./a/x.png" // and "a/./x.png" both become "a/x.png". A leading "/" is dropped because image // resolution joins src onto the page's directory anyway, so an absolute-looking -// src was never actually absolute -- and dropping it keeps attachmentFilename +// src was never actually absolute -- and dropping it keeps AttachmentFilename // from ever producing a name that decodes to an absolute path. // // A ".." prefix is preserved: an image in a shared directory above the page diff --git a/internal/convert/attachname_test.go b/internal/convert/attachname_test.go index 219ef7b..3f90110 100644 --- a/internal/convert/attachname_test.go +++ b/internal/convert/attachname_test.go @@ -36,16 +36,16 @@ func TestAttachmentNameRoundTrip(t *testing.T) { {"my docs/a b.png", "my docs%2Fa b.png"}, } for _, c := range cases { - if got := attachmentFilename(c.src); got != c.name { - t.Errorf("attachmentFilename(%q) = %q, want %q", c.src, got, c.name) + if got := AttachmentFilename(c.src); got != c.name { + t.Errorf("AttachmentFilename(%q) = %q, want %q", c.src, got, c.name) } - got, ok := attachmentSource(c.name) + got, ok := AttachmentSource(c.name) if !ok { - t.Errorf("attachmentSource(%q) refused a name we produced", c.name) + t.Errorf("AttachmentSource(%q) refused a name we produced", c.name) continue } if got != c.src { - t.Errorf("attachmentSource(%q) = %q, want %q (round trip)", c.name, got, c.src) + t.Errorf("AttachmentSource(%q) = %q, want %q (round trip)", c.name, got, c.src) } } } @@ -59,7 +59,7 @@ func TestAttachmentFilenameIsInjective(t *testing.T) { } seen := map[string]string{} for _, src := range srcs { - name := attachmentFilename(src) + name := AttachmentFilename(src) if prev, dup := seen[name]; dup { t.Errorf("%q and %q both encode to %q", prev, src, name) } @@ -80,8 +80,8 @@ func TestAttachmentFilenameNormalizes(t *testing.T) { {"/assets/x.png", "assets%2Fx.png"}, } for _, c := range cases { - if got := attachmentFilename(c.src); got != c.want { - t.Errorf("attachmentFilename(%q) = %q, want %q", c.src, got, c.want) + if got := AttachmentFilename(c.src); got != c.want { + t.Errorf("AttachmentFilename(%q) = %q, want %q", c.src, got, c.want) } } } @@ -91,8 +91,8 @@ func TestAttachmentFilenameNormalizes(t *testing.T) { // path (which is what #37's export would then write to). func TestAttachmentSourceRefusesAbsolute(t *testing.T) { for _, name := range []string{"%2Fetc%2Fpasswd.png", "%2F.png", ""} { - if got, ok := attachmentSource(name); ok { - t.Errorf("attachmentSource(%q) = %q, true; want refusal", name, got) + if got, ok := AttachmentSource(name); ok { + t.Errorf("AttachmentSource(%q) = %q, true; want refusal", name, got) } } } @@ -106,9 +106,9 @@ func TestAttachmentSourceDecodesForeignNames(t *testing.T) { {"screenshot 2026.png", "screenshot 2026.png"}, {"..%2Fup.png", "../up.png"}, } { - got, ok := attachmentSource(c.name) + got, ok := AttachmentSource(c.name) if !ok || got != c.want { - t.Errorf("attachmentSource(%q) = %q, %v; want %q, true", c.name, got, ok, c.want) + t.Errorf("AttachmentSource(%q) = %q, %v; want %q, true", c.name, got, ok, c.want) } } } diff --git a/internal/convert/images.go b/internal/convert/images.go index 0f04189..e73d66c 100644 --- a/internal/convert/images.go +++ b/internal/convert/images.go @@ -60,7 +60,7 @@ func (r *storageRenderer) renderImage( _, _ = w.WriteString(html.EscapeString(msg)) default: - filename := attachmentFilename(src) + filename := AttachmentFilename(src) if !r.seen[filename] { if r.seen == nil { r.seen = map[string]bool{} diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index a4057ce..dd69e54 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -68,7 +68,7 @@ func (r *mdRenderer) sourceFor(filename string) string { if src, ok := r.sources[filename]; ok && src != "" && !path.IsAbs(src) { return src } - if src, ok := attachmentSource(filename); ok { + if src, ok := AttachmentSource(filename); ok { return src } return filename From 33688da60750a2971e017568c887bc1ce2188365 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 5 Aug 2026 19:33:21 -0400 Subject: [PATCH 3/8] refactor(pageref): unify page argument resolution info accepted a numeric id or a markdown file; read accepted a numeric id or a page URL. Neither was a superset, so what a page argument meant depended on which command you gave it to. internal/pageref accepts all three, and both commands now use it. The change is additive on each side -- info gains URLs, read gains files -- so no existing invocation changes meaning. A file is stat'd before the numeric check, so "123.md" resolves as a file. The attachment subcommands and #37/#44 take page arguments too, which is what makes one resolver worth extracting now. Refs #9 --- cmd/info/info.go | 38 ++----------- cmd/read/read.go | 38 ++----------- cmd/read/read_test.go | 31 ----------- internal/pageref/pageref.go | 75 +++++++++++++++++++++++++ internal/pageref/pageref_test.go | 95 ++++++++++++++++++++++++++++++++ 5 files changed, 179 insertions(+), 98 deletions(-) create mode 100644 internal/pageref/pageref.go create mode 100644 internal/pageref/pageref_test.go diff --git a/cmd/info/info.go b/cmd/info/info.go index 096647e..44af099 100644 --- a/cmd/info/info.go +++ b/cmd/info/info.go @@ -10,8 +10,8 @@ import ( "strings" "github.com/mozilla/markfluence/internal/client" - "github.com/mozilla/markfluence/internal/frontmatter" "github.com/mozilla/markfluence/internal/jsonout" + "github.com/mozilla/markfluence/internal/pageref" "github.com/mozilla/markfluence/internal/pagewidth" "github.com/mozilla/markfluence/internal/ui" "github.com/spf13/cobra" @@ -27,7 +27,8 @@ var Cmd = &cobra.Command{ Use: "info ARG", Short: "Print metadata about a Confluence page", Long: "Print metadata about a Confluence page.\n\n" + - "ARG is a numeric page id or a markdown file whose frontmatter has a page_id.", + "ARG is a numeric page id, a Confluence page URL, or a markdown file\n" + + "whose frontmatter has a page_id.", Args: cobra.ExactArgs(1), RunE: run, } @@ -49,7 +50,7 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail(err.Error(), jsonout.CodeConfig) } - pageID, err := resolvePageID(args[0]) + pageID, err := pageref.Resolve(args[0]) if err != nil { return fatalFail(err.Error(), jsonout.CodeValidation) } @@ -100,25 +101,6 @@ func operationalFail(pageID string, err error, code jsonout.Code) error { return ui.SilentExit(1) } -// resolvePageID resolves the CLI argument to a page id: a markdown file's -// frontmatter page_id, or a bare numeric id. -func resolvePageID(arg string) (string, error) { - if info, err := os.Stat(arg); err == nil && !info.IsDir() { - mf, err := frontmatter.ParseFile(arg) - if err != nil { - return "", err - } - if mf.PageID() == "" { - return "", fmt.Errorf("no page_id in frontmatter of %s", arg) - } - return mf.PageID(), nil - } - if isDigits(arg) { - return arg, nil - } - return "", fmt.Errorf("%s is not a file or a numeric page id", arg) -} - // report is the resolved metadata for a page, feeding both the human "label: // value" renderer and the JSON result. Fields are captured raw (empty when // absent); each renderer decides how to present or omit them. @@ -303,15 +285,3 @@ func versionNumber(n int) string { } return strconv.Itoa(n) } - -func isDigits(s string) bool { - if s == "" { - return false - } - for _, c := range s { - if c < '0' || c > '9' { - return false - } - } - return true -} diff --git a/cmd/read/read.go b/cmd/read/read.go index 73ccc3d..6277c41 100644 --- a/cmd/read/read.go +++ b/cmd/read/read.go @@ -4,15 +4,14 @@ package read import ( "fmt" - "net/url" "os" - "regexp" "strings" "github.com/mozilla/markfluence/internal/client" "github.com/mozilla/markfluence/internal/convert" "github.com/mozilla/markfluence/internal/frontmatter" "github.com/mozilla/markfluence/internal/jsonout" + "github.com/mozilla/markfluence/internal/pageref" "github.com/mozilla/markfluence/internal/pagewidth" "github.com/mozilla/markfluence/internal/ui" "github.com/spf13/cobra" @@ -32,8 +31,9 @@ var Cmd = &cobra.Command{ Use: "read ARG", Short: "Fetch a Confluence page and print its body", Long: "Fetch a Confluence page and print its body to stdout.\n\n" + - "ARG is a numeric page id or a Confluence page URL (the modern\n" + - "/wiki/.../pages//... form or a legacy ?pageId= URL).\n\n" + + "ARG is a numeric page id, a Confluence page URL (the modern\n" + + "/wiki/.../pages//... form or a legacy ?pageId= URL), or a\n" + + "markdown file whose frontmatter has a page_id.\n\n" + "The default markdown output carries title/page_id/space/page_width\n" + "frontmatter and is a best-effort inverse of what create/update publish.", Args: cobra.ExactArgs(1), @@ -51,7 +51,7 @@ func run(cmd *cobra.Command, args []string) error { formatFlag, formatMarkdown, formatStorage), jsonout.CodeValidation) } - pageID, err := parsePageID(args[0]) + pageID, err := pageref.Resolve(args[0]) if err != nil { return fatalFail(err.Error(), jsonout.CodeValidation) } @@ -189,31 +189,3 @@ func renderFrontmatter(title, space, parent, pageID, width string) string { } return fm } - -// pagePathRE matches the numeric id in a modern Confluence page URL path, -// e.g. /wiki/spaces/ENG/pages/123456/Some+Title (the trailing slug is optional). -var pagePathRE = regexp.MustCompile(`/pages/(\d+)(?:/|$)`) - -// parsePageID resolves the CLI argument to a numeric page id: a bare numeric id, -// or a Confluence URL carrying the id in its path or a pageId query parameter. -func parsePageID(arg string) (string, error) { - if isDigits(arg) { - return arg, nil - } - if u, err := url.Parse(arg); err == nil && u.Host != "" { - if id := u.Query().Get("pageId"); isDigits(id) { - return id, nil - } - if m := pagePathRE.FindStringSubmatch(u.Path); m != nil { - return m[1], nil - } - } - return "", fmt.Errorf("%q is not a numeric page id or a Confluence page URL", arg) -} - -func isDigits(s string) bool { - if s == "" { - return false - } - return strings.IndexFunc(s, func(r rune) bool { return r < '0' || r > '9' }) == -1 -} diff --git a/cmd/read/read_test.go b/cmd/read/read_test.go index c26f6d6..384005d 100644 --- a/cmd/read/read_test.go +++ b/cmd/read/read_test.go @@ -37,34 +37,3 @@ func TestRenderFrontmatterQuotesWhenNeeded(t *testing.T) { t.Errorf("renderFrontmatter =\n%q\nwant\n%q", got, want) } } - -func TestParsePageID(t *testing.T) { - tests := []struct { - name string - arg string - want string // "" means expect an error - }{ - {"bare numeric id", "123456", "123456"}, - {"modern path with slug", "https://org.atlassian.net/wiki/spaces/ENG/pages/123456/Some+Title", "123456"}, - {"modern path no slug", "https://org.atlassian.net/wiki/spaces/ENG/pages/123456", "123456"}, - {"legacy pageId query", "https://org.atlassian.net/wiki/pages/viewpage.action?pageId=987654", "987654"}, - {"query wins over path only if numeric", "https://org.atlassian.net/wiki/x?pageId=42", "42"}, - {"non-numeric arg", "not-a-page", ""}, - {"url without an id", "https://org.atlassian.net/wiki/spaces/ENG/overview", ""}, - {"empty", "", ""}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := parsePageID(tt.arg) - if tt.want == "" { - if err == nil { - t.Fatalf("parsePageID(%q) = %q, nil; want error", tt.arg, got) - } - return - } - if err != nil || got != tt.want { - t.Fatalf("parsePageID(%q) = %q, %v; want %q", tt.arg, got, err, tt.want) - } - }) - } -} diff --git a/internal/pageref/pageref.go b/internal/pageref/pageref.go new file mode 100644 index 0000000..7314ddd --- /dev/null +++ b/internal/pageref/pageref.go @@ -0,0 +1,75 @@ +// Package pageref resolves the way a user names a Confluence page on the command +// line into a page id. +// +// Three spellings are accepted, because all three are things a user naturally +// has to hand: a bare numeric id, a Confluence page URL (pasted from a browser), +// and a markdown file whose frontmatter carries a page_id. Every command that +// takes a page argument accepts all three, so the meaning of that argument does +// not depend on which command it was given to. +package pageref + +import ( + "fmt" + "net/url" + "os" + "regexp" + "strings" + + "github.com/mozilla/markfluence/internal/frontmatter" +) + +// pagePathRE matches the numeric id in a modern Confluence page URL path, +// e.g. /wiki/spaces/ENG/pages/123456/Some+Title (the trailing slug is optional). +var pagePathRE = regexp.MustCompile(`/pages/(\d+)(?:/|$)`) + +// Resolve turns a command-line page argument into a page id. +// +// An existing file is tried first, so a numerically-named markdown file is read +// as a file rather than mistaken for an id. +func Resolve(arg string) (string, error) { + if arg == "" { + return "", fmt.Errorf("no page given") + } + if info, err := os.Stat(arg); err == nil && !info.IsDir() { + mf, err := frontmatter.ParseFile(arg) + if err != nil { + return "", err + } + if mf.PageID() == "" { + return "", fmt.Errorf("no page_id in frontmatter of %s", arg) + } + return mf.PageID(), nil + } + if IsDigits(arg) { + return arg, nil + } + if id, ok := fromURL(arg); ok { + return id, nil + } + return "", fmt.Errorf( + "%q is not a numeric page id, a Confluence page URL, or a markdown file with a page_id", arg) +} + +// fromURL pulls a page id out of a Confluence URL: the modern +// /wiki/.../pages//... path form, or a legacy ?pageId= query parameter. +func fromURL(arg string) (string, bool) { + u, err := url.Parse(arg) + if err != nil || u.Host == "" { + return "", false + } + if id := u.Query().Get("pageId"); IsDigits(id) { + return id, true + } + if m := pagePathRE.FindStringSubmatch(u.Path); m != nil { + return m[1], true + } + return "", false +} + +// IsDigits reports whether s is a non-empty run of ASCII digits. +func IsDigits(s string) bool { + if s == "" { + return false + } + return strings.IndexFunc(s, func(r rune) bool { return r < '0' || r > '9' }) == -1 +} diff --git a/internal/pageref/pageref_test.go b/internal/pageref/pageref_test.go new file mode 100644 index 0000000..0df11dc --- /dev/null +++ b/internal/pageref/pageref_test.go @@ -0,0 +1,95 @@ +package pageref + +import ( + "os" + "path/filepath" + "testing" +) + +func TestResolveNumericID(t *testing.T) { + got, err := Resolve("123456") + if err != nil || got != "123456" { + t.Fatalf("Resolve = %q, %v; want 123456", got, err) + } +} + +func TestResolveURL(t *testing.T) { + cases := []struct { + arg, want string + }{ + {"https://org.atlassian.net/wiki/spaces/ENG/pages/123456/Some+Title", "123456"}, + {"https://org.atlassian.net/wiki/spaces/ENG/pages/123456", "123456"}, + {"https://org.atlassian.net/wiki/spaces/ENG/pages/123456/", "123456"}, + {"https://org.atlassian.net/wiki/pages/viewpage.action?pageId=987", "987"}, + // A query id wins over a path that has none. + {"https://org.atlassian.net/wiki/x?pageId=42", "42"}, + } + for _, c := range cases { + got, err := Resolve(c.arg) + if err != nil || got != c.want { + t.Errorf("Resolve(%q) = %q, %v; want %q", c.arg, got, err, c.want) + } + } +} + +func TestResolveMarkdownFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "page.md") + if err := os.WriteFile(path, []byte("---\ntitle: T\npage_id: 555\n---\n\nBody\n"), 0o644); err != nil { + t.Fatal(err) + } + got, err := Resolve(path) + if err != nil || got != "555" { + t.Fatalf("Resolve = %q, %v; want 555", got, err) + } +} + +// TestResolvePrefersFileOverID pins the precedence: a file named "123.md" is a +// file. Statting first is what makes that unambiguous. +func TestResolvePrefersFileOverID(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "123.md") + if err := os.WriteFile(path, []byte("---\npage_id: 999\n---\n"), 0o644); err != nil { + t.Fatal(err) + } + got, err := Resolve(path) + if err != nil || got != "999" { + t.Fatalf("Resolve = %q, %v; want the frontmatter id 999", got, err) + } +} + +func TestResolveRejects(t *testing.T) { + dir := t.TempDir() + noID := filepath.Join(dir, "noid.md") + if err := os.WriteFile(noID, []byte("---\ntitle: T\n---\n"), 0o644); err != nil { + t.Fatal(err) + } + + cases := []struct{ name, arg string }{ + {"empty", ""}, + {"not a number or url", "banana"}, + {"a directory", dir}, + {"file without page_id", noID}, + {"url with no page id", "https://org.atlassian.net/wiki/spaces/ENG"}, + {"negative", "-5"}, + {"id with whitespace", "12 34"}, + } + for _, c := range cases { + if got, err := Resolve(c.arg); err == nil { + t.Errorf("%s: Resolve(%q) = %q, nil; want an error", c.name, c.arg, got) + } + } +} + +func TestIsDigits(t *testing.T) { + for _, s := range []string{"0", "123456"} { + if !IsDigits(s) { + t.Errorf("IsDigits(%q) = false, want true", s) + } + } + for _, s := range []string{"", "12a", "-1", " 1", "1 "} { + if IsDigits(s) { + t.Errorf("IsDigits(%q) = true, want false", s) + } + } +} From c70753f7d324b809807fee90ffb4522a81a78e80 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 5 Aug 2026 19:36:55 -0400 Subject: [PATCH 4/8] feat(cmd): add attachment-list List a page's attachments as aligned NAME/SIZE/VER/TYPE/SOURCE columns, or one JSON result per attachment so `.results[] | .filename` works directly. SOURCE is the markdown image path an attachment was published from, which makes the command a view of what a publish will and will not touch -- including the orphans left behind by the attachment-name encoding change. A dash in SOURCE means no source path was recorded, which covers both a hand-uploaded attachment and one published before markfluence recorded them; the managed field in --json tells those apart. Verified against a live page carrying one of each. No download_url is emitted: on the site URL it fails under a scoped token, and on the request base it would leak the gateway host into reader-facing output. Refs #9 --- cmd/attachmentlist/attachmentlist.go | 170 ++++++++++++++++++++++ cmd/attachmentlist/attachmentlist_test.go | 81 +++++++++++ cmd/attachmentlist/json.go | 55 +++++++ cmd/attachmentlist/json_test.go | 112 ++++++++++++++ cmd/root.go | 2 + schema/json-output/v1.json | 37 ++++- 6 files changed, 454 insertions(+), 3 deletions(-) create mode 100644 cmd/attachmentlist/attachmentlist.go create mode 100644 cmd/attachmentlist/attachmentlist_test.go create mode 100644 cmd/attachmentlist/json.go create mode 100644 cmd/attachmentlist/json_test.go diff --git a/cmd/attachmentlist/attachmentlist.go b/cmd/attachmentlist/attachmentlist.go new file mode 100644 index 0000000..5e22823 --- /dev/null +++ b/cmd/attachmentlist/attachmentlist.go @@ -0,0 +1,170 @@ +// Package attachmentlist implements the `markfluence attachment-list` command: +// list a page's attachments. +package attachmentlist + +import ( + "fmt" + "os" + "strconv" + "strings" + + "github.com/mozilla/markfluence/internal/client" + "github.com/mozilla/markfluence/internal/jsonout" + "github.com/mozilla/markfluence/internal/pageref" + "github.com/mozilla/markfluence/internal/ui" + "github.com/spf13/cobra" +) + +// command is the name used in help and as the --json command discriminator. +const command = "attachment-list" + +// Cmd is the attachment-list command. +var Cmd = &cobra.Command{ + Use: command + " ARG", + Short: "List a Confluence page's attachments", + Long: "List a Confluence page's attachments.\n\n" + + "ARG is a numeric page id, a Confluence page URL, or a markdown file\n" + + "whose frontmatter has a page_id.\n\n" + + "NAME is the name Confluence stores. For an image markfluence published\n" + + "that is the encoded source path, and SOURCE shows the markdown image\n" + + "path it came from.\n\n" + + "SOURCE is a dash when no source path is recorded: the attachment was\n" + + "uploaded by hand, or it was published before markfluence recorded one.\n" + + "Use --json, whose managed field tells those two apart.", + Args: cobra.ExactArgs(1), + RunE: run, +} + +func run(cmd *cobra.Command, args []string) error { + url, _ := cmd.Flags().GetString("url") + username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") + envFile, _ := cmd.Flags().GetString("env-file") + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) + if err != nil { + return fatalFail(err.Error(), jsonout.CodeConfig) + } + + pageID, err := pageref.Resolve(args[0]) + if err != nil { + return fatalFail(err.Error(), jsonout.CodeValidation) + } + + attachments, err := c.ListAttachments(pageID) + if err != nil { + return operationalFail(pageID, err, jsonout.CodeFor(err)) + } + + if ui.IsJSON() { + results := make([]any, 0, len(attachments)) + for _, a := range attachments { + results = append(results, buildResult(a)) + } + env := jsonout.NewEnvelope(command, results, + map[string]int{"total": len(attachments), "succeeded": len(attachments), "failed": 0}) + return jsonout.Emit(os.Stdout, env) + } + + if len(attachments) == 0 { + ui.Info("No attachments.") + return nil + } + fmt.Println(table(attachments)) + return nil +} + +// fatalFail reports a config/usage/pre-flight failure: a JSON error object on +// stderr under --json, else a human error line, exiting 2. +func fatalFail(msg string, code jsonout.Code) error { + if ui.IsJSON() { + _ = jsonout.EmitError(os.Stderr, command, msg, code) + } else { + ui.Error(msg) + } + return ui.SilentExit(2) +} + +// operationalFail reports an operational failure for the page: under --json a +// results[0] entry {ok:false,error,code}, else a human error line, exiting 1. +func operationalFail(pageID string, err error, code jsonout.Code) error { + if ui.IsJSON() { + res := map[string]any{"ok": false, "page_id": pageID, "error": err.Error(), "code": code} + env := jsonout.NewEnvelope(command, []any{res}, + map[string]int{"total": 1, "succeeded": 0, "failed": 1}) + _ = jsonout.Emit(os.Stdout, env) + } else { + ui.Error(err.Error()) + } + return ui.SilentExit(1) +} + +// table renders the attachments as aligned columns. Widths are measured over the +// rows rather than fixed, so a page of short names doesn't print a sparse table. +func table(attachments []client.Attachment) string { + rows := make([][5]string, 0, len(attachments)+1) + rows = append(rows, [5]string{"NAME", "SIZE", "VER", "TYPE", "SOURCE"}) + for _, a := range attachments { + source := "-" + if m := a.Meta(); m.Source != "" { + source = m.Source + } + rows = append(rows, [5]string{ + a.Title, + humanSize(a.Extensions.FileSize), + strconv.Itoa(a.Version.Number), + a.Extensions.MediaType, + source, + }) + } + + var widths [5]int + for _, r := range rows { + for i, cell := range r { + if n := len([]rune(cell)); n > widths[i] { + widths[i] = n + } + } + } + + var b strings.Builder + for i, r := range rows { + if i > 0 { + b.WriteByte('\n') + } + for j, cell := range r { + if j > 0 { + b.WriteString(" ") + } + // The last column needs no padding, which also avoids trailing spaces. + if j == len(r)-1 { + b.WriteString(cell) + continue + } + pad := widths[j] - len([]rune(cell)) + // SIZE and VER are numeric, so right-align them. + if j == 1 || j == 2 { + b.WriteString(strings.Repeat(" ", pad) + cell) + } else { + b.WriteString(cell + strings.Repeat(" ", pad)) + } + } + } + return b.String() +} + +// humanSize renders a byte count in the largest unit that keeps it under 1024, +// with one decimal place above bytes. +func humanSize(n int64) string { + const unit = 1024 + if n < unit { + return fmt.Sprintf("%d B", n) + } + div, exp := int64(unit), 0 + for m := n / unit; m >= unit && exp < 3; m /= unit { + div *= unit + exp++ + } + return fmt.Sprintf("%.1f %cB", float64(n)/float64(div), "KMGT"[exp]) +} diff --git a/cmd/attachmentlist/attachmentlist_test.go b/cmd/attachmentlist/attachmentlist_test.go new file mode 100644 index 0000000..815b3ea --- /dev/null +++ b/cmd/attachmentlist/attachmentlist_test.go @@ -0,0 +1,81 @@ +package attachmentlist + +import ( + "strings" + "testing" + + "github.com/mozilla/markfluence/internal/client" +) + +func TestHumanSize(t *testing.T) { + cases := []struct { + n int64 + want string + }{ + {0, "0 B"}, + {171, "171 B"}, + {1023, "1023 B"}, + {1024, "1.0 KB"}, + {24680, "24.1 KB"}, + {1024 * 1024, "1.0 MB"}, + {1258291, "1.2 MB"}, + {1024 * 1024 * 1024, "1.0 GB"}, + {1024 * 1024 * 1024 * 1024, "1.0 TB"}, + } + for _, c := range cases { + if got := humanSize(c.n); got != c.want { + t.Errorf("humanSize(%d) = %q, want %q", c.n, got, c.want) + } + } +} + +func TestTable(t *testing.T) { + managed := client.Attachment{ID: "att1", Title: "assets%2Fx.png"} + managed.Metadata.Comment = "markfluence: sha256=abc path=assets/x.png" + managed.Version.Number = 3 + managed.Extensions.MediaType = "image/png" + managed.Extensions.FileSize = 24680 + + hand := client.Attachment{ID: "att2", Title: "notes.pdf"} + hand.Version.Number = 1 + hand.Extensions.MediaType = "application/pdf" + hand.Extensions.FileSize = 171 + + got := table([]client.Attachment{managed, hand}) + lines := strings.Split(got, "\n") + if len(lines) != 3 { + t.Fatalf("got %d lines, want a header plus 2 rows:\n%s", len(lines), got) + } + if !strings.HasPrefix(lines[0], "NAME") || !strings.Contains(lines[0], "SOURCE") { + t.Errorf("header = %q", lines[0]) + } + if !strings.Contains(lines[1], "assets/x.png") { + t.Errorf("managed row is missing its source: %q", lines[1]) + } + // A hand-uploaded attachment reads as a dash, not a blank. + if !strings.HasSuffix(lines[2], "-") { + t.Errorf("hand-uploaded row should end in a dash: %q", lines[2]) + } + for i, l := range lines { + if l != strings.TrimRight(l, " ") { + t.Errorf("line %d has trailing whitespace: %q", i, l) + } + } +} + +// TestTableAlignsColumns checks the columns actually line up, which is the only +// reason to measure widths at all. +func TestTableAlignsColumns(t *testing.T) { + short := client.Attachment{ID: "a", Title: "a.png"} + short.Extensions.MediaType = "image/png" + long := client.Attachment{ID: "b", Title: "a-considerably-longer-name.png"} + long.Extensions.MediaType = "image/png" + + lines := strings.Split(table([]client.Attachment{short, long}), "\n") + col := strings.Index(lines[0], "SIZE") + for i, l := range lines[1:] { + if strings.Index(l, "B") < col { + t.Errorf("row %d size column starts before the header's: %q", i, l) + } + } +} diff --git a/cmd/attachmentlist/json.go b/cmd/attachmentlist/json.go new file mode 100644 index 0000000..e0b3d37 --- /dev/null +++ b/cmd/attachmentlist/json.go @@ -0,0 +1,55 @@ +package attachmentlist + +import "github.com/mozilla/markfluence/internal/client" + +// jsonListResult is attachment-list's --json result shape: one object per +// attachment, so `.results[] | .filename` works directly and summary.total is +// the attachment count. +// +// comment is the raw stored comment, and managed/sha256/source are what +// markfluence parses out of it -- both are emitted so a script never has to +// re-parse the comment itself, and nothing markfluence knows is hidden. +// +// source is null for a hand-uploaded attachment and also for one published +// before source paths were recorded; managed distinguishes them, which is why +// it is a field of its own rather than inferred from source being null. +// +// There is deliberately no download_url: built on the site URL it fails under a +// scoped token, and built on the request base it would leak the gateway host +// into reader-facing output. attachment-download is how bytes are fetched. +type jsonListResult struct { + OK bool `json:"ok"` + ID string `json:"id"` + Filename string `json:"filename"` + Size int64 `json:"size"` + MediaType string `json:"media_type"` + Version int `json:"version"` + Comment string `json:"comment"` + Managed bool `json:"managed"` + SHA256 *string `json:"sha256"` + Source *string `json:"source"` +} + +func buildResult(a client.Attachment) jsonListResult { + m := a.Meta() + return jsonListResult{ + OK: true, + ID: a.ID, + Filename: a.Title, + Size: a.Extensions.FileSize, + MediaType: a.Extensions.MediaType, + Version: a.Version.Number, + Comment: a.Metadata.Comment, + Managed: m.Managed, + SHA256: nullable(m.SHA256), + Source: nullable(m.Source), + } +} + +// nullable maps an empty string to a JSON null, else a pointer to the value. +func nullable(s string) *string { + if s == "" { + return nil + } + return &s +} diff --git a/cmd/attachmentlist/json_test.go b/cmd/attachmentlist/json_test.go new file mode 100644 index 0000000..a8cf3d0 --- /dev/null +++ b/cmd/attachmentlist/json_test.go @@ -0,0 +1,112 @@ +package attachmentlist + +import ( + "bytes" + "encoding/json" + "testing" + + "github.com/mozilla/markfluence/internal/client" + "github.com/mozilla/markfluence/internal/jsonout" + "github.com/mozilla/markfluence/internal/schematest" +) + +func TestSchemaConformance(t *testing.T) { + managed := client.Attachment{ID: "att1", Title: "assets%2Fx.png"} + managed.Metadata.Comment = "markfluence: sha256=abc path=assets/x.png" + managed.Version.Number = 3 + managed.Extensions.MediaType = "image/png" + managed.Extensions.FileSize = 171 + + hand := client.Attachment{ID: "att2", Title: "notes.pdf"} + hand.Extensions.MediaType = "application/pdf" + hand.Extensions.FileSize = 2048 + hand.Version.Number = 1 + + env := jsonout.NewEnvelope(command, + []any{buildResult(managed), buildResult(hand)}, + map[string]int{"total": 2, "succeeded": 2, "failed": 0}) + var buf bytes.Buffer + if err := jsonout.Emit(&buf, env); err != nil { + t.Fatalf("Emit: %v", err) + } + schematest.ValidateEnvelope(t, buf.Bytes()) + + failRes := map[string]any{"ok": false, "page_id": "9", "error": "page 9 not found", "code": jsonout.CodeNotFound} + failEnv := jsonout.NewEnvelope(command, []any{failRes}, + map[string]int{"total": 1, "succeeded": 0, "failed": 1}) + buf.Reset() + if err := jsonout.Emit(&buf, failEnv); err != nil { + t.Fatalf("Emit: %v", err) + } + schematest.ValidateEnvelope(t, buf.Bytes()) + + // A page with no attachments still has to conform. + emptyEnv := jsonout.NewEnvelope(command, nil, + map[string]int{"total": 0, "succeeded": 0, "failed": 0}) + buf.Reset() + if err := jsonout.Emit(&buf, emptyEnv); err != nil { + t.Fatalf("Emit: %v", err) + } + schematest.ValidateEnvelope(t, buf.Bytes()) +} + +func TestBuildResultManaged(t *testing.T) { + a := client.Attachment{ID: "att1", Title: "assets%2Fx.png"} + a.Metadata.Comment = "markfluence: sha256=abc path=assets/x.png" + res := buildResult(a) + if !res.Managed { + t.Error("managed = false, want true") + } + if res.Source == nil || *res.Source != "assets/x.png" { + t.Errorf("source = %v, want assets/x.png", res.Source) + } + if res.SHA256 == nil || *res.SHA256 != "abc" { + t.Errorf("sha256 = %v, want abc", res.SHA256) + } + // The stored name is reported as-is; it is what identifies the attachment. + if res.Filename != "assets%2Fx.png" { + t.Errorf("filename = %q, want the stored name", res.Filename) + } +} + +// TestBuildResultLegacyManaged covers the attachment every page published +// before the encoding change still carries: a legacy checksum comment, so it is +// managed and has a checksum but no recorded source. It must not be reported as +// hand-uploaded -- managed is what tells the two apart. +func TestBuildResultLegacyManaged(t *testing.T) { + a := client.Attachment{ID: "att3", Title: "assets_x.png"} + a.Metadata.Comment = "mzcld:checksum: e733ac00" + res := buildResult(a) + if !res.Managed { + t.Error("managed = false, want true for a legacy comment") + } + if res.SHA256 == nil || *res.SHA256 != "e733ac00" { + t.Errorf("sha256 = %v, want e733ac00", res.SHA256) + } + if res.Source != nil { + t.Errorf("source = %v, want null", res.Source) + } +} + +// TestBuildResultHandUploadedNullsMetadata is the signal attachment-list exists +// to give: an attachment publishing will not touch. +func TestBuildResultHandUploadedNullsMetadata(t *testing.T) { + a := client.Attachment{ID: "att2", Title: "notes.pdf"} + res := buildResult(a) + if res.Managed { + t.Error("managed = true, want false") + } + b, err := json.Marshal(res) + if err != nil { + t.Fatal(err) + } + var round map[string]any + if err := json.Unmarshal(b, &round); err != nil { + t.Fatal(err) + } + for _, k := range []string{"sha256", "source"} { + if round[k] != nil { + t.Errorf("%s = %v, want null", k, round[k]) + } + } +} diff --git a/cmd/root.go b/cmd/root.go index c61e5c8..2bb4f92 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -6,6 +6,7 @@ import ( "os" "strings" + "github.com/mozilla/markfluence/cmd/attachmentlist" "github.com/mozilla/markfluence/cmd/create" "github.com/mozilla/markfluence/cmd/fix" "github.com/mozilla/markfluence/cmd/info" @@ -136,4 +137,5 @@ func init() { rootCmd.AddCommand(fix.Cmd) rootCmd.AddCommand(info.Cmd) rootCmd.AddCommand(read.Cmd) + rootCmd.AddCommand(attachmentlist.Cmd) } diff --git a/schema/json-output/v1.json b/schema/json-output/v1.json index 6a1cf19..58307fb 100644 --- a/schema/json-output/v1.json +++ b/schema/json-output/v1.json @@ -2,14 +2,14 @@ "$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 info/read); 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 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.", "type": "object", "additionalProperties": false, "required": ["schema_version", "markfluence_version", "command", "results", "summary"], "properties": { "schema_version": { "const": 1 }, "markfluence_version": { "type": "string" }, - "command": { "enum": ["info", "read", "update", "create", "fix"] }, + "command": { "enum": ["info", "read", "update", "create", "fix", "attachment-list"] }, "results": { "type": "array" }, "summary": { "type": "object" } }, @@ -58,6 +58,19 @@ "summary": { "$ref": "#/$defs/fixSummary" } } } + }, + { + "if": { "properties": { "command": { "const": "attachment-list" } }, "required": ["command"] }, + "then": { + "properties": { + "results": { + "items": { + "oneOf": [{ "$ref": "#/$defs/attachmentListResult" }, { "$ref": "#/$defs/singleOpFailure" }] + } + }, + "summary": { "$ref": "#/$defs/basicSummary" } + } + } } ], "$defs": { @@ -270,8 +283,26 @@ "code": { "$ref": "#/$defs/codeOrNull" } } }, + "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 }, + "id": { "type": "string" }, + "filename": { "type": "string" }, + "size": { "type": "integer" }, + "media_type": { "type": "string" }, + "version": { "type": "integer" }, + "comment": { "type": "string" }, + "managed": { "type": "boolean" }, + "sha256": { "$ref": "#/$defs/stringOrNull" }, + "source": { "$ref": "#/$defs/stringOrNull" } + } + }, "basicSummary": { - "description": "info/read batch summary (always total:1).", + "description": "info/read batch summary (total:1), and attachment-list (total: the attachment count).", "type": "object", "additionalProperties": false, "required": ["total", "succeeded", "failed"], From 7c10a04414b2a153e1a970b45400e83463aaf1e5 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 5 Aug 2026 19:40:27 -0400 Subject: [PATCH 5/8] feat(cmd): add attachment-upload Upload or replace attachments on a page, reusing the checksum bookkeeping create/update already use so uploading by hand and publishing agree on what is current. --dry-run reuses PlanAttachments; --force uploads regardless, which is how a user repairs an attachment whose stored bytes drifted while its recorded checksum still matches. --name takes a path rather than a stored name, and markfluence encodes it, so `--name assets/x.png` produces the attachment an image written as ![](assets/x.png) resolves to without the user typing an escape. The recorded path= is always the decode of the stored name: were they allowed to disagree, a later publish would upload a second attachment under the name it computes while a restoring download put this one somewhere the markdown never references. planAttachments now records the existing id even for a skip, so a forced upload replaces in place instead of re-deriving it. Verified against a live page: created, then skipped unchanged, then --force bumped the version 1 -> 2. Refs #9 --- cmd/attachmentupload/attachmentupload.go | 218 ++++++++++++++++++ cmd/attachmentupload/attachmentupload_test.go | 117 ++++++++++ cmd/attachmentupload/json.go | 25 ++ cmd/attachmentupload/json_test.go | 70 ++++++ cmd/root.go | 2 + internal/client/client.go | 27 ++- schema/json-output/v1.json | 41 +++- 7 files changed, 498 insertions(+), 2 deletions(-) create mode 100644 cmd/attachmentupload/attachmentupload.go create mode 100644 cmd/attachmentupload/attachmentupload_test.go create mode 100644 cmd/attachmentupload/json.go create mode 100644 cmd/attachmentupload/json_test.go diff --git a/cmd/attachmentupload/attachmentupload.go b/cmd/attachmentupload/attachmentupload.go new file mode 100644 index 0000000..77239b4 --- /dev/null +++ b/cmd/attachmentupload/attachmentupload.go @@ -0,0 +1,218 @@ +// Package attachmentupload implements the `markfluence attachment-upload` +// command: upload or replace a page's attachments. +package attachmentupload + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/mozilla/markfluence/internal/client" + "github.com/mozilla/markfluence/internal/convert" + "github.com/mozilla/markfluence/internal/jsonout" + "github.com/mozilla/markfluence/internal/pageref" + "github.com/mozilla/markfluence/internal/ui" + "github.com/spf13/cobra" +) + +// command is the name used in help and as the --json command discriminator. +const command = "attachment-upload" + +var ( + nameFlag string + force bool + dryRun bool +) + +// Cmd is the attachment-upload command. +var Cmd = &cobra.Command{ + Use: command + " ARG FILE...", + Short: "Upload or replace attachments on a Confluence page", + Long: "Upload or replace attachments on a Confluence page.\n\n" + + "ARG is a numeric page id, a Confluence page URL, or a markdown file\n" + + "whose frontmatter has a page_id.\n\n" + + "Each file is attached under its base name. A file whose contents\n" + + "already match the attachment on the page is skipped, using the same\n" + + "checksum bookkeeping create/update use, so uploading by hand and\n" + + "publishing agree on what is current; --force uploads anyway.\n\n" + + "--name sets the attachment name for a single file, and takes a path:\n" + + "markfluence encodes it the way publishing would, so `--name\n" + + "assets/x.png` produces the attachment an image written as\n" + + "![](assets/x.png) resolves to.", + Args: cobra.MinimumNArgs(2), + RunE: run, +} + +func init() { + Cmd.Flags().StringVar(&nameFlag, "name", "", + "Attachment name, given as a path (requires a single FILE).") + Cmd.Flags().BoolVar(&force, "force", false, + "Upload even when the checksum shows the attachment is unchanged.") + Cmd.Flags().BoolVar(&dryRun, "dry-run", false, + "Preview what would be uploaded without writing to Confluence.") +} + +func run(cmd *cobra.Command, args []string) error { + files := args[1:] + if nameFlag != "" && len(files) != 1 { + return fatalFail("--name applies to a single attachment; pass exactly one FILE", + jsonout.CodeValidation) + } + + url, _ := cmd.Flags().GetString("url") + username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") + envFile, _ := cmd.Flags().GetString("env-file") + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) + if err != nil { + return fatalFail(err.Error(), jsonout.CodeConfig) + } + + pageID, err := pageref.Resolve(args[0]) + if err != nil { + return fatalFail(err.Error(), jsonout.CodeValidation) + } + + attachments, err := localAttachments(files, nameFlag) + if err != nil { + return fatalFail(err.Error(), jsonout.CodeIO) + } + + if dryRun && !ui.IsJSON() { + ui.Warn("DRY RUN — no changes will be written.") + } + + actions, err := plan(c, pageID, attachments) + if err != nil { + return operationalFail(pageID, err, jsonout.CodeFor(err)) + } + return report(actions) +} + +// plan performs the upload, or -- under --dry-run -- only the classification. +// --force turns every non-skip into an upload by classifying nothing as +// unchanged, which is why it is applied here rather than inside the client. +func plan(c *client.ConfluenceClient, pageID string, attachments []client.LocalAttachment) ( + []client.SyncAction, error, +) { + if dryRun { + actions, err := c.PlanAttachments(pageID, attachments) + if err != nil { + return nil, err + } + if force { + actions = forced(actions) + } + return actions, nil + } + if force { + return c.ForceUploadAttachments(pageID, attachments) + } + return c.SyncAttachments(pageID, attachments) +} + +// forced rewrites a dry-run plan for --force: what would have been skipped is +// instead updated, matching what a forced run actually does. +func forced(actions []client.SyncAction) []client.SyncAction { + out := make([]client.SyncAction, len(actions)) + for i, a := range actions { + if a.Action == "skipped" { + a.Action = "updated" + } + out[i] = a + } + return out +} + +// localAttachments resolves each file into an upload, checking readability up +// front so a batch fails before it has half-uploaded. +// +// The attachment name is the file's base name, or the encoding of --name. The +// recorded source is always the decode of the name, never the local path: if +// the two disagreed, a later publish would upload a second attachment under the +// name it computes while a download restored this one somewhere the markdown +// never references. +func localAttachments(files []string, name string) ([]client.LocalAttachment, error) { + out := make([]client.LocalAttachment, 0, len(files)) + for _, f := range files { + info, err := os.Stat(f) + if err != nil { + return nil, err + } + if info.IsDir() { + return nil, fmt.Errorf("%s is a directory", f) + } + source := filepath.Base(f) + if name != "" { + source = name + } + filename := convert.AttachmentFilename(source) + if filename == "" { + return nil, fmt.Errorf("%q is not a usable attachment name", source) + } + // Round-trip the name so source is exactly what a decode yields. + if decoded, ok := convert.AttachmentSource(filename); ok { + source = decoded + } + out = append(out, client.LocalAttachment{Path: f, Filename: filename, Source: source}) + } + return out, nil +} + +// report prints the per-file actions and returns the command's exit status. +func report(actions []client.SyncAction) error { + if ui.IsJSON() { + results := make([]any, 0, len(actions)) + skipped := 0 + for _, a := range actions { + if a.Action == "skipped" { + skipped++ + } + results = append(results, buildResult(a)) + } + env := jsonout.NewEnvelope(command, results, map[string]int{ + "total": len(actions), "succeeded": len(actions), "failed": 0, "skipped": skipped, + }) + return jsonout.Emit(os.Stdout, env) + } + for _, a := range actions { + line := fmt.Sprintf("%-8s %s", a.Action, a.Filename) + if a.Action == "skipped" { + ui.Dim(line + " (unchanged)") + continue + } + ui.Success(line) + } + return nil +} + +// fatalFail reports a config/usage/pre-flight failure: a JSON error object on +// stderr under --json, else a human error line, exiting 2. +func fatalFail(msg string, code jsonout.Code) error { + if ui.IsJSON() { + _ = jsonout.EmitError(os.Stderr, command, msg, code) + } else { + ui.Error(msg) + } + return ui.SilentExit(2) +} + +// operationalFail reports a failure against the page: under --json a results[0] +// entry {ok:false,error,code}, else a human error line, exiting 1. +func operationalFail(pageID string, err error, code jsonout.Code) error { + if ui.IsJSON() { + res := map[string]any{"ok": false, "page_id": pageID, "error": err.Error(), "code": code} + env := jsonout.NewEnvelope(command, []any{res}, + map[string]int{"total": 1, "succeeded": 0, "failed": 1, "skipped": 0}) + _ = jsonout.Emit(os.Stdout, env) + } else { + ui.Error(err.Error()) + } + return ui.SilentExit(1) +} + +// decodeName is convert.AttachmentSource, wrapped so tests can assert the +// lockstep invariant without importing the converter. +func decodeName(filename string) (string, bool) { return convert.AttachmentSource(filename) } diff --git a/cmd/attachmentupload/attachmentupload_test.go b/cmd/attachmentupload/attachmentupload_test.go new file mode 100644 index 0000000..6475405 --- /dev/null +++ b/cmd/attachmentupload/attachmentupload_test.go @@ -0,0 +1,117 @@ +package attachmentupload + +import ( + "os" + "path/filepath" + "testing" + + "github.com/mozilla/markfluence/internal/client" +) + +func writeFile(t *testing.T, dir, name string) string { + t.Helper() + path := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("bytes"), 0o644); err != nil { + t.Fatal(err) + } + return path +} + +func TestLocalAttachmentsUsesBaseName(t *testing.T) { + dir := t.TempDir() + path := writeFile(t, dir, "docs/assets/x.png") + + got, err := localAttachments([]string{path}, "") + if err != nil { + t.Fatal(err) + } + if len(got) != 1 { + t.Fatalf("got %d attachments, want 1", len(got)) + } + if got[0].Filename != "x.png" { + t.Errorf("filename = %q, want x.png", got[0].Filename) + } + if got[0].Source != "x.png" { + t.Errorf("source = %q, want x.png", got[0].Source) + } + if got[0].Path != path { + t.Errorf("path = %q, want %q", got[0].Path, path) + } +} + +// TestLocalAttachmentsNameEncodesPath is the point of --name taking a path: the +// user writes a path and markfluence produces the attachment a publish of +// ![](assets/x.png) would resolve to, without them typing an escape. +func TestLocalAttachmentsNameEncodesPath(t *testing.T) { + dir := t.TempDir() + path := writeFile(t, dir, "somewhere/else.png") + + got, err := localAttachments([]string{path}, "assets/x.png") + if err != nil { + t.Fatal(err) + } + if got[0].Filename != "assets%2Fx.png" { + t.Errorf("filename = %q, want assets%%2Fx.png", got[0].Filename) + } + if got[0].Source != "assets/x.png" { + t.Errorf("source = %q, want assets/x.png", got[0].Source) + } +} + +// TestLocalAttachmentsSourceIsAlwaysTheDecodedName is the lockstep invariant: +// if the recorded path and the stored name could disagree, a later publish +// would upload a second attachment while a restoring download put this one +// where the markdown never references it. +func TestLocalAttachmentsSourceIsAlwaysTheDecodedName(t *testing.T) { + dir := t.TempDir() + path := writeFile(t, dir, "f.png") + + for _, name := range []string{"", "assets/x.png", "./a/./b.png", "../shared/logo.png", "plain.png"} { + got, err := localAttachments([]string{path}, name) + if err != nil { + t.Fatalf("--name %q: %v", name, err) + } + decoded, ok := decodeName(got[0].Filename) + if !ok { + t.Errorf("--name %q: stored name %q does not decode", name, got[0].Filename) + continue + } + if decoded != got[0].Source { + t.Errorf("--name %q: source %q != decode of %q (%q)", + name, got[0].Source, got[0].Filename, decoded) + } + } +} + +func TestLocalAttachmentsRejectsMissingAndDirs(t *testing.T) { + dir := t.TempDir() + if _, err := localAttachments([]string{filepath.Join(dir, "nope.png")}, ""); err == nil { + t.Error("want an error for a missing file") + } + if _, err := localAttachments([]string{dir}, ""); err == nil { + t.Error("want an error for a directory") + } +} + +// TestForcedRewritesSkips covers the dry-run forecast for --force: what would +// have been skipped is reported as updated, matching what a forced run does. +func TestForcedRewritesSkips(t *testing.T) { + in := []client.SyncAction{ + {Filename: "a.png", Action: "skipped"}, + {Filename: "b.png", Action: "created"}, + {Filename: "c.png", Action: "updated"}, + } + got := forced(in) + want := []string{"updated", "created", "updated"} + for i := range want { + if got[i].Action != want[i] { + t.Errorf("[%d] action = %q, want %q", i, got[i].Action, want[i]) + } + } + if in[0].Action != "skipped" { + t.Error("forced mutated its input") + } +} diff --git a/cmd/attachmentupload/json.go b/cmd/attachmentupload/json.go new file mode 100644 index 0000000..e1198f7 --- /dev/null +++ b/cmd/attachmentupload/json.go @@ -0,0 +1,25 @@ +package attachmentupload + +import "github.com/mozilla/markfluence/internal/client" + +// jsonUploadResult is attachment-upload's --json result shape: one object per +// file. status uses the same created/updated/skipped verbs the attachments +// array on update and create already reports, so a script that understands one +// understands the other. +type jsonUploadResult struct { + OK bool `json:"ok"` + Status string `json:"status"` + DryRun bool `json:"dry_run"` + Filename string `json:"filename"` + Error *string `json:"error"` + Code *string `json:"code"` +} + +func buildResult(a client.SyncAction) jsonUploadResult { + return jsonUploadResult{ + OK: true, + Status: a.Action, + DryRun: dryRun, + Filename: a.Filename, + } +} diff --git a/cmd/attachmentupload/json_test.go b/cmd/attachmentupload/json_test.go new file mode 100644 index 0000000..6629dcf --- /dev/null +++ b/cmd/attachmentupload/json_test.go @@ -0,0 +1,70 @@ +package attachmentupload + +import ( + "bytes" + "encoding/json" + "testing" + + "github.com/mozilla/markfluence/internal/client" + "github.com/mozilla/markfluence/internal/jsonout" + "github.com/mozilla/markfluence/internal/schematest" +) + +func TestSchemaConformance(t *testing.T) { + results := []any{ + buildResult(client.SyncAction{Filename: "a.png", Action: "created"}), + buildResult(client.SyncAction{Filename: "b.png", Action: "updated"}), + buildResult(client.SyncAction{Filename: "c.png", Action: "skipped"}), + } + env := jsonout.NewEnvelope(command, results, + map[string]int{"total": 3, "succeeded": 3, "failed": 0, "skipped": 1}) + var buf bytes.Buffer + if err := jsonout.Emit(&buf, env); err != nil { + t.Fatalf("Emit: %v", err) + } + schematest.ValidateEnvelope(t, buf.Bytes()) + + failRes := map[string]any{"ok": false, "page_id": "9", "error": "page 9 not found", "code": jsonout.CodeNotFound} + failEnv := jsonout.NewEnvelope(command, []any{failRes}, + map[string]int{"total": 1, "succeeded": 0, "failed": 1, "skipped": 0}) + buf.Reset() + if err := jsonout.Emit(&buf, failEnv); err != nil { + t.Fatalf("Emit: %v", err) + } + schematest.ValidateEnvelope(t, buf.Bytes()) +} + +func TestSchemaConformanceDryRun(t *testing.T) { + dryRun = true + t.Cleanup(func() { dryRun = false }) + + res := buildResult(client.SyncAction{Filename: "a.png", Action: "created"}) + if !res.DryRun { + t.Error("dry_run = false, want true") + } + env := jsonout.NewEnvelope(command, []any{res}, + map[string]int{"total": 1, "succeeded": 1, "failed": 0, "skipped": 0}) + var buf bytes.Buffer + if err := jsonout.Emit(&buf, env); err != nil { + t.Fatalf("Emit: %v", err) + } + schematest.ValidateEnvelope(t, buf.Bytes()) +} + +func TestJSONUploadResultMarshal(t *testing.T) { + b, err := json.MarshalIndent(buildResult(client.SyncAction{Filename: "a.png", Action: "created"}), "", " ") + if err != nil { + t.Fatal(err) + } + want := `{ + "ok": true, + "status": "created", + "dry_run": false, + "filename": "a.png", + "error": null, + "code": null +}` + if string(b) != want { + t.Errorf("upload result mismatch:\n got:\n%s\n want:\n%s", b, want) + } +} diff --git a/cmd/root.go b/cmd/root.go index 2bb4f92..e003614 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/mozilla/markfluence/cmd/attachmentlist" + "github.com/mozilla/markfluence/cmd/attachmentupload" "github.com/mozilla/markfluence/cmd/create" "github.com/mozilla/markfluence/cmd/fix" "github.com/mozilla/markfluence/cmd/info" @@ -138,4 +139,5 @@ func init() { rootCmd.AddCommand(info.Cmd) rootCmd.AddCommand(read.Cmd) rootCmd.AddCommand(attachmentlist.Cmd) + rootCmd.AddCommand(attachmentupload.Cmd) } diff --git a/internal/client/client.go b/internal/client/client.go index a72415e..81a6282 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -671,6 +671,11 @@ func (c *ConfluenceClient) planAttachments(pageID string, attachments []LocalAtt } p := attachmentPlan{att: att, comment: comment, contentType: contentType} cur, ok := remote[att.Filename] + if ok { + // Recorded even for a skip, so a forced upload can replace in place + // rather than having to re-derive the id. + p.existingID = cur.ID + } switch { case !ok: p.action = "created" @@ -681,7 +686,6 @@ func (c *ConfluenceClient) planAttachments(pageID string, attachments []LocalAtt p.action = "skipped" default: p.action = "updated" - p.existingID = cur.ID } plans = append(plans, p) } @@ -706,12 +710,33 @@ func (c *ConfluenceClient) PlanAttachments(pageID string, attachments []LocalAtt // local files, using a SHA-256 stored in each attachment's comment to detect // changes. Returns one action per file. func (c *ConfluenceClient) SyncAttachments(pageID string, attachments []LocalAttachment) ([]SyncAction, error) { + return c.syncAttachments(pageID, attachments, false) +} + +// ForceUploadAttachments uploads every file regardless of its checksum, +// bumping each attachment's version. It exists for `attachment-upload --force`, +// which is how a user repairs an attachment whose stored bytes drifted while +// its recorded checksum still matches. +func (c *ConfluenceClient) ForceUploadAttachments(pageID string, attachments []LocalAttachment) ( + []SyncAction, error, +) { + return c.syncAttachments(pageID, attachments, true) +} + +// syncAttachments executes a plan. When force is set, a file the checksum says +// is unchanged is uploaded anyway and reported as updated. +func (c *ConfluenceClient) syncAttachments(pageID string, attachments []LocalAttachment, force bool) ( + []SyncAction, error, +) { plans, err := c.planAttachments(pageID, attachments) if err != nil { return nil, err } var actions []SyncAction for _, p := range plans { + if force && p.action == "skipped" { + p.action = "updated" + } switch p.action { case "created": if err := c.uploadAttachment( diff --git a/schema/json-output/v1.json b/schema/json-output/v1.json index 58307fb..24f821e 100644 --- a/schema/json-output/v1.json +++ b/schema/json-output/v1.json @@ -9,7 +9,7 @@ "properties": { "schema_version": { "const": 1 }, "markfluence_version": { "type": "string" }, - "command": { "enum": ["info", "read", "update", "create", "fix", "attachment-list"] }, + "command": { "enum": ["info", "read", "update", "create", "fix", "attachment-list", "attachment-upload"] }, "results": { "type": "array" }, "summary": { "type": "object" } }, @@ -59,6 +59,19 @@ } } }, + { + "if": { "properties": { "command": { "const": "attachment-upload" } }, "required": ["command"] }, + "then": { + "properties": { + "results": { + "items": { + "oneOf": [{ "$ref": "#/$defs/attachmentUploadResult" }, { "$ref": "#/$defs/singleOpFailure" }] + } + }, + "summary": { "$ref": "#/$defs/attachmentSummary" } + } + } + }, { "if": { "properties": { "command": { "const": "attachment-list" } }, "required": ["command"] }, "then": { @@ -283,6 +296,20 @@ "code": { "$ref": "#/$defs/codeOrNull" } } }, + "attachmentUploadResult": { + "description": "One uploaded file. status uses the same verbs as the attachments array on update/create.", + "type": "object", + "additionalProperties": false, + "required": ["ok", "status", "dry_run", "filename", "error", "code"], + "properties": { + "ok": { "type": "boolean" }, + "status": { "enum": ["created", "updated", "skipped", "failed"] }, + "dry_run": { "type": "boolean" }, + "filename": { "type": "string" }, + "error": { "$ref": "#/$defs/stringOrNull" }, + "code": { "$ref": "#/$defs/codeOrNull" } + } + }, "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", @@ -301,6 +328,18 @@ "source": { "$ref": "#/$defs/stringOrNull" } } }, + "attachmentSummary": { + "description": "attachment-upload/attachment-download batch summary.", + "type": "object", + "additionalProperties": false, + "required": ["total", "succeeded", "failed", "skipped"], + "properties": { + "total": { "type": "integer" }, + "succeeded": { "type": "integer" }, + "failed": { "type": "integer" }, + "skipped": { "type": "integer" } + } + }, "basicSummary": { "description": "info/read batch summary (total:1), and attachment-list (total: the attachment count).", "type": "object", From f23ebc59bc1238636ca4d516ded64a5f36cad731 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 5 Aug 2026 19:55:40 -0400 Subject: [PATCH 6/8] feat(cmd): add attachment-download Write a page's attachments to disk. An attachment markfluence published is restored to the markdown image path recorded in its comment, so the downloaded tree matches what the page references and previews locally in GitHub or VSCode; --flat writes stored names instead. With no NAME every attachment is downloaded; an existing file is skipped unless --force, and a NAME the page doesn't have fails that item rather than the run. Restoration reads the recorded path and never decodes the stored name. There is no way to tell a hand-uploaded "a%2Fb.png" from one markfluence published, so decoding by default would scatter a literally-named file into a/b.png; an attachment with no recorded path keeps its stored name. destPath is the only place server data becomes a filesystem path, so it clamps to --dest. ".." cannot simply be refused -- an image in a directory above its page is a supported layout -- so the resolved path is compared against the root, and escaping is an error rather than a silent clip: attachment comments are controlled by anyone who can edit the page. This takes over the clamp 018 deferred to #37. Verified against a live page: managed attachments restored to assets/markfluence-test.png and probe/notes.txt, the underscore-era orphan written under its stored name, bytes byte-identical to the source, --flat and --force as specified, and a missing name exiting 1. Refs #9 --- cmd/attachmentdownload/attachmentdownload.go | 290 ++++++++++++++++++ .../attachmentdownload_test.go | 145 +++++++++ cmd/attachmentdownload/json.go | 42 +++ cmd/attachmentdownload/json_test.go | 83 +++++ cmd/root.go | 2 + schema/json-output/v1.json | 30 +- 6 files changed, 591 insertions(+), 1 deletion(-) create mode 100644 cmd/attachmentdownload/attachmentdownload.go create mode 100644 cmd/attachmentdownload/attachmentdownload_test.go create mode 100644 cmd/attachmentdownload/json.go create mode 100644 cmd/attachmentdownload/json_test.go diff --git a/cmd/attachmentdownload/attachmentdownload.go b/cmd/attachmentdownload/attachmentdownload.go new file mode 100644 index 0000000..f3f5968 --- /dev/null +++ b/cmd/attachmentdownload/attachmentdownload.go @@ -0,0 +1,290 @@ +// Package attachmentdownload implements the `markfluence attachment-download` +// command: write a page's attachments to the filesystem. +package attachmentdownload + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/mozilla/markfluence/internal/client" + "github.com/mozilla/markfluence/internal/jsonout" + "github.com/mozilla/markfluence/internal/pageref" + "github.com/mozilla/markfluence/internal/ui" + "github.com/spf13/cobra" +) + +// command is the name used in help and as the --json command discriminator. +const command = "attachment-download" + +// Per-file outcomes, mirroring attachment-upload's verbs. +const ( + statusDownloaded = "downloaded" + statusSkipped = "skipped" + statusFailed = "failed" +) + +var ( + dest string + flat bool + force bool + + dryRun bool +) + +// Cmd is the attachment-download command. +var Cmd = &cobra.Command{ + Use: command + " ARG [NAME...]", + Short: "Download a Confluence page's attachments", + Long: "Download a Confluence page's attachments.\n\n" + + "ARG is a numeric page id, a Confluence page URL, or a markdown file\n" + + "whose frontmatter has a page_id. Each NAME is an attachment name as\n" + + "attachment-list reports it; with no NAME, every attachment is\n" + + "downloaded.\n\n" + + "An attachment markfluence published records the markdown image path it\n" + + "came from, and is written back to that path under --dest, so the\n" + + "downloaded tree matches what the page's markdown references and\n" + + "previews locally. An attachment without a recorded path is written\n" + + "under its stored name. --flat writes everything under stored names.\n\n" + + "A file that already exists is skipped unless --force.", + Args: cobra.MinimumNArgs(1), + RunE: run, +} + +func init() { + Cmd.Flags().StringVar(&dest, "dest", ".", "Directory to write attachments into.") + Cmd.Flags().BoolVar(&flat, "flat", false, + "Write every attachment under its stored name, ignoring recorded paths.") + Cmd.Flags().BoolVar(&force, "force", false, "Overwrite files that already exist.") + Cmd.Flags().BoolVar(&dryRun, "dry-run", false, + "Preview what would be written without creating any files.") +} + +func run(cmd *cobra.Command, args []string) error { + url, _ := cmd.Flags().GetString("url") + username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") + envFile, _ := cmd.Flags().GetString("env-file") + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) + if err != nil { + return fatalFail(err.Error(), jsonout.CodeConfig) + } + + pageID, err := pageref.Resolve(args[0]) + if err != nil { + return fatalFail(err.Error(), jsonout.CodeValidation) + } + + attachments, err := c.ListAttachments(pageID) + if err != nil { + return operationalFail(pageID, err, jsonout.CodeFor(err)) + } + + wanted, missing := selectAttachments(attachments, args[1:]) + if dryRun && !ui.IsJSON() { + ui.Warn("DRY RUN — no files will be written.") + } + + root, err := filepath.Abs(dest) + if err != nil { + return fatalFail(err.Error(), jsonout.CodeIO) + } + + results := make([]outcome, 0, len(wanted)+len(missing)) + for _, a := range wanted { + results = append(results, download(c, a, root)) + } + // A name the page doesn't have is that name's failure, not the run's. + for _, name := range missing { + results = append(results, outcome{ + name: name, + status: statusFailed, + err: fmt.Errorf("no attachment named %q on page %s", name, pageID), + code: jsonout.CodeNotFound, + }) + } + return report(results) +} + +// outcome is what happened to one attachment. +type outcome struct { + name string // the stored attachment name + destPath string // the local path written (or that would be) + status string + err error + code jsonout.Code +} + +// selectAttachments picks the attachments named on the command line, preserving +// the order the names were given, and reports names the page doesn't have. With +// no names, every attachment is selected in server order. +func selectAttachments(attachments []client.Attachment, names []string) ( + wanted []client.Attachment, missing []string, +) { + if len(names) == 0 { + return attachments, nil + } + byName := make(map[string]client.Attachment, len(attachments)) + for _, a := range attachments { + byName[a.Title] = a + } + for _, n := range names { + if a, ok := byName[n]; ok { + wanted = append(wanted, a) + continue + } + missing = append(missing, n) + } + return wanted, missing +} + +// download resolves one attachment's destination and writes it. +func download(c *client.ConfluenceClient, a client.Attachment, root string) outcome { + res := outcome{name: a.Title} + + path, err := destPath(root, a, flat) + if err != nil { + res.status, res.err, res.code = statusFailed, err, jsonout.CodeValidation + return res + } + res.destPath = path + + if _, err := os.Stat(path); err == nil && !force { + res.status = statusSkipped + return res + } + if dryRun { + res.status = statusDownloaded + return res + } + + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + res.status, res.err, res.code = statusFailed, err, jsonout.CodeIO + return res + } + f, err := os.Create(path) + if err != nil { + res.status, res.err, res.code = statusFailed, err, jsonout.CodeIO + return res + } + defer func() { _ = f.Close() }() + if err := c.DownloadAttachment(a, f); err != nil { + res.status, res.err, res.code = statusFailed, err, jsonout.CodeFor(err) + return res + } + res.status = statusDownloaded + return res +} + +// destPath resolves where an attachment is written, and is the only place a +// server-controlled string becomes a filesystem path. +// +// The recorded source path is used, not a decode of the attachment name: there +// is no way to tell a hand-uploaded "a%2Fb.png" from one markfluence published, +// so decoding by default would scatter a literally-named file into a/b.png. An +// attachment with no recorded source keeps its stored name. +// +// The result must stay inside root. A source path may legitimately contain ".." +// -- an image in a directory above its page is a supported layout -- so ".." +// cannot simply be refused; the resolved path is compared against root instead. +// Escaping is an error rather than a silent clip, because the path comes from an +// attachment comment, which anyone who can edit the page controls. +func destPath(root string, a client.Attachment, flat bool) (string, error) { + rel := a.Title + if !flat { + if src := a.Meta().Source; src != "" { + rel = src + } + } + // A stored name is a single path element by construction; guard anyway, since + // it is server data. + if filepath.IsAbs(rel) { + return "", fmt.Errorf("attachment %q resolves to the absolute path %q", a.Title, rel) + } + + path := filepath.Join(root, filepath.FromSlash(rel)) + if path != root && !strings.HasPrefix(path, root+string(os.PathSeparator)) { + return "", fmt.Errorf("attachment %q resolves to %q, outside the destination directory", + a.Title, path) + } + if path == root { + return "", fmt.Errorf("attachment %q has no filename", a.Title) + } + return path, nil +} + +// report prints the per-attachment outcomes and returns the command's exit +// status: 1 if any attachment failed. +func report(results []outcome) error { + failed := 0 + skipped := 0 + for _, r := range results { + switch r.status { + case statusFailed: + failed++ + case statusSkipped: + skipped++ + } + } + + if ui.IsJSON() { + out := make([]any, 0, len(results)) + for _, r := range results { + out = append(out, buildResult(r)) + } + env := jsonout.NewEnvelope(command, out, map[string]int{ + "total": len(results), "succeeded": len(results) - failed, + "failed": failed, "skipped": skipped, + }) + if err := jsonout.Emit(os.Stdout, env); err != nil { + return err + } + if failed > 0 { + return ui.SilentExit(1) + } + return nil + } + + for _, r := range results { + switch r.status { + case statusSkipped: + ui.Dim(fmt.Sprintf("%-10s %s (exists; --force to overwrite)", r.status, r.destPath)) + case statusFailed: + ui.Error(fmt.Sprintf("%-10s %s: %s", r.status, r.name, r.err)) + default: + ui.Success(fmt.Sprintf("%-10s %s", r.status, r.destPath)) + } + } + if failed > 0 { + return ui.SilentExit(1) + } + return nil +} + +// fatalFail reports a config/usage/pre-flight failure: a JSON error object on +// stderr under --json, else a human error line, exiting 2. +func fatalFail(msg string, code jsonout.Code) error { + if ui.IsJSON() { + _ = jsonout.EmitError(os.Stderr, command, msg, code) + } else { + ui.Error(msg) + } + return ui.SilentExit(2) +} + +// operationalFail reports a failure against the page: under --json a results[0] +// entry {ok:false,error,code}, else a human error line, exiting 1. +func operationalFail(pageID string, err error, code jsonout.Code) error { + if ui.IsJSON() { + res := map[string]any{"ok": false, "page_id": pageID, "error": err.Error(), "code": code} + env := jsonout.NewEnvelope(command, []any{res}, + map[string]int{"total": 1, "succeeded": 0, "failed": 1, "skipped": 0}) + _ = jsonout.Emit(os.Stdout, env) + } else { + ui.Error(err.Error()) + } + return ui.SilentExit(1) +} diff --git a/cmd/attachmentdownload/attachmentdownload_test.go b/cmd/attachmentdownload/attachmentdownload_test.go new file mode 100644 index 0000000..2a8daed --- /dev/null +++ b/cmd/attachmentdownload/attachmentdownload_test.go @@ -0,0 +1,145 @@ +package attachmentdownload + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/mozilla/markfluence/internal/client" +) + +// managed builds an attachment carrying a recorded source path. +func managed(title, source string) client.Attachment { + a := client.Attachment{ID: "att1", Title: title} + a.Metadata.Comment = "markfluence: sha256=abc path=" + source + return a +} + +func TestDestPathUsesRecordedSource(t *testing.T) { + root := filepath.Clean("/tmp/dest") + got, err := destPath(root, managed("assets%2Fx.png", "assets/x.png"), false) + if err != nil { + t.Fatal(err) + } + want := filepath.Join(root, "assets", "x.png") + if got != want { + t.Errorf("destPath = %q, want %q", got, want) + } +} + +// TestDestPathIgnoresNameWhenUnmanaged is why restoration reads the comment and +// never decodes the stored name: a hand-uploaded file literally named +// "a%2Fb.png" must not be scattered into a/b.png. +func TestDestPathIgnoresNameWhenUnmanaged(t *testing.T) { + root := filepath.Clean("/tmp/dest") + got, err := destPath(root, client.Attachment{Title: "a%2Fb.png"}, false) + if err != nil { + t.Fatal(err) + } + want := filepath.Join(root, "a%2Fb.png") + if got != want { + t.Errorf("destPath = %q, want the literal stored name %q", got, want) + } +} + +func TestDestPathFlatIgnoresSource(t *testing.T) { + root := filepath.Clean("/tmp/dest") + got, err := destPath(root, managed("assets%2Fx.png", "assets/x.png"), true) + if err != nil { + t.Fatal(err) + } + want := filepath.Join(root, "assets%2Fx.png") + if got != want { + t.Errorf("destPath = %q, want %q", got, want) + } +} + +// TestDestPathAllowsLegitimateParent covers the supported shared-asset layout: +// an image above its page encodes with "..", and as long as it still lands +// inside --dest it is fine. +func TestDestPathAllowsLegitimateParent(t *testing.T) { + root := filepath.Clean("/tmp/dest") + got, err := destPath(root, managed("..%2Fassets%2Flogo.png", "../assets/logo.png"), false) + if err == nil { + t.Fatalf("destPath = %q; a source escaping the root must be refused", got) + } + + // The same path is fine when the page sits a directory deeper. + got, err = destPath(root, managed("x", "docs/../assets/logo.png"), false) + if err != nil { + t.Fatal(err) + } + if want := filepath.Join(root, "assets", "logo.png"); got != want { + t.Errorf("destPath = %q, want %q", got, want) + } +} + +// TestDestPathRefusesEscapes is the clamp. A source path comes from an +// attachment comment, which anyone able to edit the page controls. +func TestDestPathRefusesEscapes(t *testing.T) { + root := filepath.Clean("/tmp/dest") + cases := []string{ + "../../escape.png", + "../../../.ssh/authorized_keys", + "/etc/passwd", + "/tmp/dest-sibling/x.png", + "..", + ".", + } + for _, src := range cases { + if got, err := destPath(root, managed("n.png", src), false); err == nil { + t.Errorf("destPath(path=%q) = %q, nil; want a refusal", src, got) + } + } +} + +// TestDestPathRefusesRootPrefixSibling guards a string-prefix mistake: +// "/tmp/destmore" starts with "/tmp/dest" but is not inside it. +func TestDestPathRefusesRootPrefixSibling(t *testing.T) { + root := filepath.Clean("/tmp/dest") + if got, err := destPath(root, managed("n.png", "../destmore/x.png"), false); err == nil { + t.Errorf("destPath = %q, nil; want a refusal for a sibling sharing the root's prefix", got) + } +} + +func TestSelectAttachmentsAll(t *testing.T) { + all := []client.Attachment{{Title: "a.png"}, {Title: "b.png"}} + got, missing := selectAttachments(all, nil) + if len(got) != 2 || len(missing) != 0 { + t.Fatalf("got %d wanted / %d missing, want 2/0", len(got), len(missing)) + } +} + +func TestSelectAttachmentsByNamePreservesRequestOrder(t *testing.T) { + all := []client.Attachment{{Title: "a.png"}, {Title: "b.png"}, {Title: "c.png"}} + got, missing := selectAttachments(all, []string{"c.png", "a.png"}) + if len(missing) != 0 { + t.Fatalf("missing = %v, want none", missing) + } + if got[0].Title != "c.png" || got[1].Title != "a.png" { + t.Errorf("order = %q/%q, want c.png/a.png", got[0].Title, got[1].Title) + } +} + +func TestSelectAttachmentsReportsMissing(t *testing.T) { + all := []client.Attachment{{Title: "a.png"}} + got, missing := selectAttachments(all, []string{"a.png", "nope.png"}) + if len(got) != 1 { + t.Errorf("wanted = %d, want 1", len(got)) + } + if len(missing) != 1 || missing[0] != "nope.png" { + t.Errorf("missing = %v, want [nope.png]", missing) + } +} + +// TestDestPathEscapeMessageNamesTheAttachment keeps the failure actionable: the +// user needs to know which attachment was refused. +func TestDestPathEscapeMessageNamesTheAttachment(t *testing.T) { + _, err := destPath(filepath.Clean("/tmp/dest"), managed("evil.png", "../../x"), false) + if err == nil { + t.Fatal("want an error") + } + if !strings.Contains(err.Error(), "evil.png") { + t.Errorf("error %q does not name the attachment", err) + } +} diff --git a/cmd/attachmentdownload/json.go b/cmd/attachmentdownload/json.go new file mode 100644 index 0000000..0c0f33a --- /dev/null +++ b/cmd/attachmentdownload/json.go @@ -0,0 +1,42 @@ +package attachmentdownload + +// jsonDownloadResult is attachment-download's --json result shape: one object +// per attachment. dest_path is the local path written, which is the piece a +// caller cannot derive itself -- it depends on the recorded source path, --flat, +// and --dest. It is null only when resolving the path is what failed. +type jsonDownloadResult struct { + OK bool `json:"ok"` + Status string `json:"status"` + DryRun bool `json:"dry_run"` + Filename string `json:"filename"` + DestPath *string `json:"dest_path"` + Error *string `json:"error"` + Code *string `json:"code"` +} + +func buildResult(r outcome) jsonDownloadResult { + res := jsonDownloadResult{ + OK: r.status != statusFailed, + Status: r.status, + DryRun: dryRun, + Filename: r.name, + DestPath: nullable(r.destPath), + } + if r.err != nil { + msg := r.err.Error() + res.Error = &msg + } + if r.code != "" { + code := string(r.code) + res.Code = &code + } + return res +} + +// nullable maps an empty string to a JSON null, else a pointer to the value. +func nullable(s string) *string { + if s == "" { + return nil + } + return &s +} diff --git a/cmd/attachmentdownload/json_test.go b/cmd/attachmentdownload/json_test.go new file mode 100644 index 0000000..6ea1f1d --- /dev/null +++ b/cmd/attachmentdownload/json_test.go @@ -0,0 +1,83 @@ +package attachmentdownload + +import ( + "bytes" + "encoding/json" + "errors" + "testing" + + "github.com/mozilla/markfluence/internal/jsonout" + "github.com/mozilla/markfluence/internal/schematest" +) + +func TestSchemaConformance(t *testing.T) { + results := []any{ + buildResult(outcome{name: "assets%2Fx.png", destPath: "out/assets/x.png", status: statusDownloaded}), + buildResult(outcome{name: "notes.pdf", destPath: "out/notes.pdf", status: statusSkipped}), + buildResult(outcome{ + name: "evil.png", status: statusFailed, + err: errors.New("outside the destination directory"), code: jsonout.CodeValidation, + }), + } + env := jsonout.NewEnvelope(command, results, + map[string]int{"total": 3, "succeeded": 2, "failed": 1, "skipped": 1}) + var buf bytes.Buffer + if err := jsonout.Emit(&buf, env); err != nil { + t.Fatalf("Emit: %v", err) + } + schematest.ValidateEnvelope(t, buf.Bytes()) + + failRes := map[string]any{"ok": false, "page_id": "9", "error": "page 9 not found", "code": jsonout.CodeNotFound} + failEnv := jsonout.NewEnvelope(command, []any{failRes}, + map[string]int{"total": 1, "succeeded": 0, "failed": 1, "skipped": 0}) + buf.Reset() + if err := jsonout.Emit(&buf, failEnv); err != nil { + t.Fatalf("Emit: %v", err) + } + schematest.ValidateEnvelope(t, buf.Bytes()) +} + +func TestBuildResultFailureCarriesErrorAndNullDest(t *testing.T) { + res := buildResult(outcome{ + name: "evil.png", status: statusFailed, + err: errors.New("boom"), code: jsonout.CodeValidation, + }) + if res.OK { + t.Error("ok = true, want false for a failure") + } + b, err := json.Marshal(res) + if err != nil { + t.Fatal(err) + } + var round map[string]any + if err := json.Unmarshal(b, &round); err != nil { + t.Fatal(err) + } + if round["dest_path"] != nil { + t.Errorf("dest_path = %v, want null when path resolution failed", round["dest_path"]) + } + if round["error"] != "boom" || round["code"] != "VALIDATION" { + t.Errorf("error/code = %v/%v", round["error"], round["code"]) + } +} + +func TestJSONDownloadResultMarshal(t *testing.T) { + b, err := json.MarshalIndent( + buildResult(outcome{name: "assets%2Fx.png", destPath: "out/assets/x.png", status: statusDownloaded}), + "", " ") + if err != nil { + t.Fatal(err) + } + want := `{ + "ok": true, + "status": "downloaded", + "dry_run": false, + "filename": "assets%2Fx.png", + "dest_path": "out/assets/x.png", + "error": null, + "code": null +}` + if string(b) != want { + t.Errorf("download result mismatch:\n got:\n%s\n want:\n%s", b, want) + } +} diff --git a/cmd/root.go b/cmd/root.go index e003614..451521c 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -6,6 +6,7 @@ import ( "os" "strings" + "github.com/mozilla/markfluence/cmd/attachmentdownload" "github.com/mozilla/markfluence/cmd/attachmentlist" "github.com/mozilla/markfluence/cmd/attachmentupload" "github.com/mozilla/markfluence/cmd/create" @@ -140,4 +141,5 @@ func init() { rootCmd.AddCommand(read.Cmd) rootCmd.AddCommand(attachmentlist.Cmd) rootCmd.AddCommand(attachmentupload.Cmd) + rootCmd.AddCommand(attachmentdownload.Cmd) } diff --git a/schema/json-output/v1.json b/schema/json-output/v1.json index 24f821e..5c8b75b 100644 --- a/schema/json-output/v1.json +++ b/schema/json-output/v1.json @@ -9,7 +9,7 @@ "properties": { "schema_version": { "const": 1 }, "markfluence_version": { "type": "string" }, - "command": { "enum": ["info", "read", "update", "create", "fix", "attachment-list", "attachment-upload"] }, + "command": { "enum": ["info", "read", "update", "create", "fix", "attachment-list", "attachment-upload", "attachment-download"] }, "results": { "type": "array" }, "summary": { "type": "object" } }, @@ -72,6 +72,19 @@ } } }, + { + "if": { "properties": { "command": { "const": "attachment-download" } }, "required": ["command"] }, + "then": { + "properties": { + "results": { + "items": { + "oneOf": [{ "$ref": "#/$defs/attachmentDownloadResult" }, { "$ref": "#/$defs/singleOpFailure" }] + } + }, + "summary": { "$ref": "#/$defs/attachmentSummary" } + } + } + }, { "if": { "properties": { "command": { "const": "attachment-list" } }, "required": ["command"] }, "then": { @@ -310,6 +323,21 @@ "code": { "$ref": "#/$defs/codeOrNull" } } }, + "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" }, + "status": { "enum": ["downloaded", "skipped", "failed"] }, + "dry_run": { "type": "boolean" }, + "filename": { "type": "string" }, + "dest_path": { "$ref": "#/$defs/stringOrNull" }, + "error": { "$ref": "#/$defs/stringOrNull" }, + "code": { "$ref": "#/$defs/codeOrNull" } + } + }, "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", From e0b76003fb33401ce00a4b5df17f0e0d43e54b22 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 5 Aug 2026 19:57:51 -0400 Subject: [PATCH 7/8] docs: document the attachment subcommands README gets a usage section per command, plus a note up front that every command naming a page takes an id, a URL, or a .md file -- previously true of neither info nor read alone. The --json notes gain the new status verbs and spell out what "one result per target" means per command, since the target is the attachment for the attachment-* commands. CLAUDE.md gets the three commands, internal/pageref, and the client's attachment expand, offset pagination, and download path -- including the warning never to add a CheckRedirect that forwards headers. Also corrects the root bullet's stale "four subcommands" and the cmd/ list, which had never picked up read. Closes #9 --- CLAUDE.md | 10 +++--- README.md | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c4936bb..9528ed0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,9 +46,11 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd ### Layout -- `cmd/root.go` — the cobra root: `--url`/`--username`/`--debug`/`--no-color` persistent flags, version from `internal/buildinfo`, and registration of the four subcommands. `Execute()` prints cobra-generated errors (bad args/flags) but not `ui.ErrSilent`, which marks a failure a command already reported. -- `cmd/{update,create,fix,info}/` — 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. -- `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/...`). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send` (429 for any method honoring `Retry-After`, plus 502/503/504 and network errors for idempotent methods only; exponential backoff capped), `SetContentProperty` retry-once on top (recovers a lost create-POST response), `SyncAttachments` (skip/update by a SHA-256 recorded in the attachment's comment, alongside the source path so `read` recovers image paths exactly; the comment is `markfluence: sha256=… path=…`, and the legacy `mzcld:checksum: …` form is still parsed — the skip test compares the *parsed* checksum, so the format change doesn't force a re-upload), `_links.next` pagination. `config.go` holds `Resolve` and the `.env` reader. +- `cmd/root.go` — the cobra root: `--url`/`--username`/`--debug`/`--no-color` persistent flags, version from `internal/buildinfo`, and registration of every subcommand. `Execute()` prints cobra-generated errors (bad args/flags) but not `ui.ErrSilent`, which marks a failure a command already reported. +- `cmd/{update,create,fix,info,read}/` — 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. +- `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. +- `internal/pageref` — `Resolve`, the single page-argument resolver: a numeric id, a Confluence page URL, 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. +- `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/...`). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send` (429 for any method honoring `Retry-After`, plus 502/503/504 and network errors for idempotent methods only; exponential backoff capped), `SetContentProperty` retry-once on top (recovers a lost create-POST response), `SyncAttachments` (skip/update by a SHA-256 recorded in the attachment's comment, alongside the source path so `read` recovers image paths exactly; the comment is `markfluence: sha256=… path=…`, and the legacy `mzcld:checksum: …` form is still parsed — the skip test compares the *parsed* checksum, so the format change doesn't force a re-upload), `_links.next` pagination. `ListAttachments` expands `metadata.comment,version,extensions` (`fileSize`/`mediaType` live under `extensions`) and pages by `start`/`limit` offset instead — a v1 collection omits `_links.next` when the results fit one page, and its `next` is `/wiki`-context-relative rather than the v2 paths `resolveNext` handles. `DownloadAttachment` goes through `send` (so it inherits retry/backoff) against `_links.download`, which is an API path — `/rest/api/content/{page}/child/attachment/{id}/download`, not the `/download/attachments/...` UI path — and therefore works through the gateway; it 302s to Atlassian's media host, which carries its own token, and Go drops `Authorization` cross-host, so **never** add a `CheckRedirect` that forwards headers. `config.go` holds `Resolve` and the `.env` reader. - `internal/convert` — the converter (the crux). `MdToConfluence(md *frontmatter.MarkdownFile, baseURL, spaceKey, version string) (*ConfluencePage, error)`. 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; `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); `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` (sibling-file scans, GitHub/Confluence slugs, doc-link + anchor rewriting), `tables.go` (the `` tag, stamped with Confluence's `data-layout="align-start"` so tables auto-size to their content and left-align, plus cells: an AST transformer consumes a leading `` comment in a cell and `renderTableCell` emits it as `data-highlight-colour` — rows still fall through to the GFM renderer), and `renderer.go` (code macros, text soft-break→space, images, links) do the rest. The `` and `` token substitutions happen **inside** `MdToConfluence`. - `internal/frontmatter` — flat YAML frontmatter parse/quote/`UpdateField`, and the `MarkdownFile` type (`Parse`/`ParseFile`, exported `Filename`/`Content`/`Frontmatter`/`Body`, and `Title`/`PageID`/`Space`/`Parent` accessors that normalize missing/blank/`"null"`). - `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. @@ -69,4 +71,4 @@ The design target is **semantic** (not byte-for-byte) equivalence to valid Confl ### Frontmatter-driven publishing -Each markdown file is one page. Frontmatter carries `title`, `page_id`, `space` (a key), `parent` (`null` / a `.md` path / a page id), and `page_width`. `update` requires a `page_id` (from frontmatter or `--page-id`) and errors without one; `--title`/`--page-id` override the frontmatter (single FILE only), `--page-width` overrides too (batch allowed), and `update` never writes back to files. It asserts `page_width` only when set via flag or frontmatter (otherwise the live width is left alone), and skips a file whose mtime predates the page's last version unless `--force`. `create` takes `--title`/`--page-width` overrides (`--title` single-FILE only; `--page-width` batch-ok, default `max`) and, unless `--no-persist` is given, writes `title`/`space`/`parent`/`page_id`/`page_width` back after creating; `fix` reconciles all of these (plus `page_width`) from the live page. `update`, `create`, and `fix` all take `--dry-run`, which previews the actions (pages, attachment uploads, width changes, and — for `create` — frontmatter write-backs) without writing to Confluence or files; per-file human output is identical to a real run, distinguished only by a leading `DRY RUN` banner and a `dry_run: true` field in `--json`. In a `create` dry-run a previewed page has no id/URL yet, and an in-set child's `parent` is null; the parent's source `.md` is always reported in the `parent_file` output field. Commands process multiple files (except `info`, single-arg) and exit non-zero if any fail. +Each markdown file is one page. Frontmatter carries `title`, `page_id`, `space` (a key), `parent` (`null` / a `.md` path / a page id), and `page_width`. `update` requires a `page_id` (from frontmatter or `--page-id`) and errors without one; `--title`/`--page-id` override the frontmatter (single FILE only), `--page-width` overrides too (batch allowed), and `update` never writes back to files. It asserts `page_width` only when set via flag or frontmatter (otherwise the live width is left alone), and skips a file whose mtime predates the page's last version unless `--force`. `create` takes `--title`/`--page-width` overrides (`--title` single-FILE only; `--page-width` batch-ok, default `max`) and, unless `--no-persist` is given, writes `title`/`space`/`parent`/`page_id`/`page_width` back after creating; `fix` reconciles all of these (plus `page_width`) from the live page. `update`, `create`, and `fix` all take `--dry-run`, which previews the actions (pages, attachment uploads, width changes, and — for `create` — frontmatter write-backs) without writing to Confluence or files; per-file human output is identical to a real run, distinguished only by a leading `DRY RUN` banner and a `dry_run: true` field in `--json`. In a `create` dry-run a previewed page has no id/URL yet, and an in-set child's `parent` is null; the parent's source `.md` is always reported in the `parent_file` output field. Commands process multiple files (except `info`/`read`/`attachment-list`, single-arg) and exit non-zero if any fail. A command that names a page takes it three ways via `internal/pageref` (id / URL / `.md` with a `page_id`). diff --git a/README.md b/README.md index 30cdf89..779845f 100644 --- a/README.md +++ b/README.md @@ -92,8 +92,14 @@ markfluence update --help markfluence fix --help markfluence info --help markfluence read --help +markfluence attachment-list --help +markfluence attachment-upload --help +markfluence attachment-download --help ``` +Every command that takes a page accepts it three ways: a numeric page id, a +Confluence page URL, or a Markdown file whose frontmatter has a `page_id`. + ### `create` ``` @@ -244,6 +250,90 @@ markfluence read 1234567890 --format storage > page.storage.xml markfluence read "https://org.atlassian.net/wiki/spaces/ENG/pages/1234567890/Title" ``` +### `attachment-list` + +``` +Usage: markfluence attachment-list ARG [flags] +``` + +List a page's attachments. + +```console +$ markfluence attachment-list 1234567890 +NAME SIZE VER TYPE SOURCE +assets%2Fdiagram.png 24.1 KB 3 image/png assets/diagram.png +notes.pdf 1.2 MB 1 application/pdf - +``` + +`NAME` is the name Confluence stores. For an image markfluence published that is +the percent-encoded source path (see [Body](#body)), and `SOURCE` is the Markdown +image path it came from — so the table shows at a glance which attachments a +publish manages and which it will leave alone. + +`SOURCE` is a dash when no source path is recorded: either the attachment was +uploaded by hand, or it was published before markfluence recorded source paths. +Those two look the same here; `--json` has a `managed` field that tells them +apart. Attachments left behind by the encoding change show up this way, which is +how you find them. + +### `attachment-upload` + +``` +Usage: markfluence attachment-upload ARG FILE... [flags] +``` + +Upload or replace attachments on a page, complementing the automatic sync that +`create` and `update` perform for a page's images. + +Each file is attached under its base name. A file whose contents already match +what's on the page is skipped, using the same checksum bookkeeping +`create`/`update` use, so uploading by hand and publishing agree on what's +current. `--force` uploads anyway (bumping the attachment's version), which is +how you repair an attachment whose stored bytes drifted while its checksum still +matches. `--dry-run` previews without writing. + +`--name` sets the attachment name for a single file and takes a **path**, which +markfluence encodes for you — so `--name assets/x.png` produces the attachment +that an image written as `![](assets/x.png)` resolves to. The recorded source +path always matches the stored name, so a later publish won't create a duplicate +under a different one. + +```sh +markfluence attachment-upload 1234567890 diagram.png +markfluence attachment-upload 1234567890 report.pdf notes.txt +markfluence attachment-upload 1234567890 img.png --name assets/diagram.png +markfluence attachment-upload 1234567890 diagram.png --force +``` + +### `attachment-download` + +``` +Usage: markfluence attachment-download ARG [NAME...] [flags] +``` + +Download a page's attachments. Each `NAME` is an attachment name as +`attachment-list` reports it; with no `NAME`, every attachment is downloaded. + +An attachment markfluence published is written back to the Markdown image path +recorded in its comment, so the downloaded tree matches what the page's Markdown +references and previews locally: + +```console +$ markfluence attachment-download 1234567890 --dest ./out +downloaded /out/assets/diagram.png +downloaded /out/notes.pdf +``` + +An attachment with no recorded path — hand-uploaded, or published before +markfluence recorded them — is written under its stored name. `--flat` writes +everything under stored names. `--dest` defaults to the current directory and is +created if missing. An existing file is skipped unless `--force`, and +`--dry-run` previews without writing. + +A recorded path that would resolve outside `--dest` is refused for that +attachment: the path comes from an attachment comment, which anyone who can edit +the page controls. + ### `--json` output The persistent `--json` flag makes any command emit a single machine-readable @@ -297,8 +387,14 @@ Notes on the schema: shapes (empty values are `null` or `[]`); the key *set* differs per command. `schema_version` is bumped on any breaking change. - **Status verbs** are per-command: `published`/`skipped` (`update`), - `created`/`not_created` (`create`), `changed`/`consistent` (`fix`), plus - `failed`. `info`/`read` results carry data only (no status verb). + `created`/`not_created` (`create`), `changed`/`consistent` (`fix`), + `created`/`updated`/`skipped` (`attachment-upload`), + `downloaded`/`skipped` (`attachment-download`), plus `failed`. `info`, `read`, + and `attachment-list` results carry data only (no status verb). +- **One result per target**, and the target is per-command: the page for + `info`/`read` (always one), the file for `update`/`create`/`fix`, and the + attachment for the three `attachment-*` commands — so + `.results[] | .filename` works and `summary.total` is the attachment count. - **Compound values are objects**, never display strings — `version`, `page_width`, and the `created`/`updated` author stamps on `info`. - **`create`'s two-phase abort** (a validation failure means nothing is created) From c4dd120b826a3a392d0e88ee393080cff005ee1f Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 5 Aug 2026 20:48:25 -0400 Subject: [PATCH 8/8] docs: name the page argument PAGE instead of ARG ARG was defensible when info and read each accepted a different, loosely specified thing. Since pageref unified them, all five commands take exactly one thing -- a page reference -- and the placeholder should say so. It read worst in attachment-upload ARG FILE..., where ARG drew no contrast at all. Also disambiguates NAME in attachment-list's help, which is a column there but a positional argument in attachment-download, and fills in the .md form in read's README prose, which it gained in the pageref refactor. No behavior change; positional arguments are unaffected. --- README.md | 35 +++-- assets/markfluence-test.png | Bin 0 -> 171 bytes cmd/attachmentdownload/attachmentdownload.go | 4 +- cmd/attachmentlist/attachmentlist.go | 11 +- cmd/attachmentupload/attachmentupload.go | 4 +- cmd/info/info.go | 4 +- cmd/read/read.go | 4 +- round-trip-page.md | 14 ++ table-cell-bg-probe.md | 44 +++++++ table-probe-real.md | 128 +++++++++++++++++++ table-probe.md | 97 ++++++++++++++ test-codeblock.md | 37 ++++++ test-page-no-frontmatter.md | 113 ++++++++++++++++ test-page-read.md | 113 ++++++++++++++++ test-page.md | 121 ++++++++++++++++++ 15 files changed, 705 insertions(+), 24 deletions(-) create mode 100644 assets/markfluence-test.png create mode 100644 round-trip-page.md create mode 100644 table-cell-bg-probe.md create mode 100644 table-probe-real.md create mode 100644 table-probe.md create mode 100644 test-codeblock.md create mode 100644 test-page-no-frontmatter.md create mode 100644 test-page-read.md create mode 100644 test-page.md diff --git a/README.md b/README.md index 779845f..3d79cc4 100644 --- a/README.md +++ b/README.md @@ -85,13 +85,25 @@ delete scopes. This might change in the future. ## Usage +General: + ```sh markfluence --help +``` + +Manipulating Confluence pages: + +```sh markfluence create --help markfluence update --help markfluence fix --help markfluence info --help markfluence read --help +``` + +Manipulating Confluence page attachments: + +```sh markfluence attachment-list --help markfluence attachment-upload --help markfluence attachment-download --help @@ -204,13 +216,13 @@ markfluence fix docs/foo.md --dry-run ### `info` ``` -Usage: markfluence info ARG [flags] +Usage: markfluence info PAGE [flags] ``` Print a page's metadata (id, title, status, space, parent, version, page width, -authors, dates, url). `ARG` is a numeric page id or a Markdown file whose -frontmatter has a `page_id`. `--properties` also lists all of the page's content -properties. +authors, dates, url). `PAGE` is a numeric page id, a Confluence page URL, or a +Markdown file whose frontmatter has a `page_id`. `--properties` also lists all of +the page's content properties. ```sh markfluence info 1234567890 @@ -220,12 +232,13 @@ markfluence info docs/foo.md --properties ### `read` ``` -Usage: markfluence read ARG [flags] +Usage: markfluence read PAGE [flags] ``` -Fetch a Confluence page and print its body to stdout. `ARG` is a numeric page id -or a Confluence page URL (the modern `/wiki/.../pages//...` form or a legacy -`?pageId=` URL). It composes with shell redirection. +Fetch a Confluence page and print its body to stdout. `PAGE` is a numeric page id, +a Confluence page URL (the modern `/wiki/.../pages//...` form or a legacy +`?pageId=` URL), or a Markdown file whose frontmatter has a `page_id`. It +composes with shell redirection. `--format` selects the output: @@ -253,7 +266,7 @@ markfluence read "https://org.atlassian.net/wiki/spaces/ENG/pages/1234567890/Tit ### `attachment-list` ``` -Usage: markfluence attachment-list ARG [flags] +Usage: markfluence attachment-list PAGE [flags] ``` List a page's attachments. @@ -279,7 +292,7 @@ how you find them. ### `attachment-upload` ``` -Usage: markfluence attachment-upload ARG FILE... [flags] +Usage: markfluence attachment-upload PAGE FILE... [flags] ``` Upload or replace attachments on a page, complementing the automatic sync that @@ -308,7 +321,7 @@ markfluence attachment-upload 1234567890 diagram.png --force ### `attachment-download` ``` -Usage: markfluence attachment-download ARG [NAME...] [flags] +Usage: markfluence attachment-download PAGE [NAME...] [flags] ``` Download a page's attachments. Each `NAME` is an attachment name as diff --git a/assets/markfluence-test.png b/assets/markfluence-test.png new file mode 100644 index 0000000000000000000000000000000000000000..6f87dc62a1e64fd0ed12f840b44859732276fe4c GIT binary patch literal 171 zcmeAS@N?(olHy`uVBq!ia0vp^6+mpj!2~2jc{N*rRFS8PV@SoEw-*>W859@}7zp1y zeXea|>mga212<=6zjk{3^PqYo+g&b`7hww=w=?nX63w`<)}c9%RjNe$!h&ehrB*XX Xe3+ZK$xh}Q&~^q-S3j3^P6/... form or a legacy ?pageId= URL), or a\n" + "markdown file whose frontmatter has a page_id.\n\n" + "The default markdown output carries title/page_id/space/page_width\n" + diff --git a/round-trip-page.md b/round-trip-page.md new file mode 100644 index 0000000..08183cc --- /dev/null +++ b/round-trip-page.md @@ -0,0 +1,14 @@ +--- +title: round trip page +space: ~60c36d0718e9f60071326951 +parent: null +page_id: 2904981621 +--- + +This is a round trip page. + + + +This is a thing in a section. + + diff --git a/table-cell-bg-probe.md b/table-cell-bg-probe.md new file mode 100644 index 0000000..ec91ce7 --- /dev/null +++ b/table-cell-bg-probe.md @@ -0,0 +1,44 @@ +--- +title: markfluence table cell background probe +space: ~60c36d0718e9f60071326951 +parent: null +page_id: 2942664752 +page_width: max +--- + +# markfluence table cell background probe + +Published by markfluence from `table-cell-bg-probe.md` to verify that a +`` cell marker reaches ADF as a cell `background`. Each cell below +is labeled with the swatch name that produced it; compare against the editor's +picker on "Confluence tables test". + +## All 21 swatches + +| Light | Medium | Bold | +| --- | --- | --- | +| white | light-grey | grey | +| light-blue | blue | bold-blue | +| light-teal | teal | bold-teal | +| light-green | green | bold-green | +| light-yellow | yellow | bold-yellow | +| light-red | red | bold-red | +| light-purple | purple | bold-purple | + +## Edge cases + +A colored header cell, an off-palette hex, a marker-only (empty) cell, and +alignment alongside a color: + +| colored header | off-palette hex | +| :--- | ---: | +| | right-aligned, uncolored | +| shouty marker | aligned and colored | + +These publish uncolored (each emits a warning): an unknown name, and a marker that +isn't first in its cell. + +| Cell | Result | +| --- | --- | +| unknown name | no background | +| trailing marker | no background | diff --git a/table-probe-real.md b/table-probe-real.md new file mode 100644 index 0000000..a6b58d8 --- /dev/null +++ b/table-probe-real.md @@ -0,0 +1,128 @@ +--- +title: markfluence table layout probe (real tables) +space: ~60c36d0718e9f60071326951 +parent: 76646878 +page_id: 2914451497 +page_width: max +--- + +The 29 tables from "Incident Analysis: Q2 2026" (page 2874736735), republished with +`data-layout="align-start"` and no `data-table-width` or colgroup, to see whether +auto-sizing does the right thing on real content. Compare against the original: +https://mozilla-hub.atlassian.net/wiki/spaces/~60c36d0718e9f60071326951/pages/2874736735 + +## Table 1 (6 columns) + +
Metric 2025q3 2025q4 2026q1 2026q2 Percent change
Total incidents 22 22 33 45 +36%
Total entities 14 18 15 30 +100%
S1/S2 % 30% 25% 17% 17% +0%
Automation detection % 14% 36% 21% 30% +39%
MTT-Alerted 5d 4h 19h 59m 22d 23h † 1d 1h −95% †
MTT-Mitigated 7d 16h 2d 4h 157d 14h † 3d 18h −98% †
MTT-Resolved 22d 12h 13d 4h 165d 5h † 6d 6h −96% †
+ +## Table 2 (6 columns) + +
Metric 2025q3 2025q4 2026q1 2026q2 Percent change
Total incidents 22 21 19 27 +42%
Total entities 14 17 13 27 +108%
S1/S2 % 30% 25% 33% 25% −25%
Automation detection % 14% 38% 37% 46% +25%
MTT-Alerted 5d 4h 11h 42m 2d 10h 1d 0h −58%
MTT-Mitigated 7d 16h 1d 6h 3d 18h 2d 4h −42%
MTT-Resolved 22d 12h 12d 3h 5d 9h 5d 17h +6%
+ +## Table 3 (6 columns) + +
Metric 2025q3 2025q4 2026q1 2026q2 Percent change
Total incidents 0 1 14 18 +29%
Total entities 0 1 4 4 +0%
S1/S2 % — 0% 0% 0% n/a
Automation detection % — 0% 0% 6% n/a
MTT-Alerted — 7d 17h 62d 0h † 1d 3h −98% †
MTT-Mitigated — 21d 7h 482d 7h † 6d 14h −99% †
MTT-Resolved — 35d 13h 484d 19h † 7d 1h −99% †
+ +## Table 4 (4 columns) + +
Manual Automation Total
Service 14 (52%) 12 (44%) 27
Product 17 (94%) 1 (6%) 18
Total 31 (69%) 13 (29%) 45
+ +## Table 5 (4 columns) + +
Entity # incidents % S1/S2 Link
firefox 17 53% view incidents
amo 5 40% view incidents
fenix 3 33% view incidents
firefox-ios 3 33% view incidents
lando 3 67% view incidents
vpn 3 67% view incidents
ads 2 100% view incidents
fxa 2 50% view incidents
relay 2 100% view incidents
taskcluster 2 50% view incidents
+ +## Table 6 (4 columns) + +
Key Incident title Incident entities Detail
IIM-153 Ingestion Sink Subscription Retention Regression ads, datashared, ingestion Stale PR auto-applied by Atlantis reverted a load-bearing Pub/Sub retention setting
IIM-156 Firefox.com outage due to failing database migration springfield Failing database migration took firefox.com down globally
IIM-158 Persistent 502s from Lando lando Static assets missing from GCS after a build-process change; no distributed cache
IIM-160 VPN Nimbus Rollout Collision vpn Reused messaging-component name suppressed enrollment for ~20M users
IIM-164 OHTTP Gateway unavailable for ten minutes ads, firefox, newtab MozCloud Helm chart migration hit an immutable-field patch error
IIM-169 llm-proxy prod outage for ~86 min firefox, llm-proxy Shared managed cert regenerated when one domain was removed
IIM-288 AMO Database stability issues amo Replica overload during a change window
+ +## Table 7 (4 columns) + +
Key Incident title Incident entities Detail
IIM-151 AWS SES Complaint Rate increase > 0.5% monitor No response owner/runbook for a newly-added alert class; ~28h to engage
IIM-153 Ingestion Sink Subscription Retention Regression ads, datashared, ingestion Alert metric saturated at retention, masking the failure; detected downstream
IIM-163 Weather widget / OHTTP unavailable ~13.5h firefox, merino No alerting for TLS CA changes; cloud engineering was not paged
IIM-175 VPN FxA Token Refresh Stopped vpn No alert for the subscription-deactivation rate
IIM-294 FxA WAF 4xx rate-limit blocked Guardian for 110min browser-proxy, guardian No WAF alerting; 1–2h data lag on the WAF dashboard
+ +## Table 8 (4 columns) + +
Key Incident title Incident entities Detail
IIM-153 Ingestion Sink Subscription Retention Regression ads, datashared, ingestion v1→v2 sink migration; stale PR reverted retention
IIM-156 Firefox.com outage due to failing database migration springfield Database migration failure
IIM-163 Weather widget / OHTTP unavailable ~13.5h firefox, merino Balrog migration to MozCloud altered the services.mozilla.com CAA record
IIM-164 OHTTP Gateway unavailable for ten minutes ads, firefox, newtab MozCloud Helm chart migration, immutable-field error
+ +## Table 9 (4 columns) + +
Key Incident title Incident entities Detail
IIM-157 Try pushes not created; hg replication out of sync hg, taskcluster, treeherder hg.mozilla.org replication breakage; recovery slowed by load + bots
IIM-173 Relay service degradation relay Fx151 client bug surged traffic to the Relay origin
IIM-284 Lando fails with 502/503 while pushing to try lando Bot-like traffic overloaded the database
IIM-288 AMO Database stability issues amo Replica capacity/lag under load
IIM-290 Bugzilla Cloud SQL replica at 100% CPU bugzilla Traffic spikes / unoptimized search queries; Fastly DDoS enabled
+ +## Table 10 (4 columns) + +
Key Incident title Incident entities Detail
IIM-167 TapClicks BigQuery integration failure tapclicks Third-party TapClicks→BigQuery integration failed
IIM-175 VPN FxA Token Refresh Stopped vpn FxA token revocation tripped Guardian's revoked-access path
IIM-282 Fx-CI unauthenticated RCE in web-server taskcluster RCE via the upstream sift library
IIM-294 FxA WAF 4xx rate-limit blocked Guardian for 110min browser-proxy, guardian Guardian depends on the FxA/browser-proxy path the WAF rule blocked
+ +## Table 11 (4 columns) + +
Key Incident title Incident entities Detail
IIM-159 facebook-not-loading firefox Fx150 shipped SCONE enabled-by-side-effect, tripping Facebook + Bitdefender
IIM-162 Reports of YouTube not loading on 150 firefox Style-computation / hardware-acceleration playback regression
IIM-178 Firefox Android deleting user downloads after update fenix An update / deletion-dialog behavior change lost user files
IIM-286 Sports widget missing opening World Cup game firefox Desktop/Mobile widget endpoint-contract mismatch
IIM-289 World Cup widget in iOS polling live every 15mn firefox-ios Live scores updated ~15 min late
IIM-297 World Cup widget removed with update to Fx 152.0.3 firefox Nimbus rollout ordering / targeting-query error unenrolled users
+ +## Table 12 (4 columns) + +
Key Incident title Incident entities Detail
IIM-159 facebook-not-loading firefox neqo emits the SCONE indicator unconditionally regardless of scone_enabled()
IIM-172 webserial failure to open port firefox Client regression opening serial ports
IIM-286 Sports widget missing opening World Cup game firefox Desktop/Mobile widget logic mismatch
IIM-287 Nightly-as-Release SP3 scores regressed ~10% firefox Necko disk-cache / Windows Defender disk-I/O interaction
IIM-293 Android AI Controls not blocking Shake to Summarize firefox Nimbus re-entrancy / default-value bug
IIM-296 Homepage Freeze Incident firefox-ios Swift concurrency thread contention
+ +## Table 13 (4 columns) + +
Key Incident title Incident entities Detail
IIM-283 Public disclosure of Firefox Focus for iOS UXSS bug focus-ios Publicly disclosed UXSS bug (S1)
IIM-293 Android AI Controls not blocking Shake to Summarize firefox AI Controls neither listed nor blocked a GenAI feature
+ +## Table 14 (4 columns) + +
Key Incident title Incident entities Detail
IIM-178 Firefox Android deleting user downloads after update fenix Update behavior change shipped without catching data loss
IIM-289 World Cup widget in iOS polling live every 15mn firefox-ios Polling-cadence regression shipped to release
+ +## Table 15 (4 columns) + +
Key Incident title Incident entities Detail
IIM-176 Smartcard certificates not loading in Firefox 151 firefox Client certificate handling regression
IIM-281 No push notifications to iOS firefox-ios Expired APNS certificate not deployed to production
+ +## Table 16 (3 columns) + +
Key Incident title Entities
IIM-150 Rating abuse for Metamask amo
IIM-156 Firefox.com outage due to failing database migration springfield
IIM-161 Yahoo-search-results-not-displaying firefox
IIM-162 Reports of Youtube not Loading on 150 firefox
IIM-168 bugbug pods degraded bugbug
IIM-170 Wikipedia article images not loading on first load fenix, firefox
IIM-172 webserial failure to open port firefox
IIM-174 Clear PBM session button not enabled in Fx151 firefox
IIM-176 Smartcard certificates not loading in Firefox 151 firefox
IIM-178 Firefox Android deleting user downloads after update fenix
IIM-280 Bouncer (download.mozilla.org) is down bouncer, firefox, thunderbird
IIM-283 Public disclosure of Firefox Focus for iOS UXSS bug focus-ios
IIM-284 Lando fails with 502 or 503 while pushing to try lando
IIM-285 Developers cannot submit versions on AMO amo, firefox
IIM-291 FxA elevated 4xx responses fxa
IIM-295 Startup Hangs/Extreme memory Usage firefox
IIM-297 World Cup widget removed with update to Firefox 152.0.3 firefox
+ +## Table 17 (6 columns) + +
Category Count % of Total Done Open In Progress
Improve monitoring/alerting 26 30% 11 (42%) 12 (46%) 3 (12%)
Capacity/infra change 20 23% 12 (60%) 7 (35%) 1 (5%)
Investigate/spike 10 11% 2 (20%) 7 (70%) 0
Fix code/config bug 10 11% 5 (50%) 1 (10%) 4 (40%)
Add automation 8 9% 3 (38%) 5 (63%) 0
Update runbook/docs/process 4 5% 0 3 (75%) 0
Process/access change 2 2% 2 (100%) 0 0
Other 7 8% 5 (71%) 2 (29%) 0
+ +## Table 18 (4 columns) + +
Entity # action items Done % done
amo 19 7 37%
firefox 15 6 40%
ads 12 5 42%
datashared 10 4 40%
ingestion 10 4 40%
relay 10 4 40%
vpn 9 4 44%
firefox-ios 7 4 57%
monitor 6 2 33%
merino 6 2 33%
browser-proxy 5 3 60%
guardian 5 3 60%
bugbug 5 4 80%
taskcluster 4 2 50%
lando 3 3 100%
fxa 3 1 33%
+ +## Table 19 (3 columns) + +
Type Count Examples
User-facing service degradation/outage 19 IIM-156, IIM-163, IIM-294
Data loss or data integrity issue 4 IIM-153, IIM-178, IIM-279
Internal tooling/productivity impact 3 IIM-157, IIM-158, IIM-284
Security/privacy issue 2 IIM-282, IIM-293
Telemetry/reporting impact 1 IIM-153
Revenue impact 1 IIM-164
+ +## Table 20 (4 columns) + +
Key Incident title Severity Duration
IIM-160 VPN Nimbus Rollout Collision S3 28d 15h
IIM-296 Homepage Freeze Incident S3 23d 3h
IIM-161 Yahoo-search-results-not-displaying S3 20d 19h
IIM-287 Nightly as Release SP3 scores regressed ~10% S4 20d 15h
IIM-279 atomicmail.io accounts erroneously deleted S2 9d 15h
+ +## Table 21 (3 columns) + +
Key Incident title Impact
IIM-160 VPN Nimbus Rollout Collision ~20M eligible users did not receive the VPN feature in Fx149
IIM-163 Weather widget and Online Suggest unavailable (Merino/OHTTP) 160,000–240,000 users lost OHTTP-dependent Merino features for 13.5h
IIM-294 FxA WAF 4xx rate-limit blocked Guardian for 110min All VPN users unable to connect/refresh for 110 minutes
IIM-281 No Push notifications to iOS All Firefox iOS users received no push notifications for ~2h19m
IIM-279 atomicmail.io accounts erroneously deleted 2,037 accounts erroneously deleted
+ +## Table 22 (3 columns) + +
Key Incident title Entities
IIM-150 Rating abuse for Metamask amo
IIM-161 Yahoo-search-results-not-displaying firefox
IIM-162 Reports of Youtube not Loading on 150 firefox
IIM-168 bugbug pods degraded bugbug
IIM-170 Wikipedia article images not loading on first load fenix, firefox
IIM-172 webserial failure to open port firefox
IIM-174 Clear PBM session button not enabled in Fx151 firefox
IIM-176 Smartcard certificates not loading in Firefox 151 firefox
IIM-280 Bouncer (download.mozilla.org) is down bouncer, firefox, thunderbird
IIM-283 Public disclosure of Firefox Focus for iOS UXSS bug focus-ios
IIM-285 Developers cannot submit versions on AMO amo, firefox
IIM-287 Nightly as Release SP3 scores regressed ~10% firefox
IIM-288 AMO Database stability issues amo
IIM-291 FxA elevated 4xx responses fxa
IIM-295 Startup Hangs/Extreme memory Usage firefox
+ +## Table 23 (3 columns) + +
Name Times as IC Incidents
Marco Castelluccio 4 IIM-168, IIM-177, IIM-284, IIM-290
William Durand 3 IIM-150, IIM-285, IIM-288
Sebastian Hengst 3 IIM-157, IIM-163, IIM-286
Gregory Hess 2 IIM-159, IIM-161
Dianna Smith 2 IIM-162, IIM-295
Amri Toufali 2 IIM-291, IIM-294
+ +## Table 24 (3 columns) + +
Name Incidents Roles
Hristo Ganchev 5 IC x1, Participant x4
Jon Buckley 5 IC x1, Participant x4
Mathieu Pillard 5 Participant x5
André Honeiser 4 Participant x4
Brett Kochendorfer 4 IC x1, Participant x3
Eric Maydeck 4 Participant x4
Jason Thomas 4 Participant x4
Marco Castelluccio 4 IC x4
William Durand 4 IC x3, Participant x1
Emmett Lynch 3 IC x1, Participant x2
Joe Hermann 3 Participant x3
Joe Zhou 3 IC x1, Participant x2
Luke Crouch 3 Participant x3
Sebastian Hengst 3 IC x3
Zeid Zabaneh 3 Participant x3
+ +## Table 25 (4 columns) + +
Key Incident title Entities Members and roles
IIM-151 AWS SES Complaint Rate increase > 0.5% monitor Brandon Wells (IC)
IIM-153 Ingestion Sink Subscription Retention Regression ads, datashared, ingestion Mikael Ducharme (P), Wesley Dawson (P)
IIM-156 Firefox.com outage due to failing database migration springfield Jon Buckley (IC), Hristo Ganchev (P), Rachael Crook (P), Steven Prokopienko (P)
IIM-158 Persistent 502s from Lando lando Steven Prokopienko (P)
IIM-163 Weather widget / OHTTP unavailable ~13.5h firefox, merino Brett Kochendorfer (P), Jason Thomas (P), Jon Buckley (P)
IIM-164 OHTTP Gateway unavailable for ten minutes ads, firefox, newtab Brett Kochendorfer (IC), Jason Thomas (P)
IIM-168 bugbug pods degraded bugbug Eric Maydeck (P)
IIM-169 llm-proxy prod outage for ~86 min firefox, llm-proxy Brett Kochendorfer (P), Eric Maydeck (P)
IIM-173 Relay service degradation relay Brett Kochendorfer (P), Eric Maydeck (P)
IIM-177 Phabricator is down lando, phabricator Nate Tade (P)
IIM-280 Bouncer (download.mozilla.org) is down bouncer, firefox, thunderbird Hristo Ganchev (IC), James Francis (P), Jason Thomas (P), Jon Buckley (P), Nate Tade (P)
IIM-284 Lando fails with 502 or 503 while pushing to try lando Andre Honeiser (P)
IIM-288 AMO Database stability issues amo Andre Honeiser (P), Brandon Wells (P), Hristo Ganchev (P), Wesley Dawson (P)
IIM-290 Bugzilla Cloud SQL replica at 100% CPU bugzilla Andre Honeiser (P), Hristo Ganchev (P)
IIM-291 FxA elevated 4xx responses fxa Andre Honeiser (P), Hristo Ganchev (P), Jon Buckley (P)
IIM-294 FxA WAF 4xx rate-limit blocked Guardian for 110min browser-proxy, guardian Jason Thomas (P)
+ +## Table 26 (3 columns) + +
Member Incidents Roles
Hristo Ganchev 5 IC x1, P x4
Jon Buckley 4 IC x1, P x3
Jason Thomas 4 P x4
Brett Kochendorfer 4 IC x1, P x3
Andre Honeiser 4 P x4
Eric Maydeck 3 P x3
Brandon Wells 2 IC x1, P x1
Wesley Dawson 2 P x2
Steven Prokopienko 2 P x2
Nate Tade 2 P x2
Mikael Ducharme 1 P x1
Rachael Crook 1 P x1
James Francis 1 P x1
+ +## Table 27 (3 columns) + +
Key Incident title What needs to be done?
IIM-150 Rating abuse for Metamask Add an impact statement; add contributing factors; expand the description
IIM-156 Firefox.com outage due to failing database migration Add contributing factors; expand the description; file at least one action item
IIM-161 Yahoo-search-results-not-displaying Add an impact statement; add contributing factors; expand the description; file at least one action item
IIM-162 Reports of Youtube not Loading on 150 Add an impact statement; add contributing factors; expand the description; file at least one action item
IIM-168 bugbug pods degraded Add an impact statement; add contributing factors; expand the description
IIM-170 Wikipedia article images not loading on first load Add an impact statement; add contributing factors; expand the description; file at least one action item
IIM-172 webserial failure to open port Add an impact statement; add contributing factors; file at least one action item
IIM-174 Clear PBM session button not enabled in Fx151 Add an impact statement; add contributing factors; expand the description; file at least one action item
IIM-176 Smartcard certificates not loading in Firefox 151 Add an impact statement; add contributing factors; expand the description; file at least one action item
IIM-280 Bouncer (download.mozilla.org) is down Add an impact statement; add contributing factors; expand the description
IIM-283 Public disclosure of Firefox Focus for iOS UXSS bug Add an impact statement; add contributing factors; expand the description; file at least one action item
IIM-285 Developers cannot submit versions on AMO Add an impact statement; add contributing factors; expand the description
IIM-291 FxA elevated 4xx responses Add an impact statement; add contributing factors; expand the description
IIM-295 Startup Hangs/Extreme memory Usage Add an impact statement; add contributing factors; expand the description; file at least one action item
+ +## Table 28 (3 columns) + +
Key Incident title What needs to be done?
IIM-159 facebook-not-loading File at least one action item
IIM-178 Firefox Android deleting user downloads after update Add contributing factors; file at least one action item
IIM-284 Lando fails with 502 or 503 while pushing to try Add contributing factors
IIM-287 Nightly as Release SP3 scores regressed ~10% Add an impact statement; file at least one action item
IIM-288 AMO Database stability issues Add an impact statement
IIM-289 World Cup Widget in iOS polling live every 15mn Expand the description; file at least one action item
IIM-297 World Cup widget removed with update to Firefox 152.0.3 Add contributing factors; file at least one action item
+ +## Table 29 (7 columns) + +
Period (UTC) Period (Eastern Time) Period (Pacific Time) Incidents % Manual Detection Automated Detection
00:00–04:00 20:00–00:00 (evening) 17:00–21:00 (evening) 9 20% 8 1
04:00–08:00 00:00–04:00 (overnight) 21:00–01:00 (late evening) 1 2% 0 1
08:00–12:00 04:00–08:00 (early morning) 01:00–05:00 (overnight) 6 13% 2 4
12:00–16:00 08:00–12:00 (business hours) 05:00–09:00 (early morning) 7 16% 5 2
16:00–20:00 12:00–16:00 (business hours) 09:00–13:00 (business hours) 12 27% 7 5
20:00–24:00 16:00–20:00 (evening) 13:00–17:00 (business hours) 6 13% 6 0
Unknown — — 4 9% 3 0
diff --git a/table-probe.md b/table-probe.md new file mode 100644 index 0000000..b3d38fb --- /dev/null +++ b/table-probe.md @@ -0,0 +1,97 @@ +--- +title: markfluence table storage probe +space: ~60c36d0718e9f60071326951 +parent: 76646878 +page_id: 2913796912 +page_width: max +--- + +Each table below is hand-written raw storage testing one hypothesis about +Confluence's table attributes. Everything is pasted storage (no blank lines inside +a table, so each stays a single CommonMark HTML block and passes through verbatim). + +## T1 — bare table, no attributes (baseline) + +

T1 head A

T1 head B

bare

no attrs at all

+ +## T2 — data-layout="center", no data-table-width + +

T2 head A

T2 head B

layout=center

no width attr

+ +## T3 — data-layout="align-start", no data-table-width + +

T3 head A

T3 head B

layout=align-start

no width attr

+ +## T4 — data-layout="wide" (undocumented guess) + +

T4 head A

T4 head B

layout=wide

is this value accepted?

+ +## T5 — data-layout="full-width" (undocumented guess) + +

T5 head A

T5 head B

layout=full-width

is this value accepted?

+ +## T6 — align-start + data-table-width="1110" (known-good control) + +

T6 head A

T6 head B

layout+width

copied from the editor-authored page

+ +## T7 — colgroup in percentages instead of px + +

T7 head A

T7 head B

25%

75% — do percentage widths work?

+ +## T8 — colgroup px with no data-table-width + +

T8 head A

T8 head B

200px

600px, but no data-table-width

+ +## T9 — cell background: hex, named, and on a th + +

T9 head, hex on th

T9 head B

hex #ffebe6

named "grey" (Server-era spelling)

+ +## T10 — what markfluence emits today, plus legacy align attributes + +
T10 head AT10 head B
thead, no p-wrappersalign="right" on the td
+ +## T11 — text-align on a header cell's paragraph + +

T11 right-aligned head

T11 centered head

does header alignment

survive?

+ +## T12 — numbered column (isNumberColumnEnabled guesses) + +

T12 head A

T12 head B

data-number-column

="true"

+ +## T13 — control: an ordinary GFM table through the converter + +| Feature | Works? | +| :------ | -----: | +| GFM | yes | + +## T14 — percentage colgroup with NO data-table-width + +

T14 head A

T14 head B

30%

70%, no data-table-width — does % resolve?

+ +## T15 — full-width with percentage colgroup + +

T15 head A

T15 head B

20%

80% under full-width

+ +## T16 — full-width + data-table-width + percentage colgroup + +

T16 A (20%)

T16 B (80%)

narrow col

wide col — are the 20/80 proportions honored under full-width?

+ +## T17 — full-width + data-table-width + px colgroup + +

T17 A (200px)

T17 B (910px)

narrow col

wide col — px proportions under full-width?

+ +## T18 — wide + data-table-width + percentage colgroup (control) + +

T18 A (20%)

T18 B (80%)

narrow col

wide col — same, but layout=wide

+ +## T19 — full-width, NO data-table-width, px colgroup + +

T19 A (200px)

T19 B (910px)

px widths

no data-table-width, full-width layout

+ +## T20 — wide, NO data-table-width, px colgroup + +

T20 A (200px)

T20 B (910px)

px widths

no data-table-width, wide layout

+ +## T21 — bare table, NO layout, NO data-table-width, px colgroup + +

T21 A (200px)

T21 B (910px)

px widths

no layout at all

diff --git a/test-codeblock.md b/test-codeblock.md new file mode 100644 index 0000000..c72b9cb --- /dev/null +++ b/test-codeblock.md @@ -0,0 +1,37 @@ +--- +title: markfluence test codeblock +space: ~60c36d0718e9f60071326951 +parent: null +page_id: 2941386773 +page_width: max +--- + +# Narrow Code block + +```python +def hello(name): + print(f"hello, {name}") +``` + + +# Wide code block + +```go +// Package client is an HTTP client for the Confluence REST API. It wraps +// net/http with basic auth and the handful of calls markfluence needs. +// +// Requests are built as absolute URLs off the base URL. Pages and content +// properties use the Confluence v2 API; attachment writes and the user lookup +// use v1 (/wiki/rest/api/...) since v2 doesn't cover them. +// +// A client carries two bases. baseURL is where requests go; siteURL is the +// human-facing site the pages live on. They differ only when a cloud ID selects +// the platform API gateway (see Config): a scoped API token -- the kind a service +// account gets -- is rejected against the site domain and must go through +// https://api.atlassian.com/ex/confluence/{cloudId} instead. The path suffixes are +// identical under the gateway, so every call below is written against baseURL +// unchanged. siteURL exists because the gateway host must never reach a reader: +// it's wrong in printed URLs and, worse, would be written into published page +// content by the converter's link rewriting. +package client +``` diff --git a/test-page-no-frontmatter.md b/test-page-no-frontmatter.md new file mode 100644 index 0000000..b1b6e73 --- /dev/null +++ b/test-page-no-frontmatter.md @@ -0,0 +1,113 @@ + + +# Overview + +This page exercises markfluence's conversion features so you can eyeball the +published result. It has **bold**, _italic_, `inline code`, and a +[link to Atlassian](https://www.atlassian.com). + + + +- A bullet +- Another bullet + - Nested bullet +1. Ordered one +2. Ordered two + +## GitHub-style callout + +> [!NOTE] +> This is a GitHub-style callout; markfluence converts it to a Confluence panel. + +## Code block + +```python +def hello(name): + print(f"hello, {name}") +``` + +## Table + +| Feature | Works? | +| --------- | ------ | +| Tables | yes | +| Callouts | yes | + +# Raw Confluence storage format + +Everything below is pasted storage format (`ac:` elements), emitted verbatim. + +## Info panel with converted markdown body + + + + +This is an **info** panel. The body is markdown — note the blank lines around it — +so this [link](https://example.com) and list convert: + +- alpha +- beta + + + + +## Status macro (parameters, no body) + + +Green +DONE + + +## Expand macro + + +Click to expand + + +Hidden **markdown** content, revealed on click. + + + + +## Two-column layout + + + + + +### Left column + +Some **markdown** in the left cell, with a +[link](https://developer.atlassian.com). + + + + +### Right column + +- first +- second + + + + + +## A storage example in a code fence (should stay literal, not activate) + +```xml + +``` + +# Images + +A **local** image (uploaded as a page attachment): + +![Local test image](assets/markfluence-test.png) + +The **same local image sized and centered** via the JSON-in-title properties: + +![Sized local image](assets/markfluence-test.png '{"width":"64","align":"center"}') + +A **remote** image (referenced by URL, not attached): + +![Remote placeholder](https://placehold.co/150x60/png) diff --git a/test-page-read.md b/test-page-read.md new file mode 100644 index 0000000..c2a86d2 --- /dev/null +++ b/test-page-read.md @@ -0,0 +1,113 @@ +--- +title: markfluence test page - round trip +space: ~60c36d0718e9f60071326951 +parent: 76646878 +page_id: 2904948757 +page_width: max +--- + + + +# Overview + +This page exercises markfluence's conversion features so you can eyeball the published result. It has **bold**, *italic*, `inline code`, and a [link to Atlassian](https://www.atlassian.com). + +markfluence vdev 2026-07-22T21:14:28Z + +- A bullet +- Another bullet + - Nested bullet + +1. Ordered one +2. Ordered two + +## GitHub-style callout + +> [!NOTE] +> This is a GitHub-style callout; markfluence converts it to a Confluence panel. + +## Code block + +```python +def hello(name): + print(f"hello, {name}") +``` + +## Table + +| Feature | Works? | +| --- | --- | +| Tables | yes | +| Callouts | yes | + +# Raw Confluence storage format + +Everything below is pasted storage format (`ac:` elements), emitted verbatim. + +## Info panel with converted markdown body + +> [!NOTE] +> This is an **info** panel. The body is markdown — note the blank lines around it — so this [link](https://example.com) and list convert: +> +> - alpha +> - beta + +## Status macro (parameters, no body) + + +Green +DONE + + +## Expand macro + + +Click to expand + + +Hidden **markdown** content, revealed on click. + + + + +## Two-column layout + + + + + +### Left column + +Some **markdown** in the left cell, with a [link](https://developer.atlassian.com). + + + + +### Right column + +- first +- second + + + + + +## A storage example in a code fence (should stay literal, not activate) + +```xml + +``` + +# Images + +A **local** image (uploaded as a page attachment): + +![Local test image](assets_markfluence-test.png) + +The **same local image sized and centered** via the JSON-in-title properties: + +![Sized local image](assets_markfluence-test.png '{"width":64,"align":"center"}') + +A **remote** image (referenced by URL, not attached): + +![Remote placeholder](https://placehold.co/150x60/png) diff --git a/test-page.md b/test-page.md new file mode 100644 index 0000000..8bb4fbd --- /dev/null +++ b/test-page.md @@ -0,0 +1,121 @@ +--- +title: markfluence test page +space: ~60c36d0718e9f60071326951 +parent: 76646878 +page_id: 2848423944 +page_width: max +--- + + + +# Overview + +This page exercises markfluence's conversion features so you can eyeball the +published result. It has **bold**, _italic_, `inline code`, and a +[link to Atlassian](https://www.atlassian.com). + + + +- A bullet +- Another bullet + - Nested bullet +1. Ordered one +2. Ordered two + +## GitHub-style callout + +> [!NOTE] +> This is a GitHub-style callout; markfluence converts it to a Confluence panel. + +## Code block + +```python +def hello(name): + print(f"hello, {name}") +``` + +## Table + +| Feature | Works? | +| --------- | ------ | +| Tables | yes | +| Callouts | yes | + +# Raw Confluence storage format + +Everything below is pasted storage format (`ac:` elements), emitted verbatim. + +## Info panel with converted markdown body + + + + +This is an **info** panel. The body is markdown — note the blank lines around it — +so this [link](https://example.com) and list convert: + +- alpha +- beta + + + + +## Status macro (parameters, no body) + + +Green +DONE + + +## Expand macro + + +Click to expand + + +Hidden **markdown** content, revealed on click. + + + + +## Two-column layout + + + + + +### Left column + +Some **markdown** in the left cell, with a +[link](https://developer.atlassian.com). + + + + +### Right column + +- first +- second + + + + + +## A storage example in a code fence (should stay literal, not activate) + +```xml + +``` + +# Images + +A **local** image (uploaded as a page attachment): + +![Local test image](assets/markfluence-test.png) + +The **same local image sized and centered** via the JSON-in-title properties: + +![Sized local image](assets/markfluence-test.png '{"width":"64","align":"center"}') + +A **remote** image (referenced by URL, not attached): + +![Remote placeholder](https://placehold.co/150x60/png)