Merge remote-tracking branch 'origin/main' into pitfalls

# Conflicts:
#	cli/apple/iap-review.ts

cli/apple/iap-review.ts

@@ -1,46 +1,100 @@
-// Upload the App Review screenshot of every in-app purchase (idempotent: a screenshot whose MD5 matches is kept) and
+// Upload the App Review screenshot of every in-app purchase (idempotent: a screenshot whose MD5 matches and that
-// print each product's state. Nothing is submitted.
+// Apple processed is kept) and print each product's state with what is still missing. Nothing is submitted.
 //   storebound apple iap-review [--dry]
 // Screenshot per product: products[].reviewScreenshot, else apple.reviewShots/<product id>.(png|jpg).
-// Size: a real device screenshot size (e.g. 1290 x 2796); other sizes fail with IMAGE_INCORRECT_DIMENSIONS.
+// Size: an App Store screenshot size (iPhone or iPad, e.g. 1320 x 2868 or 1290 x 2796); Apple processes the upload
+// after it is committed and fails other sizes with IMAGE_INCORRECT_DIMENSIONS, so the size is checked before the
+// upload and the delivery state is read back after it (an upload is only "done" when Apple says COMPLETE).
 // Config products missing from the store's list (it lags a minute after `apple iap` creates them) are named with a
-// warning and the exit code is 1, so nothing is skipped silently (docs/pitfalls.md).
+// warning and the exit code is 1, so nothing is skipped silently (docs/pitfalls.md). --dry ends with the plan table.
 import { existsSync, readFileSync } from 'node:fs';
-import { join } from 'node:path';
+import { join, relative } from 'node:path';
 import { sellsOn } from '../lib/config';
 import type { Ctx } from '../lib/ctx';
 import { need } from '../lib/ctx';
 import { dryPlan, type PlanRow } from '../lib/dry';
 import { done, log, warn } from '../lib/log';
 import { reportMissing } from '../lib/store-lag';
+import { IMAGE_SPECS, imageInfo } from '../shots/spec';
 import { ascClient, md5 } from './asc';
 
