diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f32a40..92bb873 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## Unreleased +- Play: `play status` and `validate` list every build still active on a testing track below the production build, with the `play release --version-code -- completed` that replaces it (an old build with a policy issue, e.g. READ_MEDIA_IMAGES, blocks Send changes for review for the whole app). A completed production release the store page does not show yet reads "Not live yet: in review or blocked before review" instead of "In review" (CLI and desktop). A commit with `--not-sent-for-review` that Play answers with 400 "Changes are sent for review automatically" is redone without the parameter (pitfalls #129). + ## 0.8.68 · 2026-11-07 - Desktop app: `storebound` works in a terminal with only the app installed. On every start the app puts a launcher for its own CLI on the PATH (Windows: `%LOCALAPPDATA%\storebound\bin`, added to the user PATH once; macOS / Linux: `~/.local/bin`, added to `~/.zprofile` / `~/.profile` when missing). An npm / bun install stays first, a file that is not the app's launcher is never overwritten, AppImage runs skip it, `STOREBOUND_NO_CLI_SHIM=1` turns it off. diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index b34f6bb..181d7c2 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -153,7 +153,8 @@ export const en = { 'phase.releasedUnknown': "Couldn't check the store page", 'phase.pageUnknown': "Couldn't check the store page", 'phase.pageStale': "couldn't check the store page, as of {when}", - 'phase.reviewLive': 'In review (live: {version})', + 'phase.reviewLive': 'Not live yet (live: {version}): in review or blocked before review', + 'phase.playPending': 'Not live yet: in review or blocked before review', 'roll.publishReady': 'If the release is approved, publish it in Publishing overview ("Publish N changes"). The agent prepares it with `{cmd}`; the final press is for the owner.', 'roll.publishOpen': 'Open Publishing overview', 'phase.reviewManaged': 'In review / ready to publish (managed publishing)', diff --git a/apps/desktop/src/i18n/tr.ts b/apps/desktop/src/i18n/tr.ts index 03dc74d..417f0dd 100644 --- a/apps/desktop/src/i18n/tr.ts +++ b/apps/desktop/src/i18n/tr.ts @@ -155,7 +155,8 @@ export const tr: Record = { 'phase.releasedUnknown': 'Mağaza sayfası kontrol edilemedi', 'phase.pageUnknown': 'Mağaza sayfası kontrol edilemedi', 'phase.pageStale': 'mağaza sayfası kontrol edilemedi, {when} itibarıyla', - 'phase.reviewLive': 'İncelemede (yayında: {version})', + 'phase.reviewLive': 'Henüz yayında değil (yayında: {version}): incelemede ya da incelemeden önce takıldı', + 'phase.playPending': 'Henüz yayında değil: incelemede ya da incelemeden önce takıldı', 'roll.publishReady': 'Sürüm onaylandıysa Yayınlama özetinde yayınlayın ("Publish N changes"). Ajan `{cmd}` ile hazırlar; son basış sahibine aittir.', 'roll.publishOpen': 'Yayınlama özetini aç', 'phase.reviewManaged': 'İncelemede / yayına hazır (yönetilen yayınlama)', diff --git a/apps/desktop/src/ui/kit.tsx b/apps/desktop/src/ui/kit.tsx index d118760..6e58bff 100644 --- a/apps/desktop/src/ui/kit.tsx +++ b/apps/desktop/src/ui/kit.tsx @@ -25,7 +25,7 @@ export function phaseLabel(r: StoreRelease | null | undefined): { text: string; const stale = r.pageStaleAt ? ` · ${t('phase.pageStale', { when: ago(r.pageStaleAt) })}` : ''; if (r.store === 'android' && r.phase === 'released') return { text: t('phase.pageUnknown') + stale, tone: 'neutral' }; if (r.store === 'android' && (r.phase === 'review' || r.phase === 'sent')) - return { text: (r.managed ? t('phase.reviewManaged') : r.liveVersion ? t('phase.reviewLive', { version: r.liveVersion }) : t('phase.review')) + stale, tone: 'warn' }; + return { text: (r.managed ? t('phase.reviewManaged') : r.liveVersion ? t('phase.reviewLive', { version: r.liveVersion }) : t('phase.playPending')) + stale, tone: 'warn' }; if (r.store === 'android' && r.phase === 'live' && stale) return { text: t('phase.live') + stale, tone: 'ok' }; if (r.phase === 'testing' && r.track) return { text: `${t('phase.testing')} · ${r.track}`, tone: 'info' }; return { text: t(`phase.${r.phase}`), tone: PHASE_TONE[r.phase] }; diff --git a/cli/lib/portfolio.ts b/cli/lib/portfolio.ts index ae906e6..8448f5f 100644 --- a/cli/lib/portfolio.ts +++ b/cli/lib/portfolio.ts @@ -161,11 +161,15 @@ export function playReleases(data: unknown): StoreRelease[] { return out; } +/** A completed Play production release the store page does not show yet. The API has no review state: it may be in + * review, or blocked before review (an old build on another track with a policy issue, pitfall 129). Never "In review". */ +export const PLAY_NOT_LIVE = 'Not live yet: in review or blocked before review (the API cannot tell; see Publishing overview)'; + /** Plain words for a Play release's phase, decided from the store page (CLI text; the desktop has the same words in * i18n, apps/desktop/src/ui/kit.tsx phaseLabel). */ export function playReleaseLabel(r: StoreRelease): string { const base = ((): string => { - if (r.phase === 'review' || r.phase === 'sent') return r.managed ? 'In review / ready to publish (managed publishing)' : r.liveVersion ? `In review (live: ${r.liveVersion})` : 'In review'; + if (r.phase === 'review' || r.phase === 'sent') return r.managed ? 'In review / ready to publish (managed publishing)' : r.liveVersion ? `${PLAY_NOT_LIVE} (live: ${r.liveVersion})` : PLAY_NOT_LIVE; if (r.phase === 'released') return "Couldn't check the store page"; if (r.phase === 'live') return 'Live'; if (r.phase === 'rollout') return `Rolling out ${Math.round((r.fraction ?? 0) * 1000) / 10}%`; diff --git a/cli/lib/validate-rules.ts b/cli/lib/validate-rules.ts index ad3593b..f7854e5 100644 --- a/cli/lib/validate-rules.ts +++ b/cli/lib/validate-rules.ts @@ -1,6 +1,7 @@ // Store readiness rules for `storebound validate`. Pure: the command collects a snapshot from both APIs (read-only), // these functions turn it into an ordered list of findings. Order = the order you would fix things in. +import { oldTrackBuilds } from '../play/old-builds'; import { consoleLink, consoleUrl } from './console-links'; import { describePending, type Pending, repairable } from '../apple/screenshot-health'; import { type FixAction, fixAppleListing, fixPlayListing, fixReviewDetails } from './fix-actions'; @@ -409,6 +410,7 @@ export function playRules(s: PlaySnapshot): Finding[] { if (r.status === 'halted') add('warn', 'tracks', `${t.track}: release ${r.versionCodes.join(',')} is halted`, 'storebound play rollout --resume (owner)'); if (r.status === 'inProgress') add('ok', 'tracks', `${t.track}: rollout ${Math.round((r.userFraction ?? 0) * 100)}% of ${r.versionCodes.join(',')}`); } + for (const o of oldTrackBuilds(s.tracks)) add('warn', 'old-builds', `${o.track}: old build ${o.versionCodes.join(',')} (${o.status}) is still active below production ${o.productionCode}; Play reviews it with every submission and a policy issue in it (e.g. READ_MEDIA_IMAGES / READ_MEDIA_VIDEO) blocks Send changes for review (pitfall 129)`, `storebound play release --version-code ${o.productionCode} ${['internal','alpha','beta'].includes(o.track) ? '--' + o.track : '--track ' + o.track} completed`); if (anyRelease && !f.some(x => x.code === 'tracks')) add('ok', 'tracks', s.tracks.filter(t => t.releases.length).map(t => `${t.track}: ${t.releases.map(r => r.status).join('/')}`).join(', ')); const prods = new Map(s.products.map(p => [p.productId, p] as const)); diff --git a/cli/play/client.ts b/cli/play/client.ts index 81239dd..290b2ea 100644 --- a/cli/play/client.ts +++ b/cli/play/client.ts @@ -7,7 +7,7 @@ import { noteGoogle } from '../lib/quota'; import { AmbiguousWriteError, isAmbiguous, retrySafe } from '../lib/upsert'; import * as rt from '../lib/rt'; import { withMachineLock } from '../lib/lock'; -import { clearPending, isReviewRefused, notSentMessage, publishingUrl, recordPending, withNotSent } from './review-pending'; +import { clearPending, isAutoSendOnly, isReviewRefused, notSentMessage, publishingUrl, recordPending, withNotSent } from './review-pending'; /** A read's edit is gone: replaced by a newer edit of the app, committed elsewhere or expired. */ export const EDIT_GONE = /Edit has been deleted|editAlreadyCommitted|already been committed|edit.*(expired|not found)|outside of this Edit/i; @@ -118,9 +118,15 @@ export async function playClient(root: string, packageName: string) { } }; try { await commit(); } catch (err) { - if (notSent || !isReviewRefused(err)) throw err; - refused(); - await commit(); + if (notSent && isAutoSendOnly(err)) { + notSent = false; + log('play', 'Play answered "Changes are sent for review automatically" (managed publishing is off): committing again without changesNotSentForReview; Play sends the changes for review itself'); + await commit(); + } else { + if (notSent || !isReviewRefused(err)) throw err; + refused(); + await commit(); + } } openEdit = null; log('play', `edit ${id} committed${notSent ? ' with changesNotSentForReview=true' : ''}`); diff --git a/cli/play/old-builds.ts b/cli/play/old-builds.ts new file mode 100644 index 0000000..5b039ec --- /dev/null +++ b/cli/play/old-builds.ts @@ -0,0 +1,42 @@ +// Builds still active on a testing track below the production build (pitfall 129). Play reviews every active build +// with each submission: a policy issue in an old one (a media permission it asked for, "Use alternative system +// pickers") blocks "Send changes for review" for the whole app, while the API still reads production as completed. +// The Developer API cannot read a bundle's permissions, so each old build is named with the command that replaces it. +// Pure. + +export interface TrackLike { track: string; releases?: { name?: string; versionCodes?: string[]; status: string }[] } +export interface OldBuild { track: string; versionCodes: number[]; name: string | null; status: string; productionCode: number } + +/** Permissions Play reviews as policy-sensitive (Photo and video permissions, All files access). */ +export const SENSITIVE_PERMISSIONS = ['READ_MEDIA_IMAGES', 'READ_MEDIA_VIDEO', 'READ_EXTERNAL_STORAGE', 'MANAGE_EXTERNAL_STORAGE', 'WRITE_EXTERNAL_STORAGE']; + +const ACTIVE = new Set(['completed', 'inProgress', 'halted']); +const codes = (r: { versionCodes?: string[] }): number[] => (r.versionCodes ?? []).map(Number).filter(Number.isFinite); + +export function oldTrackBuilds(tracks: TrackLike[]): OldBuild[] { + const prod = tracks.find(t => t.track === 'production'); + const prodCodes = (prod?.releases ?? []).filter(r => ACTIVE.has(r.status)).flatMap(codes); + if (!prodCodes.length) return []; + const productionCode = Math.max(...prodCodes); + const out: OldBuild[] = []; + for (const t of tracks) { + if (t.track === 'production') continue; + for (const r of t.releases ?? []) { + const cs = codes(r); + if (!ACTIVE.has(r.status) || !cs.length || Math.max(...cs) >= productionCode) continue; + out.push({ track: t.track, versionCodes: cs, name: r.name ?? null, status: r.status, productionCode }); + } + } + return out; +} + +/** The track flag of `play release` for a track name (custom closed tracks go by --track). */ +const trackFlag = (track: string): string => (['internal', 'alpha', 'beta'].includes(track) ? `--${track}` : `--track ${track}`); + +export function oldBuildLines(old: OldBuild[]): string[] { + if (!old.length) return []; + return [ + ` old builds still active (${old.length}): Play reviews them with every submission; a policy issue in one (e.g. ${SENSITIVE_PERMISSIONS.slice(0, 2).join(' / ')}) blocks Send changes for review. The API cannot read their permissions:`, + ...old.map(o => ` ${o.track} [${o.versionCodes.join(',')}] ${o.name ?? ''} ${o.status} -> replace: storebound play release --version-code ${o.productionCode} ${trackFlag(o.track)} completed`), + ]; +} diff --git a/cli/play/review-pending.ts b/cli/play/review-pending.ts index 2bb927d..a7fc809 100644 --- a/cli/play/review-pending.ts +++ b/cli/play/review-pending.ts @@ -25,6 +25,14 @@ export function isReviewRefused(err: unknown): boolean { return reasons.every(r => /badRequest|invalid|failedPrecondition|precondition/i.test(r)); } +/** The opposite 400: with managed publishing off Play refuses changesNotSentForReview=true (live: "Changes are sent for + * review automatically"); the commit is redone without it. "cannot be sent ... automatically" is the other refusal. */ +export function isAutoSendOnly(err: unknown): boolean { + const s = String(err instanceof Error ? err.message : err); + const m = /-> 400 (.*)$/s.exec(s); + return !!m && /are sent for review automatically/i.test(m[1]!); +} + /** The query string with changesNotSentForReview=true added to `base` ("?a=1" or ''). */ export function withNotSent(base: string | undefined, on: boolean): string { const parts = [(base ?? '').replace(/^\?/, ''), on ? 'changesNotSentForReview=true' : ''].filter(Boolean); diff --git a/cli/play/status.ts b/cli/play/status.ts index 74214cc..86b292a 100644 --- a/cli/play/status.ts +++ b/cli/play/status.ts @@ -11,6 +11,7 @@ import { playReleases, playReleaseLabel } from '../lib/portfolio'; import { playClient } from './client'; import { listOneTimeProducts } from './products'; import { readyLines, readyToPublish } from '../lib/play-publish'; +import { oldBuildLines, oldTrackBuilds } from './old-builds'; import { clearPending, pendingLine, publishingUrl, readPending } from './review-pending'; interface Track { track: string; releases?: { name?: string; versionCodes?: string[]; status: string; userFraction?: number }[] } @@ -33,7 +34,7 @@ export async function run(ctx: Ctx): Promise { // the App category is not in the API (edits.details has no category field): a console step const cat = playCategoryOf(config); const category = { config: cat?.category ?? null, source: cat?.source ?? null, api: false, plan: 'storebound console plan play-store-settings' }; - if (ctx.args.has('json')) { printJson({ details, category, tracks, listings, products, publicPage, managedPublishing: config.play?.managedPublishing ?? null, reviewPending, readyToPublish: readyToPublish({ tracks, publicPage, managedPublishing: config.play?.managedPublishing ?? null }) }); return; } + if (ctx.args.has('json')) { printJson({ details, category, tracks, listings, products, publicPage, managedPublishing: config.play?.managedPublishing ?? null, reviewPending, oldBuilds: oldTrackBuilds(tracks), readyToPublish: readyToPublish({ tracks, publicPage, managedPublishing: config.play?.managedPublishing ?? null }) }); return; } console.log(`${play.packageName}\n details: ${details.defaultLanguage} · ${details.contactEmail ?? '-'} · ${details.contactWebsite ?? '-'}`); console.log(` category: ${cat ? `${cat.category} (${cat.source})` : 'not in the config (play.category)'} · not readable by the API: Store settings is a console step (${category.plan})`); console.log(` listings (${listings.length}): ${listings.map(l => l.language).join(', ') || '-'}`); @@ -44,6 +45,7 @@ export async function run(ctx: Ctx): Promise { console.log(` public store page: ${publicPage.state === 'public' ? `published (200), live version ${publicPage.versionName ?? (publicPage.varies ? 'varies with device' : 'not shown')}` : publicPage.state === 'notPublic' ? 'not public yet (404)' : `couldn't check it (${publicPage.error ?? 'not read'})`}${publicPage.stale ? ` (last known, ${publicPage.checkedAt.slice(0, 16).replace('T', ' ')} UTC)` : ''}`); for (const r of playReleases({ tracks, publicPage, managedPublishing: config.play?.managedPublishing ?? null }).filter(x => x.track === 'production')) console.log(` production ${r.version ?? '?'} (${r.build ?? '?'}): ${playReleaseLabel(r)}`); for (const l of readyLines(readyToPublish({ tracks, publicPage, managedPublishing: config.play?.managedPublishing ?? null }))) console.log(l); + for (const l of oldBuildLines(oldTrackBuilds(tracks))) console.log(l); if (pending) console.log(` review: ${pendingLine(pending)}`); console.log(` products (${products.length}):`); for (const p of products) console.log(` ${p.productId.padEnd(40)} ${(p.purchaseOptions ?? []).map(o => `${o.purchaseOptionId}:${o.state ?? '?'}`).join(' ')}`); diff --git a/skills/store-pitfalls/SKILL.md b/skills/store-pitfalls/SKILL.md index ec81a14..5110489 100644 --- a/skills/store-pitfalls/SKILL.md +++ b/skills/store-pitfalls/SKILL.md @@ -1594,3 +1594,19 @@ that fails without the fix (see CONTRIBUTING.md). version and build only, and the dry run names what it left out and why. - **Never:** attach iOS-only products to the Mac submission or switch Game Center on for a Mac app without it. - **Guard:** `macosSubmitSkips` in `cli/lib/config.ts`, `tests/resubmit.test.ts`. + +## 129. Play said "In review" while an old testing build blocked Send changes for review + +- **Symptom:** `play status` showed production as "In review" for days; Publishing overview had "Send changes for + review" greyed out. An old build still active on a testing track (alpha) asked for READ_MEDIA_IMAGES / + READ_MEDIA_VIDEO ("Use alternative system pickers"); the current production build did not. Play reviews every active + build, so the policy issue of the old one held the whole app back. A later commit with `--not-sent-for-review` + answered 400 "Changes are sent for review automatically" (managed publishing off). +- **Do:** replace each old build: `storebound play release --version-code --alpha completed` (or + `--beta`, `--internal`, `--track `); the changes then go into review by themselves. `play status` and + `validate` list every active build below production with that command. +- **Never:** read a completed production release that the store page does not show as "In review": the API has no + review state, so storebound says "Not live yet: in review or blocked before review". +- **Guard:** `cli/play/old-builds.ts` (play status, validate `old-builds`), `PLAY_NOT_LIVE` in `cli/lib/portfolio.ts`, + `isAutoSendOnly` in `cli/play/review-pending.ts` (the commit is redone without changesNotSentForReview), + `tests/play-old-builds.test.ts`. diff --git a/tests/play-old-builds.test.ts b/tests/play-old-builds.test.ts new file mode 100644 index 0000000..e654d97 --- /dev/null +++ b/tests/play-old-builds.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, test } from 'bun:test'; +import { oldBuildLines, oldTrackBuilds } from '../cli/play/old-builds'; +import { isAutoSendOnly, isReviewRefused } from '../cli/play/review-pending'; + +const tracks = [ + { track: 'production', releases: [{ name: '2.0.0 (10)', versionCodes: ['10'], status: 'completed' }] }, + { track: 'alpha', releases: [{ name: '1.0 (2)', versionCodes: ['2'], status: 'completed' }] }, + { track: 'beta', releases: [{ versionCodes: ['10'], status: 'completed' }] }, + { track: 'internal', releases: [{ versionCodes: ['3'], status: 'draft' }] }, + { track: 'qa-closed', releases: [{ versionCodes: ['4'], status: 'completed' }] }, +]; + +describe('old builds on testing tracks (pitfall 129)', () => { + test('active builds below production are listed with the command that replaces them; drafts and current ones are not', () => { + const old = oldTrackBuilds(tracks); + expect(old.map(o => `${o.track}:${o.versionCodes.join(',')}`)).toEqual(['alpha:2', 'qa-closed:4']); + const text = oldBuildLines(old).join('\n'); + expect(text).toContain('storebound play release --version-code 10 --alpha completed'); + expect(text).toContain('--track qa-closed completed'); + }); + test('no production release: nothing to compare', () => { + expect(oldTrackBuilds(tracks.filter(t => t.track !== 'production'))).toEqual([]); + }); + test('the "are sent for review automatically" 400 is told apart from the "cannot be sent" one', () => { + const auto = new Error('POST commit -> 400 {"error":{"message":"Changes are sent for review automatically.","errors":[{"reason":"badRequest"}]}}'); + const refused = new Error('POST commit -> 400 {"error":{"message":"Changes cannot be sent for review automatically.","errors":[{"reason":"badRequest"}]}}'); + expect(isAutoSendOnly(auto)).toBe(true); + expect(isAutoSendOnly(refused)).toBe(false); + expect(isReviewRefused(refused)).toBe(true); + }); +}); diff --git a/tests/play-public-version-match.test.ts b/tests/play-public-version-match.test.ts index 647850e..f756b8b 100644 --- a/tests/play-public-version-match.test.ts +++ b/tests/play-public-version-match.test.ts @@ -7,7 +7,7 @@ import { trainsOf } from '../cli/release'; import { readFileSync } from 'node:fs'; import { join } from 'node:path'; import { clearPlayPublicCache, pageVersion, pageVersionInfo, playPublicPage } from '../cli/lib/play-public'; -import { appleReleases, playReleaseLabel, playReleases, releaseTrains, sameVersion, versionKey } from '../cli/lib/portfolio'; +import { appleReleases, PLAY_NOT_LIVE, playReleaseLabel, playReleases, releaseTrains, sameVersion, versionKey } from '../cli/lib/portfolio'; import { laneOf, laneText, releasePlan, storeNow, versionHistory } from '../cli/lib/train'; const realFetch = globalThis.fetch; @@ -45,7 +45,7 @@ describe('version matching across the stores', () => { expect(p.ios?.release?.version).toBe('1.0'); expect(p.android?.release?.version).toBe('1.0.0'); expect(p.done).toBe(false); - expect(p.android!.steps.map(x => x.text).join(' ')).toContain('1.0: In review'); + expect(p.android!.steps.map(x => x.text).join(' ')).toContain('1.0: Not live yet'); }); }); @@ -53,7 +53,7 @@ describe('Google Play "Live" needs the public store page', () => { test('completed production + page 404 = in review, never live', () => { const [prod] = playReleases(PLAY_STATUS_FIRST_404); expect(prod).toMatchObject({ phase: 'review', state: 'completed', publicPage: 'notPublic', fraction: 1 }); - expect(playReleaseLabel(prod!)).toBe('In review'); + expect(playReleaseLabel(prod!)).toBe(PLAY_NOT_LIVE); const lane = laneOf('android', prod!); expect(lane.at).toBe(2); // the review stage, not live / rollout expect(lane.share).toBeNull(); @@ -77,14 +77,14 @@ describe('Google Play "Live" needs the public store page', () => { expect(playReleaseLabel(r)).toBe('Live'); }); test('rule: page 404, managed publishing off: In review; on: In review / ready to publish', () => { - expect(playReleaseLabel(prodRel({ state: 'notPublic', versionName: null }))).toBe('In review'); + expect(playReleaseLabel(prodRel({ state: 'notPublic', versionName: null }))).toBe(PLAY_NOT_LIVE); expect(playReleaseLabel(prodRel({ state: 'notPublic', versionName: null }, '1.1.0', 'completed', { managedPublishing: true }))).toBe('In review / ready to publish (managed publishing)'); }); test('rule: page live with an older version: In review (live: )', () => { const r = prodRel({ state: 'public', versionName: '1.0.0' }); expect(r).toMatchObject({ phase: 'review', liveVersion: '1.0.0' }); - expect(playReleaseLabel(r)).toBe('In review (live: 1.0.0)'); - expect(laneText(laneOf('android', r))).toBe('in review (live: 1.0.0)'); + expect(playReleaseLabel(r)).toBe(`${PLAY_NOT_LIVE} (live: 1.0.0)`); + expect(laneText(laneOf('android', r))).toBe(`not live yet: in review or blocked before review (the API cannot tell; see Publishing overview) (live: 1.0.0)`); expect(storeNow([r], 'android').now).toBeNull(); }); test('rule: rollout, halted, draft', () => {