diff --git a/cli/api.ts b/cli/api.ts index b91de2e..b8b5568 100644 --- a/cli/api.ts +++ b/cli/api.ts @@ -3,7 +3,7 @@ // storebound api list [--store apple|play] [--search text] [--json] // storebound api coverage [--store apple|play] [--json] // storebound api apple|play [--param k=v ...] [--body file.json] [--upload file] -// [--out file] [--all-pages] [--json] [--dry] [--yes] +// [--out file] [--all-pages] [--json] [--dry] [--yes [--confirm ]] // storebound api get apple|play ... GET only (what the MCP tool api_get runs) // // GETs run right away. Writes print the exact request and are only sent with --yes in an interactive terminal; @@ -28,12 +28,13 @@ export const USAGE = `usage: storebound api list [--store apple|play] [--search text] [--json] storebound api coverage [--store apple|play] [--json] storebound api apple|play [--param k=v ...] [--body file.json] [--upload file] - [--out file] [--all-pages] [--json] [--dry] [--yes] + [--out file] [--all-pages] [--json] [--dry] [--yes [--confirm ]] storebound api get apple|play [--param k=v ...] [--all-pages] [--json] (GET only) GETs run right away. Writes print the exact request and are sent only with --yes in a terminal; owner-only operations (submit, release, delete, cancel, prices, testers, review replies, experiments) also ask you to type the -operation id. Full table: docs/api-coverage.md`; +operation id. Agents without a terminal: --yes --confirm (must equal the operation id; plain writes +only, never owner-only operations). Full table: docs/api-coverage.md`; export interface Row { store: Store; id: string; method: string; path: string; gate: GateKind; wrapped: string } @@ -144,6 +145,7 @@ export async function run(args: Args): Promise { kind, method: op.method, dry: args.has('dry'), yes: args.has('yes'), getOnly, tty: stdinIsTTY() && !!process.stdout.isTTY, agent: process.env.STOREBOUND_SERVE === '1' || process.env.STOREBOUND_MCP === '1', + confirm: args.get('confirm'), opId: op.id, }); if (decision.action === 'refuse') { const instead = op.method !== 'GET' ? dedicatedCommand(st, op.id) : null; diff --git a/cli/api/gate.ts b/cli/api/gate.ts index 95bd857..599721e 100644 --- a/cli/api/gate.ts +++ b/cli/api/gate.ts @@ -2,7 +2,8 @@ // // read GET (and a few POSTs that only compute or query, listed below): runs right away. // write any other POST / PATCH / PUT / DELETE: prints the exact request; sent only with --yes in an interactive -// terminal, never under MCP (`storebound mcp`) or the desktop app (`storebound serve`). +// terminal, or with --yes --confirm (equal to the resolved id) from an agent's shell without a +// terminal; never under MCP (`storebound mcp`) or the desktop app (`storebound serve`). // owner writes that submit, release, delete, cancel, change prices, invite or remove testers, send review replies, // start or stop experiments, refund or rotate keys (AGENTS.md rule 1): a write that also needs the operation id // typed at the prompt. Never reachable through MCP or serve. @@ -78,6 +79,10 @@ export interface GateInput { agent: boolean; /** `storebound api get` (the MCP api_get tool): plain GETs only */ getOnly: boolean; + /** --confirm : the non-interactive confirmation for an agent in a shell (plain writes only) */ + confirm?: string; + /** the operation id the call resolved to (what --confirm must equal) */ + opId?: string; } export type Decision = @@ -87,9 +92,13 @@ export type Decision = | { action: 'refuse'; why: string }; /** --yes without a terminal: refused loudly (exit 1, stderr), never a silent dry run (pitfall 77). */ -export const NOT_A_TTY = 'refused: writes through `storebound api` need an interactive terminal (stdout is not a TTY: piped or redirected). Nothing was sent. Run it without a pipe or redirect, or use the dedicated storebound command.'; +export const NOT_A_TTY = 'refused: writes through `storebound api` need an interactive terminal (stdout is not a TTY: piped or redirected). Nothing was sent. Run it without a pipe or redirect, add --confirm (agents, plain writes only), or use the dedicated storebound command.'; -/** What to do with one call. Pure. */ +/** --confirm never stands in for the owner's typed id on an owner-only operation. */ +export const OWNER_NO_CONFIRM = 'refused: owner-only operation: --confirm does not apply; the owner types the operation id in an interactive terminal. Nothing was sent.'; + +/** What to do with one call. Pure. A plain write without a terminal is sent only with --yes and --confirm + * equal to the resolved operation id (the agent path); owner-only operations always need the owner's terminal. */ export function decide(g: GateInput): Decision { if (g.getOnly && g.method.toUpperCase() !== 'GET') return { action: 'refuse', why: 'api get runs GET operations only' }; if (g.kind === 'read') return g.dry ? { action: 'dry', why: '--dry' } : { action: 'send' }; @@ -97,6 +106,12 @@ export function decide(g: GateInput): Decision { ? 'owner-only operation: never through MCP or the desktop app; the owner runs it in a terminal' : 'writes through `storebound api` are not available to MCP or the desktop app; run it in a terminal' }; if (g.dry || !g.yes) return { action: 'dry', why: g.dry ? '--dry' : 'writes are dry by default: add --yes (in a terminal) to send' }; + if (g.confirm !== undefined) { + // an agent's typed confirmation: only for plain writes, only when it names this exact operation + if (g.kind === 'owner') return { action: 'refuse', why: OWNER_NO_CONFIRM }; + if (!g.opId || g.confirm !== g.opId) return { action: 'refuse', why: `refused: --confirm ${g.confirm} does not equal the operation id ${g.opId ?? '?'}. Nothing was sent.` }; + return { action: 'send' }; + } if (!g.tty) return { action: 'refuse', why: NOT_A_TTY }; return g.kind === 'owner' ? { action: 'confirm', typed: true } : { action: 'send' }; } diff --git a/cli/api/wrapped.gen.ts b/cli/api/wrapped.gen.ts index 7da7541..3fe7002 100644 --- a/cli/api/wrapped.gen.ts +++ b/cli/api/wrapped.gen.ts @@ -42,8 +42,12 @@ export const WRAPPED: { apple: Record; play: Record; play: Record; play: Record Buffer.from(b).toString('base64url'); /** One App Store Connect API call may take at most this long (then retried when safe, else AmbiguousWriteError). */ @@ -154,22 +155,9 @@ export async function ascClient(root: string, bundleId: string, appIdHint?: stri return info; } - /** Reserve -> PUT the parts -> commit with the MD5 (screenshots, IAP review screenshots, app previews). `attrs` go into - * the reservation (app previews: mimeType, previewFrameTimeCode). */ - async function uploadAsset(type: string, rel: Record, file: string, attrs: Record = {}): Promise { - const buf = readFileSync(file); - // basename, not split('/'): a Windows path has backslashes, and the store would get the whole path as the file name - const name = basename(file); - const res = (await asc('POST', `/v1/${type}`, { data: { type, attributes: { fileName: name, fileSize: buf.length, ...attrs }, relationships: rel } })).data; - for (const op of res.attributes.uploadOperations as J[]) { - const headers: Record = {}; - for (const h of op.requestHeaders) headers[h.name] = h.value; - const r = await fetch(op.url, { method: op.method, headers, body: buf.subarray(op.offset, op.offset + op.length) }); - if (!r.ok) throw new Error(`upload ${name} part ${op.offset} -> ${r.status}`); - } - await asc('PATCH', `/v1/${type}/${res.id}`, { data: { type, id: res.id, attributes: { uploaded: true, sourceFileChecksum: md5(buf) } } }); - return res.id; - } + /** Reserve -> PUT the parts -> commit with the MD5 (uploadAssetVia). */ + const uploadAsset = (type: string, rel: Record, file: string, attrs: Record = {}): Promise => + uploadAssetVia({ asc, fetch: (u, i) => fetch(u, i) }, type, rel, file, attrs); /** * Makes a screenshot or preview set hold exactly `files`, in this order, touching as little as possible: the leading @@ -206,3 +194,37 @@ export async function ascClient(root: string, bundleId: string, appIdHint?: stri return { asc, all, maybe, getAs, bytes, app, editableVersion, editableAppInfo, uploadAsset, syncAssets, keyId: k.keyId }; } + +/** One upload part may take at most this long. */ +export const PART_TIMEOUT_MS = 300_000; + +/** + * Reserve -> PUT the parts -> commit with the MD5 (screenshots, IAP review screenshots, app previews). `attrs` go into + * the reservation (app previews: mimeType, previewFrameTimeCode). Parts go without `Expect: 100-continue` + * (partHeaders) and anything but a 2xx is a failed part. A failed part never leads to a commit: the reservation + * (AWAITING_UPLOAD) is deleted and the whole upload tried once more with a new one; a second failure throws. + */ +export async function uploadAssetVia(d: { asc: (method: string, path: string, body?: unknown) => Promise; fetch: (url: string, init: RequestInit) => Promise }, + type: string, rel: Record, file: string, attrs: Record = {}): Promise { + const buf = readFileSync(file); + // basename, not split('/'): a Windows path has backslashes, and the store would get the whole path as the file name + const name = basename(file); + for (let attempt = 1; ; attempt++) { + const res = (await d.asc('POST', `/v1/${type}`, { data: { type, attributes: { fileName: name, fileSize: buf.length, ...attrs }, relationships: rel } })).data; + let failed = ''; + for (const op of (res.attributes.uploadOperations ?? []) as J[]) { + try { + const r = await d.fetch(op.url, { method: op.method, headers: partHeaders(op), body: buf.subarray(op.offset, op.offset + op.length), signal: AbortSignal.timeout(PART_TIMEOUT_MS) }); + if (r.status < 200 || r.status > 299) failed = `part ${op.offset} -> ${r.status}`; + } catch (e) { failed = `part ${op.offset} -> ${(e as Error).message}`; } + if (failed) break; + } + if (!failed) { + await d.asc('PATCH', `/v1/${type}/${res.id}`, { data: { type, id: res.id, attributes: { uploaded: true, sourceFileChecksum: md5(buf) } } }); + return res.id; + } + // never commit after a failed part: remove the stranded AWAITING_UPLOAD reservation, then one more try + try { await d.asc('DELETE', `/v1/${type}/${res.id}`); } catch { /* the next sync or --repair-stuck removes it */ } + if (attempt >= 2) throw new Error(`upload ${name} ${failed} (twice; the reservation was deleted, nothing committed)`); + } +} diff --git a/cli/apple/creative-assets.ts b/cli/apple/creative-assets.ts index ac65f1c..7539022 100644 --- a/cli/apple/creative-assets.ts +++ b/cli/apple/creative-assets.ts @@ -34,6 +34,7 @@ import { ffprobe } from '../shots/report'; import { imageInfo } from '../shots/spec'; import { checkCreativeImage, checkCreativeVideo, type CreativeFile, creativeFiles, PLACEMENT_TYPE, type Placement } from '../shots/creative'; import { type Asc, ascClient, md5, type J } from './asc'; +import { partHeaders } from './screenshot-health'; import { AmbiguousWriteError } from '../lib/upsert'; import { addItem, ensureDraft, OtherSubmissionError } from './submission'; @@ -190,10 +191,8 @@ export async function run(ctx: Ctx): Promise { const res = (await c.asc('POST', `/v1/${type}`, { data: { type, attributes: { fileName: basename(f.file), fileSize: buf.length, category: 'CREATIVE_ASSETS', referenceName: REF(sum) }, relationships: { assetLibrary: { data: { type: 'appAssetLibraries', id: lib.id } } } } })).data as J; for (const op of (res.attributes.uploadOperations ?? []) as J[]) { - const headers: Record = {}; - for (const h of op.requestHeaders ?? []) headers[h.name] = h.value; - const r = await fetch(op.url, { method: op.method, headers, body: buf.subarray(op.offset, op.offset + op.length), signal: AbortSignal.timeout(300_000) }); - if (!r.ok) throw new Error(`upload ${basename(f.file)} part ${op.offset} -> ${r.status}`); + const r = await fetch(op.url, { method: op.method, headers: partHeaders(op), body: buf.subarray(op.offset, op.offset + op.length), signal: AbortSignal.timeout(300_000) }); + if (r.status < 200 || r.status > 299) { await c.asc('DELETE', `/v1/${type}/${res.id}`).catch(() => {}); throw new Error(`upload ${basename(f.file)} part ${op.offset} -> ${r.status} (reservation deleted, nothing committed)`); } } return (await c.asc('PATCH', `/v1/${type}/${res.id}`, { data: { type, id: res.id, attributes: { uploaded: true } } })).data as J; } diff --git a/cli/apple/resubmit.ts b/cli/apple/resubmit.ts index dab7493..ef834a5 100644 --- a/cli/apple/resubmit.ts +++ b/cli/apple/resubmit.ts @@ -25,6 +25,7 @@ import { type BuildStep, formatRows, labelOf, planIapSubmission, REJECTED_APP, t import { allSubscriptions } from '../lib/subscriptions'; import { ascClient, type J } from './asc'; import { readSubmitState, submitPlanned } from './iap-submit'; +import { explain409, isScreenshotProcessing409, pendingScreenshots, updateLedger, versionScreenshotRows } from './screenshot-health'; import { openSubmissions } from './submission'; import { ensureGcSwitch, gameCenterInUse, gcSwitchPlan, readGcSwitch } from './game-center'; import { allVersions, parsePlatform, pickPlatform, platformLabel, platformOf, versionsPath } from './platform'; @@ -118,5 +119,13 @@ export async function submitVersion(ctx: Ctx, mode: 'submit' | 'resubmit'): Prom log(TAG, `build ${number} attached to ${versionString}`); } if (gc && gcPlan.length) await ensureGcSwitch(c, gc, TAG); - await submitPlanned(ctx, c, { appId, ids, state, tag: TAG, withVersion: true, appVersionId: v.id, platform, quiet: true, resolve }); + try { + await submitPlanned(ctx, c, { appId, ids, state, tag: TAG, withVersion: true, appVersionId: v.id, platform, quiet: true, resolve }); + } catch (e) { + if (!isScreenshotProcessing409(e)) throw e; + // 409 STATE_ERROR.ASSET_IN_POST_PROCESSING.APP_SCREENSHOT: name every screenshot that is not COMPLETE + const rows = await versionScreenshotRows(c, v.id); + const pending = pendingScreenshots(rows, updateLedger(root, rows), Date.now()); + throw new Error(`${explain409(pending)}\nLast answer: ${(e as Error).message}`); + } } diff --git a/cli/apple/screenshot-health.ts b/cli/apple/screenshot-health.ts new file mode 100644 index 0000000..c167a1f --- /dev/null +++ b/cli/apple/screenshot-health.ts @@ -0,0 +1,147 @@ +// Screenshots that hold up a submission: App Store Connect answers 409 STATE_ERROR.ASSET_IN_POST_PROCESSING.APP_SCREENSHOT +// to a submit while any screenshot of the version is not COMPLETE. Seen live: a part upload answered with a non-2xx +// behind `Expect: 100-continue` was still committed, and the screenshot then sat in UPLOAD_COMPLETE for hours (never +// processed), blocking the submit. This module names every such screenshot (locale, display type, position, file), +// tells stuck ones (UPLOAD_COMPLETE longer than STUCK_MS) from ones still processing, and replaces only those in +// place (`apple screenshots --repair-stuck`). +// +// Apple gives no timestamp on a screenshot, so "how long in UPLOAD_COMPLETE" comes from a local ledger +// (.storebound/screenshot-states.json: screenshot id -> first time this machine saw it UPLOAD_COMPLETE, or committed it). +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { basename, dirname, join } from 'node:path'; +import type { J } from './asc'; + +/** UPLOAD_COMPLETE longer than this is stuck, not processing (Apple normally finishes in well under a minute). */ +export const STUCK_MS = 5 * 60_000; + +export interface ShotRow { id: string; locale: string; displayType: string; position: number; fileName: string; state: string; checksum?: string | null } +export type PendingKind = 'processing' | 'stuck' | 'failed' | 'awaiting-upload'; +export interface Pending extends ShotRow { kind: PendingKind; ageMs?: number } + +/** The submit / validate error this module explains. Pure. */ +export const isScreenshotProcessing409 = (e: unknown): boolean => + /-> 409\b/.test(String((e as Error)?.message ?? e)) && /ASSET_IN_POST_PROCESSING(\.APP_SCREENSHOT)?/.test(String((e as Error)?.message ?? e)); + +const ledgerFile = (root: string): string => join(root, '.storebound', 'screenshot-states.json'); + +export function readLedger(root: string): Record { + try { return JSON.parse(readFileSync(ledgerFile(root), 'utf8')) as Record; } catch { return {}; } +} + +/** Notes the first time each UPLOAD_COMPLETE screenshot of `rows` was seen (a known one keeps its time; one in another + * state is dropped; screenshots not in `rows` are left as they are). */ +export function updateLedger(root: string, rows: { id: string; state: string }[], now = Date.now()): Record { + const next: Record = { ...readLedger(root) }; + for (const r of rows) { if (r.state === 'UPLOAD_COMPLETE') next[r.id] = next[r.id] ?? now; else delete next[r.id]; } + try { mkdirSync(dirname(ledgerFile(root)), { recursive: true }); writeFileSync(ledgerFile(root), JSON.stringify(next, null, 2) + '\n'); } catch { /* read-only project: ages stay unknown */ } + return next; +} + +/** Records a screenshot this run just committed (its UPLOAD_COMPLETE clock starts now). */ +export function noteCommitted(root: string, id: string, now = Date.now()): void { + const l = readLedger(root); + l[id] = l[id] ?? now; + try { mkdirSync(dirname(ledgerFile(root)), { recursive: true }); writeFileSync(ledgerFile(root), JSON.stringify(l, null, 2) + '\n'); } catch { /* ignore */ } +} + +/** Every screenshot that is not COMPLETE, classified. A screenshot with no known first-seen time counts as processing + * (the next look starts its clock). Pure. */ +export function pendingScreenshots(rows: ShotRow[], seen: Record, now: number, stuckMs = STUCK_MS): Pending[] { + const out: Pending[] = []; + for (const r of rows) { + if (r.state === 'COMPLETE') continue; + if (r.state === 'FAILED') { out.push({ ...r, kind: 'failed' }); continue; } + if (r.state === 'AWAITING_UPLOAD') { out.push({ ...r, kind: 'awaiting-upload' }); continue; } + const at = seen[r.id]; + const ageMs = at === undefined ? undefined : now - at; + out.push({ ...r, kind: r.state === 'UPLOAD_COMPLETE' && ageMs !== undefined && ageMs > stuckMs ? 'stuck' : 'processing', ...(ageMs !== undefined ? { ageMs } : {}) }); + } + return out; +} + +/** Can --repair-stuck replace it (stuck, failed or a stranded reservation)? Processing ones are left to finish. */ +export const repairable = (p: Pending): boolean => p.kind !== 'processing'; + +const mins = (ms: number): string => `${Math.round(ms / 60_000)} min`; + +/** One line per pending screenshot. Pure. */ +export function describePending(list: Pending[]): string[] { + return list.map(p => `${p.locale} ${p.displayType} #${p.position} ${p.fileName}: ${p.state}${p.ageMs !== undefined ? ` for ${mins(p.ageMs)}` : ''} -> ${ + p.kind === 'stuck' ? 'STUCK (not processing)' : p.kind === 'failed' ? 'FAILED' : p.kind === 'awaiting-upload' ? 'stranded reservation (never uploaded)' : 'still processing'}`); +} + +/** The explanation for a 409 ASSET_IN_POST_PROCESSING: what holds the submit and what to do. Pure. */ +export function explain409(list: Pending[]): string { + if (!list.length) return 'App Store Connect says a screenshot is still processing, but every screenshot of the version now reads COMPLETE: run the same command again.'; + const bad = list.filter(repairable); + return `App Store Connect refuses the submit while ${list.length} screenshot(s) are not COMPLETE:\n${describePending(list).map(l => ' ' + l).join('\n')}\n` + + (bad.length ? `${bad.length} will never finish on their own: storebound apple screenshots --repair-stuck --dry, then without --dry (replaces only these, at their positions), then submit again.` + : `They are still processing (under ${mins(STUCK_MS)}): run the same command again in a few minutes.`); +} + +/** Every screenshot of one version, with its locale, display type and 1-based position. */ +export async function versionScreenshotRows(c: { all: (p: string) => Promise }, versionId: string): Promise { + const rows: ShotRow[] = []; + for (const loc of await c.all(`/v1/appStoreVersions/${versionId}/appStoreVersionLocalizations?limit=50`)) + for (const set of await c.all(`/v1/appStoreVersionLocalizations/${loc.id}/appScreenshotSets?limit=50`)) + rows.push(...setRows(await c.all(`/v1/appScreenshotSets/${set.id}/appScreenshots?limit=50`), loc.attributes.locale, set.attributes.screenshotDisplayType)); + return rows; +} + +export const setRows = (shots: J[], locale: string, displayType: string): ShotRow[] => shots.map((s, i) => ({ + id: s.id, locale, displayType, position: i + 1, fileName: s.attributes?.fileName ?? '?', state: s.attributes?.assetDeliveryState?.state ?? 'UNKNOWN', checksum: s.attributes?.sourceFileChecksum, +})); + +/** Request headers of one upload part, without `Expect` (100-continue): seen live, a part sent with it got a non-2xx + * that looked like progress and the asset never processed. Pure. */ +export function partHeaders(op: { requestHeaders?: { name: string; value: string }[] }): Record { + const h: Record = {}; + for (const x of op.requestHeaders ?? []) if (x.name.toLowerCase() !== 'expect') h[x.name] = x.value; + return h; +} + +export interface RepairStep { id: string; position: number; fileName: string; file: string | null; kind: PendingKind } + +/** What --repair-stuck does in one set: each repairable screenshot with the local file of the same name (else the file + * at its position). Pure. */ +export function repairPlan(shots: ShotRow[], pending: Pending[], files: string[]): RepairStep[] { + const ids = new Set(pending.filter(repairable).map(p => p.id)); + return shots.filter(s => ids.has(s.id)).map(s => ({ + id: s.id, position: s.position, fileName: s.fileName, kind: pending.find(p => p.id === s.id)!.kind, + file: files.find(f => basename(f) === s.fileName) ?? files[s.position - 1] ?? null, + })); +} + +export interface RepairClient { + asc: (method: string, path: string, body?: unknown) => Promise; + all: (path: string) => Promise; + uploadAsset: (type: string, rel: Record, file: string) => Promise; +} + +/** Replaces the planned screenshots of one set at their own positions: delete, upload the same file, put the order + * back, then read the set again and check the count did not change. Re-runnable: a step whose screenshot is gone + * already is skipped, and a set that still holds the same count afterwards is left as it is. */ +export async function repairSet(c: RepairClient, setId: string, steps: RepairStep[], say: (m: string) => void = () => {}): Promise<{ replaced: number }> { + const before = (await c.all(`/v1/appScreenshotSets/${setId}/appScreenshots?limit=50`)).map(x => x.id as string); + const order = [...before]; + let replaced = 0; + for (const s of steps) { + const at = order.indexOf(s.id); + if (at < 0) continue; // replaced by an earlier run + if (!s.file || !existsSync(s.file)) throw new Error(`#${s.position} ${s.fileName}: no local file to re-upload (looked for the same name in the renders); nothing changed for it`); + await c.asc('DELETE', `/v1/appScreenshots/${s.id}`); + const id = await c.uploadAsset('appScreenshots', { appScreenshotSet: { data: { type: 'appScreenshotSets', id: setId } } }, s.file); + order[at] = id; + replaced++; + say(`#${s.position} ${s.fileName}: replaced (${s.kind})`); + } + if (replaced) { + const now = (await c.all(`/v1/appScreenshotSets/${setId}/appScreenshots?limit=50`)).map(x => x.id as string); + if (now.join() !== order.join() && order.every(id => now.includes(id))) + await c.asc('PATCH', `/v1/appScreenshotSets/${setId}/relationships/appScreenshots`, { data: order.map(id => ({ type: 'appScreenshots', id })) }); + const after = (await c.all(`/v1/appScreenshotSets/${setId}/appScreenshots?limit=50`)).map(x => x.id as string); + if (after.length !== before.length) throw new Error(`set ${setId}: ${before.length} screenshots before the repair, ${after.length} after: re-run storebound apple screenshots to restore the set`); + if (after.join() !== order.join()) throw new Error(`set ${setId}: order after the repair differs from the original positions: re-run storebound apple screenshots`); + } + return { replaced }; +} diff --git a/cli/apple/screenshots.ts b/cli/apple/screenshots.ts index 3d45e1f..afcc0d5 100644 --- a/cli/apple/screenshots.ts +++ b/cli/apple/screenshots.ts @@ -1,6 +1,7 @@ // Store screenshots of every locale on the editable App Store version (idempotent: a set whose files already match // by MD5 and order is left alone, otherwise it is emptied and re-uploaded). // storebound apple screenshots [--src store/renders] [--only en-US,tr] [--jobs 4] [--variant b] [--dry] +// storebound apple screenshots --repair-stuck [--dry] (only the screenshots stuck outside COMPLETE, in place) // Source per language: //ios69/NN.png (6.9", 1320 x 2868 -> APP_IPHONE_67), // ios65/NN.png (6.5", 1284 x 2778 -> APP_IPHONE_65) and, only with apple.ipad, ipad13/NN.png (13" iPad, 2064 x 2752 -> // APP_IPAD_PRO_3GEN_129: the API has no "13 inch" value, the 13" display set keeps the 3rd gen 12.9" name). With @@ -21,6 +22,7 @@ import { pulledChecksum } from '../lib/art-pull'; import { consoleLink } from '../lib/console-links'; import { IPHONE_DUO_DISPLAY, IPHONE_DUO_NOT_IN_API } from '../shots/spec'; import { type Asc, ascClient, EDITABLE, keptPrefix, md5, setUpToDate, type J } from './asc'; +import { pendingScreenshots, repairPlan, repairSet, setRows, updateLedger } from './screenshot-health'; import { allVersions, parsePlatform, pickPlatform, platformLabel, stateOf, versionLabel } from './platform'; /** Render size key -> ASC screenshotDisplayType (iPhone sets). */ @@ -104,6 +106,8 @@ export async function run(ctx: Ctx): Promise { const syncSet = (locId: string, sets: J[], type: string, files: string[], tag: string) => syncScreenshotSet(c, { rel: 'appStoreVersionLocalization', type: 'appStoreVersionLocalizations', id: locId }, sets, type, files, tag, ctx.args.has('dry')); + if (ctx.args.has('repair-stuck')) { await repairStuck(ctx, c, root, locs, src, platform, only); return; } + const tasks: (() => Promise)[] = []; for (const loc of locs) { const locale: string = loc.attributes.locale; @@ -132,3 +136,35 @@ export async function run(ctx: Ctx): Promise { } done('apple', bad ? `${bad} screenshots failed processing (re-run to retry)` : 'screenshots done'); } + +/** --repair-stuck: only the screenshots that are not COMPLETE and will not finish on their own (UPLOAD_COMPLETE for + * over 5 min, FAILED, or a stranded AWAITING_UPLOAD reservation) are replaced at their own positions with the same + * local file (same name in the renders). Everything else stays; still-processing ones are left to finish. --dry lists + * the plan. Re-runnable: a run cut midway leaves the rest for the next run, which skips what is already COMPLETE. */ +async function repairStuck(ctx: Ctx, c: Asc, root: string, locs: J[], src: string, platform: string, only: string[]): Promise { + const dry = ctx.args.has('dry'); + const langOf = byAppleLocale(ctx.loaded.config.languages); + const dirOf = Object.fromEntries(Object.entries(displaySets(ctx.loaded.config.apple, platform)).map(([d, t]) => [t, d] as const)); + let total = 0, held = 0; + for (const loc of locs) { + const locale: string = loc.attributes.locale; + if (only.length && !only.includes(locale)) continue; + for (const set of await c.all(`/v1/appStoreVersionLocalizations/${loc.id}/appScreenshotSets?limit=50`)) { + const type: string = set.attributes.screenshotDisplayType; + const rows = setRows(await c.all(`/v1/appScreenshotSets/${set.id}/appScreenshots?limit=50`), locale, type); + const pending = pendingScreenshots(rows, updateLedger(root, rows), Date.now()); + held += pending.filter(p => p.kind === 'processing').length; + const lang = langOf.get(locale); + const dir = dirOf[type]; + const files = lang && dir ? numberedImages(join(src, lang.id, dir)).filter(f => f.endsWith('.png')) : []; + const steps = repairPlan(rows, pending, files); + if (!steps.length) continue; + total += steps.length; + const tag = `${locale} ${type}`; + if (dry) { for (const st of steps) log('apple', `${tag}: would replace #${st.position} ${st.fileName} (${st.kind}) with ${st.file ?? 'NO LOCAL FILE'}`); continue; } + await repairSet(c, set.id, steps, m => log('apple', `${tag} ${m}`)); + } + } + if (held) log('apple', `${held} screenshot(s) still processing (under 5 min): left alone; run validate again in a few minutes`); + done('apple', total ? `${dry ? 'dry run: would replace' : 'replaced'} ${total} stuck screenshot(s)` : 'no stuck screenshots'); +} diff --git a/cli/apple/upload.ts b/cli/apple/upload.ts index 661cc4d..cc38e77 100644 --- a/cli/apple/upload.ts +++ b/cli/apple/upload.ts @@ -32,6 +32,7 @@ import { createOnce } from '../lib/upsert'; import { elapsed, heartbeat, type HeartbeatOptions, mb, share, withHeartbeat } from '../lib/progress'; import { pump } from '../lib/runner'; import * as rt from '../lib/rt'; +import { partHeaders } from './screenshot-health'; const TAG = 'upload'; @@ -94,8 +95,7 @@ export async function sendParts(o: SendPartsOptions): Promise { try { for (const [i, op] of o.ops.entries()) { current = i + 1; - const headers: Record = {}; - for (const h of op.requestHeaders ?? []) headers[h.name] = h.value; + const headers = partHeaders(op); const body = new Uint8Array(slice(o.file, op.offset, op.length)); for (let attempt = 1; ; attempt++) { const left = o.deadline - now(); diff --git a/cli/commands.ts b/cli/commands.ts index d851e4c..b58f9e6 100644 --- a/cli/commands.ts +++ b/cli/commands.ts @@ -78,7 +78,7 @@ export const COMMANDS: Record Promise, string]> = { 'apple cpp': [() => import('./apple/cpp'), 'custom product pages: list | create | upload --name | submit --name | visible --name (submit / visible ask first)'], 'apple events': [() => import('./apple/events'), 'in-app events: list | create | upload --name | submit --name (submit asks first)'], 'apple listing': [() => import('./apple/listing'), 'version, build, name / subtitle / description / keywords per locale; pull [--write]: live texts back into the listing file (diff first); --platform ios|macos|tvos|visionos (default: the platform that has the version this needs; asks when several do)'], - 'apple screenshots': [() => import('./apple/screenshots'), '6.9" + 6.5" (+ 13" iPad) screenshots per locale; --variant ; --platform ios|macos|tvos|visionos (default: the platform that has the version this needs; asks when several do)'], + 'apple screenshots': [() => import('./apple/screenshots'), '6.9" + 6.5" (+ 13" iPad) screenshots per locale; --variant ; --repair-stuck [--dry] (replace only screenshots not COMPLETE, at their positions); --platform ios|macos|tvos|visionos (default: the platform that has the version this needs; asks when several do)'], 'apple appinfo': [() => import('./apple/appinfo'), 'categories, age rating, content rights, Free price, territories'], 'apple review-details': [() => import('./apple/review-details'), 'App Review contact, demo account (from .storebound/, masked) + notes'], 'apple status': [() => import('./apple/status'), 'versions, builds, IAPs, review submissions (--wait-review, --cancel-submission ); --platform ios|macos|tvos|visionos (default: the platform that has the version this needs; asks when several do)'], diff --git a/cli/data/flags.json b/cli/data/flags.json index 938d845..9c2cd6a 100644 --- a/cli/data/flags.json +++ b/cli/data/flags.json @@ -43,6 +43,7 @@ "api": [ "all-pages", "body", + "confirm", "out", "param", "search", @@ -84,6 +85,7 @@ "off", "platform", "prune", + "repair-stuck", "src", "variant", "write" @@ -151,6 +153,7 @@ "not-sent-for-review", "platform", "prune", + "repair-stuck", "src", "traffic", "variant", @@ -198,6 +201,7 @@ "not-sent-for-review", "platform", "prune", + "repair-stuck", "src", "variant", "write" @@ -895,6 +899,7 @@ "play", "prune", "react-native", + "repair-stuck", "signature", "src", "unity", @@ -968,6 +973,7 @@ "complete", "concurrency", "configuration", + "confirm", "contact-from", "cordova", "cordova-android", @@ -1099,6 +1105,7 @@ "release-date", "remove", "render", + "repair-stuck", "resolve-iaps", "resume", "review-sent", @@ -1281,7 +1288,8 @@ "storebound apple review-details [--contact-from ] [--dry]" ], "apple screenshots": [ - "storebound apple screenshots [--src store/renders] [--only en-US,tr] [--jobs 4] [--variant b] [--dry]" + "storebound apple screenshots [--src store/renders] [--only en-US,tr] [--jobs 4] [--variant b] [--dry]", + "storebound apple screenshots --repair-stuck [--dry] (only the screenshots stuck outside COMPLETE, in place)" ], "apple signing": [ "storebound apple signing status [--json] read-only: capabilities, certificates (with / without a", diff --git a/cli/lib/validate-rules.ts b/cli/lib/validate-rules.ts index 491fb2a..8470651 100644 --- a/cli/lib/validate-rules.ts +++ b/cli/lib/validate-rules.ts @@ -2,6 +2,7 @@ // these functions turn it into an ordered list of findings. Order = the order you would fix things in. import { consoleLink, consoleUrl } from './console-links'; +import { describePending, type Pending, repairable } from '../apple/screenshot-health'; import { type FixAction, fixAppleListing, fixPlayListing, fixReviewDetails } from './fix-actions'; import { iphoneDuoNote, macNote } from '../shots/spec'; @@ -59,6 +60,9 @@ export interface AppleSnapshot { version: { versionString: string; state: string; copyright?: string | null; platform?: string } | null; /** Header and Search Results files uploaded but not approved yet, per locale and placement (apple creative-assets). */ creativeWaiting?: { locale: string; placement: string; file: string; text: string }[]; + /** Screenshots of the version that are not COMPLETE (cli/apple/screenshot-health.ts): a submit answers 409 + * ASSET_IN_POST_PROCESSING while any is left. */ + screenshotsPending?: Pending[]; /** Editable or rejected versions of the app's other platforms (an app's macOS version has its own state). */ otherPlatformVersions?: { versionString: string; state: string; platform: string }[]; /** app.copyright from the config (undefined: not set). */ @@ -211,6 +215,13 @@ export function appleRules(s: AppleSnapshot): Finding[] { else if (EDITABLE.includes(s.version.state)) add('ok', 'version', `version ${vl}: ${s.version.state}`); else if (IN_FLIGHT.includes(s.version.state)) add('warn', 'version', `version ${vl} is ${s.version.state} (nothing to prepare)`); else add('warn', 'version', `version ${vl}: ${s.version.state}`); + { + const pend = s.screenshotsPending ?? []; + const lines = describePending(pend); + pend.forEach((p, i) => repairable(p) + ? add('blocker', 'screenshot-stuck', lines[i]!, 'storebound apple screenshots --repair-stuck --dry, then without --dry (replaces only this screenshot at its position)') + : add('warn', 'screenshot-processing', lines[i]!, 'wait a few minutes: a submit answers 409 ASSET_IN_POST_PROCESSING until it is COMPLETE')); + } for (const w of s.creativeWaiting ?? []) add('warn', 'creative-waiting', `${w.locale} ${w.placement}: ${w.file} ${w.text}`, 'storebound apple creative-assets (a re-run places it once approved; --add-for-review sends it to review)'); for (const o of s.otherPlatformVersions ?? []) { const name = PLATFORM_NAMES[o.platform] ?? o.platform; diff --git a/cli/validate.ts b/cli/validate.ts index 7819d81..4d43db3 100644 --- a/cli/validate.ts +++ b/cli/validate.ts @@ -20,6 +20,7 @@ import { allVersions, type AscPlatform, parsePlatform, platformOf, stateOf, vers import { capabilityReport } from './apple/signing'; import { firstProductReport } from './apple/first-products'; import { requiredDisplayTypes } from './apple/screenshots'; +import { pendingScreenshots, setRows, type ShotRow, updateLedger } from './apple/screenshot-health'; import { type Loaded, playCategoryOf, playCodes, profileOf, sellsOn } from './lib/config'; import { reviewDemoAccount } from './lib/credentials'; import { allSubscriptions, basePlanId } from './lib/subscriptions'; @@ -80,6 +81,7 @@ export async function collectApple(ctx: Ctx): Promise { let build: AppleSnapshot['build'] = null; const locales: AppleSnapshot['locales'] = []; let reviewDetail: AppleSnapshot['reviewDetail'] = null; + const shotRows: ShotRow[] = []; if (v) { const b = (await c.maybe(`/v1/appStoreVersions/${v.id}/build`))?.data; if (b) { @@ -92,7 +94,11 @@ export async function collectApple(ctx: Ctx): Promise { for (const l of await c.all(`/v1/appStoreVersions/${v.id}/appStoreVersionLocalizations?limit=50`)) { const sets = await c.all(`/v1/appStoreVersionLocalizations/${l.id}/appScreenshotSets?limit=50`); const screenshots: Record = {}; - for (const s of sets) screenshots[s.attributes.screenshotDisplayType] = (await c.all(`/v1/appScreenshotSets/${s.id}/appScreenshots?limit=50`)).length; + for (const s of sets) { + const shots = await c.all(`/v1/appScreenshotSets/${s.id}/appScreenshots?limit=50`); + screenshots[s.attributes.screenshotDisplayType] = shots.length; + shotRows.push(...setRows(shots, l.attributes.locale, s.attributes.screenshotDisplayType)); + } const ia = infoLocs.get(l.attributes.locale) ?? {}; locales.push({ locale: l.attributes.locale, name: ia.name, subtitle: ia.subtitle, description: l.attributes.description, keywords: l.attributes.keywords, screenshots }); } @@ -132,6 +138,7 @@ export async function collectApple(ctx: Ctx): Promise { version: v ? { versionString: v.attributes.versionString, state: v.attributes.appVersionState ?? v.attributes.appStoreState, copyright: v.attributes.copyright ?? null, platform } : null, otherPlatformVersions, creativeWaiting: await creativeWaitingOf(ctx, c, appId), + screenshotsPending: pendingScreenshots(shotRows, updateLedger(root, shotRows), Date.now()), configCopyright: config.app.copyright, build, locales, reviewDetail, ageRating, expectedLocales: config.languages.map(l => l.apple).filter((x): x is string => !!x), diff --git a/docs/api-coverage.md b/docs/api-coverage.md index 22354bd..b257ca2 100644 --- a/docs/api-coverage.md +++ b/docs/api-coverage.md @@ -12,7 +12,7 @@ Specs: Apple [App Store Connect API 4.5.1](https://developer.apple.com/sample-co | API | operations | GET | other | wrapped by a command | owner only | |---|---:|---:|---:|---:|---:| -| App Store Connect API 4.5.1 | 1303 | 826 | 477 | 218 | 166 | +| App Store Connect API 4.5.1 | 1303 | 826 | 477 | 222 | 166 | | Google Play Android Developer API v3 (revision 20261008) | 145 | 45 | 100 | 35 | 49 | | Google Play Developer Reporting API v1beta1 (revision 20261008) | 25 | 15 | 10 | 6 | 0 | @@ -282,17 +282,17 @@ Specs: Apple [App Store Connect API 4.5.1](https://developer.apple.com/sample-co | `appPriceSchedules_manualPrices_getToManyRelated` | GET | `/v1/appPriceSchedules/{id}/manualPrices` | read | | | `appPriceSchedules_manualPrices_getToManyRelationship` | GET | `/v1/appPriceSchedules/{id}/relationships/manualPrices` | read | | | **appScreenshotSets** (6) | | | | | -| `appScreenshotSets_appScreenshots_getToManyRelated` | GET | `/v1/appScreenshotSets/{id}/appScreenshots` | read | apple/ppo, apple/screenshots, art, validate | +| `appScreenshotSets_appScreenshots_getToManyRelated` | GET | `/v1/appScreenshotSets/{id}/appScreenshots` | read | apple/ppo, apple/screenshot-health, apple/screenshots, art, validate | | `appScreenshotSets_appScreenshots_getToManyRelationship` | GET | `/v1/appScreenshotSets/{id}/relationships/appScreenshots` | read | | -| `appScreenshotSets_appScreenshots_replaceToManyRelationship` | PATCH | `/v1/appScreenshotSets/{id}/relationships/appScreenshots` | write (dry by default) | | +| `appScreenshotSets_appScreenshots_replaceToManyRelationship` | PATCH | `/v1/appScreenshotSets/{id}/relationships/appScreenshots` | write (dry by default) | apple/screenshot-health | | `appScreenshotSets_createInstance` | POST | `/v1/appScreenshotSets` | write (dry by default) | apple/screenshots | | `appScreenshotSets_deleteInstance` | DELETE | `/v1/appScreenshotSets/{id}` | owner only (typed confirm) | | | `appScreenshotSets_getInstance` | GET | `/v1/appScreenshotSets/{id}` | read | | | **appScreenshots** (4) | | | | | -| `appScreenshots_createInstance` | POST | `/v1/appScreenshots` | write (dry by default) | | -| `appScreenshots_deleteInstance` | DELETE | `/v1/appScreenshots/{id}` | owner only (typed confirm) | | +| `appScreenshots_createInstance` | POST | `/v1/appScreenshots` | write (dry by default) | apple/screenshot-health | +| `appScreenshots_deleteInstance` | DELETE | `/v1/appScreenshots/{id}` | owner only (typed confirm) | apple/screenshot-health | | `appScreenshots_getInstance` | GET | `/v1/appScreenshots/{id}` | read | | -| `appScreenshots_updateInstance` | PATCH | `/v1/appScreenshots/{id}` | write (dry by default) | | +| `appScreenshots_updateInstance` | PATCH | `/v1/appScreenshots/{id}` | write (dry by default) | apple/screenshot-health | | **appStoreReviewAttachments** (4) | | | | | | `appStoreReviewAttachments_createInstance` | POST | `/v1/appStoreReviewAttachments` | write (dry by default) | | | `appStoreReviewAttachments_deleteInstance` | DELETE | `/v1/appStoreReviewAttachments/{id}` | owner only (typed confirm) | | @@ -337,7 +337,7 @@ Specs: Apple [App Store Connect API 4.5.1](https://developer.apple.com/sample-co | **appStoreVersionLocalizations** (14) | | | | | | `appStoreVersionLocalizations_appPreviewSets_getToManyRelated` | GET | `/v1/appStoreVersionLocalizations/{id}/appPreviewSets` | read | apple/previews, art | | `appStoreVersionLocalizations_appPreviewSets_getToManyRelationship` | GET | `/v1/appStoreVersionLocalizations/{id}/relationships/appPreviewSets` | read | | -| `appStoreVersionLocalizations_appScreenshotSets_getToManyRelated` | GET | `/v1/appStoreVersionLocalizations/{id}/appScreenshotSets` | read | apple/screenshots, art, validate | +| `appStoreVersionLocalizations_appScreenshotSets_getToManyRelated` | GET | `/v1/appStoreVersionLocalizations/{id}/appScreenshotSets` | read | apple/screenshot-health, apple/screenshots, art, validate | | `appStoreVersionLocalizations_appScreenshotSets_getToManyRelationship` | GET | `/v1/appStoreVersionLocalizations/{id}/relationships/appScreenshotSets` | read | | | `appStoreVersionLocalizations_createInstance` | POST | `/v1/appStoreVersionLocalizations` | write (dry by default) | listing-store | | `appStoreVersionLocalizations_deleteInstance` | DELETE | `/v1/appStoreVersionLocalizations/{id}` | owner only (typed confirm) | | @@ -371,7 +371,7 @@ Specs: Apple [App Store Connect API 4.5.1](https://developer.apple.com/sample-co | `appStoreVersions_appStoreVersionExperimentsV2_getToManyRelationship` | GET | `/v1/appStoreVersions/{id}/relationships/appStoreVersionExperimentsV2` | read | | | `appStoreVersions_appStoreVersionExperiments_getToManyRelated` | GET | `/v1/appStoreVersions/{id}/appStoreVersionExperiments` | read | | | `appStoreVersions_appStoreVersionExperiments_getToManyRelationship` | GET | `/v1/appStoreVersions/{id}/relationships/appStoreVersionExperiments` | read | | -| `appStoreVersions_appStoreVersionLocalizations_getToManyRelated` | GET | `/v1/appStoreVersions/{id}/appStoreVersionLocalizations` | read | apple/creative-assets, apple/listing, apple/previews, apple/screenshots, art, listing-store, onboard, validate | +| `appStoreVersions_appStoreVersionLocalizations_getToManyRelated` | GET | `/v1/appStoreVersions/{id}/appStoreVersionLocalizations` | read | apple/creative-assets, apple/listing, apple/previews, apple/screenshot-health, apple/screenshots, art, listing-store, onboard, validate | | `appStoreVersions_appStoreVersionLocalizations_getToManyRelationship` | GET | `/v1/appStoreVersions/{id}/relationships/appStoreVersionLocalizations` | read | | | `appStoreVersions_appStoreVersionPhasedRelease_getToOneRelated` | GET | `/v1/appStoreVersions/{id}/appStoreVersionPhasedRelease` | read | apple/release, apple/status | | `appStoreVersions_appStoreVersionPhasedRelease_getToOneRelationship` | GET | `/v1/appStoreVersions/{id}/relationships/appStoreVersionPhasedRelease` | read | | diff --git a/skills/store-api/SKILL.md b/skills/store-api/SKILL.md index 485ab59..dae8e94 100644 --- a/skills/store-api/SKILL.md +++ b/skills/store-api/SKILL.md @@ -40,6 +40,9 @@ storebound api apple salesReports_getCollection --param ... --out report.gz # whatever the flags. `--yes` with stdout not a TTY (piped `| tail`, redirected `> out.json`, an agent's shell) is refused: exit 1 and "refused: writes through `storebound api` need an interactive terminal ... Nothing was sent." on stderr (store-pitfalls 78). Never pipe or redirect a write; use the dedicated command the message names. + An agent without a terminal sends a plain (non-owner) write with `--yes --confirm `, the id equal + to the one the call resolved to (store-pitfalls 92), after showing the dry run. `--confirm` never applies to + owner-only operations. 3. Owner-only operations (cli/api/gate.ts: every DELETE, submissions and their cancel, release requests, phased release changes, product page test start / stop, price schedules and price migrations, review replies, tester and user invitations, Play `edits.commit`, product activation, refunds, revokes and cancels, app signing) also ask for diff --git a/skills/store-pitfalls/SKILL.md b/skills/store-pitfalls/SKILL.md index b9b143d..cc5b7da 100644 --- a/skills/store-pitfalls/SKILL.md +++ b/skills/store-pitfalls/SKILL.md @@ -1113,3 +1113,26 @@ that fails without the fix (see CONTRIBUTING.md). check `apple status` for the draft's state before re-running anything else. - **Guard:** `willSubmit` in `cli/lib/review-submit.ts` (a non-empty READY_FOR_REVIEW draft that holds the named app version, or for `iap-submit` an asked-for item, is submitted); `tests/resubmit.test.ts`. + +## 91. Submit refused: 409 STATE_ERROR.ASSET_IN_POST_PROCESSING.APP_SCREENSHOT (screenshots stuck in UPLOAD_COMPLETE) +- **Symptom:** `apple submit` answered 409 `ASSET_IN_POST_PROCESSING.APP_SCREENSHOT` for hours. A few screenshots + sat in UPLOAD_COMPLETE and never became COMPLETE: their part upload had gone out with `Expect: 100-continue`, got + a non-2xx answer, and the upload was committed anyway. +- **Do:** `storebound validate` (or the failed submit) now lists every screenshot that is not COMPLETE: locale, + display type, position, file name, state and age. UPLOAD_COMPLETE for over 5 minutes is **stuck**, not processing + (a blocker); younger ones are a warning, wait. Then `storebound apple screenshots --repair-stuck --dry`, then + without `--dry`: only the stuck / FAILED / AWAITING_UPLOAD ones are deleted, the same local file (same name in the + renders) uploaded again and put back at its position; the set count is checked afterwards. Re-runnable. +- **Guard:** `cli/apple/screenshot-health.ts` (`pendingScreenshots`, `explain409`, `repairPlan`, `repairSet`, + ledger `.storebound/screenshot-states.json` for the UPLOAD_COMPLETE age), `uploadAssetVia` in `cli/apple/asc.ts` + (parts without `Expect`, any non-2xx part is a failure, the AWAITING_UPLOAD reservation is deleted and the upload + tried once more, never committed after a failed part), `tests/screenshot-health.test.ts`. + +## 92. Agents could not send any `storebound api` write (no TTY) +- **Symptom:** an agent's shell has no TTY, so every write through `storebound api` was refused (pitfall 78), even + a harmless one the owner had already agreed to, and the owner was asked to run it by hand. +- **Do:** show the dry run, then `storebound api apple|play ... --yes --confirm `. The + `--confirm` value must equal the operation id the call resolved to (also with `METHOD path`), or nothing is sent. + Owner-only operations (submit, release, delete, prices, testers, replies, ...) refuse `--confirm`: the owner + still types the id in a terminal. MCP and the desktop app still never send writes. +- **Guard:** `decide` / `OWNER_NO_CONFIRM` in `cli/api/gate.ts`, `tests/api.test.ts`. diff --git a/tests/api.test.ts b/tests/api.test.ts index ac4bf1b..41bd5b4 100644 --- a/tests/api.test.ts +++ b/tests/api.test.ts @@ -5,7 +5,7 @@ import { appleRows, generate, playRows, scanSource, templateRegex, wrappedOps } import { APPLE_OPS } from '../cli/api/apple.gen'; import { PLAY_APIS, PLAY_OPS } from '../cli/api/play.gen'; import { allOps, appleApi, backoffMs, buildRequest, getOp, mergePages, nextRequest, type Op, playApi, resolveOp, send, sendAll } from '../cli/api/client'; -import { decide, gateOf, type GateInput, OWNER_ONLY, ownerReason } from '../cli/api/gate'; +import { decide, gateOf, type GateInput, OWNER_NO_CONFIRM, OWNER_ONLY, ownerReason } from '../cli/api/gate'; import { coverage, describeRequest, parseParams, rows, search } from '../cli/api'; import { parseArgs } from '../cli/lib/args'; import { TOOLS, toArgv, validateInput } from '../cli/mcp-tools'; @@ -180,6 +180,20 @@ describe('write gating', () => { expect(decide({ ...base, getOnly: true, method: 'POST' }).action).toBe('refuse'); expect(decide({ ...base, getOnly: true })).toEqual({ action: 'send' }); }); + test('--confirm : an agent without a terminal sends a plain write, never an owner-only one', () => { + const w = { ...base, kind: 'write' as const, method: 'PATCH', yes: true, tty: false, opId: 'appStoreVersions_updateInstance' }; + expect(decide({ ...w, confirm: 'appStoreVersions_updateInstance' })).toEqual({ action: 'send' }); + expect(decide({ ...w, confirm: 'appInfos_updateInstance' })).toMatchObject({ action: 'refuse', why: expect.stringContaining('does not equal') }); + expect(decide({ ...w, confirm: 'x', opId: undefined }).action).toBe('refuse'); + // still dry without --yes, still nothing under MCP / desktop + expect(decide({ ...w, yes: false, confirm: w.opId }).action).toBe('dry'); + expect(decide({ ...w, agent: true, confirm: w.opId }).action).toBe('refuse'); + // owner-only: --confirm never replaces the owner's typed id, with or without a terminal + const o = { ...w, kind: 'owner' as const, method: 'DELETE', opId: 'appScreenshots_deleteInstance' }; + expect(decide({ ...o, confirm: o.opId })).toMatchObject({ action: 'refuse', why: OWNER_NO_CONFIRM }); + expect(decide({ ...o, tty: true, confirm: o.opId })).toMatchObject({ action: 'refuse', why: OWNER_NO_CONFIRM }); + expect(decide({ ...o, tty: true })).toEqual({ action: 'confirm', typed: true }); + }); test('the dry run shows the exact request without the credential', () => { const r = buildRequest(op('apple', 'appStoreVersions_updateInstance'), { id: '1' }, { data: { type: 'appStoreVersions', id: '1' } }); const text = describeRequest(r); diff --git a/tests/idempotency.test.ts b/tests/idempotency.test.ts index 26d6ae3..bef1580 100644 --- a/tests/idempotency.test.ts +++ b/tests/idempotency.test.ts @@ -529,9 +529,12 @@ describe('crash midway and unclear answers', () => { const md5 = (b: Uint8Array) => new Bun.CryptoHasher('md5').update(b).digest('hex'); s.asc.put({ type: 'appScreenshots', id: 'shot-a', attributes: { sourceFileChecksum: md5(png(1)), assetDeliveryState: { state: 'COMPLETE' }, fileName: '01.png' }, rels: { appScreenshotSet: set.id } }); s.asc.put({ type: 'appScreenshots', id: 'shot-x', attributes: { sourceFileChecksum: 'stale', assetDeliveryState: { state: 'COMPLETE' }, fileName: 'old.png' }, rels: { appScreenshotSet: set.id } }); - s.failPart = n => n === 2; // the 2nd uploaded part (03.png) fails + s.failPart = n => n === 2 || n === 3; // 03.png fails, and its one retry too const r1 = await capture(() => appleScreenshots(p.ctx(['--only', 'en-US']))).catch(e => ({ out: String(e), err: '', exit: 1 })); expect(r1.out + r1.err).toContain('03.png part 0 -> 400'); + expect(r1.out + r1.err).toContain('nothing committed'); + // the failed reservations were deleted, not left AWAITING_UPLOAD + expect(s.asc.children(set.id, 'appScreenshots').filter(x => (x.attributes.assetDeliveryState as { state: string } | undefined)?.state !== 'COMPLETE')).toEqual([]); expect(s.asc.children(set.id, 'appScreenshots').map(x => x.id)).toContain('shot-a'); s.failPart = undefined; const r2 = await capture(() => appleScreenshots(p.ctx(['--only', 'en-US']))); diff --git a/tests/screenshot-health.test.ts b/tests/screenshot-health.test.ts new file mode 100644 index 0000000..a0dc24d --- /dev/null +++ b/tests/screenshot-health.test.ts @@ -0,0 +1,127 @@ +// Screenshots that hold up a submit (409 ASSET_IN_POST_PROCESSING.APP_SCREENSHOT): classification (stuck vs still +// processing), the 409 explanation, validate findings, --repair-stuck in place, and the part upload rules (no +// `Expect: 100-continue`, a failed part deletes the reservation and is retried once, never committed). No network. +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { uploadAssetVia } from '../cli/apple/asc'; +import { describePending, explain409, isScreenshotProcessing409, partHeaders, pendingScreenshots, readLedger, repairPlan, repairSet, setRows, STUCK_MS, updateLedger, type ShotRow } from '../cli/apple/screenshot-health'; +import { appleRules } from '../cli/lib/validate-rules'; + +let dir = ''; +beforeAll(() => { dir = mkdtempSync(join(tmpdir(), 'sb-shots-')); for (const n of ['01.png', '02.png', '03.png']) writeFileSync(join(dir, n), `png ${n}`); }); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +const shot = (id: string, fileName: string, state: string) => ({ id, type: 'appScreenshots', attributes: { fileName, assetDeliveryState: { state }, sourceFileChecksum: 'x' } }); +const NOW = 10_000_000; + +describe('classification', () => { + const rows: ShotRow[] = setRows([shot('a', '01.png', 'COMPLETE'), shot('b', '02.png', 'UPLOAD_COMPLETE'), shot('c', '03.png', 'UPLOAD_COMPLETE'), shot('d', '04.png', 'FAILED'), shot('e', '05.png', 'AWAITING_UPLOAD')], 'en-US', 'APP_IPHONE_67'); + test('only non-COMPLETE rows; UPLOAD_COMPLETE over 5 min is stuck, younger or unknown is processing', () => { + const p = pendingScreenshots(rows, { b: NOW - STUCK_MS - 1, c: NOW - 60_000 }, NOW); + expect(p.map(x => [x.id, x.kind, x.position])).toEqual([['b', 'stuck', 2], ['c', 'processing', 3], ['d', 'failed', 4], ['e', 'awaiting-upload', 5]]); + expect(pendingScreenshots(rows, {}, NOW).find(x => x.id === 'b')!.kind).toBe('processing'); + const lines = describePending(p); + expect(lines[0]).toBe('en-US APP_IPHONE_67 #2 02.png: UPLOAD_COMPLETE for 5 min -> STUCK (not processing)'); + expect(lines[1]).toContain('still processing'); + }); + test('the 409 is recognised and explained with the repair command', () => { + const e = new Error('PATCH /v1/reviewSubmissions/1 -> 409 STATE_ERROR.ASSET_IN_POST_PROCESSING.APP_SCREENSHOT: screenshots are processing'); + expect(isScreenshotProcessing409(e)).toBe(true); + expect(isScreenshotProcessing409(new Error('-> 409 not ready to be submitted'))).toBe(false); + const p = pendingScreenshots(rows, { b: NOW - STUCK_MS - 1 }, NOW); + expect(explain409(p)).toContain('--repair-stuck'); + expect(explain409(p)).toContain('#2 02.png'); + expect(explain409(pendingScreenshots(rows.slice(0, 3), {}, NOW))).toContain('still processing'); + }); + test('validate: stuck is a blocker, processing a warning', () => { + const p = pendingScreenshots(rows.slice(0, 3), { b: NOW - STUCK_MS - 1 }, NOW); + const f = appleRules({ version: null, build: null, expectedLocales: [], locales: [], requiredDisplayTypes: [], reviewDetail: null, ageRating: null, priceSchedule: true, availability: true, + iaps: [], expectedProducts: [], privacyUrl: { url: 'https://example.com/privacy', status: 200 }, screenshotsPending: p }).filter(x => x.code.startsWith('screenshot-')); + expect(f.map(x => [x.code, x.level])).toEqual([['screenshot-stuck', 'blocker'], ['screenshot-processing', 'warn']]); + expect(f[0]!.fix).toContain('--repair-stuck'); + }); + test('ledger keeps the first-seen time and drops screenshots that left UPLOAD_COMPLETE', () => { + updateLedger(dir, [{ id: 'b', state: 'UPLOAD_COMPLETE' }, { id: 'z', state: 'UPLOAD_COMPLETE' }], 100); + updateLedger(dir, [{ id: 'b', state: 'UPLOAD_COMPLETE' }, { id: 'z', state: 'COMPLETE' }], 900); + expect(readLedger(dir)).toEqual({ b: 100 }); + }); +}); + +/** A fake screenshot set: list, delete, upload (append), reorder. */ +function fakeSet(start: ReturnType[]) { + let items = [...start]; + let n = 0; + const calls: string[] = []; + return { + calls, get items() { return items; }, + all: async (_p: string) => items.map(x => ({ ...x })), + asc: async (method: string, path: string, b?: unknown) => { + const body = b as { data: { id: string }[] } | undefined; + calls.push(`${method} ${path}`); + if (method === 'DELETE') items = items.filter(x => !path.endsWith('/' + x.id)); + if (method === 'PATCH') items = body!.data.map(d => items.find(x => x.id === d.id)!); + return {}; + }, + uploadAsset: async (_t: string, _r: unknown, file: string) => { const id = `new${++n}`; calls.push(`UPLOAD ${file.split('/').pop()}`); items.push(shot(id, file.split('/').pop()!, 'COMPLETE')); return id; }, + }; +} + +describe('--repair-stuck', () => { + test('replaces only the stuck one, same file, same position, count unchanged; a re-run does nothing', async () => { + const set = fakeSet([shot('a', '01.png', 'COMPLETE'), shot('b', '02.png', 'UPLOAD_COMPLETE'), shot('c', '03.png', 'UPLOAD_COMPLETE')]); + const rows = setRows(set.items, 'en-US', 'APP_IPHONE_67'); + const pending = pendingScreenshots(rows, { b: NOW - STUCK_MS - 1, c: NOW - 1000 }, NOW); + const files = ['01.png', '02.png', '03.png'].map(f => join(dir, f)); + const steps = repairPlan(rows, pending, files); + expect(steps.map(s => [s.id, s.position, s.file])).toEqual([['b', 2, join(dir, '02.png')]]); + expect(await repairSet(set, 'S', steps)).toEqual({ replaced: 1 }); + expect(set.items.map(x => [x.id, x.attributes.fileName])).toEqual([['a', '01.png'], ['new1', '02.png'], ['c', '03.png']]); + expect(set.calls.filter(c => !c.startsWith('PATCH'))).toEqual(['DELETE /v1/appScreenshots/b', 'UPLOAD 02.png']); + // re-run with the same (now stale) plan: the stuck id is gone, nothing is touched + const before = set.calls.length; + expect(await repairSet(set, 'S', steps)).toEqual({ replaced: 0 }); + expect(set.calls.length).toBe(before); + }); + test('no local file: nothing deleted', async () => { + const set = fakeSet([shot('b', 'gone.png', 'FAILED')]); + const rows = setRows(set.items, 'en-US', 'APP_IPHONE_67'); + const steps = repairPlan(rows, pendingScreenshots(rows, {}, NOW), []); + await expect(repairSet(set, 'S', steps)).rejects.toThrow('no local file'); + expect(set.calls).toEqual([]); + }); +}); + +describe('part uploads', () => { + test('Expect is never sent', () => { + expect(partHeaders({ requestHeaders: [{ name: 'Content-Type', value: 'image/png' }, { name: 'Expect', value: '100-continue' }, { name: 'expect', value: '100-continue' }] })).toEqual({ 'Content-Type': 'image/png' }); + }); + function store(partStatuses: number[]) { + const calls: string[] = []; + let r = 0; + return { + calls, + asc: async (method: string, path: string) => { + calls.push(`${method} ${path}`); + return method === 'POST' ? { data: { id: `r${++r}`, attributes: { uploadOperations: [{ method: 'PUT', url: 'https://upload.example.com/p', offset: 0, length: 4, requestHeaders: [{ name: 'Expect', value: '100-continue' }] }] } } } : {}; + }, + fetch: async (_u: string, i: RequestInit) => { + expect(Object.keys(i.headers as Record).map(k => k.toLowerCase())).not.toContain('expect'); + const s = partStatuses.shift()!; calls.push(`PART ${s}`); return new Response(null, { status: s }); + }, + }; + } + const rel = { appScreenshotSet: { data: { type: 'appScreenshotSets', id: 'S' } } }; + test('a non-2xx part (also 100 / 3xx): the reservation is deleted, retried once, committed only after a good part', async () => { + const s = store([302, 200]); + expect(await uploadAssetVia(s, 'appScreenshots', rel, join(dir, '01.png'))).toBe('r2'); + expect(s.calls).toEqual(['POST /v1/appScreenshots', 'PART 302', 'DELETE /v1/appScreenshots/r1', 'POST /v1/appScreenshots', 'PART 200', 'PATCH /v1/appScreenshots/r2']); + }); + test('two failed parts: throws, never commits', async () => { + const s = store([500, 403]); + await expect(uploadAssetVia(s, 'appScreenshots', rel, join(dir, '01.png'))).rejects.toThrow('nothing committed'); + expect(s.calls.some(c => c.startsWith('PATCH'))).toBe(false); + expect(s.calls.filter(c => c.startsWith('DELETE'))).toEqual(['DELETE /v1/appScreenshots/r1', 'DELETE /v1/appScreenshots/r2']); + }); +});