diff --git a/.changeset/pr-aware-upload-context.md b/.changeset/pr-aware-upload-context.md new file mode 100644 index 00000000..2777fc72 --- /dev/null +++ b/.changeset/pr-aware-upload-context.md @@ -0,0 +1,24 @@ +--- +"@buildinternet/uploads": minor +--- + +Make bare `put`/`screenshot` PR-aware (issue #700). Three changes: + +- The bare-upload nudge (issue #393) is now concrete: it names the actual open + PR and a ready-made follow-up naming the actual uploaded key(s), e.g. + `uploads attach --pr 1250 f/abc123.webp`. It's now surfaced in the `hint` + field for `--format json` and in the local stdio MCP `put`/`screenshot` + tool responses, not only on stderr. +- Default behavior change: a bare `put`/`screenshot` on a git branch that + maps to exactly one open PR now behaves as if `--pr ` had been passed — + stable key, managed comment sync — instead of the previous branch-staging + default. Opt out per-call with `--no-pr`, or globally with + `UPLOADS_NO_AUTO_PR=1` (env or config file). Never fires outside a git + checkout, on the default branch, with `--no-git`, when any explicit + destination flag is set, or when no single open PR can be resolved — those + cases fall back to the existing branch-staging/dated-layout behavior + unchanged. +- `uploads hook pre-pr-screenshot` now also suggests promoting + staged-but-unattached files (`uploads attach --promote --pr `) when it + detects them ahead of `gh pr create`, alongside its existing "stage + screenshots first" advisory. diff --git a/docs/cli.md b/docs/cli.md index 7ff7b327..9b251727 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -139,6 +139,42 @@ uploads put ./after.png --pr 123 --alt "Dashboard after" # key: gh///pull/123/after.webp (PNG optimized to WebP; extension follows the output) ``` +**Auto-PR context** (issue #700). A bare `put`/`screenshot` — no +`--pr`/`--issue`/`--branch`/`--key`/`--ref`/`--prefix`/`--destination` — on a +git branch that maps to exactly one open PR now behaves as if `--pr ` had +been passed: stable `gh/` key, managed comment sync, same as the explicit +form above. A one-line note announces it: + +```bash +uploads put ./after.png +# note: branch maps to open PR #1250 — auto-attached (stable key + managed comment sync). +# key: gh///pull/1250/after.webp +``` + +This is a default-behavior change from the previous "stage to the branch +prefix" default (still issue #403's behavior — see below) whenever a PR +already exists for the branch. Opt out per-call with `--no-pr`, or globally +with `UPLOADS_NO_AUTO_PR=1` (env or config file). It never fires outside a +git checkout, on the default branch, with `--no-git`, or when no single open +PR can be resolved for the branch — those cases fall back to branch staging, +then the plain dated layout, unchanged. When it doesn't fire and the upload +still lands on the dated layout with a detectable PR (e.g. an explicit `--ref` +opts out of both staging and auto-PR), a similar one-line nudge names the PR +and a ready-made follow-up naming the actual uploaded key(s): + +```bash +uploads put ./after.png --ref manual +# note: on branch feature/thing (PR #1250 open) — rerun with --pr 1250 for a +# stable key plus a managed comment that collects this PR's media, or stage +# pre-PR files with: uploads attach --branch. Already uploaded? +# uploads attach --pr 1250 f/.webp +``` + +Both notes appear in the `hint` field for `--format json` and in the stdio +MCP `put`/`screenshot` tool responses, not only on stderr — suppress either +with `--quiet`, `UPLOADS_NO_NUDGE=1` (env or config), or the MCP tools' +`noPr`/`noGit` arguments as appropriate. + **Managed attachments comment** creates or updates a single comment listing every file attached to that PR or issue. `put --pr`/`--issue` syncs it by default (posts via the GitHub App bot, falling back to local `gh` when the @@ -327,6 +363,21 @@ reference it defers to. covers callouts and redaction (`uploads annotate` / `uploads screenshot --annotate`). See [api.md](api.md) for the REST routes. +`uploads hook pre-pr-screenshot` is a `gh pr create` PreToolUse hook +(installed by `uploads install`/`hooks-install`). It's non-blocking and +fail-open (silent when unconfigured or on any error). Two mutually exclusive +advisories: + +- staged-but-unattached files already exist for the branch (`gh.status=staged`) + → a promote suggestion (issue #700): `uploads attach --promote --pr ` + once the PR this command is about to open exists (or a bare + `uploads attach --promote`, which infers the PR from the branch). +- nothing is staged, but the branch touches UI files (`.astro`/`.tsx`/`.jsx`/ + `.vue`/`.svelte`/`.html`/`.css`/`.scss`/`.less`, or an `/email/` path) → + the original (issue #379) suggestion to stage screenshots first. + +Disable with `UPLOADS_HOOK_DISABLE=1`. + The same install step also wires a fail-open pre-PR screenshot reminder for **Grok** and **Cursor** when those tools are present. **Claude Code** and **Codex** get that reminder from their plugins instead (both run diff --git a/packages/uploads/src/commands.ts b/packages/uploads/src/commands.ts index 183a4f79..161f0be6 100644 --- a/packages/uploads/src/commands.ts +++ b/packages/uploads/src/commands.ts @@ -240,6 +240,7 @@ Options: --no-git Don't derive --repo from git (or UPLOADS_NO_GIT=1) --auto Resolve current PR/issue and stamp gh.* metadata (default on) --no-auto Skip gh.* auto-resolution (also skipped by --no-git or UPLOADS_NO_AUTO_META=1) + --no-pr Skip auto-PR context (or UPLOADS_NO_AUTO_PR=1) — see below --workspace, -w Override workspace (wins over UPLOADS_WORKSPACE and token inference) --format human|url|markdown|json --pr Attach to a pull request: key gh///pull// (stable URL, no hash) @@ -263,9 +264,17 @@ Options: --dry-run Print key + public URL without uploading; reports if the key would replace (or, on a strict key, be refused). Not with --gallery -A bare put (no --pr/--issue/--key) on a non-default git branch prints a one-line -nudge toward --pr/attach --branch (stderr in human mode, a "hint" field in ---format json). Suppress with --quiet, UPLOADS_NO_NUDGE=1, or config UPLOADS_NO_NUDGE=1. +A bare put (no --pr/--issue/--key/--ref/--prefix/--destination) on a git branch +that maps to exactly one open PR now behaves as if --pr had been passed +(issue #700): stable gh/ key, managed comment sync — instead of the #403 +branch-staging default. A one-line note announces this (stderr in human mode, +the "hint" field in --format json). Opt out with --no-pr, UPLOADS_NO_AUTO_PR=1, +or config UPLOADS_NO_AUTO_PR=1; never fires outside a git repo, on the default +branch, with --no-git, or when no single open PR can be resolved (falls back to +branch staging, then the plain dated layout). When it doesn't fire and the +upload lands on the dated layout with a detectable PR, a similar one-line nudge +names the PR and a ready-made follow-up (uploads attach --pr ...). +Suppress either note with --quiet, UPLOADS_NO_NUDGE=1, or config UPLOADS_NO_NUDGE=1. Exit codes: 0 ok · 2 usage/token/file · 3 auth/policy · 4 network · 1 other (incl. partial multi-file failure). Scripted formats (json|url|markdown) also print failures on stdout. @@ -1865,45 +1874,67 @@ async function runAttachPromoteOnly( const PUT_NUDGE_GH_TIMEOUT_MS = 3000; /** - * The bare-put nudge's wording (issue #393): teaches `--pr`/`attach --branch` - * as an upgrade from a targetless `put`. `pr` present → names the PR; - * otherwise a generic variant that still points at `--pr `. Used - * verbatim for both the human-mode stderr line and the JSON `hint` field. + * The bare-put nudge's wording (issue #393, made concrete by issue #700): + * teaches `--pr`/`attach --branch` as an upgrade from a targetless `put`. + * `pr` present → names the PR and, once upload `keys` are known, appends a + * ready-made follow-up command naming them verbatim (e.g. `uploads attach + * --pr 1250 f/abc123.webp`); otherwise a generic variant that still points + * at `--pr `. Used verbatim for both the human-mode stderr line and the + * JSON `hint` field. */ -function putNudgeText(branch: string, pr: number | undefined): string { +export function putNudgeText(branch: string, pr: number | undefined, keys: string[] = []): string { const prClause = pr !== undefined ? ` (PR #${pr} open) — rerun with --pr ${pr}` : ` — rerun with --pr `; - return ( + const base = `note: on branch ${branch}${prClause} for a stable key plus a managed comment ` + - `that collects this PR's media, or stage pre-PR files with: uploads attach --branch` + `that collects this PR's media, or stage pre-PR files with: uploads attach --branch`; + if (pr === undefined || keys.length === 0) return base; + return `${base}. Already uploaded? uploads attach --pr ${pr} ${keys.join(" ")}`; +} + +/** + * Auto-PR note (issue #700): announces at the moment it fires that a bare + * put/screenshot on this branch was auto-attached to `pr` — the default + * behavior change this issue introduces — and how to opt out. + */ +export function autoPrNoteText(pr: number): string { + return ( + `note: branch maps to open PR #${pr} — auto-attached (stable key + managed comment sync). ` + + `Opt out with --no-pr or UPLOADS_NO_AUTO_PR=1.` ); } /** - * Best-effort bare-put nudge (issue #393): fires only when `put` has no - * targeting flag at all (`--pr`/`--issue`/`--key`; `--branch` too, though + * Best-effort bare-put/screenshot nudge context (issue #393): resolves the + * branch and, when detectable, the open PR for it — fires only when there is + * no targeting flag at all (`--pr`/`--issue`/`--key`; `--branch` too, though * `put` doesn't currently accept it — defensive parity with `attach`), is * inside a git repo (reusing `deriveRepoFromGit`, the same detection the * default screenshot key's repo segment uses), and the current branch isn't * the default one. Never throws — any failure (not a repo, detached HEAD, * `gh` missing/unauthenticated/timed out) degrades to "no nudge" or, once a - * branch is already known, to the generic no-PR wording. Must never affect - * put's exit code, stdout, or upload behavior. + * branch is already known, to a context with `pr: undefined` (the generic + * no-PR wording). Must never affect put's exit code, stdout, or upload + * behavior. Callers turn the result into text via `putNudgeText`, once any + * upload keys are known. */ -function resolvePutNudge(opts: { - ctx: CliContext; - flags: CommandFlags["flags"]; +export function resolvePutNudgeContext(opts: { + quiet: boolean; + noNudge: boolean; ghTarget: GhTarget | undefined; keyHint: string | undefined; + /** True when an explicit `--branch`-style flag was given (CLI `attach` + * parity; `put`/MCP `put` don't accept one — pass false there). */ + hasBranchFlag?: boolean; noGit: boolean; - defaults: PutDefaults; + repoArg: string | undefined; run: CommandRunner; -}): string | undefined { - const { ctx, flags, ghTarget, keyHint, noGit, defaults, run } = opts; - if (ctx.quiet) return undefined; - if (defaults.noNudge) return undefined; +}): { branch: string; pr: number | undefined } | undefined { + const { quiet, noNudge, ghTarget, keyHint, hasBranchFlag, noGit, repoArg, run } = opts; + if (quiet) return undefined; + if (noNudge) return undefined; if (ghTarget || keyHint || noGit) return undefined; - if (flags.has("--branch")) return undefined; // not a real put flag today; defensive only + if (hasBranchFlag) return undefined; // not a real put flag today; defensive only try { if (deriveRepoFromGit(run) === undefined) return undefined; // not a (usable) git repo let branch: string; @@ -1925,18 +1956,87 @@ function resolvePutNudge(opts: { // fast/fake, and execFileSync's `timeout` option is meaningless // against anything that isn't actually shelling out. const timed = run === execRunner ? timedExecRunner(PUT_NUDGE_GH_TIMEOUT_MS) : run; - const repoArg = flagString(flags, "--repo") ?? defaults.repo; const repo = resolveRepo(repoArg, timed); pr = resolveCurrentPullRequest(repo, timed).num; } catch { pr = undefined; // gh missing/unauthenticated/timed out/no open PR — generic wording } - return putNudgeText(branch, pr); + return { branch, pr }; } catch { return undefined; } } +/** + * Auto-PR context (issue #700): when a bare put/screenshot has no explicit + * destination flag at all (`--pr`/`--issue`/`--key`/`--ref`/`--prefix`/ + * `--destination`, and for `screenshot` no explicit `--branch`) and runs on a + * branch that maps to exactly one open PR, this resolves that PR so the + * caller can behave as if `--pr ` had been passed — stable key + managed + * comment sync — instead of the #403/#469 staging default or the plain dated + * layout. `resolveCurrentPullRequest`'s `gh pr view ` lookup is + * already the unambiguous case: it names the single open PR whose head is + * that branch, or fails (no open PR, or `gh` unavailable/unauthenticated) — + * there is no "ambiguous, more than one" state to further disambiguate. + * Opt-out: `noAutoPr` (the caller folds in `--no-pr` and + * `UPLOADS_NO_AUTO_PR=1`/config). Never fires outside a git checkout, on the + * default branch, or with `--no-git`; any failure (not a repo, detached + * HEAD, gh missing/unauthenticated/timed out, no open PR) degrades to + * undefined so the caller falls back to its normal staging/dated behavior. + */ +export function resolveAutoPrTarget(opts: { + ghTarget: GhTarget | undefined; + keyHint: string | undefined; + refArg: string | undefined; + prefixArg: string | undefined; + destinationArg: string | undefined; + /** Explicit `--branch` (screenshot only) also opts out — put has no + * `--branch` flag today, so callers pass undefined there. */ + branchArg?: string | undefined; + noGit: boolean; + noAutoPr: boolean; + repoArg: string | undefined; + run: CommandRunner; +}): GhTarget | undefined { + const { + ghTarget, + keyHint, + refArg, + prefixArg, + destinationArg, + branchArg, + noGit, + noAutoPr, + repoArg, + run, + } = opts; + if (noAutoPr) return undefined; + if (ghTarget || keyHint || noGit) return undefined; + if (refArg || prefixArg || destinationArg || branchArg !== undefined) return undefined; + try { + if (deriveRepoFromGit(run) === undefined) return undefined; // not a (usable) git repo + let branch: string; + try { + branch = resolveCurrentBranch(run); + } catch { + return undefined; // detached HEAD, or git unavailable + } + const defaultBranch = resolveDefaultBranch(run); + const onDefaultBranch = defaultBranch + ? branch === defaultBranch + : branch === "main" || branch === "master"; // undetermined: err toward the old default + if (onDefaultBranch) return undefined; + + // Same bounded-timeout treatment as the #393 nudge's `gh pr view` call — + // this must never be felt as a hang. + const timed = run === execRunner ? timedExecRunner(PUT_NUDGE_GH_TIMEOUT_MS) : run; + const repo = resolveRepo(repoArg, timed); + return resolveCurrentPullRequest(repo, timed); + } catch { + return undefined; // gh/git unavailable, no open PR, or repo unresolvable + } +} + /** * Bare-put branch-staging trigger (issue #403): put on a non-default git * branch stages to the branch prefix by default — the branch becomes the @@ -2227,10 +2327,40 @@ export async function runPut( } const multi = files.length > 1; + // Resolved early (issue #700): both the auto-PR opt-out default and the + // `--no-git`-gated staging/auto-PR detection below need it before the rest + // of put's flag parsing. + const defaults = resolvePutDefaults({ envFile: ctx.envFile }); + const noGit = flagBool(parsed.flags, "--no-git") || defaults.noGit === true; + const keyHint = flagString(parsed.flags, "--key"); const destFlag = flagString(parsed.flags, "--destination"); const prefixFlag = flagString(parsed.flags, "--prefix"); const ghTarget = ghTargetFromFlags(parsed.flags, run); + if (parsed.flags.has("--no-pr") && typeof parsed.flags.get("--no-pr") === "string") { + throw new UsageError("--no-pr takes no value"); + } + const noAutoPr = flagBool(parsed.flags, "--no-pr") || defaults.noAutoPr === true; + // Auto-PR context (issue #700): a bare put (no --pr/--issue/--key/--ref/ + // --prefix/--destination, not --no-git/--no-pr) on a branch that maps to + // exactly one open PR behaves as if `--pr ` had been passed — see + // resolveAutoPrTarget. Supersedes both the #403 staging default and the + // #393 nudge for this case; computed before the gh.* metadata resolution + // below since it takes over that resolution entirely. + const autoPrTarget: GhTarget | undefined = ghTarget + ? undefined + : resolveAutoPrTarget({ + ghTarget, + keyHint, + refArg: flagString(parsed.flags, "--ref"), + prefixArg: prefixFlag, + destinationArg: destFlag, + noGit, + noAutoPr, + repoArg: flagString(parsed.flags, "--repo") ?? defaults.repo, + run, + }); + const effectiveGhTarget = ghTarget ?? autoPrTarget; // Comment sync runs by default with --pr/--issue (matches `attach`); opt // out with --no-comment. --comment is accepted as a redundant no-op for // back-compat with scripts written before this default flipped (#537). @@ -2264,7 +2394,7 @@ export async function runPut( if (parsed.flags.has("--no-auto") && typeof parsed.flags.get("--no-auto") === "string") { throw new UsageError("--no-auto takes no value"); } - if (parsed.flags.has("--no-comment") && !ghTarget) { + if (parsed.flags.has("--no-comment") && !effectiveGhTarget) { throw new UsageError("--no-comment requires --pr or --issue"); } if (multi) { @@ -2299,7 +2429,7 @@ export async function runPut( destination: destFlag, prefix: prefixFlag, key: keyHint, - ghAttachment: Boolean(ghTarget), + ghAttachment: Boolean(effectiveGhTarget), }); } catch (err) { throw new UsageError(err instanceof Error ? err.message : String(err)); @@ -2314,7 +2444,6 @@ export async function runPut( throw new UsageError(`invalid --format: ${raw}`); })(); - const defaults = resolvePutDefaults({ envFile: ctx.envFile }); const optimizeOpts = optimizeOptionsFromFlags(parsed.flags, defaults); const frameOpts = frameOptionsFromFlags(parsed.flags); const contentTypeOverride = flagString(parsed.flags, "--content-type"); @@ -2329,17 +2458,16 @@ export async function runPut( })() : defaults.width; - const noGit = flagBool(parsed.flags, "--no-git") || defaults.noGit === true; - // Bare-put branch staging (issue #403): a bare put (no --pr/--issue/--key/ // --ref/--prefix/--destination, not --no-git) on a non-default git branch // stages to the branch prefix — identical key/metadata to `attach // --branch` — instead of the dated layout. Computed before gh.* metadata // resolution below since it takes over that resolution entirely (branch // metadata, not PR/issue metadata) and supersedes the #393 nudge for this - // case. + // case. `effectiveGhTarget` (explicit --pr/--issue OR the #700 auto-PR + // match) wins over staging, same as it wins over the dated layout. const stagingTarget = resolvePutStagingTarget({ - ghTarget, + ghTarget: effectiveGhTarget, keyHint, refArg: flagString(parsed.flags, "--ref"), prefixArg: prefixFlag, @@ -2349,15 +2477,16 @@ export async function runPut( run, }); - // gh.* metadata: explicit --pr/--issue target wins over --meta; staging - // wins over --meta the same way (matches attach --branch); otherwise - // best-effort auto resolution (on by default) where --meta wins. --no-git, - // --no-auto, or UPLOADS_NO_AUTO_META disable auto; --auto forces past the - // config default but never past --no-git (no repo to resolve). + // gh.* metadata: explicit --pr/--issue target (or the #700 auto-PR match) + // wins over --meta; staging wins over --meta the same way (matches attach + // --branch); otherwise best-effort auto resolution (on by default) where + // --meta wins. --no-git, --no-auto, or UPLOADS_NO_AUTO_META disable auto; + // --auto forces past the config default but never past --no-git (no repo + // to resolve). let metadata = userMeta; let attachedRef: string | undefined; - if (ghTarget) { - const merged = { ...userMeta, ...ghMetadataFromTargetWithTitle(ghTarget, run) }; + if (effectiveGhTarget) { + const merged = { ...userMeta, ...ghMetadataFromTargetWithTitle(effectiveGhTarget, run) }; validateMetaMap(merged); // enforce 24-key/8KB caps on the merged map (matches attach) metadata = merged; attachedRef = merged["gh.ref"]; @@ -2411,25 +2540,29 @@ export async function runPut( const contextNudge = !ctx.quiet && !defaults.noNudge && !noGit ? noProjectContextNudge(metadata) : undefined; - // Bare-put nudge (issue #393): only relevant when staging didn't take over - // — once `stagingTarget` resolves, staging IS the upgrade the nudge used to - // point at, so this is skipped entirely rather than firing redundantly. - // Still fires as before for a bare put that lands on the dated layout with - // a detectable PR (e.g. an explicit --ref/--prefix opts out of staging). - // Computed once, used for both the trailing stderr line (human mode) and - // the JSON `hint` field below. Best-effort — see resolvePutNudge; never - // affects exit code, stdout, or the upload. - const nudge = stagingTarget - ? undefined - : resolvePutNudge({ - ctx, - flags: parsed.flags, - ghTarget, - keyHint, - noGit, - defaults, - run, - }); + // Bare-put nudge (issue #393): only relevant when neither auto-PR nor + // staging took over — once `effectiveGhTarget`/`stagingTarget` resolves, + // that IS the upgrade the nudge used to point at, so this is skipped + // entirely rather than firing redundantly. Still fires as before for a + // bare put that lands on the dated layout with a detectable PR (e.g. an + // explicit --ref/--prefix opts out of staging AND auto-PR, or --no-pr/ + // UPLOADS_NO_AUTO_PR opts out of auto-PR specifically). The concrete + // key-naming text (issue #700) is finished below, once upload keys exist. + // Best-effort — see resolvePutNudgeContext; never affects exit code, + // stdout, or the upload. + const nudgeContext = + effectiveGhTarget || stagingTarget + ? undefined + : resolvePutNudgeContext({ + quiet: ctx.quiet, + noNudge: defaults.noNudge === true, + ghTarget, + keyHint, + hasBranchFlag: parsed.flags.has("--branch"), + noGit, + repoArg: flagString(parsed.flags, "--repo") ?? defaults.repo, + run, + }); // Staging note (issue #403): same suppression as the #393 nudge // (--quiet, UPLOADS_NO_NUDGE=1 env/config); staging itself is NOT gated by @@ -2439,6 +2572,14 @@ export async function runPut( ? putStagingNoteText(stagingTarget.branch) : undefined; + // Auto-PR note (issue #700): announces the default-behavior change at the + // moment it fires, so a bare put that silently became a --pr attach isn't + // a surprise — names the PR and how to opt out. Same suppression as the + // other advisories (--quiet, UPLOADS_NO_NUDGE=1); NOT gated by --no-pr/ + // UPLOADS_NO_AUTO_PR since those are what prevent it from firing at all. + const autoPrNote = + autoPrTarget && !ctx.quiet && !defaults.noNudge ? autoPrNoteText(autoPrTarget.num) : undefined; + const logHuman = !ctx.quiet && format === "human"; if (logHuman) { if (multi) { @@ -2457,7 +2598,7 @@ export async function runPut( files, nameOverride: nameFlag, explicitKey: keyHint, - ghTarget, + ghTarget: effectiveGhTarget, ghBranchTarget: stagingTarget, prefix: resolvedPrefix ?? defaults.prefix, repo: flagString(parsed.flags, "--repo") ?? defaults.repo, @@ -2479,6 +2620,19 @@ export async function runPut( throw firstError instanceof Error ? firstError : new Error(String(firstError)); } + // Concrete bare-put nudge text (issue #700): built once upload keys exist, + // so the ready-made follow-up names them, e.g. + // "uploads attach --pr 1250 f/abc123.webp". Falls back to the plain + // #393 wording when there are no successful uploads to name. + const nudge = + nudgeContext && uploads.length > 0 + ? putNudgeText( + nudgeContext.branch, + nudgeContext.pr, + uploads.map((u) => u.key), + ) + : undefined; + // Stage-time binding warning (issue #398/#400): same check `attach // --branch` runs, now also on the bare-put staging path. Best-effort — see // resolveStageBindingWarning; never affects exit code or the upload. @@ -2487,21 +2641,20 @@ export async function runPut( ? await resolveStageBindingWarning({ ctx, defaults, repo: stagingTarget.repo }) : undefined; // Lever 3 (issue #469): tip when a --pr/--issue put lands an image with no - // `path` meta. Only relevant on the ghTarget path — the bare-put paths - // above (staging/auto/dated) aren't attached to a PR/issue yet, so there's - // nothing to look up from a page later. + // `path` meta. Only relevant on the (explicit or auto) gh target path — the + // bare-put staging/dated paths aren't attached to a PR/issue yet, so + // there's nothing to look up from a page later. const pathHint = - ghTarget && uploads.length > 0 && !ctx.quiet + effectiveGhTarget && uploads.length > 0 && !ctx.quiet ? pathMetaHintFor(uploads, sentMetadata) : undefined; - // One JSON `hint` slot, shared with the #393 nudge (mutually exclusive with - // it — nudge is undefined whenever staging took over). When staging fires, - // prefer the more actionable binding warning over the generic staging note - // (mirrors attach --branch, whose only JSON hint content IS the binding - // warning); stderr prints the nudge/staging-note and binding-warning lines - // independently, below. pathHint only ever fires on the ghTarget path, so - // it never competes with the other three. - const jsonHint = nudge ?? bindingWarning ?? stagingNote ?? pathHint ?? contextNudge; + // One JSON `hint` slot, shared across every advisory this command can + // surface. `autoPrNote` and `nudge` are mutually exclusive with each other + // and with `stagingNote` (each corresponds to a different destination the + // upload landed on); pathHint only ever fires on the gh-target path, so it + // never competes with the other three. Same precedence stderr prints, + // below. + const jsonHint = autoPrNote ?? nudge ?? bindingWarning ?? stagingNote ?? pathHint ?? contextNudge; type GalleryOutcome = { id: string; @@ -2531,9 +2684,14 @@ export async function runPut( let comment: AttachmentsCommentResult | undefined; let commentError: string | undefined; - if (wantComment && ghTarget && !dryRun && uploads.length > 0) { + if (wantComment && effectiveGhTarget && !dryRun && uploads.length > 0) { try { - comment = await syncAttachmentsComment(ctx.client, ghTarget, run, ctx.config.workspace); + comment = await syncAttachmentsComment( + ctx.client, + effectiveGhTarget, + run, + ctx.config.workspace, + ); if (logHuman) process.stderr.write( `>> attachments comment ${comment.action}${commentViaSuffix(comment.via)}\n`, @@ -2595,6 +2753,7 @@ export async function runPut( `warning: could not upload ${failure.file}: ${failure.error.message}\n`, ); } + if (autoPrNote) process.stderr.write(`${autoPrNote}\n`); if (nudge) process.stderr.write(`${nudge}\n`); if (stagingNote) process.stderr.write(`${stagingNote}\n`); if (bindingWarning) process.stderr.write(`${bindingWarning}\n`); @@ -2659,6 +2818,7 @@ export async function runPut( `warning: upload succeeded but adding it to gallery ${gallery.id} failed: ${gallery.error.message}\n`, ); } + if (autoPrNote && format !== "json") process.stderr.write(`${autoPrNote}\n`); if (nudge && format !== "json") process.stderr.write(`${nudge}\n`); if (stagingNote && format !== "json") process.stderr.write(`${stagingNote}\n`); if (bindingWarning && format !== "json") process.stderr.write(`${bindingWarning}\n`); diff --git a/packages/uploads/src/commands/hook.ts b/packages/uploads/src/commands/hook.ts index 89fc51c0..7a12dd45 100644 --- a/packages/uploads/src/commands/hook.ts +++ b/packages/uploads/src/commands/hook.ts @@ -1,7 +1,11 @@ /** * `uploads hook pre-pr-screenshot` — agent PreToolUse / beforeShellExecution - * handler. When the shell command is `gh pr create`, the branch touches UI - * files, and nothing is staged on uploads.sh, emit a non-blocking advisory. + * handler, triggered on `gh pr create`. Two advisories, mutually exclusive: + * - staged-but-unattached files exist for the branch (any reason they got + * there) → a promote suggestion (issue #700): `uploads attach --promote` + * once the PR this command is about to open exists. + * - nothing is staged, but the branch touches UI files → the original + * (issue #379) "consider staging screenshots" advisory. * * Always fail-open. Disable with UPLOADS_HOOK_DISABLE=1. */ @@ -25,9 +29,12 @@ Invoked by Claude Code / Codex / Grok / Cursor hooks. Never blocks. Harness manifests no-op (exit 0, no output) when this binary is not on PATH. pre-pr-screenshot - If the shell command is \`gh pr create\`, the branch touches UI files, and - no screenshots are staged for the branch, emit an advisory to stage with - \`uploads attach … --branch\`. + If the shell command is \`gh pr create\`: + - staged-but-unattached files exist for the branch → suggest promoting + them into the PR's managed comment once it exists (issue #700): + \`uploads attach --promote --pr \`. + - otherwise, if the branch touches UI files → suggest staging with + \`uploads attach … --branch\`. Disable with UPLOADS_HOOK_DISABLE=1. `; @@ -177,20 +184,37 @@ export async function runPrePrScreenshot(deps: HookDeps): Promise const branch = git.branch(); if (!branch) return null; + const staged = await (deps.countStaged ?? defaultCountStaged)(branch); + if (staged === null) return null; // error/unconfigured → fail open + + // Promote suggestion (issue #700): staged-but-unattached files already + // exist for this branch right as its PR is about to open. The PR doesn't + // exist yet at this PreToolUse point, so its number isn't knowable here — + // the wording still gives the exact command shape, and a bare + // `attach --promote` (which infers the PR from the branch) works too. + if (staged > 0) { + const fork = (deps.isFork ?? (() => defaultIsFork(cwd)))(); + const forkNote = + fork === true + ? " Note: this looks like a fork branch, so staged screenshots won't auto-promote into the PR comment yet (see issue #317) — attach them manually if you use uploads." + : ""; + const message = + `${staged} file${staged === 1 ? "" : "s"} staged for branch '${branch}' on uploads.sh ` + + `${staged === 1 ? "isn't" : "aren't"} attached to a pull request yet. Once this PR opens, run ` + + "`uploads attach --promote --pr ` (or a bare `uploads attach --promote`, which infers " + + `the PR from the branch) to collect ${staged === 1 ? "it" : "them"} into the managed attachments comment.${forkNote}`; + return formatAdvisory(message, isCursorHookInput(raw)); + } + const testFiles = deps.testFiles ?? process.env.UPLOADS_HOOK_TEST_FILES; const changed = testFiles ? testFiles.split("\n").filter(Boolean) : git.changedFiles(); if (!anyVisual(changed)) return null; - const staged = await (deps.countStaged ?? defaultCountStaged)(branch); - // null = error/unconfigured → fail open; >0 = already staged - if (staged === null || staged > 0) return null; - const fork = (deps.isFork ?? (() => defaultIsFork(cwd)))(); const forkNote = fork === true ? " Note: this looks like a fork branch, so staged screenshots won't auto-promote into the PR comment yet (see issue #317) — attach them manually if you use uploads." : ""; - const message = `This PR touches UI files (astro/tsx/jsx/vue/svelte/html/css/scss/less or an /email/ path) but no screenshots are staged for branch '${branch}' on uploads.sh. ` + `Consider running \`uploads attach --branch --state after\` (and a --state before if useful) before or after opening the PR — the managed attachments comment assembles from staged files automatically.${forkNote}`; diff --git a/packages/uploads/src/commands/screenshot.ts b/packages/uploads/src/commands/screenshot.ts index 583a0ff7..29a12bc0 100644 --- a/packages/uploads/src/commands/screenshot.ts +++ b/packages/uploads/src/commands/screenshot.ts @@ -29,6 +29,10 @@ import { mergeStagingMeta, writeReplacedNote, resolveGhPrefixSafe, + resolveAutoPrTarget, + resolvePutNudgeContext, + putNudgeText, + autoPrNoteText, type BranchTarget, } from "../commands.js"; import { resolvePutDefaults } from "../config.js"; @@ -105,9 +109,18 @@ sets the whole object key verbatim (no folding). After capture, screenshots share the put upload pipeline: optional --frame, optimize-by-default, --pr/--issue attachment + --comment, --gallery, --meta. -Branch staging by default (pre-PR): with no --pr/--issue/--branch/--key/--ref/ ---prefix/--destination, a screenshot taken on a non-default git branch stages -under gh///branch// instead of the dated +Auto-PR context (issue #700): with no --pr/--issue/--branch/--key/--ref/ +--prefix/--destination, a screenshot taken on a branch that maps to exactly +one open PR behaves as if --pr had been passed — stable gh/ key, managed +comment sync (with --comment) — instead of branch staging below. A one-line +note announces this. Opt out with --no-pr, UPLOADS_NO_AUTO_PR=1, or config +UPLOADS_NO_AUTO_PR=1; never fires outside a git repo, on the default branch, +with --no-git, or when no single open PR can be resolved. + +Branch staging by default (pre-PR): when auto-PR above doesn't apply and none +of --pr/--issue/--branch/--key/--ref/--prefix/--destination is given, a +screenshot taken on a non-default git branch stages under +gh///branch// instead of the dated screenshots///... layout — same key/metadata as an explicit --branch, carrying every derived fact (path/url/env/viewport, --state) along. Staged files auto-attach with full metadata the first time you attach to that @@ -163,6 +176,7 @@ Options: --optimize-max-edge Max long edge when optimizing (default: 2400) --optimize-quality <1-100> WebP quality (default: 85) --keep-exif Keep EXIF/XMP/ICC when optimizing + --no-pr Skip auto-PR context (or UPLOADS_NO_AUTO_PR=1) — see above --pr Attach to a pull request (stable URL, no hash) --issue Attach to an issue --branch [name] Stage against a branch, pre-PR (default: current git branch): @@ -390,10 +404,37 @@ export async function runScreenshot( const putDefaults = resolvePutDefaults({ envFile: ctx.envFile }, rawDefaults); const noGit = flagBool(parsed.flags, "--no-git") || putDefaults.noGit === true; + if (parsed.flags.has("--no-pr") && typeof parsed.flags.get("--no-pr") === "string") { + throw new UsageError("--no-pr takes no value"); + } + const noAutoPr = flagBool(parsed.flags, "--no-pr") || putDefaults.noAutoPr === true; const branchRepo = branchArg !== undefined ? resolveRepo(flagString(parsed.flags, "--repo"), run) : undefined; + // Auto-PR context (issue #700): when no --branch/--pr/--issue/--key/--ref/ + // --prefix/--destination is given, git use isn't disabled, and --no-pr/ + // UPLOADS_NO_AUTO_PR hasn't opted out, a screenshot taken on a branch that + // maps to exactly one open PR behaves as if --pr had been passed — + // stable key + managed comment sync — instead of the #469 auto-staging + // default below. Mirrors put's #700 handling exactly (resolveAutoPrTarget). + const autoPrTarget = + ghTarget || branchArg !== undefined + ? undefined + : resolveAutoPrTarget({ + ghTarget, + keyHint, + refArg: flagString(parsed.flags, "--ref"), + prefixArg: prefixFlag, + destinationArg: destFlag, + branchArg, + noGit, + noAutoPr, + repoArg: flagString(parsed.flags, "--repo") ?? putDefaults.repo, + run, + }); + const effectiveGhTarget = ghTarget ?? autoPrTarget; + // Auto branch staging (issue #469 lever 1): mirrors bare `put`'s auto-staging // (issue #403). When no --branch/--pr/--issue/--key/--ref/--prefix/--destination // is given and git use isn't disabled, a screenshot taken on a non-default @@ -402,11 +443,12 @@ export async function runScreenshot( // `screenshots///...` layout. This is what lets derived // metadata (path/url/env/viewport, --state) ride through to PR-open // promotion when the capture happens before the PR exists. Skipped - // entirely when --branch was given explicitly (already handled above). + // entirely when --branch was given explicitly (already handled above), or + // when the #700 auto-PR match above already took over. const autoStagingTarget: BranchTarget | undefined = branchArg === undefined ? resolvePutStagingTarget({ - ghTarget, + ghTarget: effectiveGhTarget, keyHint, refArg: flagString(parsed.flags, "--ref"), prefixArg: prefixFlag, @@ -425,7 +467,7 @@ export async function runScreenshot( destination: destFlag, prefix: prefixFlag, key: keyHint, - ghAttachment: Boolean(ghTarget) || stagingTarget !== undefined, + ghAttachment: Boolean(effectiveGhTarget) || stagingTarget !== undefined, }); } catch (err) { throw new UsageError(err instanceof Error ? err.message : String(err)); @@ -456,8 +498,8 @@ export async function runScreenshot( }); let metadata: Record | undefined = withFacts; - if (ghTarget) { - metadata = { ...withFacts, ...ghMetadataFromTargetWithTitle(ghTarget, run) }; + if (effectiveGhTarget) { + metadata = { ...withFacts, ...ghMetadataFromTargetWithTitle(effectiveGhTarget, run) }; validateMetaMap(metadata); } else if (stagingTarget !== undefined) { metadata = mergeStagingMeta(withFacts, stagingTarget); @@ -552,10 +594,10 @@ export async function runScreenshot( // Resolved once (issue #631), only when it's actually needed for the // upload about to happen (never for the noUpload/no-target bailouts // above) — never per file (screenshot only ever uploads one). - const ghPrefix = ghTarget + const ghPrefix = effectiveGhTarget ? await resolveGhPrefixSafe(ctx.client, { - repo: ghTarget.repo, - target: { kind: ghTarget.kind, num: ghTarget.num }, + repo: effectiveGhTarget.repo, + target: { kind: effectiveGhTarget.kind, num: effectiveGhTarget.num }, }) : stagingTarget !== undefined ? await resolveGhPrefixSafe(ctx.client, { @@ -564,6 +606,27 @@ export async function runScreenshot( }) : undefined; + // Bare-screenshot nudge context (issue #393/#700): only relevant when + // neither auto-PR nor staging took over — mirrors put's handling exactly. + // Resolved before upload; finished into text below once the key is known. + const nudgeContext = + effectiveGhTarget || stagingTarget + ? undefined + : resolvePutNudgeContext({ + quiet: ctx.quiet, + noNudge: putDefaults.noNudge === true, + ghTarget, + keyHint, + hasBranchFlag: branchArg !== undefined, + noGit, + repoArg: flagString(parsed.flags, "--repo") ?? putDefaults.repo, + run, + }); + const autoPrNote = + autoPrTarget && !ctx.quiet && !putDefaults.noNudge + ? autoPrNoteText(autoPrTarget.num) + : undefined; + const alt = altFlag ?? basename(captured.filename); const { result, prepared, markdown } = await uploadPreparedImage( ctx.client, @@ -572,7 +635,7 @@ export async function runScreenshot( { frame: frameOpts, optimize: optimizeOpts, - ghTarget, + ghTarget: effectiveGhTarget, ghBranchTarget: stagingTarget, ghPrefix, key: keyHint, @@ -619,11 +682,22 @@ export async function runScreenshot( } } + // Concrete bare-screenshot nudge text (issue #700): built once the upload + // key exists, so the ready-made follow-up names it verbatim. + const nudge = nudgeContext + ? putNudgeText(nudgeContext.branch, nudgeContext.pr, [result.key]) + : undefined; + let comment: AttachmentsCommentResult | undefined; let commentError: string | undefined; - if (wantComment && ghTarget) { + if (wantComment && effectiveGhTarget) { try { - comment = await syncAttachmentsComment(ctx.client, ghTarget, run, ctx.config.workspace); + comment = await syncAttachmentsComment( + ctx.client, + effectiveGhTarget, + run, + ctx.config.workspace, + ); if (logHuman) process.stderr.write( `>> attachments comment ${comment.action}${commentViaSuffix(comment.via)}\n`, @@ -658,6 +732,8 @@ export async function runScreenshot( ); } } + if (autoPrNote) process.stderr.write(`${autoPrNote}\n`); + if (nudge) process.stderr.write(`${nudge}\n`); if (bindingWarning) process.stderr.write(`${bindingWarning}\n`); if (contextNudge) process.stderr.write(`${contextNudge}\n`); process.stderr.write("\n"); @@ -666,17 +742,25 @@ export async function runScreenshot( // One JSON `hint` slot (mirrors bare put): the clip note (issue #652) wins // first — it's about the just-captured image itself, more immediately // actionable than the other three, which are about upload/staging - // mechanics. Then the binding warning, more actionable than the generic - // staging note; a replaced-object note (issue #618) is lowest priority — - // it only surfaces when nothing else already claimed the slot. Since state - // folds into the derived key, replaced + state means a same-side re-capture, + // mechanics. Then the auto-PR note and the #393/#700 nudge (issue #700), + // then the binding warning, more actionable than the generic staging note; + // a replaced-object note (issue #618) is lowest priority — it only + // surfaces when nothing else already claimed the slot. Since state folds + // into the derived key, replaced + state means a same-side re-capture, // which is the intended replace-in-place flow — word it as informational, // not as a problem. const replacedHint = result.replaced && explicitMeta.state ? `re-capture replaced the previous state=${explicitMeta.state} object at ${result.key} — expected for repeat captures of the same URL + state` : undefined; - const jsonHint = clipHint ?? bindingWarning ?? stagingNote ?? replacedHint ?? contextNudge; + const jsonHint = + clipHint ?? + autoPrNote ?? + nudge ?? + bindingWarning ?? + stagingNote ?? + replacedHint ?? + contextNudge; switch (format) { case "json": diff --git a/packages/uploads/src/config-file.ts b/packages/uploads/src/config-file.ts index 211d42eb..0aee15ee 100644 --- a/packages/uploads/src/config-file.ts +++ b/packages/uploads/src/config-file.ts @@ -19,6 +19,7 @@ export const UPLOADS_CONFIG_KEYS = [ "UPLOADS_NO_AUTO_META", "UPLOADS_SCREENSHOT_VIA", "UPLOADS_NO_NUDGE", + "UPLOADS_NO_AUTO_PR", ] as const; export type UploadsConfigKey = (typeof UPLOADS_CONFIG_KEYS)[number]; @@ -39,6 +40,9 @@ export interface PutDefaults { noAutoMeta?: boolean; /** When true, `put` never prints the bare-put --pr/attach nudge (issue #393). */ noNudge?: boolean; + /** When true, a bare put/screenshot never auto-assumes an unambiguous open + * PR's context (issue #700) — falls back to the #403/#469 staging default. */ + noAutoPr?: boolean; } const PUT_DEFAULT_KEY_MAP: Record = { @@ -51,6 +55,7 @@ const PUT_DEFAULT_KEY_MAP: Record = { keepExif: "UPLOADS_KEEP_EXIF", noAutoMeta: "UPLOADS_NO_AUTO_META", noNudge: "UPLOADS_NO_NUDGE", + noAutoPr: "UPLOADS_NO_AUTO_PR", }; function isTruthyConfigFlag(value: string | undefined): boolean { @@ -70,6 +75,7 @@ export function putDefaultsToConfigValues(defaults: PutDefaults): UploadsConfigV if (defaults.keepExif) out.UPLOADS_KEEP_EXIF = "1"; if (defaults.noAutoMeta) out.UPLOADS_NO_AUTO_META = "1"; if (defaults.noNudge) out.UPLOADS_NO_NUDGE = "1"; + if (defaults.noAutoPr) out.UPLOADS_NO_AUTO_PR = "1"; return out; } @@ -87,6 +93,7 @@ function parsePutDefaultsFromRaw(raw: UploadsConfigValues): PutDefaults { if (isTruthyConfigFlag(raw.UPLOADS_KEEP_EXIF)) out.keepExif = true; if (isTruthyConfigFlag(raw.UPLOADS_NO_AUTO_META)) out.noAutoMeta = true; if (isTruthyConfigFlag(raw.UPLOADS_NO_NUDGE)) out.noNudge = true; + if (isTruthyConfigFlag(raw.UPLOADS_NO_AUTO_PR)) out.noAutoPr = true; return out; } @@ -103,6 +110,7 @@ function parsePutDefaultsFromEnv(): PutDefaults { if (process.env.UPLOADS_KEEP_EXIF) raw.UPLOADS_KEEP_EXIF = process.env.UPLOADS_KEEP_EXIF; if (process.env.UPLOADS_NO_AUTO_META) raw.UPLOADS_NO_AUTO_META = process.env.UPLOADS_NO_AUTO_META; if (process.env.UPLOADS_NO_NUDGE) raw.UPLOADS_NO_NUDGE = process.env.UPLOADS_NO_NUDGE; + if (process.env.UPLOADS_NO_AUTO_PR) raw.UPLOADS_NO_AUTO_PR = process.env.UPLOADS_NO_AUTO_PR; return parsePutDefaultsFromRaw(raw); } @@ -169,6 +177,7 @@ export function mergePutDefaults(...layers: PutDefaults[]): PutDefaults { if (layer.keepExif != null) out.keepExif = layer.keepExif; if (layer.noAutoMeta != null) out.noAutoMeta = layer.noAutoMeta; if (layer.noNudge != null) out.noNudge = layer.noNudge; + if (layer.noAutoPr != null) out.noAutoPr = layer.noAutoPr; } return out; } diff --git a/packages/uploads/src/mcp/tools.ts b/packages/uploads/src/mcp/tools.ts index ff36054b..b5652174 100644 --- a/packages/uploads/src/mcp/tools.ts +++ b/packages/uploads/src/mcp/tools.ts @@ -13,10 +13,14 @@ import { ghMergedList, makeGhTarget, mergeStagingMeta, + resolveAutoPrTarget, resolveGhPrefixSafe, + resolvePutNudgeContext, resolvePutStagingTarget, resolveStaged, syncAttachmentsComment, + autoPrNoteText, + putNudgeText, type AttachmentsCommentResult, uploadAttachments, uploadPreparedImage, @@ -452,10 +456,15 @@ export function createUploadsMcpTools(opts: { }, ...frameProps, noGit: { type: "boolean", description: "Don't derive the repo segment from git." }, + noPr: { + type: "boolean", + description: + "Skip auto-PR context (issue #700): without pr/issue/key/ref/prefix/destination, a call on a branch mapping to exactly one open PR otherwise behaves as if pr had been passed (stable key + managed comment sync when comment is set). Also opts out via UPLOADS_NO_AUTO_PR=1.", + }, comment: { type: "boolean", description: - "With pr/issue: create or update the managed attachments comment. Posts as uploads-sh[bot] when the GitHub App is installed on the repo; otherwise via local gh auth (best-effort).", + "With pr/issue (or auto-detected PR context): create or update the managed attachments comment. Posts as uploads-sh[bot] when the GitHub App is installed on the repo; otherwise via local gh auth (best-effort).", }, dryRun: { type: "boolean", @@ -535,17 +544,41 @@ export function createUploadsMcpTools(opts: { const frameOpts = mcpFrameOptions(args); const optimizeOpts = mcpOptimizeOptions(args, defaults); const noGit = optBool(args, "noGit") || defaults.noGit === true; + const noAutoPr = optBool(args, "noPr") || defaults.noAutoPr === true; const alt = optString(args, "alt"); const width = optPosInt(args, "width") ?? defaults.width; const contentType = optString(args, "contentType"); + // Auto-PR context (issue #700): local stdio MCP put mirrors the CLI + // default — no pr/issue/key/ref/prefix/destination, not noGit/noPr, + // on a branch that maps to exactly one open PR behaves as if `pr` + // had been passed (stable key + managed comment sync) instead of + // the #403 staging default below. Never throws — see + // resolveAutoPrTarget. + const autoPrTarget = target + ? undefined + : resolveAutoPrTarget({ + ghTarget: target, + keyHint: keyArg, + refArg, + prefixArg, + destinationArg: destArg, + noGit, + noAutoPr, + repoArg: optString(args, "repo") ?? defaults.repo, + run, + }); + const effectiveTarget = target ?? autoPrTarget; + // Bare-put branch staging (issue #403): local stdio MCP put mirrors // the CLI default — no pr/issue/key/ref/prefix/destination, not // noGit, on a non-default git branch stages to the branch prefix // (identical key/metadata to `attach --branch`) instead of the // dated layout. Never throws — see resolvePutStagingTarget. + // effectiveTarget (explicit pr/issue OR the #700 auto-PR match) + // wins over staging, same as it wins over the dated layout. const stagingTarget = resolvePutStagingTarget({ - ghTarget: target, + ghTarget: effectiveTarget, keyHint: keyArg, refArg, prefixArg, @@ -554,6 +587,24 @@ export function createUploadsMcpTools(opts: { repoArg: optString(args, "repo") ?? defaults.repo, run, }); + + // Bare-put nudge context (issue #393/#700): only relevant when + // neither auto-PR nor staging took over. Finished into a hint once + // upload keys exist, below. + const nudgeCtx = + effectiveTarget || stagingTarget + ? undefined + : resolvePutNudgeContext({ + quiet: false, + noNudge: defaults.noNudge === true, + ghTarget: target, + keyHint: keyArg, + noGit, + repoArg: optString(args, "repo") ?? defaults.repo, + run, + }); + const autoPrHint = autoPrTarget ? autoPrNoteText(autoPrTarget.num) : undefined; + // Derived `repo` metadata (spec: 2026-08-11-screenshots-project-grouping-design.md). // Same derivation the CLI does; MCP always derives (no --no-auto), so // this is only suppressed by noGit. metadataProp's contract: omitting @@ -575,7 +626,7 @@ export function createUploadsMcpTools(opts: { const putShared = { client, - ghTarget: target, + ghTarget: effectiveTarget, ghBranchTarget: stagingTarget, prefix: resolvedPrefix ?? defaults.prefix, repo: optString(args, "repo") ?? defaults.repo, @@ -604,11 +655,24 @@ export function createUploadsMcpTools(opts: { if (uploads.length === 0 && failures.length > 0) { throw new ToolBatchError(batchFailureMessage(failures), { uploads, failures }); } - if (wantComment && target && uploads.length > 0) { - const { comment, commentError } = await syncComment(client, target, config.workspace); - return { uploads, failures, comment, commentError }; + const hint = + autoPrHint ?? + (nudgeCtx && uploads.length > 0 + ? putNudgeText( + nudgeCtx.branch, + nudgeCtx.pr, + uploads.map((u) => u.key), + ) + : undefined); + if (wantComment && effectiveTarget && uploads.length > 0) { + const { comment, commentError } = await syncComment( + client, + effectiveTarget, + config.workspace, + ); + return { uploads, failures, comment, commentError, ...(hint ? { hint } : {}) }; } - return { uploads, failures }; + return { uploads, failures, ...(hint ? { hint } : {}) }; } // Single-file: contentBase64 still supported; paths go through uploadPuts. @@ -622,7 +686,7 @@ export function createUploadsMcpTools(opts: { { frame: frameOpts, optimize: optimizeOpts, - ghTarget: target, + ghTarget: effectiveTarget, ghBranchTarget: stagingTarget, key: keyArg, prefix: resolvedPrefix ?? defaults.prefix, @@ -645,9 +709,24 @@ export function createUploadsMcpTools(opts: { outputBytes: prepared.outputBytes, filename: prepared.filename, }; - if (wantComment && target) { - const { comment, commentError } = await syncComment(client, target, config.workspace); - return { ...result, markdown, optimize, frame: prepared.frame, comment, commentError }; + const hint = + autoPrHint ?? + (nudgeCtx ? putNudgeText(nudgeCtx.branch, nudgeCtx.pr, [result.key]) : undefined); + if (wantComment && effectiveTarget) { + const { comment, commentError } = await syncComment( + client, + effectiveTarget, + config.workspace, + ); + return { + ...result, + markdown, + optimize, + frame: prepared.frame, + comment, + commentError, + ...(hint ? { hint } : {}), + }; } return { ...result, @@ -655,6 +734,7 @@ export function createUploadsMcpTools(opts: { optimize, frame: prepared.frame, ...(dryRun ? { dryRun: true } : {}), + ...(hint ? { hint } : {}), }; } @@ -668,6 +748,9 @@ export function createUploadsMcpTools(opts: { throw firstError instanceof Error ? firstError : new Error(String(firstError)); } const u = uploads[0]!; + const hint = + autoPrHint ?? + (nudgeCtx ? putNudgeText(nudgeCtx.branch, nudgeCtx.pr, [u.key]) : undefined); const flat = { workspace: u.workspace, key: u.key, @@ -681,9 +764,14 @@ export function createUploadsMcpTools(opts: { optimize: u.optimize, frame: u.frame, ...(dryRun ? { dryRun: true } : {}), + ...(hint ? { hint } : {}), }; - if (wantComment && target) { - const { comment, commentError } = await syncComment(client, target, config.workspace); + if (wantComment && effectiveTarget) { + const { comment, commentError } = await syncComment( + client, + effectiveTarget, + config.workspace, + ); return { ...flat, comment, commentError }; } return flat; @@ -798,10 +886,15 @@ export function createUploadsMcpTools(opts: { keepExif: { type: "boolean", description: "Keep EXIF/XMP/ICC when optimizing." }, ...frameProps, noGit: { type: "boolean", description: "Don't derive the repo segment from git." }, + noPr: { + type: "boolean", + description: + "Skip auto-PR context (issue #700): without pr/issue/key/ref/prefix/destination, a call on a branch mapping to exactly one open PR otherwise behaves as if pr had been passed. Also opts out via UPLOADS_NO_AUTO_PR=1.", + }, comment: { type: "boolean", description: - "With pr/issue: create/update the managed attachments comment (best-effort).", + "With pr/issue (or auto-detected PR context): create/update the managed attachments comment (best-effort).", }, galleryId: { type: "string", @@ -861,17 +954,40 @@ export function createUploadsMcpTools(opts: { const frameOpts = mcpFrameOptions(args); const optimizeOpts = mcpOptimizeOptions(args, defaults); const noGit = optBool(args, "noGit") || defaults.noGit === true; + const noAutoPr = optBool(args, "noPr") || defaults.noAutoPr === true; const alt = optString(args, "alt"); const width = optPosInt(args, "width") ?? defaults.width; + // Auto-PR context (issue #700): mirrors the CLI screenshot command + // and the put tool above — no pr/issue/key/ref/prefix/destination, + // not noGit/noPr, on a branch that maps to exactly one open PR + // behaves as if `pr` had been passed (stable key + managed comment + // sync) instead of the #469 auto-staging default below. Never + // throws — see resolveAutoPrTarget. + const autoPrTarget = target + ? undefined + : resolveAutoPrTarget({ + ghTarget: target, + keyHint: keyArg, + refArg, + prefixArg, + destinationArg: destArg, + noGit, + noAutoPr, + repoArg: optString(args, "repo") ?? defaults.repo, + run, + }); + const effectiveTarget = target ?? autoPrTarget; + // Auto branch staging (issue #469 lever 1): mirrors the CLI screenshot // command and the put tool above (issue #403) — no pr/issue/key/ref/ // prefix/destination, not noGit, on a non-default git branch stages // to the branch prefix (identical key/metadata to `attach --branch`) // instead of the dated `screenshots///...` layout. Never - // throws — see resolvePutStagingTarget. + // throws — see resolvePutStagingTarget. effectiveTarget (explicit + // pr/issue OR the #700 auto-PR match) wins over staging. const stagingTarget = resolvePutStagingTarget({ - ghTarget: target, + ghTarget: effectiveTarget, keyHint: keyArg, refArg, prefixArg, @@ -881,13 +997,29 @@ export function createUploadsMcpTools(opts: { run, }); + // Bare-screenshot nudge context (issue #393/#700): only relevant + // when neither auto-PR nor staging took over. + const nudgeCtx = + effectiveTarget || stagingTarget + ? undefined + : resolvePutNudgeContext({ + quiet: false, + noNudge: defaults.noNudge === true, + ghTarget: target, + keyHint: keyArg, + noGit, + repoArg: optString(args, "repo") ?? defaults.repo, + run, + }); + const autoPrHint = autoPrTarget ? autoPrNoteText(autoPrTarget.num) : undefined; + let resolvedPrefix: string | undefined; try { resolvedPrefix = resolvePutPrefix({ destination: destArg, prefix: prefixArg, key: keyArg, - ghAttachment: Boolean(target) || stagingTarget !== undefined, + ghAttachment: Boolean(effectiveTarget) || stagingTarget !== undefined, }); } catch (err) { usage(err instanceof Error ? err.message : String(err)); @@ -970,10 +1102,10 @@ export function createUploadsMcpTools(opts: { // Resolved once (issue #631), only now that upload is actually // about to happen — never per file (screenshot uploads exactly one). - const ghPrefix = target + const ghPrefix = effectiveTarget ? await resolveGhPrefixSafe(client, { - repo: target.repo, - target: { kind: target.kind, num: target.num }, + repo: effectiveTarget.repo, + target: { kind: effectiveTarget.kind, num: effectiveTarget.num }, }) : stagingTarget ? await resolveGhPrefixSafe(client, { @@ -989,7 +1121,7 @@ export function createUploadsMcpTools(opts: { { frame: frameOpts, optimize: optimizeOpts, - ghTarget: target, + ghTarget: effectiveTarget, ghBranchTarget: stagingTarget, ghPrefix, key: keyArg, @@ -1037,14 +1169,23 @@ export function createUploadsMcpTools(opts: { frame: prepared.frame, gallery, ...(dryRun ? { dryRun: true } : {}), - // Full-page height cap note (issue #652), mirrors the CLI's stderr - // note + `hint` field. + // Hint precedence (mirrors the CLI): the full-page height cap note + // (issue #652) is about the just-captured image itself, more + // immediately actionable than the #700 auto-PR/nudge notes below. ...(captured.capped?.clipped ? { hint: screenshotModule.clipHintText(captured.capped.maxHeightPx, "maxHeight") } - : {}), + : autoPrHint + ? { hint: autoPrHint } + : nudgeCtx + ? { hint: putNudgeText(nudgeCtx.branch, nudgeCtx.pr, [result.key]) } + : {}), }; - if (wantComment && target) { - const { comment, commentError } = await syncComment(client, target, config.workspace); + if (wantComment && effectiveTarget) { + const { comment, commentError } = await syncComment( + client, + effectiveTarget, + config.workspace, + ); return { ...flat, comment, commentError }; } return flat; diff --git a/packages/uploads/test/commands-put.test.ts b/packages/uploads/test/commands-put.test.ts index 3160afc4..af7787ee 100644 --- a/packages/uploads/test/commands-put.test.ts +++ b/packages/uploads/test/commands-put.test.ts @@ -2073,6 +2073,96 @@ describe("runPut branch staging (issue #403)", () => { }); }); +describe("runPut auto-PR context (issue #700)", () => { + const withPr = { + branch: "feature/thing", + defaultBranch: "main", + originUrl: "git@github.com:o/r.git", + repo: "o/r", + pr: 1250, + }; + + it("behaves as if --pr had been passed when the branch maps to exactly one open PR", async () => { + const { client, puts } = fakeClient(); + const stderr = await captureStderr(() => + runPut({ ...ctxWith(client), quiet: false }, [tmpFile()], false, nudgeRunner(withPr)), + ); + expect(puts[0]?.key).toBe("gh/o/r/pull/1250/shot.png"); + expect(puts[0]?.metadata?.["gh.number"]).toBe("1250"); + expect(stderr).toContain("branch maps to open PR #1250"); + expect(stderr).toContain("--no-pr"); + }); + + it("skips staging and lands on the stable gh/ key, not the branch-staging prefix", async () => { + const { client, puts } = fakeClient(); + await runPut(ctxWith(client), [tmpFile()], false, nudgeRunner(withPr)); + expect(puts[0]?.key).not.toContain("/branch/"); + }); + + it("includes an additive JSON hint field naming the PR", async () => { + const { client } = fakeClient(); + const stdout = await captureStdout(() => + runPut( + { ...ctxWith(client), quiet: false, json: true }, + [tmpFile()], + false, + nudgeRunner(withPr), + ), + ); + const payload = JSON.parse(stdout) as { hint?: string }; + expect(payload.hint).toContain("PR #1250"); + }); + + it("opts out with --no-pr, falling back to branch staging", async () => { + const { client, puts } = fakeClient(); + await runPut(ctxWith(client), [tmpFile(), "--no-pr"], false, nudgeRunner(withPr)); + expect(puts[0]?.key).toBe("gh/o/r/branch/feature-thing/shot.png"); + }); + + it("opts out with UPLOADS_NO_AUTO_PR=1, falling back to branch staging", async () => { + const { client, puts } = fakeClient(); + const prev = process.env.UPLOADS_NO_AUTO_PR; + process.env.UPLOADS_NO_AUTO_PR = "1"; + try { + await runPut(ctxWith(client), [tmpFile()], false, nudgeRunner(withPr)); + } finally { + if (prev === undefined) delete process.env.UPLOADS_NO_AUTO_PR; + else process.env.UPLOADS_NO_AUTO_PR = prev; + } + expect(puts[0]?.key).toBe("gh/o/r/branch/feature-thing/shot.png"); + }); + + it("never fires when an explicit --pr/--issue/--key/--ref/--prefix/--destination is set", async () => { + const { client, puts } = fakeClient(); + await runPut( + ctxWith(client), + [tmpFile(), "--key", "screenshots/explicit.png"], + false, + nudgeRunner(withPr), + ); + expect(puts[0]?.key).toBe("screenshots/explicit.png"); + }); + + it("never fires on the default branch", async () => { + const { client, puts } = fakeClient(); + await runPut(ctxWith(client), [tmpFile()], false, nudgeRunner({ ...withPr, branch: "main" })); + expect(puts[0]?.key).toBeUndefined(); + }); + + it("never fires with --no-git", async () => { + const { client, puts } = fakeClient(); + await runPut(ctxWith(client), [tmpFile(), "--no-git"], false, noRun); + expect(puts[0]?.key).toBeUndefined(); + }); + + it("falls back to staging when there is no open PR for the branch", async () => { + const { client, puts } = fakeClient(); + const { pr: _pr, ...noPr } = withPr; + await runPut(ctxWith(client), [tmpFile()], false, nudgeRunner(noPr)); + expect(puts[0]?.key).toBe("gh/o/r/branch/feature-thing/shot.png"); + }); +}); + describe("put promotes EXIF facts", () => { async function retinaPng(): Promise { const sharp = (await import("sharp")).default; diff --git a/packages/uploads/test/commands-screenshot.test.ts b/packages/uploads/test/commands-screenshot.test.ts index 1bb24f1d..ea537716 100644 --- a/packages/uploads/test/commands-screenshot.test.ts +++ b/packages/uploads/test/commands-screenshot.test.ts @@ -932,6 +932,119 @@ describe("runScreenshot auto branch staging (issue #469 lever 1)", () => { }); }); +/** stagingRunner + a `gh pr view ` stub for the #700 auto-PR lookup. */ +function autoPrRunner(opts: { + branch?: string; + defaultBranch?: string; + originUrl?: string; + repo?: string; + pr?: number; +}): CommandRunner { + return (cmd, args) => { + if (cmd === "gh" && args[0] === "pr" && args[1] === "view") { + if (opts.pr === undefined) throw new Error("no pull request found"); + return `${opts.pr}\n`; + } + return stagingRunner(opts)(cmd, args); + }; +} + +describe("runScreenshot auto-PR context (issue #700)", () => { + const withPr = { + branch: "feature/thing", + defaultBranch: "main", + originUrl: "git@github.com:o/r.git", + repo: "o/r", + pr: 1250, + }; + + it("behaves as if --pr had been passed when the branch maps to exactly one open PR", async () => { + const { client, puts } = fakeClient(); + const stderr = await captureStderr(() => + runScreenshot( + { ...ctxWith(client), quiet: false }, + ["https://example.com"], + false, + autoPrRunner(withPr), + fakeCapture("remote"), + ), + ); + expect(puts[0]?.key).toBe("gh/o/r/pull/1250/example-com.png"); + expect(puts[0]?.metadata?.["gh.number"]).toBe("1250"); + expect(stderr).toContain("branch maps to open PR #1250"); + }); + + it("opts out with --no-pr, falling back to branch staging", async () => { + const { client, puts } = fakeClient(); + const code = await runScreenshot( + ctxWith(client), + ["https://example.com", "--no-pr"], + false, + autoPrRunner(withPr), + fakeCapture("remote"), + ); + expect(code).toBe(0); + expect(puts[0]?.key).toBe("gh/o/r/branch/feature-thing/example-com.png"); + }); + + it("opts out with UPLOADS_NO_AUTO_PR=1, falling back to branch staging", async () => { + const { client, puts } = fakeClient(); + const prev = process.env.UPLOADS_NO_AUTO_PR; + process.env.UPLOADS_NO_AUTO_PR = "1"; + try { + await runScreenshot( + ctxWith(client), + ["https://example.com"], + false, + autoPrRunner(withPr), + fakeCapture("remote"), + ); + } finally { + if (prev === undefined) delete process.env.UPLOADS_NO_AUTO_PR; + else process.env.UPLOADS_NO_AUTO_PR = prev; + } + expect(puts[0]?.key).toBe("gh/o/r/branch/feature-thing/example-com.png"); + }); + + it("never fires with an explicit --branch (staging wins)", async () => { + const { client, puts } = fakeClient(); + const code = await runScreenshot( + ctxWith(client), + ["https://example.com", "--branch"], + false, + autoPrRunner(withPr), + fakeCapture("remote"), + ); + expect(code).toBe(0); + expect(puts[0]?.key).toBe("gh/o/r/branch/feature-thing/example-com.png"); + }); + + it("never fires on the default branch", async () => { + const { client, puts } = fakeClient(); + await runScreenshot( + ctxWith(client), + ["https://example.com"], + false, + autoPrRunner({ ...withPr, branch: "main" }), + fakeCapture("remote"), + ); + expect(puts[0]?.key).toBeUndefined(); + }); + + it("falls back to staging when there is no open PR for the branch", async () => { + const { client, puts } = fakeClient(); + const { pr: _pr, ...noPr } = withPr; + await runScreenshot( + ctxWith(client), + ["https://example.com"], + false, + autoPrRunner(noPr), + fakeCapture("remote"), + ); + expect(puts[0]?.key).toBe("gh/o/r/branch/feature-thing/example-com.png"); + }); +}); + describe("runScreenshot gh.title metadata (issue #267)", () => { it("stamps gh.title alongside the base gh.* pairs when the title resolves", async () => { const { client, puts } = fakeClient(); diff --git a/packages/uploads/test/config-put-defaults.test.ts b/packages/uploads/test/config-put-defaults.test.ts index cf390152..f6b3b199 100644 --- a/packages/uploads/test/config-put-defaults.test.ts +++ b/packages/uploads/test/config-put-defaults.test.ts @@ -42,3 +42,27 @@ describe("resolvePutDefaults noNudge (issue #393)", () => { expect(resolvePutDefaults(undefined, { fromEnvFile: {}, fromUser }).noNudge).toBe(true); }); }); + +describe("resolvePutDefaults noAutoPr (issue #700)", () => { + const prev = process.env.UPLOADS_NO_AUTO_PR; + afterEach(() => { + if (prev === undefined) delete process.env.UPLOADS_NO_AUTO_PR; + else process.env.UPLOADS_NO_AUTO_PR = prev; + }); + + it("is undefined by default (auto-PR stays on)", () => { + delete process.env.UPLOADS_NO_AUTO_PR; + expect(resolvePutDefaults({}).noAutoPr).toBeUndefined(); + }); + + it("reads UPLOADS_NO_AUTO_PR=1 from env", () => { + process.env.UPLOADS_NO_AUTO_PR = "1"; + expect(resolvePutDefaults({}).noAutoPr).toBe(true); + }); + + it("reads UPLOADS_NO_AUTO_PR=1 from a config/env-file layer (same key, put config path)", () => { + delete process.env.UPLOADS_NO_AUTO_PR; + const fromUser = { UPLOADS_NO_AUTO_PR: "1" } as const; + expect(resolvePutDefaults(undefined, { fromEnvFile: {}, fromUser }).noAutoPr).toBe(true); + }); +}); diff --git a/packages/uploads/test/hook-pre-pr-screenshot.test.ts b/packages/uploads/test/hook-pre-pr-screenshot.test.ts index c623d781..38666973 100644 --- a/packages/uploads/test/hook-pre-pr-screenshot.test.ts +++ b/packages/uploads/test/hook-pre-pr-screenshot.test.ts @@ -92,13 +92,54 @@ describe("runPrePrScreenshot", () => { expect(out).toBeNull(); }); - it("is silent when screenshots are already staged", async () => { + it("suggests promoting when screenshots are already staged (issue #700)", async () => { const out = await runPrePrScreenshot({ stdin: JSON.stringify({ tool_input: { command: "gh pr create" } }), git, countStaged: async () => 2, + isFork: () => false, }); - expect(out).toBeNull(); + expect(out).toBeTruthy(); + const parsed = JSON.parse(out!); + expect(parsed.hookSpecificOutput.additionalContext).toMatch(/2 files staged/); + expect(parsed.hookSpecificOutput.additionalContext).toMatch(/feat\/ui/); + expect(parsed.hookSpecificOutput.additionalContext).toMatch( + /uploads attach --promote --pr /, + ); + }); + + it("singularizes the promote suggestion for exactly one staged file", async () => { + const out = await runPrePrScreenshot({ + stdin: JSON.stringify({ tool_input: { command: "gh pr create" } }), + git, + countStaged: async () => 1, + isFork: () => false, + }); + const parsed = JSON.parse(out!); + expect(parsed.hookSpecificOutput.additionalContext).toMatch(/1 file staged/); + expect(parsed.hookSpecificOutput.additionalContext).toMatch(/isn't attached/); + }); + + it("appends the fork note to the promote suggestion", async () => { + const out = await runPrePrScreenshot({ + stdin: JSON.stringify({ tool_input: { command: "gh pr create" } }), + git, + countStaged: async () => 2, + isFork: () => true, + }); + expect(out).toMatch(/fork branch/); + expect(out).toMatch(/#317/); + }); + + it("promote suggestion fires even when the diff has no UI files", async () => { + const out = await runPrePrScreenshot({ + stdin: JSON.stringify({ tool_input: { command: "gh pr create" } }), + git: { ...git, changedFiles: () => ["packages/api/src/index.ts"] }, + countStaged: async () => 1, + isFork: () => false, + }); + expect(out).toBeTruthy(); + expect(out).toMatch(/uploads attach --promote/); }); it("is silent when find fails open", async () => { diff --git a/packages/uploads/test/mcp-screenshot.test.ts b/packages/uploads/test/mcp-screenshot.test.ts index 9224a3c8..646d0c39 100644 --- a/packages/uploads/test/mcp-screenshot.test.ts +++ b/packages/uploads/test/mcp-screenshot.test.ts @@ -334,3 +334,61 @@ describe("mcp screenshot tool auto branch staging (issue #469 lever 1)", () => { expect(puts[0]?.key).toBeUndefined(); }); }); + +/** branchStagingRunner + a `gh pr view ` stub for the #700 auto-PR lookup. */ +function autoPrRunner(opts: { + branch?: string; + defaultBranch?: string; + originUrl?: string; + repo?: string; + pr?: number; +}): CommandRunner { + return (cmd, args) => { + if (cmd === "gh" && args[0] === "pr" && args[1] === "view") { + if (opts.pr === undefined) throw new Error("no pull request found"); + return `${opts.pr}\n`; + } + return branchStagingRunner(opts)(cmd, args); + }; +} + +describe("mcp screenshot tool auto-PR context (issue #700)", () => { + const withPr = { + branch: "feature/thing", + defaultBranch: "main", + originUrl: "git@github.com:o/r.git", + repo: "o/r", + pr: 1250, + }; + + it("behaves as if pr had been passed when the branch maps to exactly one open PR", async () => { + const { server, puts } = serverWith({ runner: autoPrRunner(withPr) }); + const res = await rpc(server, "tools/call", { + name: "screenshot", + arguments: { target: "https://example.com" }, + }); + expect(res.result.isError).toBe(false); + expect(puts[0]?.key).toBe("gh/o/r/pull/1250/example-com.png"); + expect(res.result.structuredContent.hint).toContain("branch maps to open PR #1250"); + }); + + it("opts out with noPr, falling back to auto branch staging", async () => { + const { server, puts } = serverWith({ runner: autoPrRunner(withPr) }); + const res = await rpc(server, "tools/call", { + name: "screenshot", + arguments: { target: "https://example.com", noPr: true }, + }); + expect(res.result.isError).toBe(false); + expect(puts[0]?.key).toBe("gh/o/r/branch/feature-thing/example-com.png"); + }); + + it("falls back to staging when there is no open PR for the branch", async () => { + const { server, puts } = serverWith({ runner: autoPrRunner({ ...withPr, pr: undefined }) }); + const res = await rpc(server, "tools/call", { + name: "screenshot", + arguments: { target: "https://example.com" }, + }); + expect(res.result.isError).toBe(false); + expect(puts[0]?.key).toBe("gh/o/r/branch/feature-thing/example-com.png"); + }); +}); diff --git a/packages/uploads/test/mcp.test.ts b/packages/uploads/test/mcp.test.ts index 45a366ff..ed478c1e 100644 --- a/packages/uploads/test/mcp.test.ts +++ b/packages/uploads/test/mcp.test.ts @@ -869,6 +869,74 @@ describe("tools/call put branch staging (issue #403)", () => { }); }); +/** branchStagingRunner + a `gh pr view ` stub for the #700 auto-PR lookup. */ +function autoPrRunner(opts: { + branch?: string; + defaultBranch?: string; + originUrl?: string; + repo?: string; + pr?: number; +}): CommandRunner { + return (cmd, args) => { + if (cmd === "gh" && args[0] === "pr" && args[1] === "view") { + if (opts.pr === undefined) throw new Error("no pull request found"); + return `${opts.pr}\n`; + } + return branchStagingRunner(opts)(cmd, args); + }; +} + +describe("tools/call put auto-PR context (issue #700)", () => { + const withPr = { + branch: "feature/thing", + defaultBranch: "main", + originUrl: "git@github.com:o/r.git", + repo: "o/r", + pr: 1250, + }; + + it("behaves as if pr had been passed when the branch maps to exactly one open PR", async () => { + const { server, puts } = serverWith({ runner: autoPrRunner(withPr) }); + const res = await rpc(server, "tools/call", { + name: "put", + arguments: { contentBase64: PNG_B64, filename: "shot.png" }, + }); + expect(res.result.isError).toBe(false); + expect(puts[0].key).toBe("gh/o/r/pull/1250/shot.png"); + expect(res.result.structuredContent.hint).toContain("branch maps to open PR #1250"); + }); + + it("opts out with noPr, falling back to branch staging", async () => { + const { server, puts } = serverWith({ runner: autoPrRunner(withPr) }); + const res = await rpc(server, "tools/call", { + name: "put", + arguments: { contentBase64: PNG_B64, filename: "shot.png", noPr: true }, + }); + expect(res.result.isError).toBe(false); + expect(puts[0].key).toBe("gh/o/r/branch/feature-thing/shot.png"); + }); + + it("never fires when an explicit destination is set", async () => { + const { server, puts } = serverWith({ runner: autoPrRunner(withPr) }); + const res = await rpc(server, "tools/call", { + name: "put", + arguments: { contentBase64: PNG_B64, filename: "shot.png", destination: "screenshots" }, + }); + expect(res.result.isError).toBe(false); + expect(puts[0].key).toBe("generated/key.png"); + }); + + it("falls back to staging when there is no open PR for the branch", async () => { + const { server, puts } = serverWith({ runner: autoPrRunner({ ...withPr, pr: undefined }) }); + const res = await rpc(server, "tools/call", { + name: "put", + arguments: { contentBase64: PNG_B64, filename: "shot.png" }, + }); + expect(res.result.isError).toBe(false); + expect(puts[0].key).toBe("gh/o/r/branch/feature-thing/shot.png"); + }); +}); + describe("tools/call attach", () => { it("infers the current PR and uploads stable keys with markdown", async () => { const { run, calls } = ghRunner();