feat: attachment subcommands (list, upload, download) - #58
Merged
Conversation
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
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
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
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
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  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
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
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
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds standalone attachment management, complementing the automatic sync that
create/updatealready do for a page's images. Closes #9.Design notes and rejected alternatives are in
_plans/019_attachment-subcommands.md.Commands
Flat and noun-first so cobra's alphabetized help keeps them together and
attachment-<TAB>completes as a group.attachment-listshows what a publish will and won't touch:SOURCEis the Markdown image path an attachment was published from. A dash means none is recorded — hand-uploaded, or published before markfluence recorded them;--json'smanagedtells those apart. The second row above is a real underscore-era orphan from the encoding change in #18, which this command is how you find.Decisions worth review
Upload naming. Files attach under their base name.
--nametakes a path and markfluence encodes it, so--name assets/x.pngproduces the attachmentresolves to without anyone typing an escape. The recordedpath=is always the decode of the stored name — if they could disagree, a later publish would upload a second attachment under the name it computes while a restoring download put this one where the Markdown never references it.Download restores layout by default. Round-trip is the point; a download yielding
docs%2Fassets%2Fx.pngleaks Confluence's no-slash restriction into your filesystem and breaks local preview. Restoration reads the recordedpath=and never decodes the stored name — a hand-uploadeda%2Fb.pngis indistinguishable from a published one, so decoding by default would scatter a literally-named file intoa/b.png.--flatopts out.The traversal clamp.
destPathis the only place server data becomes a filesystem path...can't simply be refused (an image above its page is a supported layout per #18), so the resolved path is compared against--dest, and an escape is an error for that attachment rather than a silent clip — attachment comments are controlled by anyone who can edit the page. #18 deferred this clamp to #37; it lands here instead, since this is the first code that writes attachment bytes.No
download_urlin--json. On the site URL it 401s under a scoped token; on the request base it would leak the gateway host into reader-facing output.Also in here
internal/pagerefunifies page-argument resolution.infotook an id or a.mdfile;readtook an id or a URL; neither was a superset. Now all three work everywhere — additive on both sides. The placeholder isPAGErather thanARG, which only made sense while the commands disagreed.Attachment(fileSize/mediaType/version/download link),DownloadAttachment,ForceUploadAttachments, andListAttachmentsnow paginates bystart/limitoffset instead of a hardcodedlimit=250that silently truncated.Verified against the live API
The v1 shape was probed before the client was written, and corrected one assumption:
_links.downloadis/rest/api/content/{page}/child/attachment/{id}/download, not the/download/attachments/…UI path — being an API path, it works through the gateway. It 302s toapi.media.atlassian.comwith its own short-lived token. Go dropsAuthorizationon a cross-host redirect, so site credentials never reach that host;TestDownloadAttachmentDoesNotLeakCredentialsOnRedirectpins it, addressing the media server aslocalhostagainst a127.0.0.1origin so the hop is genuinely cross-host rather than passing vacuously.End to end against a real page: upload created → skipped unchanged →
--forcebumped v1→v2; download restoredassets/markfluence-test.pngbyte-identical to the source, the orphan under its stored name,--flatand--forceas specified, and a missing name exiting 1.make test && make lint && make vetpass.