mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-21 01:53:33 +00:00
xarmian
c70186547a
* feat(mcp): wire attachment list + show; reject upload/download/view as CLI-only (TASK-968 partial)
Final slice of TASK-968's surface expansion. Two attachment commands
wired (the metadata-only ones), three rejected as noRemoteEquivalent
(those that take a local filesystem `<path>` argument), and the
noRemoteEquivalent map gets a small refactor to carry per-entry
rationale clauses.
New commands:
- attachment list → custom dispatcher with --item ref→UUID
resolution, --attached/--unattached
mutex fold, full filter pass-through
(category, collection, sort, limit,
offset).
- attachment show → HEAD /api/v1/workspaces/{ws}/attachments/{id}
packaging response headers
(Content-Type, -Length, -Disposition,
ETag, Last-Modified) into the CLI's
--format json shape:
{id, mime, size, filename?, etag?,
last_modified?}
Rejected as noRemoteEquivalent (with per-command rationale):
- attachment upload → needs a local filesystem `<path>` arg;
agents fetch raw bytes via the
attachment URL directly.
- attachment download → writes to a local filesystem `<out-path>`;
agents read raw bytes via the URL.
- attachment view → writes to a local filesystem path and
prints it; agents read bytes via the URL.
Refactor: noRemoteEquivalent went from `map[string]struct{}` to
`map[string]string` where the value is a rationale clause appended
to the error message. The previous generic message ("operates on
local pad client / config state, not the workspace") was misleading
for attachments (which DO operate on workspace state, just via a
local filesystem argument). Per-entry rationale lets each rejection
point at the alternative path agents should use — e.g. github
commands now suggest `item update --field github_pr=...`,
attachment commands point at `attachment show` + the URL.
The `--item TASK-5` resolution on `attachment list` reuses the
existing resolveItemRef helper (introduced in PR #346 for the link
commands), so the OAuth-scope hook (d.Apply) applies uniformly to
the prefetch — no scope bypass.
`parseAttachmentFilename` reproduces the CLI helper of the same
name. Handles both the bare `filename="value"` form and the RFC
5987 `filename*=UTF-8''<urlencoded>` form, preferring the latter
when both appear (spec-compliant carrier for non-ASCII names).
Tests:
- attachment list happy path with full query string forwarding
- --attached/--unattached fold + mutex rejection
- --item ref → UUID resolution end-to-end (and abort-on-resolution-
failure pin)
- attachment show: header → JSON extraction with all five fields
populated
- --variant query forwarding
- 404 surfaces as IsError
- parseAttachmentFilename: standard/quoted/unquoted/RFC5987 cases
- Per-entry rationale verified: github vs attachment messages
differ (the refactor's behavioural test).
- Integration smoke: attachment list against fresh workspace
(returns total=0); upload/download/view rejected with stable
"no remote equivalent" prefix.
TASK-968 is now functionally complete: 38 commands wired across
PR #346, #347, #348, #349, this PR, plus 13 commands explicitly
rejected as noRemoteEquivalent. The route table expansion lifts
the dispatcher from TASK-965's seed of 1 command to the full
mid-tier MCP surface PLAN-943's TASK-950 needs.
Parent: PLAN-943.
* fix(mcp): use mime.ParseMediaType for Content-Disposition parsing per Codex review (round 1)
Codex caught that the previous strings.Split(";") approach in
parseAttachmentFilename chopped quoted filenames containing semicolons
at the first internal `;`, returning `"a"` for
`attachment; filename="a;b.png"`. The CLI's helper of the same name
uses mime.ParseMediaType which respects the quote boundaries, so
the MCP path was diverging from CLI behaviour.
Fix: replace the hand-rolled splitter with mime.ParseMediaType +
filepath.Base(name) — matching the CLI exactly. mime.ParseMediaType
also handles the RFC 5987 `filename*=UTF-8''<urlencoded>` form
automatically, so we no longer need the explicit precedence check
either.
filepath.Base is the same defensive base the CLI applies even
though the server is supposed to sanitize before emitting the
header — keeps a stray `../` from sneaking through.
Tests added:
- PreservesSemicolonsInQuotedFilename pins the
`attachment; filename="a;b.png"` regression Codex flagged.
- AppliesBasenameDefense pins the directory-stripping behaviour
that filepath.Base provides.
Pre-existing tests (StandardForm, PrefersFilenameStarOverFilename,
HandlesQuotedAndUnquoted) still pass — mime.ParseMediaType handles
all three cases correctly.
Parent: PLAN-943.