+/** Every iPhone / iPad screenshot size App Store Connect accepts (portrait and landscape). */
+export const REVIEW_SHOT_SIZES: [number, number][] = Object.values(IMAGE_SPECS)
+  .filter(s => s.store === 'apple' && /^APP_(IPHONE|IPAD)/.test(s.type))
+  .flatMap(s => s.sizes.flatMap(([w, h]) => [[w, h], [h, w]] as [number, number][]));
+
+/** null when the image can be used as an App Review screenshot, else why not. Pure. */
+export function reviewShotProblem(buf: Uint8Array): string | null {
+  const i = imageInfo(buf);
+  if (i.format === 'unknown') return 'not a PNG or JPEG';
+  if (!REVIEW_SHOT_SIZES.some(([w, h]) => w === i.w && h === i.h))
+    return `${i.w} x ${i.h} is not an App Store screenshot size (use e.g. 1320 x 2868 or 1290 x 2796 for iPhone, 2064 x 2752 for iPad; Apple fails other sizes with IMAGE_INCORRECT_DIMENSIONS)`;
+  return null;
+}
+
+type Shot = { id: string; attributes: { sourceFileChecksum?: string; fileName?: string; assetDeliveryState?: { state?: string; errors?: { code?: string; description?: string }[] } } };
+const stateOf = (s: Shot | undefined): string => s?.attributes.assetDeliveryState?.state ?? 'NONE';
+const errorsOf = (s: Shot | undefined): string => (s?.attributes.assetDeliveryState?.errors ?? []).map(e => e.code ?? e.description).filter(Boolean).join(', ');
+
 export async function run(ctx: Ctx): Promise<void> {
   const { config, root, resolve } = ctx.loaded;
+  const dry = ctx.args.has('dry');
   const c = await ascClient(root, need(config.apple?.bundleId, 'apple.bundleId'), config.apple?.appId);
   const appId = await c.app();
   const defs = new Map((config.products ?? []).map(p => [p.id, p]));
-  const dry = ctx.args.has('dry');
+  const shotOf = (id: string) => c.maybe(`/v2/inAppPurchases/${id}/appStoreReviewScreenshot`).then(r => r?.data as Shot | undefined);
-  const listed = await c.all(`/v1/apps/${appId}/inAppPurchasesV2?limit=200`);
+  const iaps = await c.all(`/v1/apps/${appId}/inAppPurchasesV2?limit=200`);
   const want = (config.products ?? []).filter(p => sellsOn(config, p, 'apple')).map(p => p.id);
-  const missing = reportMissing('apple', 'products', want, listed.map(p => p.attributes.productId as string), { fail: true });
+  const missing = reportMissing('apple', 'products', want, iaps.map(p => p.attributes.productId as string), { fail: true });
   const rows: PlanRow[] = missing.map(id => ({ item: id, action: 'skip', detail: 'not in the store\'s list yet (run apple iap first, or re-run in a minute)' }));
-  for (const p of listed) {
+  let problems = 0;
+  for (const p of iaps) {
     const sku: string = p.attributes.productId;
     const def = defs.get(sku);
     const file = def?.reviewScreenshot ? resolve(def.reviewScreenshot)
       : ['png', 'jpg', 'jpeg'].map(e => resolve(join(config.apple?.reviewShots ?? 'store/iap-review', `${sku}.${e}`))).find(existsSync);
-    if (!file || !existsSync(file)) { warn('apple', `${sku}: no review screenshot (set products[].reviewScreenshot)`); rows.push({ item: sku, action: 'skip', detail: 'no review screenshot' }); continue; }
+    if (!file || !existsSync(file)) { warn('apple', `${sku}: no review screenshot (set products[].reviewScreenshot)`); rows.push({ item: sku, action: 'skip', detail: 'no review screenshot' }); problems++; continue; }
     const buf = readFileSync(file);
-    const cur = (await c.maybe(`/v2/inAppPurchases/${p.id}/appStoreReviewScreenshot`))?.data;
+    const bad = reviewShotProblem(buf);
-    if (cur && cur.attributes.sourceFileChecksum === md5(buf) && cur.attributes.assetDeliveryState?.state !== 'FAILED') { log('apple', `${sku}: screenshot up to date`); rows.push({ item: sku, action: 'keep' }); continue; }
+    const cur = await shotOf(p.id);
-    if (dry) { rows.push({ item: sku, action: cur ? 'replace' : 'upload', detail: `${file} (state ${p.attributes.state ?? '?'})` }); continue; }
+    const curState = stateOf(cur);
+    if (bad) {
+      warn('apple', `${sku}: ${relative(root, file)}: ${bad}${cur ? `; App Store Connect has ${curState}${errorsOf(cur) ? ` (${errorsOf(cur)})` : ''}` : ''}`);
+      rows.push({ item: sku, action: 'skip', detail: bad }); problems++; continue;
+    }
+    if (cur && cur.attributes.sourceFileChecksum === md5(buf) && curState !== 'FAILED') { log('apple', `${sku}: screenshot up to date (${curState})`); rows.push({ item: sku, action: 'keep' }); continue; }
+    const why = !cur ? 'none in App Store Connect' : curState === 'FAILED' ? `the one there FAILED${errorsOf(cur) ? ` (${errorsOf(cur)})` : ''}` : 'a different file is there';
+    if (dry) { log('apple', `${sku}: would upload ${relative(root, file)} (${why})`); rows.push({ item: sku, action: cur ? 'replace' : 'upload', detail: `${relative(root, file)} (${why})` }); continue; }
     if (cur) await c.asc('DELETE', `/v1/inAppPurchaseAppStoreReviewScreenshots/${cur.id}`);
     await c.uploadAsset('inAppPurchaseAppStoreReviewScreenshots', { inAppPurchaseV2: { data: { type: 'inAppPurchases', id: p.id } } }, file);
-    log('apple', `${sku}: screenshot uploaded`);
+    // Apple processes the asset after the commit: read the state back (COMPLETE, or FAILED with the reason)
+    let after: Shot | undefined, state = 'UPLOAD_COMPLETE';
+    for (let i = 0; i < 10; i++) {
+      await Bun.sleep(i ? 3000 : 1500);
+      after = await shotOf(p.id); state = stateOf(after);
+      if (state === 'COMPLETE' || state === 'FAILED') break;
     }
-  if (dry) { console.log(dryPlan('apple iap-review', rows)); done('apple', missing.length ? `iap-review checked (dry run): ${missing.length} product(s) missing from the store's list, exit 1` : 'iap-review checked (dry run)'); return; }
+    if (state === 'FAILED') { warn('apple', `${sku}: Apple rejected the screenshot: ${errorsOf(after) || 'FAILED'}`); problems++; }
-  await Bun.sleep(3000);
+    else log('apple', `${sku}: screenshot uploaded (${state})`);
-  for (const p of await c.all(`/v1/apps/${appId}/inAppPurchasesV2?limit=200`)) log('apple', `${p.attributes.productId}: ${p.attributes.state}`);
+  }
-  done('apple', 'iap-review done (READY_TO_SUBMIT = complete; first IAPs go to review with a version, from the IAP page)');
+  // what keeps a product in MISSING_METADATA (Apple's rule: a localization, a price, availability and the screenshot)
+  for (const p of await c.all(`/v1/apps/${appId}/inAppPurchasesV2?limit=200`)) {
+    const state: string = p.attributes.state;
+    if (state !== 'MISSING_METADATA') { log('apple', `${p.attributes.productId}: ${state}`); continue; }
+    const lack: string[] = [];
+    const locs = await c.maybe(`/v2/inAppPurchases/${p.id}/inAppPurchaseLocalizations?limit=50`);
+    if (!locs?.data?.length) lack.push('a localization (name + description)');
+    const price = await c.maybe(`/v2/inAppPurchases/${p.id}/iapPriceSchedule`);
+    if (!price?.data) lack.push('a price');
+    const avail = await c.maybe(`/v2/inAppPurchases/${p.id}/inAppPurchaseAvailability`);
+    if (!avail?.data) lack.push('availability (territories)');
+    const shot = await shotOf(p.id);
+    if (!shot) lack.push('the review screenshot');
+    else if (stateOf(shot) === 'FAILED') lack.push(`a working review screenshot (FAILED: ${errorsOf(shot) || 'unknown'})`);
+    else if (stateOf(shot) !== 'COMPLETE') lack.push(`Apple to finish processing the screenshot (${stateOf(shot)}; check again in a minute)`);
+    log('apple', `${p.attributes.productId}: MISSING_METADATA${lack.length ? `: needs ${lack.join(', ')}` : ' (all parts present: Apple may still be updating the state)'}`);
+  }
+  if (dry) console.log(dryPlan('apple iap-review', rows));
+  if (problems) process.exitCode = 1;
+  done('apple', dry ? 'iap-review dry run (nothing uploaded)' : 'iap-review done (READY_TO_SUBMIT = complete; first IAPs go to review with a version, from the IAP page)');
 }

cli/check.ts

@@ -14,6 +14,7 @@ import { dataSafetyRows } from './lib/data-safety';
 import { reviewDemoAccount } from './lib/credentials';
 import { log, printJson, warn } from './lib/log';
 import { profileIssues } from './lib/profile';
+import { reviewShotProblem } from './apple/iap-review';
 import { allSubscriptions } from './lib/subscriptions';
 import { type CopyFile, validateCopy } from './shots/lib';
 
@@ -78,6 +79,13 @@ export async function run(ctx: Ctx): Promise<void> {
     if (config.apple && len(l.description) > APPLE_IAP_LIMITS.description) problems.push(`product ${p.id} ${lang}: description ${len(l.description)} > ${APPLE_IAP_LIMITS.description} (App Store limit)`);
     if (len(l.description) > 200) problems.push(`product ${p.id} ${lang}: description ${len(l.description)} > 200 (Play limit)`);
   }
+  // App Review screenshots of in-app purchases: Apple fails any non-screenshot size after upload (IMAGE_INCORRECT_DIMENSIONS)
+  if (config.apple) for (const f of new Set((config.products ?? []).map(p => p.reviewScreenshot).filter((x): x is string => !!x))) {
+    const file = resolve(f);
+    if (!existsSync(file)) continue; // reported with the other missing files below
+    const bad = reviewShotProblem(readFileSync(file));
+    if (bad) problems.push(`products[].reviewScreenshot ${f}: ${bad}`);
+  }
   if (config.shots) tryDo('shots copy', () => {
     const copy = JSON.parse(readFileSync(resolve(config.shots!.copy), 'utf8')) as CopyFile;
     const e = validateCopy(copy, config.shots!, config.languages.map(l => l.id));

tests/pitfalls.test.ts

@@ -18,6 +18,8 @@ afterEach(() => { restore(); if (env.STOREBOUND_LIST_SETTLE_MS === undefined) de
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const page = (data: unknown[]): any => ({ data, links: { self: 'https://api.appstoreconnect.apple.com/v1/x' }, meta: { paging: { total: data.length, limit: 200 } } });
 const product = (id: string): Product => ({ id, type: 'non_consumable', usd: 2.99, listings: { en: { title: 'No Ads', description: 'Removes ads' } } });
+// a PNG header of an App Store screenshot size (the size check reads IHDR only)
+const png = (w = 1290, h = 2796) => { const b = Buffer.alloc(33); b.set([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a], 0); b.writeUInt32BE(13, 8); b.write('IHDR', 12); b.writeUInt32BE(w, 16); b.writeUInt32BE(h, 20); b[24] = 8; b[25] = 2; return b; };
 const PRO = 'com.example.notes.pro', GEMS = 'com.example.notes.gems';
 const CONFIG = { ...BASE, products: [product(PRO), { ...product(GEMS), type: 'consumable' as const }] };
 
@@ -29,23 +31,23 @@ describe('store list lag after create', () => {
 
   test('apple iap-review --dry: a product missing from /inAppPurchasesV2 is named with the lag hint and exits 1', async () => {
     ascEnv();
-    const p = project(CONFIG, { [`store/iap-review/${PRO}.png`]: new Uint8Array([1, 2, 3]), [`store/iap-review/${GEMS}.png`]: new Uint8Array([4, 5, 6]) });
+    const p = project(CONFIG, { [`store/iap-review/${PRO}.png`]: png(), [`store/iap-review/${GEMS}.png`]: png() });
     // as answered a few seconds after `apple iap` created both: only the older product is listed
     const calls = mockFetch([
-      { match: /^\/v1\/apps\/6400000101\/inAppPurchasesV2\?limit=200/, answer: page([{ type: 'inAppPurchases', id: 'i-pro', attributes: { productId: PRO, state: 'MISSING_METADATA' } }]) },
+      { match: /^\/v1\/apps\/6400000101\/inAppPurchasesV2\?limit=200/, answer: page([{ type: 'inAppPurchases', id: 'i-pro', attributes: { productId: PRO, state: 'READY_TO_SUBMIT' } }]) },
       { match: /^\/v2\/inAppPurchases\/i-pro\/appStoreReviewScreenshot/, answer: notFound() },
     ]);
     const r = await capture(() => iapReview(p.ctx(['--dry'])));
     expect(r.err).toContain(`1 of 2 products not in the store's list: ${GEMS} (${LAG_HINT})`);
     expect(r.out).toMatch(new RegExp(`${GEMS.replace(/\./g, '\\.')}\\s+skip\\s+not in the store's list yet`));
-    expect(r.out).toMatch(new RegExp(`${PRO.replace(/\./g, '\\.')}\\s+upload\\s+`));
+    expect(r.out).toMatch(new RegExp(`${PRO.replace(/\./g, '\\.')}\\s+upload\\s+store/iap-review/${PRO.replace(/\./g, '\\.')}\\.png \\(none in App Store Connect\\)`));
     expect(r.exit).toBe(1);
     expect(calls.filter(c => c.method !== 'GET')).toHaveLength(0);
   });
 
   test('apple iap-review --dry: every product listed exits 0', async () => {
     ascEnv();
-    const p = project({ ...CONFIG, products: [product(PRO)] }, { [`store/iap-review/${PRO}.png`]: new Uint8Array([1]) });
+    const p = project({ ...CONFIG, products: [product(PRO)] }, { [`store/iap-review/${PRO}.png`]: png() });
     mockFetch([
       { match: /^\/v1\/apps\/6400000101\/inAppPurchasesV2\?limit=200/, answer: page([{ type: 'inAppPurchases', id: 'i-pro', attributes: { productId: PRO, state: 'READY_TO_SUBMIT' } }]) },
       { match: /^\/v2\/inAppPurchases\/i-pro\/appStoreReviewScreenshot/, answer: notFound() },

tests/wave1.test.ts

@@ -457,3 +457,14 @@ describe('config + scrub', () => {
     expect(hits.map(h => h.rule).sort()).toEqual(['apple-id-like', 'email', 'ip-address', 'private-key']);
   });
 });
+
+describe('iap review screenshot size', () => {
+  const png = (w: number, h: number) => { const b = Buffer.alloc(33); b.set([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a], 0); b.writeUInt32BE(13, 8); b.write('IHDR', 12); b.writeUInt32BE(w, 16); b.writeUInt32BE(h, 20); b[24] = 8; b[25] = 2; return b; };
+  test('only App Store screenshot sizes pass (1320 x 2706 was rejected by Apple with IMAGE_INCORRECT_DIMENSIONS)', async () => {
+    const { reviewShotProblem } = await import('../cli/apple/iap-review');
+    expect(reviewShotProblem(png(1320, 2706))).toContain('IMAGE_INCORRECT_DIMENSIONS');
+    expect(reviewShotProblem(png(1320, 2868))).toBeNull();
+    expect(reviewShotProblem(png(2796, 1290))).toBeNull(); // landscape
+    expect(reviewShotProblem(png(2064, 2752))).toBeNull(); // iPad 13"
+  });
+});