feat: canonical upload metadata vocabulary#361
Merged
Conversation
…XIF facts Three pure modules that later tasks wire in: - metadata-vocab: the ten canonical keys, the state enum with near-miss suggestions, and a capped merge that drops derived keys before explicit ones. - capture-facts: url/path/env/theme/viewport from a screenshot target. env is only ever local; inferring prod from 'not localhost' would mislabel staging. - image-facts: an EXIF allowlist (viewport/device/software/captured). GPS, serial numbers and personal-name tags are excluded by construction. Adds exif-reader; sharp exposes the raw EXIF buffer but does not parse it.
Both are sugar for the matching --meta keys; the value is --help discoverability and fail-fast validation of --state. A near-miss key like route= or page= now warns and suggests the canonical spelling, but is never rewritten — a wrong guess is worse than a nag.
screenshot now derives url/path/env/theme/viewport from what it captured. Adds derivedMetaEnabled() as the single switch for the derived tier, shared with gh.* auto-resolution. It intentionally ignores --no-git: that flag means 'don't shell out to git', which says nothing about a viewport or a URL path, and gating on it silently produced zero metadata for captures outside a repo. gh.* still requires git, checked separately at its own call site.
METADATA_DESCRIPTION suggested 'page' and 'resolution', two of the exact near-miss spellings the CLI now warns about — the schema in front of every agent call was teaching the wrong vocabulary. It now names the canonical keys and points at path as the one to search by. state is enum-constrained in the schema and re-validated in the handler, since a schema is a hint rather than an enforcement boundary.
put now reads each file's EXIF before the optimizer strips it and promotes the allowlist into queryable metadata. Per-file by construction, so one image's camera never labels another's upload. Resolves the spec's open question against apps/api/src/files-core.ts:313: any metadata sent on a put fully replaces that key's stored set. Derived keys count, so a re-upload that derives anything now replaces where it previously preserved — matching the existing gh.* tier. Pinned with a regression test and called out in the changeset; --no-auto is the opt-out. Also retires the page=/app= examples from CLI help, which taught a spelling the CLI now warns about.
The new flags had been inserted between --meta and its wrapped explanation, so the re-upload note read as if it described --app.
Cleanup pass over the vocabulary change. Two capability gaps it closes: - `uploads attach --state/--app` parsed the flags, documented them in ATTACH_HELP and completions, and the github-screenshots skill recommended them — but nothing ever read them. The flags were a silent no-op. - EXIF promotion was wired into one caller (uploadPuts' loop) rather than the shared bytes tail, so MCP put/screenshot and CLI attach derived nothing, while the shared MCP metadata description already claimed uploads.sh derives these "automatically where it can". Moved into uploadPreparedImage and uploadAttachmentBatch; MCP screenshot now derives capture facts too. Dedup: metadataArgWithCanonical replaces three near-identical MCP merge blocks; dropUnsafeMetaValues and warnNearMissMeta replace copies in capture-facts/image-facts and runPut/runScreenshot/runAttach; MCP state validation now delegates to validateStateValue, so MCP callers get the same near-miss suggestions the CLI gives instead of a generic enum list. safeCaptureFacts moves to capture-facts.ts and is shared by both surfaces.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (2)
🚫 Excluded labels (none allowed) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-auth | 18773f6 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 09:31 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-api | 18773f6 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 09:31 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-web | 18773f6 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 09:31 PM |
Zach Dunn (zachdunn)
added a commit
that referenced
this pull request
Jul 21, 2026
…d docs (#364) Follow-ups to #361, found by auditing the surfaces it did not touch. The hosted MCP worker had its own hardcoded copy of the metadata tool description, so it still told every connected agent to use 'page' and 'resolution' — the exact near-miss spellings the CLI now warns about, and the thing #361 set out to fix. It now composes the shared METADATA_DESCRIPTION. Its put tool also gains state/app; it cannot derive the EXIF keys (no image decoding in the Workers runtime), so those two are its whole canonical surface. The public docs said 'EXIF metadata is stripped'. That is now only half the story: EXIF still leaves the uploaded bytes, but an allowlist is promoted into metadata that renders publicly. Reworded so the privacy claim matches behavior, with the denials and the --no-auto opt-out spelled out. Also fixes a pre-existing missing space before the Reference link (the Astro whitespace collapse, live since #254) in the paragraph next door.
This was referenced Jul 21, 2026
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.
Gives uploads a small canonical metadata vocabulary — mostly derived — so that
uploads find path=/settings state=afterstill works months after the shot wastaken and the filename stopped meaning anything.
The vocabulary
Ten keys. Eight are derived; two are worth your keystrokes.
urlhttps://app.example/settingspath/settingsenvlocalonlylocalthemedarkviewport1280x800@2xdeviceApple iPhone 16 ProsoftwareFigmacaptured2026-07-20T20:35:39state--statebefore|afterapp--appweb|iosscreenshotknows its own target, so it stamps the first five.put/attachread each image's EXIF before the optimizer strips it and promote an
allowlist.
--stateis the highest-value thing a human or agent can add, sincebefore/after is the dominant pattern in PR screenshots and nothing can infer it.
envis only everlocal. Inferringprodfrom "not localhost" would mislabelevery staging and preview URL, and wrong metadata is worse than none.
Two behavior changes worth reading before merging
EXIF that was previously discarded now becomes public.
deviceandsoftwarerender on the public/f/page. This is deliberate and allowlisted —all GPS tags, body/lens serial numbers,
Artist/Copyright/OwnerNameandfree-form user comments are never promoted, and that denial is tested directly.
But it is a real escalation and shouldn't be merged without a nod.
Derived metadata makes re-uploads destructive.
apps/api/src/files-core.tsfull-replaces a key's metadata whenever any is sent. Derived keys count, so a
re-upload that derives something now replaces a set that was previously left
untouched. This matches the pre-existing
gh.*tier rather than being new inkind, and it's pinned by a regression test.
--no-autois the opt-out.Notes on the shape
METADATA_DESCRIPTIONsuggested
pageandresolution— two of the exact near-miss spellings theCLI now warns about. Every agent using the MCP server had been told to use
them. That's plausibly a real source of the inconsistency this fixes.
--no-gitdeliberately does not disable the derived tier, though anearlier draft of the design said it should. Implementing it that way, a test
helper reached for
--no-git— the natural choice outside a repo — andsilently got zero metadata.
--no-gitmeans "don't shell out to git" and saysnothing about a viewport.
gh.*still requires git, checked separately.route=,page=) warn and suggest the canonical spelling butare never rewritten — a wrong guess is worse than a nag.
dropped, and a full budget never fails an upload.
Verification
2134 tests pass across the monorepo; typecheck and lint clean. Beyond unit
tests, the built binary was pointed at a local listener to confirm the real
wire format — a 1624×1154 @144dpi PNG produces
viewport=812x577@2xalongsidestateandappasX-Uploads-Meta-*headers.exif-reader's output shapewas verified against real EXIF bytes rather than assumed, and pinned with a
round-trip test, since the hand-built fixtures would have passed either way.
The final commit is a cleanup pass that closed two gaps found by review:
uploads attach --state/--appwas parsed, documented, and recommended by theskill but never read — a silent no-op; and EXIF promotion was wired into one
caller rather than the shared upload tail, so MCP and attach derived nothing
while the tool description already claimed otherwise. Both now go through
uploadPreparedImage.Not included
Title/description metadata was designed and then cut as not-yet-needed, so the
"nicer PR comments" payoff isn't here. A follow-up issue for surfacing
pathand
statein the managed 📎 comment is drafted and still needs filing.