feat(uploads): auto gh.* metadata on uploads put#162
Conversation
…ccess note Final-review fix wave for uploads put auto gh.* metadata: validate the merged metadata map (not just --meta alone) on both the explicit and auto paths, with the auto path falling back to userMeta on cap overflow instead of failing the upload; make --no-git always disable auto resolution even when --auto is passed; add a human-format success note when gh.* is attached (silent on skip); add fail-fast guards for stray --auto/--no-auto values; and mention --no-git in the disable lists (PUT_HELP, SKILL.md) and design doc.
|
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 | 374f32f | Commit Preview URL Branch Preview URL |
Jul 14 2026, 03:59 AM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/uploads-cli/SKILL.md (1)
379-382: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win"Recognized keys" list is missing the new
UPLOADS_NO_AUTO_METAconfig key.
config-file.ts'sPUT_DEFAULT_KEY_MAPmapsnoAutoMetatoUPLOADS_NO_AUTO_META, making it settable/persistable viauploads config setjust likeUPLOADS_NO_GIT/UPLOADS_NO_OPTIMIZE/UPLOADS_KEEP_EXIF, but this list wasn't updated to include it.📝 Suggested fix
Recognized keys: `UPLOADS_API_URL`, `UPLOADS_WORKSPACE`, `UPLOADS_TOKEN`, `UPLOADS_DEFAULT_PREFIX`, `UPLOADS_DEFAULT_REPO`, `UPLOADS_DEFAULT_REF`, -`UPLOADS_DEFAULT_WIDTH`, `UPLOADS_NO_GIT`, `UPLOADS_NO_OPTIMIZE`, `UPLOADS_KEEP_EXIF`. +`UPLOADS_DEFAULT_WIDTH`, `UPLOADS_NO_GIT`, `UPLOADS_NO_OPTIMIZE`, `UPLOADS_KEEP_EXIF`, +`UPLOADS_NO_AUTO_META`.As per coding guidelines, "Keep the checked-in
uploads-cliskill synchronized with CLI commands and flags."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/uploads-cli/SKILL.md` around lines 379 - 382, The recognized configuration keys list in SKILL.md must include UPLOADS_NO_AUTO_META. Update the list alongside UPLOADS_NO_GIT, UPLOADS_NO_OPTIMIZE, and UPLOADS_KEEP_EXIF, preserving the existing distinction that UPLOADS_EMBED_PUBLIC_BASE_URL is environment-only.Source: Coding guidelines
🧹 Nitpick comments (3)
packages/uploads/test/commands-put.test.ts (1)
428-484: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing integration test for
UPLOADS_NO_AUTO_METAgatingrunPut.This suite exercises
--no-autosuppressing auto resolution but never exercises theUPLOADS_NO_AUTO_METAenv var throughrunPutend-to-end (onlyresolvePutDefaultsis unit-tested for it, inconfig-put-defaults.test.ts). Since it's one of the three documented opt-outs (--no-auto,--no-git,UPLOADS_NO_AUTO_META=1) and gates a real metadata-write side effect, an integration test would close the loop between the config layer andrunPut'sdefaults.noAutoMeta !== truecheck.♻️ Suggested additional test
it("UPLOADS_NO_AUTO_META=1 suppresses auto resolution end-to-end", async () => { const prev = process.env.UPLOADS_NO_AUTO_META; process.env.UPLOADS_NO_AUTO_META = "1"; try { const { client, puts } = fakeClient(); await runPut(ctxWith(client), [tmpFile(), "--repo", "o/r"], false, ghRunner({ pr: 481 })); expect(puts[0].metadata).toBeUndefined(); } finally { if (prev === undefined) delete process.env.UPLOADS_NO_AUTO_META; else process.env.UPLOADS_NO_AUTO_META = prev; } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/uploads/test/commands-put.test.ts` around lines 428 - 484, Add an integration test in the “runPut auto gh.* metadata (default path)” suite that sets UPLOADS_NO_AUTO_META to "1", invokes runPut with a resolvable PR, and verifies no metadata is written. Restore the environment variable in a finally block to preserve test isolation.skills/uploads-cli/SKILL.md (1)
231-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAuto-path
--metaprecedence is undocumented here (asymmetric with the explicit-path note below).Lines 300-305 clarify that for
--pr/--issue,gh.*wins over a same-key--meta. On this default/auto path the precedence is the opposite (--metawins over auto-derivedgh.*, per the "explicit --meta wins over auto-derived gh.*" test), but that isn't stated here, which could confuse readers into assuming both paths behave the same.📝 Suggested addition
On the default `screenshots/…` path, `put` also auto-derives GitHub context and stamps `gh.repo`/`gh.kind`/`gh.number`/`gh.ref` from the current branch's PR (or a numeric `--ref`), so the file's `/f/` page shows an "Attached to" link. This is on by default and best-effort; disable it with `--no-auto`, `--no-git`, or `UPLOADS_NO_AUTO_META=1`. +A `--meta gh.*` on this path overrides the auto-derived value (opposite of the +`--pr`/`--issue` precedence below, where the target's own `gh.*` always wins).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/uploads-cli/SKILL.md` around lines 231 - 235, Update the default screenshots/… auto-path documentation to state that explicit --meta values take precedence over auto-derived gh.repo, gh.kind, gh.number, and gh.ref values. Keep the existing best-effort auto-derivation and --no-auto/--no-git/UPLOADS_NO_AUTO_META controls unchanged, and distinguish this precedence from the explicit --pr/--issue behavior documented below.docs/superpowers/plans/2026-07-13-uploads-put-auto-gh-metadata.md (1)
461-465: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a language to the fenced CLI-help snippet.
markdownlint flags this fence for missing a language identifier (MD040).
📝 Proposed fix
-``` +```text --auto Resolve current PR/issue and stamp gh.* metadata (default on) --no-auto Skip gh.* auto-resolution on the default path (or UPLOADS_NO_AUTO_META=1)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/superpowers/plans/2026-07-13-uploads-put-auto-gh-metadata.mdaround
lines 461 - 465, Add the text language identifier to the fenced CLI-help snippet
containing the --auto and --no-auto options, changing only the fence marker
while preserving its contents.</details> <!-- cr-comment:v1:455cf42e0faf55d6a780926b --> _Source: Linters/SAST tools_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@packages/uploads/src/commands.ts:
- Around line 643-655: Update the auto-target branch in the metadata merge flow
so attachedRef is sourced from the validated merged metadata rather than
directly from autoMeta. Set it consistently with the final metadata persisted
when validation succeeds, while preserving the existing fallback to userMeta
when merged metadata exceeds the caps.
Outside diff comments:
In@skills/uploads-cli/SKILL.md:
- Around line 379-382: The recognized configuration keys list in SKILL.md must
include UPLOADS_NO_AUTO_META. Update the list alongside UPLOADS_NO_GIT,
UPLOADS_NO_OPTIMIZE, and UPLOADS_KEEP_EXIF, preserving the existing distinction
that UPLOADS_EMBED_PUBLIC_BASE_URL is environment-only.
Nitpick comments:
In@docs/superpowers/plans/2026-07-13-uploads-put-auto-gh-metadata.md:
- Around line 461-465: Add the text language identifier to the fenced CLI-help
snippet containing the --auto and --no-auto options, changing only the fence
marker while preserving its contents.In
@packages/uploads/test/commands-put.test.ts:
- Around line 428-484: Add an integration test in the “runPut auto gh.* metadata
(default path)” suite that sets UPLOADS_NO_AUTO_META to "1", invokes runPut with
a resolvable PR, and verifies no metadata is written. Restore the environment
variable in a finally block to preserve test isolation.In
@skills/uploads-cli/SKILL.md:
- Around line 231-235: Update the default screenshots/… auto-path documentation
to state that explicit --meta values take precedence over auto-derived gh.repo,
gh.kind, gh.number, and gh.ref values. Keep the existing best-effort
auto-derivation and --no-auto/--no-git/UPLOADS_NO_AUTO_META controls unchanged,
and distinguish this precedence from the explicit --pr/--issue behavior
documented below.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `478d080a-33e6-4680-ae21-2d75f5ae7260` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 528d8959735c2022a1099750c9635b7a4e386ae9 and ae007da4f497fcceafb6e97b67f37086f9e33996. </details> <details> <summary>📒 Files selected for processing (10)</summary> * `.changeset/put-auto-gh-metadata.md` * `docs/superpowers/plans/2026-07-13-uploads-put-auto-gh-metadata.md` * `docs/superpowers/specs/2026-07-13-uploads-put-auto-gh-metadata-design.md` * `packages/uploads/src/commands.ts` * `packages/uploads/src/config-file.ts` * `packages/uploads/src/github-gh.ts` * `packages/uploads/test/commands-put.test.ts` * `packages/uploads/test/config-put-defaults.test.ts` * `packages/uploads/test/github-gh.test.ts` * `skills/uploads-cli/SKILL.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
- auto path: source the success-note ref from the merged map (consistent with the stored metadata and the explicit-target path) - docs: add UPLOADS_NO_AUTO_META to the recognized config keys, document auto-path --meta precedence, fence the plan-doc help snippet - test: UPLOADS_NO_AUTO_META=1 gating runPut end-to-end
Zach Dunn (zachdunn)
deleted the
claude/buildinternet-bucket-metadata-1cf770
branch
1 participant
Summary
uploads putnow stamps the fourgh.*metadata pairs (gh.repo,gh.kind,gh.number,gh.ref) whenever it has a GitHub target, so hosted screenshots show an "Attached to" PR/issue link on their/f/file page. One uniform rule: any resolved GitHub target — explicit or auto — stamps metadata.Motivated by the
buildinternetbucket sweep (done separately against prod): its 65 legacyscreenshots/objects had lost their PR linkage precisely because the defaultputpath captured nogh.*. This closes that gap at the source.What changed
put --pr/--issuenow stampsgh.*— previously it wrote the stablegh/…key but no queryable metadata (onlyuploads attachdid). Fixed so any explicit target stamps it, validating the merged map client-side (matchesattach).screenshots/…path auto-resolves gh context — on by default: resolves the current branch's PR viagh pr view, or classifies a numeric--refas pull/issue viagh api. Best-effort — missinggh, no PR, an API error, or an over-cap metadata merge all upload normally without failing. Key layout is unchanged (auto never producesgh/keys).--auto/--no-autoflags and aUPLOADS_NO_AUTO_METAenv/config default (mirrorsUPLOADS_NO_GIT).--no-gitalso disables auto (no repo to resolve). A human-format>> attached to <ref>note prints on success; silent on skip.--meta; on the auto path explicit--metawins over auto-derived pairs.Testing
@buildinternet/uploads; typecheck clean.ghinvocation shapes through the injectableCommandRunnerseam (no network): branch→PR resolution, numeric-ref classification, no-PR skip,--no-auto/--no-gitgating, cap-overflow drop on the auto path (never fails), success-note present/absent, and stray-value guards.Notes
minor). Thegithub-screenshotsskill is untouched — it inherits this throughuploads put.docs/superpowers/.--no-gitgate, success note) before this PR.Summary by CodeRabbit
New Features
uploads putnow automatically attaches GitHub context to screenshot uploads when available, enabling “Attached to” links and searchable metadata.--autoand--no-autocontrols, plusUPLOADS_NO_AUTO_META=1configuration.Documentation