From 992f5bcc0ed117979b6bf7a136021c91442497ac Mon Sep 17 00:00:00 2001 From: CRT Date: Thu, 15 Oct 2026 06:14:49 +0800 Subject: [PATCH] desktop: a Fix button on scan and validate findings storebound can fix itself. Findings carry action { method, args } (cli/lib/fix-actions.ts); privacy-labels runs scan.write (scan --write-only, the dialog shows the config diff), Play listing gaps play.listing, a missing App Store localization or copyright apple.listing (editable version only), the App Review contact apple.review_details; each a dry run, the native dialog, then the confirmed run. --write-only is forbidden for MCP tools; tests pin the mapping, the round trip on a fixture and that no fix reaches a submit, release or publish --- CHANGELOG.md | 5 ++ apps/desktop/src/dev/fixtures.ts | 7 ++ apps/desktop/src/i18n/en.ts | 7 ++ apps/desktop/src/i18n/tr.ts | 7 ++ apps/desktop/src/lib/model.ts | 6 +- apps/desktop/src/screens/tabs/Checks.tsx | 22 ++++- cli/data/flags.json | 5 +- cli/lib/fix-actions.ts | 23 ++++++ cli/lib/scan/index.ts | 5 ++ cli/lib/scan/rules.ts | 3 + cli/lib/validate-rules.ts | 26 +++--- cli/mcp-tools.ts | 2 +- cli/scan.ts | 54 ++++++++---- cli/serve.ts | 7 ++ cli/validate.ts | 2 +- docs/desktop.md | 8 +- tests/fix-actions.test.ts | 100 +++++++++++++++++++++++ tests/serve.test.ts | 5 +- 18 files changed, 257 insertions(+), 37 deletions(-) create mode 100644 cli/lib/fix-actions.ts create mode 100644 tests/fix-actions.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b6cc110..93c2292 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +- **Desktop: a Fix button on findings** that storebound can fix itself. Scan and validate findings carry a + machine-readable `action` (`--json`); the Checks tab runs it as a write with its dry run in the native dialog + before anything changes. privacy-labels writes the detected answers into the config (`scan --write-only`, the + dialog shows the config diff); missing Play listings, a missing App Store localization or copyright and the App + Review contact push the listing files / config to the store (text only). Nothing submits, releases or publishes. - **Desktop:** long findings (a file path in a build folder) wrap inside the Store readiness card instead of pushing the store chips out of it; the release timeline's rail no longer breaks after the current step; the rollout bar stays under its stage. diff --git a/apps/desktop/src/dev/fixtures.ts b/apps/desktop/src/dev/fixtures.ts index d0d4953..7b56da5 100644 --- a/apps/desktop/src/dev/fixtures.ts +++ b/apps/desktop/src/dev/fixtures.ts @@ -339,6 +339,7 @@ const CATALOG: Record = { const VALIDATE: Record = { 'p-prism': { blockers: 1, findings: [ + { store: 'play', level: 'blocker', code: 'listing', msg: 'ja-JP: missing shortDescription', fix: 'storebound play listing', action: { method: 'play.listing', args: {} } }, { store: 'play', level: 'blocker', code: 'product-missing', msg: 'starter_pack is in the config but not on Google Play', fix: 'storebound play products --only starter_pack (dry run first)' }, { store: 'apple', level: 'warn', code: 'screenshots-ipad', msg: 'ar-SA has no 13" iPad screenshots', fix: 'storebound shots render --lang ar, then storebound apple screenshots --only ar-SA' }, { store: 'apple', level: 'warn', code: 'whats-new', msg: '2.4.0 has no "What\'s New" for ja', fix: 'storebound whats-new --since v2.3.0' }, @@ -363,6 +364,9 @@ const SCAN_PRISM: ScanResult['findings'] = [ evidence: [{ file: 'storebound.config.ts', line: 41, text: 'no_ads (non_consumable)' }], fix: 'Add a visible "Restore Purchases" button on the store screen or in Settings.', skill: 'store-iap-setup', ref: 'https://developer.apple.com/app-store/review/guidelines/' }, { rule: 'demo-account', guideline: '2.1(a)', store: 'apple', severity: 'blocker', title: 'Sign-in is required but there is no demo account for App Review', evidence: [{ file: 'storebound.config.ts', line: 17, text: 'reviewAccess: { signInRequired: true }' }], fix: 'Add the App Review demo account in .storebound/ (review-access skill).', skill: 'review-access', ref: 'https://developer.apple.com/app-store/review/guidelines/', accepted: 'leaderboards only; the game plays without signing in' }, + // a deterministic fix (scan.write: the detected answers into the config) + { rule: 'privacy-labels', guideline: 'Play: User Data (Data safety)', store: 'play', severity: 'medium', title: 'Data safety likely misses PSL_DEVICE_ID: an SDK collects it, dataSafety does not declare it', + evidence: [{ file: 'Packages/manifest.json', line: 4, text: 'com.applovin.mediation.ads 8.1.0' }], fix: 'Confirm with the SDK\'s Data safety guidance, add it with `storebound scan --write` (or an edit of the config), then `storebound play data-safety`.', skill: 'store-declarations', ref: 'https://support.google.com/googleplay/android-developer/answer/10787469', action: { method: 'scan.write', args: {} } }, ]; const SCAN: Record = { 'p-prism': { stacks: ['unity'], sdks: [{ id: 'applovin', name: 'AppLovin MAX', category: 'ads', tracking: true }, { id: 'unity-ads', name: 'Unity Ads', category: 'ads', tracking: true }, { id: 'gameanalytics', name: 'GameAnalytics', category: 'analytics', tracking: false }], @@ -593,6 +597,9 @@ async function answer(cmd: string, a: Record): Promise if (method === 'reviews.approve' && d) d.approved = !args.undo; if (method === 'reviews.reply' && d) { d.reply = String(args.text); d.approved = false; d.issues = []; } if (method === 'owner.reply_send' && d) d.sent = iso(0); + // a Fix button's write: the finding it fixed is gone on the next read + if (method === 'scan.write') { const i = SCAN_PRISM.findIndex(f => f.rule === 'privacy-labels'); if (i >= 0) SCAN_PRISM.splice(i, 1); SCAN['p-prism']!.counts = scanCounts(SCAN_PRISM); } + if (method === 'play.listing') { const v = VALIDATE[a.projectId as string]; if (v) v.findings = v.findings.filter(f => f.code !== 'listing'); } const res: ToolResult = { method, ok: true, exitCode: 0, dryRun: false, command: `storebound ${method.replace('.', ' ')}`, output: `[${method}] done (fixture)` }; return { confirmed: true, stage: 'done', dryRun: { ...res, dryRun: true }, result: res }; } diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index 6940a7c..309b810 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -437,6 +437,13 @@ export const en = { 'checks.scanAsk': '{count} declaration answers only you can give', 'checks.scanAsk_one': '1 declaration answer only you can give', 'checks.scanWrite': 'Detected Data safety / App Privacy answers differ from the config: storebound scan --write adds them', + 'checks.fix': 'Fix', + 'checks.fixing': 'Fix: {what}', + 'checks.fixed': 'Fixed. Checking again', + 'checks.fixHint.scan.write': 'Writes the detected Data safety / App Privacy answers into the config. You see the diff before anything is written.', + 'checks.fixHint.play.listing': 'Pushes the listing file to Google Play (text only, nothing is published). You see the dry run first.', + 'checks.fixHint.apple.listing': 'Pushes the listing file and copyright to the App Store version (text only, nothing is submitted). You see the dry run first.', + 'checks.fixHint.apple.review_details': 'Sends the App Review contact and notes from the config (nothing is submitted). You see the dry run first.', 'checks.accepted': 'Accepted: {reason}', 'checks.sev.blocker': 'Blocker', 'checks.sev.high': 'High', diff --git a/apps/desktop/src/i18n/tr.ts b/apps/desktop/src/i18n/tr.ts index 8e4f3a5..1cc92ed 100644 --- a/apps/desktop/src/i18n/tr.ts +++ b/apps/desktop/src/i18n/tr.ts @@ -439,6 +439,13 @@ export const tr: Record = { 'checks.scanAsk': 'Yalnızca senin cevaplayabileceğin {count} beyan sorusu', 'checks.scanAsk_one': 'Yalnızca senin cevaplayabileceğin 1 beyan sorusu', 'checks.scanWrite': 'Bulunan Data safety / App Privacy cevapları ayardan farklı: storebound scan --write ekler', + 'checks.fix': 'Düzelt', + 'checks.fixing': 'Düzelt: {what}', + 'checks.fixed': 'Düzeltildi. Yeniden kontrol ediliyor', + 'checks.fixHint.scan.write': 'Bulunan Data safety / App Privacy cevaplarını ayara yazar. Bir şey yazılmadan önce farkı görürsün.', + 'checks.fixHint.play.listing': "Mağaza metni dosyasını Google Play'e gönderir (yalnız metin, yayınlanmaz). Önce deneme çalıştırmasını görürsün.", + 'checks.fixHint.apple.listing': 'Mağaza metnini ve telif hakkını App Store sürümüne gönderir (yalnız metin, incelemeye gönderilmez). Önce deneme çalıştırmasını görürsün.', + 'checks.fixHint.apple.review_details': 'App Review iletişim bilgisini ve notları ayardan gönderir (incelemeye gönderilmez). Önce deneme çalıştırmasını görürsün.', 'checks.accepted': 'Kabul edildi: {reason}', 'checks.sev.blocker': 'Engel', 'checks.sev.high': 'Yüksek', diff --git a/apps/desktop/src/lib/model.ts b/apps/desktop/src/lib/model.ts index 6eed9e5..82da3e1 100644 --- a/apps/desktop/src/lib/model.ts +++ b/apps/desktop/src/lib/model.ts @@ -39,13 +39,15 @@ export interface TestFlightStatus { groups: TfGroup[]; builds: TfBuild[]; missin export interface CatalogRow { id: string; store: 'apple' | 'play'; level: 'error' | 'warn' | 'ok'; code: string; msg: string } export interface ParityRow { id: string; region: string; currency: string; apple: number; play: number; diffPct: number } export interface CatalogDiff { errors: number; rows: CatalogRow[]; parity: ParityRow[] } -export interface Finding { store: 'apple' | 'play'; level: 'blocker' | 'warn' | 'ok'; code: string; msg: string; fix?: string } +/** A finding's machine-readable fix (cli/lib/fix-actions.ts): the write the Fix button runs (dry run, dialog, apply). */ +export interface FixAction { method: string; args: Record } +export interface Finding { store: 'apple' | 'play'; level: 'blocker' | 'warn' | 'ok'; code: string; msg: string; fix?: string; action?: FixAction } export interface ValidateResult { blockers: number; findings: Finding[] } /** `storebound scan --json` (offline rejection-risk scan of the project files), the parts the Checks tab shows. */ export type ScanSeverity = 'blocker' | 'high' | 'medium' | 'low'; export interface ScanFinding { rule: string; guideline: string; store: 'apple' | 'play'; severity: ScanSeverity; title: string; - evidence: { file: string; line: number; text?: string }[]; fix: string; skill: string; ref: string; accepted?: string; + evidence: { file: string; line: number; text?: string }[]; fix: string; skill: string; ref: string; accepted?: string; action?: FixAction; } export interface ScanResult { stacks: string[]; diff --git a/apps/desktop/src/screens/tabs/Checks.tsx b/apps/desktop/src/screens/tabs/Checks.tsx index 32dc2a4..2dc5b01 100644 --- a/apps/desktop/src/screens/tabs/Checks.tsx +++ b/apps/desktop/src/screens/tabs/Checks.tsx @@ -3,8 +3,9 @@ import type { Project } from '../../api'; import { t } from '../../i18n'; import { ago } from '../../lib/format'; -import type { Finding, ScanResult, ScanSeverity, ValidateResult } from '../../lib/model'; -import { useData, useRead } from '../../store'; +import type { Finding, FixAction, ScanResult, ScanSeverity, ValidateResult } from '../../lib/model'; +import { useData, useRead, useWrite } from '../../store'; +import { WriteButton } from '../../ui/WriteButton'; import { Icon } from '../../ui/icons'; import { Card, Chip, Empty, ErrorNotice, ReadControl, Skeleton } from '../../ui/kit'; import { exportName, validateMd } from '../../lib/exports'; @@ -12,11 +13,25 @@ import { useExports } from '../../ui/Exports'; const LEVEL: Record = { blocker: ['bad', 'alertCircle'], warn: ['warn', 'alert'], ok: ['ok', 'check'] }; +/** Fix methods the window runs from a finding (cli/lib/fix-actions.ts FIX_METHODS): local config writes and store + * text pushes, each a dry run, the native dialog showing it, then the real run. Anything else gets no button. */ +export const FIX_METHODS = new Set(['scan.write', 'play.listing', 'apple.listing', 'apple.review_details']); +export const fixable = (a: FixAction | undefined): a is FixAction => !!a && FIX_METHODS.has(a.method) && !!a.args && typeof a.args === 'object'; + +/** The Fix button of one finding: its action as a confirm-gated write. */ +function FixButton({ action, what, w }: { action: FixAction; what: string; w: ReturnType }) { + return ( + [0])} + w={{ method: action.method, args: action.args, title: t('checks.fixing', { what }), done: t('checks.fixed') }}>{t('checks.fix')} + ); +} + const SEV: Record = { blocker: ['bad', 'alertCircle'], high: ['bad', 'alert'], medium: ['warn', 'alert'], low: ['info', 'info'] }; /** The `storebound scan` card: rejection risks with guideline, evidence file:line, fix and skill. */ function ScanCard({ project }: { project: Project }) { const s = useRead(project.id, 'scan', { network: false, title: t('checks.scan') }); + const w = useWrite(project.id); const findings = Array.isArray(s.data?.findings) ? s.data!.findings : []; const sdks = Array.isArray(s.data?.sdks) ? s.data!.sdks : []; const asks = (Array.isArray(s.data?.declarations?.items) ? s.data!.declarations.items : []).filter(x => x.status === 'ask').length; @@ -39,6 +54,7 @@ function ScanCard({ project }: { project: Project }) { {f.accepted ? {t('checks.accepted', { reason: f.accepted })} : {f.fix} {f.skill}} + {!f.accepted && fixable(f.action) && } {t(f.accepted ? 'checks.sev.accepted' : `checks.sev.${f.severity}` as Parameters[0])} {f.store === 'apple' ? 'App Store' : 'Google Play'} @@ -60,6 +76,7 @@ export function ChecksTab({ project }: { project: Project }) { const d = useData(); const e = d.statusOf(project); const v = useRead(project.id, 'validate', { network: true, title: t('checks.validate') }); + const w = useWrite(project.id); const name = e?.summary?.name ?? project.name; useExports('checks', [{ id: 'validate-md', label: t('export.validate'), icon: 'shield', kind: 'md', file: exportName(name, 'store readiness'), disabled: v.data ? false : t('export.readFirst'), build: () => validateMd(name, v.data!, v.at) }]); const problems = [...(e?.errors ?? []), ...(e?.check?.problems ?? [])]; @@ -95,6 +112,7 @@ export function ChecksTab({ project }: { project: Project }) { {f.msg} {f.fix && {f.fix}} + {f.level !== 'ok' && fixable(f.action) && } {f.store === 'apple' ? 'App Store' : 'Google Play'} ); diff --git a/cli/data/flags.json b/cli/data/flags.json index 1b7939b..36cb38f 100644 --- a/cli/data/flags.json +++ b/cli/data/flags.json @@ -730,7 +730,8 @@ "store", "threshold", "unity", - "write" + "write", + "write-only" ], "serve": [ "stdio" @@ -1087,6 +1088,7 @@ "with-version", "workspace", "write", + "write-only", "xdg-mime" ] }, @@ -1400,6 +1402,7 @@ "scan": [ "storebound scan [--dir ] [--config ] [--store apple|play] [--json] [--no-declarations]", "storebound scan --write also write the detected dataSafety / appPrivacy answers into the config", + "storebound scan --write-only [--dry] only that write (with --dry: the config diff), no report (desktop Fix button)", "storebound scan --originality [--against ] [--threshold 0.7] also the 4.3 / Play repetitive content" ], "shots capture": [ diff --git a/cli/lib/fix-actions.ts b/cli/lib/fix-actions.ts new file mode 100644 index 0000000..3a29007 --- /dev/null +++ b/cli/lib/fix-actions.ts @@ -0,0 +1,23 @@ +// Machine-readable fixes of scan and validate findings: the desktop's "Fix" button runs `action.method` with +// `action.args` as a write (dry run, the native dialog showing it, then the real run with a host-minted token), so the +// window never parses a finding's text. Only fixes an existing command fully determines get an action; the rest has +// none. FIX_METHODS is the whole list: local config writes and store text pushes, never a submit, release or publish +// (tests/fix-actions.test.ts). + +export interface FixAction { method: FixMethod; args: Record } + +export const FIX_METHODS = ['scan.write', 'play.listing', 'apple.listing', 'apple.review_details'] as const; +export type FixMethod = typeof FIX_METHODS[number]; + +/** Scan rules `storebound scan --write` fixes: the detected Data safety / App Privacy answers go into the config. */ +export const SCAN_WRITE_RULES = new Set(['privacy-labels']); + +/** The fix of one scan finding: `scan --write` while it would change the config (detected answers only). */ +export function scanFixAction(rule: string, o: { configPath: string | null; declarationsChanged: boolean }): FixAction | undefined { + return SCAN_WRITE_RULES.has(rule) && o.configPath && o.declarationsChanged ? { method: 'scan.write', args: {} } : undefined; +} + +/** Validate findings fixed by pushing the listing files / config to the store (text only, never submitted). */ +export const fixPlayListing = (): FixAction => ({ method: 'play.listing', args: {} }); +export const fixAppleListing = (version: string): FixAction => ({ method: 'apple.listing', args: { version } }); +export const fixReviewDetails = (): FixAction => ({ method: 'apple.review_details', args: {} }); diff --git a/cli/lib/scan/index.ts b/cli/lib/scan/index.ts index f660501..28f8b4e 100644 --- a/cli/lib/scan/index.ts +++ b/cli/lib/scan/index.ts @@ -5,6 +5,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { join, relative, resolve } from 'node:path'; import { CONFIG_NAMES, loadConfig, type ShipConfig } from '../config'; import { reviewDemoAccount } from '../credentials'; +import { scanFixAction } from '../fix-actions'; import { proposeDeclarations, type Proposal } from './declare'; import { detectProject, ECO_LABEL, type ProjectFacts, type Stack, where } from './project'; import type { OriginalityReport } from '../originality'; @@ -62,6 +63,10 @@ export async function runScan(dir: string, o: { config?: string } = {}): Promise return { type: d.type, missing: [...new Set(missing)].join(', '), evidence: d.evidence }; }).filter(d => d.missing) : []; const findings = scanRules({ facts, config, configFile, listings, demoAccount: !!reviewDemoAccount(dir), labelGaps: declarations.labelGaps, dataMerges }); + for (const f of findings) { + const action = f.accepted ? undefined : scanFixAction(f.rule, { configPath, declarationsChanged: declarations.changed }); + if (action) f.action = action; + } const counts = { blocker: 0, high: 0, medium: 0, low: 0, accepted: 0 }; for (const f of findings) { if (f.accepted) counts.accepted++; else counts[f.severity]++; } const notes: string[] = []; diff --git a/cli/lib/scan/rules.ts b/cli/lib/scan/rules.ts index dce9ec0..ea7b3e1 100644 --- a/cli/lib/scan/rules.ts +++ b/cli/lib/scan/rules.ts @@ -33,6 +33,7 @@ // error of Play Console (no policy page; developer.android.com/build/configure-app-module on choosing the id). // The other Google Play rules (sensitive permissions, prominent disclosure, Families, advertising ID, 16 KB pages, // testing for personal accounts, Play metadata, ads, payments, impersonation) are in play-rules.ts with their sources. +import type { FixAction } from '../fix-actions'; import { templateBlockers } from '../template-answers'; import { BETA_WORDS, competitorRe, OTHER_PLATFORMS, PLACEHOLDER_MARKERS, PLACEHOLDER_TEXT, PRICE_CLAIMS } from '../listing-rules'; import type { ShipConfig } from '../config'; @@ -62,6 +63,8 @@ export interface ScanFinding { ref: string; /** Owner accepted it (`scan.ignore` in the config) with this reason. */ accepted?: string; + /** The desktop's Fix button: a write fully determined by an existing command (lib/fix-actions.ts). */ + action?: FixAction; } export interface ScanInput { diff --git a/cli/lib/validate-rules.ts b/cli/lib/validate-rules.ts index 1af5958..621c6fb 100644 --- a/cli/lib/validate-rules.ts +++ b/cli/lib/validate-rules.ts @@ -1,6 +1,8 @@ // 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 { type FixAction, fixAppleListing, fixPlayListing, fixReviewDetails } from './fix-actions'; + export type Level = 'blocker' | 'warn' | 'ok'; /** Screenshot sets App Store Connect takes instead of a required one (App Store Connect Help, "Screenshot @@ -13,7 +15,9 @@ export const ACCEPTED_INSTEAD: Record = { APP_IPAD_PRO_3GEN_129: ['APP_IPAD_PRO_129'], APP_IPAD_PRO_129: ['APP_IPAD_PRO_3GEN_129'], }; -export interface Finding { store: 'apple' | 'play'; level: Level; code: string; msg: string; fix?: string } +export interface Finding { store: 'apple' | 'play'; level: Level; code: string; msg: string; fix?: string; + /** The desktop's Fix button: a write fully determined by an existing command (lib/fix-actions.ts). */ + action?: FixAction } export interface UrlCheck { url: string; status: number | null; error?: string; body?: string } @@ -168,12 +172,14 @@ export function copyrightRules(s: Pick f.push({ store: 'apple', level, code, msg, fix }); + const add = (level: Level, code: string, msg: string, fix?: string, action?: FixAction) => f.push({ store: 'apple', level, code, msg, fix, ...(action ? { action } : {}) }); + const listing = s.version && EDITABLE.includes(s.version.state) ? fixAppleListing(s.version.versionString) : undefined; if (!s.version) add('blocker', 'version', 'no App Store version being prepared', 'storebound apple listing --version x.y.z'); else if (EDITABLE.includes(s.version.state)) add('ok', 'version', `version ${s.version.versionString}: ${s.version.state}`); @@ -193,7 +199,7 @@ export function appleRules(s: AppleSnapshot): Finding[] { } const have = new Map(s.locales.map(l => [l.locale, l] as const)); - for (const loc of s.expectedLocales) if (!have.has(loc)) add('blocker', 'locale', `${loc}: no localization on the version`, 'storebound apple listing'); + for (const loc of s.expectedLocales) if (!have.has(loc)) add('blocker', 'locale', `${loc}: no localization on the version`, 'storebound apple listing', listing); for (const l of s.locales) { const miss = (['description', 'keywords', 'subtitle'] as const).filter(k => !l[k]); if (miss.length) add('blocker', 'metadata', `${l.locale}: missing ${miss.join(', ')}`, 'store/appstore.md, then storebound apple listing'); @@ -207,10 +213,10 @@ export function appleRules(s: AppleSnapshot): Finding[] { if (s.locales.length && !f.some(x => x.code === 'metadata' || x.code === 'screenshots' || x.code === 'locale')) add('ok', 'metadata', `${s.locales.length} locales with text and screenshots`); const r = s.reviewDetail; - if (!r) add('blocker', 'review-details', 'no App Review information', 'storebound apple review-details'); + if (!r) add('blocker', 'review-details', 'no App Review information', 'storebound apple review-details', fixReviewDetails()); else { const miss = (['contactFirstName', 'contactLastName', 'contactPhone', 'contactEmail'] as const).filter(k => !r[k]); - if (miss.length) add('blocker', 'review-details', `review contact missing ${miss.join(', ')}`, 'storebound apple review-details'); + if (miss.length) add('blocker', 'review-details', `review contact missing ${miss.join(', ')}`, 'storebound apple review-details', fixReviewDetails()); else if (r.demoAccountRequired && !r.demoAccountName) add('blocker', 'review-details', 'demo account required but not given'); else add('ok', 'review-details', 'review contact set'); if (!r.notes) add('warn', 'review-notes', 'no review notes: say where IAPs, Restore Purchases, ads and the privacy policy are', 'apple.info.reviewNotes'); @@ -257,14 +263,14 @@ export const normAdsLine = (l: string) => l.split('#')[0]!.split(',').map(p => p export function playRules(s: PlaySnapshot): Finding[] { const f: Finding[] = []; - const add = (level: Level, code: string, msg: string, fix?: string) => f.push({ store: 'play', level, code, msg, fix }); + const add = (level: Level, code: string, msg: string, fix?: string, action?: FixAction) => f.push({ store: 'play', level, code, msg, fix, ...(action ? { action } : {}) }); const have = new Map(s.listings.map(l => [l.language, l] as const)); const fallback: Record = {}; - if (!have.has(s.defaultLanguage)) add('blocker', 'listing', `no listing for the default language ${s.defaultLanguage}`, 'storebound play listing'); - for (const lang of s.expectedLanguages) if (!have.has(lang)) add('blocker', 'listing', `${lang}: no listing`, 'storebound play listing'); + if (!have.has(s.defaultLanguage)) add('blocker', 'listing', `no listing for the default language ${s.defaultLanguage}`, 'storebound play listing', fixPlayListing()); + for (const lang of s.expectedLanguages) if (!have.has(lang)) add('blocker', 'listing', `${lang}: no listing`, 'storebound play listing', fixPlayListing()); for (const l of s.listings) { const miss = (['title', 'shortDescription', 'fullDescription'] as const).filter(k => !l[k]); - if (miss.length) add('blocker', 'listing', `${l.language}: missing ${miss.join(', ')}`, 'storebound play listing'); + if (miss.length) add('blocker', 'listing', `${l.language}: missing ${miss.join(', ')}`, 'storebound play listing', fixPlayListing()); for (const [type, n] of Object.entries(playRequiredImages(s.tablets))) { if ((l.images[type] ?? 0) >= n) continue; // Play shows the default language's graphics in a language that has none of its own (store-pitfalls 45): only diff --git a/cli/mcp-tools.ts b/cli/mcp-tools.ts index 77e57da..a7fc8f7 100644 --- a/cli/mcp-tools.ts +++ b/cli/mcp-tools.ts @@ -424,7 +424,7 @@ export const TOOLS: ToolDef[] = [ ]; /** CLI flags / commands that must never be reachable through MCP. Tests assert none of the tools can produce them. */ -export const FORBIDDEN_ARGS = ['--write', '--yes', '--manual-release', '--cancel-submission', '--production', 'rollout', 'resume', 'complete', +export const FORBIDDEN_ARGS = ['--write', '--write-only', '--yes', '--manual-release', '--cancel-submission', '--production', 'rollout', 'resume', 'complete', '--submit-review', '--remove', '--invite', 'testers', 'send', 'start', 'stop']; /** diff --git a/cli/scan.ts b/cli/scan.ts index 2549aaa..e7c0ef3 100644 --- a/cli/scan.ts +++ b/cli/scan.ts @@ -1,6 +1,7 @@ // Offline, read-only scan of the project itself for App Store and Google Play rejection risks, before you submit. // storebound scan [--dir ] [--config ] [--store apple|play] [--json] [--no-declarations] // storebound scan --write also write the detected dataSafety / appPrivacy answers into the config +// storebound scan --write-only [--dry] only that write (with --dry: the config diff), no report (desktop Fix button) // storebound scan --originality [--against ] [--threshold 0.7] also the 4.3 / Play repetitive content // check against your other apps (storebound originality; folders default to originality.against in the config) // Reads the project files (Xcode project, Info.plist, entitlements, privacy manifests, Podfile.lock, Package.resolved, @@ -16,7 +17,8 @@ import type { Args } from './lib/args'; import { CONFIG_NAMES } from './lib/config'; import { log, printJson, warn } from './lib/log'; import { upsertBlock } from './lib/scan/config-edit'; -import { addOriginality, formatScan, runScan } from './lib/scan'; +import { addOriginality, formatScan, runScan, type ScanReport } from './lib/scan'; +import { compactDiff } from './lib/diff'; import { compareAgainst } from './originality'; /** Project folder: --dir, else the folder of the nearest storebound.config.* above the working directory, else it. */ @@ -48,25 +50,43 @@ export async function run(args: Args): Promise { report.counts = { blocker: 0, high: 0, medium: 0, low: 0, accepted: 0 }; for (const f of report.findings) { if (f.accepted) report.counts.accepted++; else report.counts[f.severity]++; } } else if (store) throw new Error('--store apple | play'); - if (args.has('json')) printJson(report); - else console.log(formatScan(report, { declarations: !args.has('no-declarations') })); + // --write-only (the desktop's Fix button, scan.write): only the write and its diff, no report, no blocker exit + const writeOnly = args.has('write-only'); + if (!writeOnly) { + if (args.has('json')) printJson(report); + else console.log(formatScan(report, { declarations: !args.has('no-declarations') })); + } - if (args.has('write')) { + if (args.has('write') || writeOnly) { const d = report.declarations; - if (!configPath) warn('scan', '--write: no storebound.config.ts in this project (storebound init creates one)'); - else if (!d.changed) log('scan', `--write: nothing to change in ${report.config}`); - else if (args.has('dry')) log('scan', `--dry: would write ${[(d.diff.some(x => x.change !== 'kept' && x.change !== 'same') || d.accountCreationChange) && 'dataSafety', (d.appPrivacyChange || d.appPrivacyTypes) && 'appPrivacy'].filter(Boolean).join(' + ')} into ${report.config}`); + if (!configPath) { + if (writeOnly) throw new Error('no storebound.config.ts in this project (storebound init creates one)'); + warn('scan', '--write: no storebound.config.ts in this project (storebound init creates one)'); + } else if (!d.changed) log('scan', `--write: nothing to change in ${report.config}`); else { - let text = readFileSync(configPath, 'utf8'); - const ds = d.dataSafety!; - const missing = (['encryptedInTransit'] as const).filter(k => typeof ds[k] !== 'boolean'); - if (d.diff.some(x => x.change === 'add' || x.change === 'merge') || d.accountCreationChange) - text = upsertBlock(text, configPath, 'dataSafety', ds, missing.length ? `storebound scan: answer ${missing.join(' and ')} (ask the owner; store-declarations skill)` : undefined); - if (d.appPrivacyChange || d.appPrivacyTypes) text = upsertBlock(text, configPath, 'appPrivacy', d.appPrivacy, - d.appPrivacyTypes ? 'storebound scan: types proposed from the detected SDKs; answer linked / tracking per type (ask the owner; store-declarations skill)' : undefined); - writeFileSync(configPath, text); - log('scan', `wrote the detected answers into ${report.config}${missing.length ? `; still to answer: dataSafety.${missing.join(', dataSafety.')}` : ''}. Review the diff (git diff) before pushing it to a store.`); + const before = readFileSync(configPath, 'utf8'); + const { text, missing } = declarationsWrite(before, configPath, d); + const what = [(d.diff.some(x => x.change !== 'kept' && x.change !== 'same') || d.accountCreationChange) && 'dataSafety', (d.appPrivacyChange || d.appPrivacyTypes) && 'appPrivacy'].filter(Boolean).join(' + '); + if (args.has('dry')) { + log('scan', `--dry: would write ${what} into ${report.config}`); + console.log([`--- ${report.config}`, ...compactDiff(before, text)].join('\n')); + } else { + writeFileSync(configPath, text); + log('scan', `wrote the detected answers into ${report.config}${missing.length ? `; still to answer: dataSafety.${missing.join(', dataSafety.')}` : ''}. Review the diff (git diff) before pushing it to a store.`); + } } } - if (report.counts.blocker) process.exitCode = 1; + if (report.counts.blocker && !writeOnly) process.exitCode = 1; +} + +/** The config text with the detected answers written in (dataSafety / appPrivacy blocks; comments and order kept). */ +export function declarationsWrite(before: string, configPath: string, d: ScanReport['declarations']): { text: string; missing: string[] } { + let text = before; + const ds = d.dataSafety!; + const missing = (['encryptedInTransit'] as const).filter(k => typeof ds[k] !== 'boolean'); + if (d.diff.some(x => x.change === 'add' || x.change === 'merge') || d.accountCreationChange) + text = upsertBlock(text, configPath, 'dataSafety', ds, missing.length ? `storebound scan: answer ${missing.join(' and ')} (ask the owner; store-declarations skill)` : undefined); + if (d.appPrivacyChange || d.appPrivacyTypes) text = upsertBlock(text, configPath, 'appPrivacy', d.appPrivacy, + d.appPrivacyTypes ? 'storebound scan: types proposed from the detected SDKs; answer linked / tracking per type (ask the owner; store-declarations skill)' : undefined); + return { text, missing }; } diff --git a/cli/serve.ts b/cli/serve.ts index 35b92da..c750e89 100644 --- a/cli/serve.ts +++ b/cli/serve.ts @@ -65,6 +65,11 @@ export const JSON_WRITES = new Set(['console_fill', 'owner_apple_submit', 'owner /** Write tools whose command may ask its own question (confirm()): a verified host token answers it, as for owner actions. */ const TOKEN_ANSWERS = new Set(['console_fill']); +/** The one desktop exception to FORBIDDEN_ARGS: `scan --write-only` (scan.write, the Fix button of a privacy-labels + * finding) writes the detected declarations into the config, only after the native dialog showed its diff and the + * host minted a token. The MCP server (AI tools) never gets it (tests/fix-actions.test.ts). */ +export const DESKTOP_LOCAL_WRITES: Record = { scan_write: ['--write-only'] }; + /** * Methods only the desktop profile has (not MCP tools): offline reads of the project's own files, local writes the * owner confirms in a native dialog, and `console.fill`. That one is a write: its dry run returns the answers it would @@ -132,6 +137,8 @@ export const DESKTOP_ONLY: ToolDef[] = [ props: { from: { type: 'string', pattern: ART_FROM_RE.source, description: 'The picked file (absolute path)' }, to: { type: 'string', pattern: ART_TO_RE.source, description: 'The image to replace, relative to the app folder' } }, required: ['from', 'to'], args: i => ['--from', String(i.from), '--to', String(i.to)] }, + { name: 'scan_write', title: 'Write the detected declarations', command: ['scan', DESKTOP_LOCAL_WRITES.scan_write![0]!], write: true, network: false, idempotent: true, + description: 'The Fix of a scan finding (privacy-labels): writes the Data safety / App Privacy answers the scan detected from the SDKs into the config (dataSafety / appPrivacy blocks; the rest of the file kept). The dry run shows the config diff; "ask the owner" answers stay questions. Nothing goes to a store and nothing is committed.' }, { name: 'prices_current', title: 'Current prices', command: ['prices', 'export'], write: false, network: true, description: 'Read-only: the current price of every product on both stores per region (Apple manual prices, with with_automatic also the equalized ones). Writes nothing.', props: { store: { type: 'string', enum: ['apple', 'play'] }, with_automatic: { type: 'boolean', default: false } }, diff --git a/cli/validate.ts b/cli/validate.ts index b5103ec..aadee92 100644 --- a/cli/validate.ts +++ b/cli/validate.ts @@ -175,7 +175,7 @@ export function scanAsFindings(f: ScanFinding[], stores: { apple: boolean; play: return f.filter(x => !x.accepted && x.severity !== 'low' && stores[x.store]).map(x => ({ store: x.store, level: x.severity === 'blocker' ? 'blocker' as const : 'warn' as const, code: `scan-${x.rule}`, msg: `${x.guideline}: ${x.title}${x.evidence[0] ? ` (${x.evidence[0].file}:${x.evidence[0].line})` : ''}`, - fix: `${x.fix} · ${x.skill} skill`, + fix: `${x.fix} · ${x.skill} skill`, ...(x.action ? { action: x.action } : {}), })); } diff --git a/docs/desktop.md b/docs/desktop.md index be2c4ed..729969d 100644 --- a/docs/desktop.md +++ b/docs/desktop.md @@ -65,7 +65,13 @@ Types of the protocol: `cli/lib/desktop-protocol.ts`; pure portfolio logic: `cli worst devices, top issues, App Store metrics and signatures), Reviews (`reviews.list` + `reviews.drafts`: star distribution, filters, the local draft with its send checks, Approve via `reviews.approve`), TestFlight (`apple.testflight_status`), Checks (offline `scan`: rejection risks with guideline, evidence file:line, fix - and skill, owner-accepted ones marked; offline `check`; `validate` with fix hints). + and skill, owner-accepted ones marked; offline `check`; `validate` with fix hints). A finding whose fix an + existing command fully determines carries `action: { method, args }` (cli/lib/fix-actions.ts) and gets a **Fix** + button: a confirm-gated write (dry run, the native dialog showing it, then the real run). Mapped today: + privacy-labels -> `scan.write` (`scan --write-only`: the detected Data safety / App Privacy answers into the + config, the dialog shows the config diff), Play listing gaps -> `play.listing`, App Store missing localization and + copyright -> `apple.listing` (editable version only), App Review contact -> `apple.review_details`. No fix reaches + a submit, release or publish (tests/fix-actions.test.ts). - **First-run guide**: what the app does, add or start an app folder (`project.init` = `storebound init --profile`, confirmed in a native dialog), store keys into the shared credential profiles and which app uses which, local tools, first read. diff --git a/tests/fix-actions.test.ts b/tests/fix-actions.test.ts new file mode 100644 index 0000000..8fe0734 --- /dev/null +++ b/tests/fix-actions.test.ts @@ -0,0 +1,100 @@ +// The desktop's Fix button (lib/fix-actions.ts): which findings get a machine-readable fix, the scan.write round trip +// (dry run with the config diff -> confirm token -> real write) on a fixture, and that no fix reaches a submit, +// release or publish. +import { describe, expect, test } from 'bun:test'; +import { cpSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { mint } from '../cli/lib/confirm-token'; +import { FIX_METHODS, scanFixAction } from '../cli/lib/fix-actions'; +import { runScan } from '../cli/lib/scan'; +import { appleRules, copyrightRules, playRules } from '../cli/lib/validate-rules'; +import { FORBIDDEN_ARGS, TOOLS, toArgv } from '../cli/mcp-tools'; +import { describeMethods, DESKTOP_LOCAL_WRITES, FORBIDDEN_METHOD_WORDS, Session } from '../cli/serve'; +import { scanAsFindings } from '../cli/validate'; + +const FX = join(import.meta.dir, 'fixtures', 'scan'); +const SECRET_HEX = Array.from({ length: 32 }, (_, i) => i.toString(16).padStart(2, '0')).join(''); +const SECRET = Buffer.from(SECRET_HEX, 'hex'); +const ENV = { REVIEW_DEMO_USER: '', REVIEW_DEMO_PASSWORD: '' }; + +describe('finding -> fix action', () => { + test('scan: privacy-labels gets scan.write while the write would change the config; nothing else does', () => { + expect(scanFixAction('privacy-labels', { configPath: '/x/storebound.config.ts', declarationsChanged: true })).toEqual({ method: 'scan.write', args: {} }); + expect(scanFixAction('privacy-labels', { configPath: '/x/storebound.config.ts', declarationsChanged: false })).toBeUndefined(); + expect(scanFixAction('privacy-labels', { configPath: null, declarationsChanged: true })).toBeUndefined(); + for (const r of ['ats', 'play-fgs', 'export-compliance', 'demo-account', 'att', 'template-answers']) + expect(scanFixAction(r, { configPath: '/x/c.ts', declarationsChanged: true })).toBeUndefined(); + }); + test('validate: listing text and review contact map to the push commands; screenshots, builds and compliance do not', () => { + const version = { versionString: '1.2.0', state: 'PREPARE_FOR_SUBMISSION', copyright: null as string | null | undefined }; + const base = { version, build: null, locales: [], expectedLocales: ['en-US'], requiredDisplayTypes: [], reviewDetail: null, configCopyright: '2026 Example', ageRating: null, priceSchedule: true, availability: true, iaps: [], expectedProducts: [], privacyUrl: { url: 'https://example.com/privacy', status: 200 } }; + const apple = appleRules(base as unknown as Parameters[0]); + const by = (code: string) => apple.filter(f => f.code === code); + expect(by('locale')[0]!.action).toEqual({ method: 'apple.listing', args: { version: '1.2.0' } }); + expect(by('review-details')[0]!.action).toEqual({ method: 'apple.review_details', args: {} }); + expect(by('build')[0]!.action).toBeUndefined(); + expect(copyrightRules({ version, configCopyright: '2026 Example' })[0]!.action).toEqual({ method: 'apple.listing', args: { version: '1.2.0' } }); + expect(copyrightRules({ version, configCopyright: undefined })[0]!.action).toBeUndefined(); + // a version in review is not editable: no push + const inReview = appleRules({ ...base, version: { ...version, state: 'WAITING_FOR_REVIEW' } } as unknown as Parameters[0]); + expect(inReview.find(f => f.code === 'locale')!.action).toBeUndefined(); + const play = playRules({ defaultLanguage: 'en-US', expectedLanguages: ['en-US'], listings: [], privacyUrl: { url: 'https://example.com/privacy', status: 200 }, website: null, appAds: null, appAdsLines: [], adsLikely: false, tracks: [], products: [], expectedProducts: [] } as unknown as Parameters[0]); + expect(play.filter(f => f.code === 'listing').map(f => f.action)).toEqual([{ method: 'play.listing', args: {} }, { method: 'play.listing', args: {} }]); + expect(play.filter(f => f.code !== 'listing').some(f => f.action)).toBe(false); + }); + test('no fix reaches a submit, release, publish or an owner action; every fix method is a desktop write', () => { + const ms = new Map(describeMethods().map(m => [m.method, m.kind])); + for (const m of FIX_METHODS) { + expect(m).not.toMatch(FORBIDDEN_METHOD_WORDS); + expect(m.startsWith('owner.')).toBe(false); + expect(ms.get(m)).toBe('write'); + } + // scan --write-only is desktop-only: the MCP server (AI tools) never produces it + expect(FORBIDDEN_ARGS).toContain('--write-only'); + expect(DESKTOP_LOCAL_WRITES.scan_write).toEqual(['--write-only']); + for (const t of TOOLS) for (const s of [{}, { dry_run: false }]) expect(toArgv(t, s).argv).not.toContain('--write-only'); + }); +}); + +describe('scan.write round trip', () => { + test('dry run shows the config diff and writes nothing; the confirmed run writes; the finding is gone', async () => { + const dir = mkdtempSync(join(tmpdir(), 'sb-fix-')); + const prev = { ...process.env }; + Object.assign(process.env, ENV); + try { + cpSync(join(FX, 'android'), dir, { recursive: true }); + const cfg = join(dir, 'storebound.config.ts'); + const before = readFileSync(cfg, 'utf8'); + const scan = await runScan(dir); + const f = scan.report.findings.find(x => x.rule === 'privacy-labels' && !x.accepted)!; + expect(f.action).toEqual({ method: 'scan.write', args: {} }); + // validate carries the scan's action along + expect(scanAsFindings([f], { apple: true, play: true })[0]!.action).toEqual(f.action); + + const s = new Session(() => {}); + let id = 0; + const call = async (method: string, params: unknown = {}) => (await s.handle({ jsonrpc: '2.0', id: ++id, method, params }))! as Record; + await call('session.hello', { secret: SECRET_HEX, client: { name: 'test', version: '1' } }); + const p = { project: dir, args: f.action!.args }; + const dry = (await call(f.action!.method, p)).result as { ok: boolean; dryRun: boolean; output: string }; + expect(dry).toMatchObject({ ok: true, dryRun: true }); + expect(dry.output).toContain('+'); + expect(dry.output).toContain('PSL_DEVICE_ID'); + expect(readFileSync(cfg, 'utf8')).toBe(before); + // without the host's token the real run is refused + expect(((await call(f.action!.method, { ...p, dryRun: false })).error as { code: number }).code).toBeDefined(); + expect(readFileSync(cfg, 'utf8')).toBe(before); + const real = (await call(f.action!.method, { ...p, dryRun: false, confirmToken: mint(SECRET, f.action!.method, p) })).result as { ok: boolean }; + expect(real.ok).toBe(true); + expect(readFileSync(cfg, 'utf8')).toContain('PSL_DEVICE_ID: { shared: true'); + // a fresh process (this one has the old config module cached) + const again = JSON.parse(Bun.spawnSync(['bun', join(import.meta.dir, '..', 'cli', 'index.ts'), 'scan', '--dir', dir, '--json'], { env: { ...process.env, ...ENV } }).stdout.toString()) as { declarations: { changed: boolean }; findings: { rule: string }[] }; + expect(again.declarations.changed).toBe(false); + expect(again.findings.some(x => x.rule === 'privacy-labels')).toBe(false); + } finally { + for (const k of Object.keys(ENV)) if (prev[k] === undefined) delete process.env[k]; else process.env[k] = prev[k]; + rmSync(dir, { recursive: true, force: true }); + } + }, 60_000); +}); diff --git a/tests/serve.test.ts b/tests/serve.test.ts index 14e6d29..7b90829 100644 --- a/tests/serve.test.ts +++ b/tests/serve.test.ts @@ -6,7 +6,7 @@ import { canonical, mac, MAX_TTL_MS, mint, parseSecret, TokenGuard } from '../cl import { ERR } from '../cli/lib/desktop-protocol'; import { childEnv, pickCallEnv, type RunOptions, type Running } from '../cli/lib/runner'; import { FORBIDDEN_ARGS, TOOLS, toArgv, validateInput } from '../cli/mcp-tools'; -import { describeMethods, DESKTOP_ONLY, DESKTOP_TOOLS, FORBIDDEN_METHOD_WORDS, JSON_TOOLS, methodName, OWNER_SUBMIT, OWNER_TOOLS, Session } from '../cli/serve'; +import { describeMethods, DESKTOP_LOCAL_WRITES, DESKTOP_ONLY, DESKTOP_TOOLS, FORBIDDEN_METHOD_WORDS, JSON_TOOLS, methodName, OWNER_SUBMIT, OWNER_TOOLS, Session } from '../cli/serve'; import { buildOne, hostTarget, leaks } from '../tools/build-sidecars'; type J = Record; @@ -113,7 +113,8 @@ describe('serve --stdio: methods', () => { } expect(TOOLS.some(t => OWNER_SUBMIT.has(t.name) || t.name.startsWith('owner_'))).toBe(false); // every desktop tool still goes through the MCP rules: no forbidden flag can come out of any of them - for (const t of DESKTOP_TOOLS) expect(t.command.some(c => FORBIDDEN_ARGS.includes(c))).toBe(false); + for (const t of DESKTOP_TOOLS) expect(t.command.some(c => FORBIDDEN_ARGS.includes(c) && !DESKTOP_LOCAL_WRITES[t.name]?.includes(c))).toBe(false); + expect(Object.keys(DESKTOP_LOCAL_WRITES)).toEqual(['scan_write']); expect(new Set(names).size).toBe(names.length); }); test('typed descriptors for every tool; json flags only on read tools', () => {