Compare commits

...
Author SHA1 Message Date
evanbaconandClaude Opus 4.6 59ac890d2a fix(metadata): improve review attachment pull behavior and document ASC limitation
The App Store Connect API does not expose a download URL for review
attachments (unlike AppScreenshot which has imageAsset.templateUrl).
This is a confirmed API limitation (expo/third-party#146), not a gap
in our implementation.

Changes:
- Remove directory/placeholder file creation during pull — only record
  the path in config for round-trip support
- Replace terse warning with informational message explaining WHY
  download is not possible and what the user should do
- Add code comment referencing the investigation (expo/third-party#146)
- Add test verifying no directory is created on disk during pull

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-04-10 13:36:07 -07:00
evanbaconandClaude Opus 4.6 1f4260cc47 feat(metadata): support App Store review attachment
Adds an `attachment` field to the `review` block of the Apple metadata
config so users can ship a single file (e.g. demo instructions PDF) to
the App Store review team alongside contact + demo account info. Push
uploads the file via `AppStoreReviewDetail.uploadAttachmentAsync` and
skips re-uploading when the local MD5 matches the existing attachment.
Pull records the existing attachment's filename under
`store/apple/review-attachment/` (App Store Connect does not currently
expose a download URL for review attachments, so we cannot fetch the
bytes back).

Removes the `// TODO: add attachment` comments in
`apple/config/reader.ts` and `apple/config/writer.ts`.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-04-09 16:41:08 -07:00
8 changed files with 461 additions and 6 deletions

No files matched your search

+4
View File
@@ -685,6 +685,10 @@
"markdownDescription": "Additional information about your app that can help during the review process.\n\n**Do not include demo account details.**",
"minLength": 2,
"maxLength": 4000
},
"attachment": {
"type": "string",
"description": "Relative path (from project root) to a file shared with the App Store review team. The file is uploaded as an App Store review attachment and only re-uploaded when its content changes."
}
}
},
@@ -216,3 +216,35 @@ describe('getAppClip', () => {
expect(reader.getAppClipLocalizedInfo('de-DE')).toBeNull();
});
});
describe('getReviewAttachment', () => {
it('returns null when review is not configured', () => {
const reader = new AppleConfigReader({});
expect(reader.getReviewAttachment()).toBeNull();
});
it('returns null when review has no attachment', () => {
const reader = new AppleConfigReader({
review: {
firstName: 'Evan',
lastName: 'Bacon',
email: '[email protected]',
phone: '+1 555 555 5555',
},
});
expect(reader.getReviewAttachment()).toBeNull();
});
it('returns the configured attachment path', () => {
const reader = new AppleConfigReader({
review: {
firstName: 'Evan',
lastName: 'Bacon',
email: '[email protected]',
phone: '+1 555 555 5555',
attachment: 'store/apple/review-attachment/demo.pdf',
},
});
expect(reader.getReviewAttachment()).toBe('store/apple/review-attachment/demo.pdf');
});
});
@@ -345,6 +345,30 @@ describe('setReviewDetails', () => {
demoRequired: undefined,
});
});
it('preserves an existing review attachment when re-setting review details', () => {
const writer = new AppleConfigWriter();
writer.setReviewDetails(nameOnlyReviewDetails);
writer.setReviewAttachment('store/apple/review-attachment/demo.pdf');
writer.setReviewDetails(nameAndDemoReviewDetails);
expect(writer.schema.review?.attachment).toBe('store/apple/review-attachment/demo.pdf');
});
});
describe('setReviewAttachment', () => {
it('initializes a review block with the attachment path', () => {
const writer = new AppleConfigWriter();
writer.setReviewAttachment('store/apple/review-attachment/demo.pdf');
expect(writer.schema.review?.attachment).toBe('store/apple/review-attachment/demo.pdf');
});
it('clears the attachment when null is passed', () => {
const writer = new AppleConfigWriter();
writer.setReviewDetails(nameOnlyReviewDetails);
writer.setReviewAttachment('store/apple/review-attachment/demo.pdf');
writer.setReviewAttachment(null);
expect(writer.schema.review?.attachment).toBeUndefined();
});
});
describe('setAppClipDefaultExperience', () => {
@@ -224,10 +224,14 @@ export class AppleConfigReader {
demoAccountPassword: review.demoPassword,
demoAccountRequired: review.demoRequired,
notes: review.notes,
// TODO: add attachment
};
}
/** Get the configured review attachment path (relative to the project root), or null if not set. */
public getReviewAttachment(): string | null {
return this.schema.review?.attachment ?? null;
}
/** Get screenshots configuration for a specific locale */
public getScreenshots(locale: string): AppleScreenshots | null {
return this.schema.info?.[locale]?.screenshots ?? null;
@@ -190,6 +190,7 @@ export class AppleConfigWriter {
}
public setReviewDetails(attributes: AttributesOf<AppStoreReviewDetail>): void {
const existingAttachment = this.schema.review?.attachment;
this.schema.review = {
firstName: attributes.contactFirstName ?? '',
lastName: attributes.contactLastName ?? '',
@@ -199,10 +200,25 @@ export class AppleConfigWriter {
demoPassword: optional(attributes.demoAccountPassword),
demoRequired: optional(attributes.demoAccountRequired),
notes: optional(attributes.notes),
// TODO: add attachment
attachment: existingAttachment,
};
}
/** Set the review attachment path (relative to project root) on the review block. */
public setReviewAttachment(attachmentPath: string | null): void {
this.schema.review = this.schema.review ?? {
firstName: '',
lastName: '',
email: '',
phone: '',
};
if (attachmentPath) {
this.schema.review.attachment = attachmentPath;
} else {
delete this.schema.review.attachment;
}
}
/** Set screenshots for a specific locale */
public setScreenshots(locale: string, screenshots: AppleScreenshots): void {
this.schema.info = this.schema.info ?? {};
@@ -1,4 +1,6 @@
import { AppStoreReviewAttachment, AppStoreReviewDetail, AppStoreVersion } from '@expo/apple-utils';
import crypto from 'crypto';
import fs from 'fs';
import nock from 'nock';
import { requestContext } from './fixtures/requestContext';
@@ -9,6 +11,10 @@ import { AppReviewDetailTask } from '../app-review-detail';
jest.mock('../../../../ora');
jest.mock('../../config/writer');
function md5(value: Buffer | string): string {
return crypto.createHash('md5').update(value).digest('hex');
}
describe(AppReviewDetailTask, () => {
describe('prepareAsync', () => {
it('loads review details from app version', async () => {
@@ -51,6 +57,78 @@ describe(AppReviewDetailTask, () => {
expect(writer.setReviewDetails).not.toBeCalled();
});
it('records the review attachment placeholder path when one exists', async () => {
const writer = jest.mocked(new AppleConfigWriter());
const attachment = new AppStoreReviewAttachment(requestContext, 'ATTACH_1', {
fileName: 'demo-instructions.pdf',
fileSize: 1024,
sourceFileChecksum: 'abc123',
uploaded: true,
} as any);
const reviewDetail = new AppStoreReviewDetail(requestContext, 'stub-id', {
appStoreReviewAttachments: [attachment],
} as any);
const existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(true);
await new AppReviewDetailTask().downloadAsync({
config: writer,
context: { reviewDetail, projectDir: '/test/project' } as any,
});
expect(writer.setReviewDetails).toBeCalledWith(reviewDetail.attributes);
expect(writer.setReviewAttachment).toBeCalledWith(
'store/apple/review-attachment/demo-instructions.pdf'
);
existsSyncSpy.mockRestore();
});
it('logs an info message when attachment exists remotely but not on disk', async () => {
const writer = jest.mocked(new AppleConfigWriter());
const attachment = new AppStoreReviewAttachment(requestContext, 'ATTACH_1', {
fileName: 'demo-instructions.pdf',
fileSize: 1024,
sourceFileChecksum: 'abc123',
uploaded: true,
} as any);
const reviewDetail = new AppStoreReviewDetail(requestContext, 'stub-id', {
appStoreReviewAttachments: [attachment],
} as any);
const existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(false);
const mkdirSpy = jest.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined);
await new AppReviewDetailTask().downloadAsync({
config: writer,
context: { reviewDetail, projectDir: '/test/project' } as any,
});
// Should still record the path in config for round-trip support
expect(writer.setReviewAttachment).toBeCalledWith(
'store/apple/review-attachment/demo-instructions.pdf'
);
// Should NOT create directories on disk (no placeholder file)
expect(mkdirSpy).not.toBeCalled();
existsSyncSpy.mockRestore();
mkdirSpy.mockRestore();
});
it('does not call setReviewAttachment when there is no attachment', async () => {
const writer = jest.mocked(new AppleConfigWriter());
const reviewDetail = new AppStoreReviewDetail(requestContext, 'stub-id', {
appStoreReviewAttachments: [],
} as any);
await new AppReviewDetailTask().downloadAsync({
config: writer,
context: { reviewDetail, projectDir: '/test/project' } as any,
});
expect(writer.setReviewAttachment).not.toBeCalled();
});
});
describe('uploadAsync', () => {
@@ -141,5 +219,182 @@ describe(AppReviewDetailTask, () => {
expect(createReviewDetailScope.isDone()).toBeTruthy();
expect(updateReviewDetailScope.isDone()).toBeTruthy();
});
describe('attachment sync', () => {
const fileBytes = Buffer.from('file-bytes');
const fileChecksum = md5(fileBytes);
let existsSyncSpy: jest.SpyInstance;
let readFileSyncSpy: jest.SpyInstance;
beforeEach(() => {
existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(true);
readFileSyncSpy = jest.spyOn(fs, 'readFileSync').mockReturnValue(fileBytes as any);
});
afterEach(() => {
existsSyncSpy.mockRestore();
readFileSyncSpy.mockRestore();
jest.restoreAllMocks();
nock.cleanAll();
});
it('uploads a new attachment when none exists', async () => {
nock('https://api.appstoreconnect.apple.com')
.patch(`/v1/${AppStoreReviewDetail.type}/APP_STORE_REVIEW_DETAILS_1`)
.reply(200, require('./fixtures/appStoreReviewDetails/patch-200.json'));
const reviewDetail = new AppStoreReviewDetail(
requestContext,
'APP_STORE_REVIEW_DETAILS_1',
{ appStoreReviewAttachments: [] } as any
);
const uploadSpy = jest
.spyOn(AppStoreReviewDetail.prototype, 'uploadAttachmentAsync')
.mockResolvedValue({} as any);
await new AppReviewDetailTask().uploadAsync({
config: new AppleConfigReader({
review: {
firstName: 'Evan',
lastName: 'Bacon',
email: '[email protected]',
phone: '+1 555 555 5555',
attachment: 'store/apple/review-attachment/demo.pdf',
},
}),
context: {
version: new AppStoreVersion(requestContext, 'stub-id', {} as any),
reviewDetail,
projectDir: '/test/project',
} as any,
});
expect(uploadSpy).toBeCalledTimes(1);
expect(uploadSpy).toBeCalledWith(
expect.stringContaining('store/apple/review-attachment/demo.pdf')
);
});
it('skips upload when checksum matches existing attachment', async () => {
nock('https://api.appstoreconnect.apple.com')
.patch(`/v1/${AppStoreReviewDetail.type}/APP_STORE_REVIEW_DETAILS_1`)
.reply(200, require('./fixtures/appStoreReviewDetails/patch-200.json'));
const existing = new AppStoreReviewAttachment(requestContext, 'ATTACH_1', {
fileName: 'demo.pdf',
fileSize: fileBytes.length,
sourceFileChecksum: fileChecksum,
uploaded: true,
} as any);
const reviewDetail = new AppStoreReviewDetail(
requestContext,
'APP_STORE_REVIEW_DETAILS_1',
{ appStoreReviewAttachments: [existing] } as any
);
const uploadSpy = jest
.spyOn(AppStoreReviewDetail.prototype, 'uploadAttachmentAsync')
.mockResolvedValue({} as any);
const deleteSpy = jest.spyOn(existing, 'deleteAsync').mockResolvedValue(undefined);
await new AppReviewDetailTask().uploadAsync({
config: new AppleConfigReader({
review: {
firstName: 'Evan',
lastName: 'Bacon',
email: '[email protected]',
phone: '+1 555 555 5555',
attachment: 'store/apple/review-attachment/demo.pdf',
},
}),
context: {
version: new AppStoreVersion(requestContext, 'stub-id', {} as any),
reviewDetail,
projectDir: '/test/project',
} as any,
});
expect(uploadSpy).not.toBeCalled();
expect(deleteSpy).not.toBeCalled();
});
it('replaces stale attachment when checksum differs', async () => {
nock('https://api.appstoreconnect.apple.com')
.patch(`/v1/${AppStoreReviewDetail.type}/APP_STORE_REVIEW_DETAILS_1`)
.reply(200, require('./fixtures/appStoreReviewDetails/patch-200.json'));
const existing = new AppStoreReviewAttachment(requestContext, 'ATTACH_OLD', {
fileName: 'old.pdf',
fileSize: 5,
sourceFileChecksum: 'different-checksum',
uploaded: true,
} as any);
const reviewDetail = new AppStoreReviewDetail(
requestContext,
'APP_STORE_REVIEW_DETAILS_1',
{ appStoreReviewAttachments: [existing] } as any
);
const uploadSpy = jest
.spyOn(AppStoreReviewDetail.prototype, 'uploadAttachmentAsync')
.mockResolvedValue({} as any);
const deleteSpy = jest.spyOn(existing, 'deleteAsync').mockResolvedValue(undefined);
await new AppReviewDetailTask().uploadAsync({
config: new AppleConfigReader({
review: {
firstName: 'Evan',
lastName: 'Bacon',
email: '[email protected]',
phone: '+1 555 555 5555',
attachment: 'store/apple/review-attachment/demo.pdf',
},
}),
context: {
version: new AppStoreVersion(requestContext, 'stub-id', {} as any),
reviewDetail,
projectDir: '/test/project',
} as any,
});
expect(deleteSpy).toBeCalledTimes(1);
expect(uploadSpy).toBeCalledTimes(1);
});
it('warns and skips when local attachment file is missing', async () => {
nock('https://api.appstoreconnect.apple.com')
.patch(`/v1/${AppStoreReviewDetail.type}/APP_STORE_REVIEW_DETAILS_1`)
.reply(200, require('./fixtures/appStoreReviewDetails/patch-200.json'));
existsSyncSpy.mockReturnValue(false);
const reviewDetail = new AppStoreReviewDetail(
requestContext,
'APP_STORE_REVIEW_DETAILS_1',
{ appStoreReviewAttachments: [] } as any
);
const uploadSpy = jest
.spyOn(AppStoreReviewDetail.prototype, 'uploadAttachmentAsync')
.mockResolvedValue({} as any);
await new AppReviewDetailTask().uploadAsync({
config: new AppleConfigReader({
review: {
firstName: 'Evan',
lastName: 'Bacon',
email: '[email protected]',
phone: '+1 555 555 5555',
attachment: 'store/apple/review-attachment/missing.pdf',
},
}),
context: {
version: new AppStoreVersion(requestContext, 'stub-id', {} as any),
reviewDetail,
projectDir: '/test/project',
} as any,
});
expect(uploadSpy).not.toBeCalled();
});
});
});
});
@@ -1,5 +1,8 @@
import { AppStoreReviewDetail } from '@expo/apple-utils';
import { AppStoreReviewAttachment, AppStoreReviewDetail } from '@expo/apple-utils';
import chalk from 'chalk';
import crypto from 'crypto';
import fs from 'fs';
import path from 'path';
import Log from '../../../log';
import { logAsync } from '../../utils/log';
@@ -10,6 +13,14 @@ export type AppReviewData = {
reviewDetail: AppStoreReviewDetail;
};
/** Default directory (relative to project root) for downloaded review attachments. */
const REVIEW_ATTACHMENT_DIR = path.join('store', 'apple', 'review-attachment');
function md5OfFile(absolutePath: string): string {
const bytes = fs.readFileSync(absolutePath);
return crypto.createHash('md5').update(bytes).digest('hex');
}
/** Handle all contact, demo account, or instruction info that are required for the App Store review team. */
export class AppReviewDetailTask extends AppleTask {
public name = (): string => 'app review detail';
@@ -22,9 +33,41 @@ export class AppReviewDetailTask extends AppleTask {
}
public async downloadAsync({ config, context }: TaskDownloadOptions): Promise<void> {
if (context.reviewDetail) {
config.setReviewDetails(context.reviewDetail.attributes);
if (!context.reviewDetail) {
return;
}
config.setReviewDetails(context.reviewDetail.attributes);
// ASC limitation — see expo/third-party#146. AppStoreReviewAttachment has no
// download URL (contrast with AppScreenshot.imageAsset.templateUrl). The API
// only exposes fileName, fileSize, sourceFileChecksum, uploadOperations, and
// assetDeliveryState. We record the expected path in the config so that the
// pull -> push round-trip preserves the attachment entry, but we cannot
// download the actual file bytes.
const attachments = context.reviewDetail.attributes.appStoreReviewAttachments ?? [];
const attachment = attachments[0];
if (!attachment) {
return;
}
const fileName = attachment.attributes.fileName || 'attachment';
const relativePath = path.join(REVIEW_ATTACHMENT_DIR, fileName);
const absolutePath = path.resolve(context.projectDir, relativePath);
if (!fs.existsSync(absolutePath)) {
Log.log(
chalk`{yellow Review attachment "${chalk.bold(
fileName
)}" exists in App Store Connect but cannot be downloaded.}\n` +
chalk` App Store Connect does not expose a download URL for review attachments\n` +
chalk` (unlike screenshots which have {dim imageAsset.templateUrl}). Only {dim fileName},\n` +
chalk` {dim fileSize}, and {dim sourceFileChecksum} are available from the API.\n` +
chalk` To keep the attachment in sync, place the file at: {bold ${relativePath}}`
);
}
config.setReviewAttachment(relativePath);
}
public async uploadAsync({ config, context }: TaskUploadOptions): Promise<void> {
@@ -54,10 +97,82 @@ export class AppReviewDetailTask extends AppleTask {
);
}
// Capture existing attachments before the patch response overwrites them. The
// PATCH response doesn't include the related `appStoreReviewAttachments`, so
// we need to reuse the version we fetched in `prepareAsync` to make a
// checksum-based skip decision below.
const existingAttachments = context.reviewDetail.attributes.appStoreReviewAttachments ?? [];
context.reviewDetail = await logAsync(() => context.reviewDetail.updateAsync(reviewDetail), {
pending: `Updating store review details for ${chalk.bold(versionString)}...`,
success: `Updated store review details for ${chalk.bold(versionString)}`,
failure: `Failed updating store review details for ${chalk.bold(versionString)}`,
});
await this.syncAttachmentAsync({
attachmentPath: config.getReviewAttachment(),
reviewDetail: context.reviewDetail,
existingAttachments,
projectDir: context.projectDir,
});
}
/**
* Sync the App Store review attachment.
* - When no attachment is configured, do nothing (existing remote attachments are left alone).
* - When the configured file matches an existing attachment by checksum, skip re-upload.
* - Otherwise delete any existing attachment(s) and upload the new file.
*/
private async syncAttachmentAsync({
attachmentPath,
reviewDetail,
existingAttachments,
projectDir,
}: {
attachmentPath: string | null;
reviewDetail: AppStoreReviewDetail;
existingAttachments: AppStoreReviewAttachment[];
projectDir: string;
}): Promise<void> {
if (!attachmentPath) {
return;
}
const absolutePath = path.resolve(projectDir, attachmentPath);
if (!fs.existsSync(absolutePath)) {
Log.warn(chalk`{yellow Review attachment not found: ${absolutePath}}`);
return;
}
const fileName = path.basename(absolutePath);
const localChecksum = md5OfFile(absolutePath);
const matching = existingAttachments.find(
attachment =>
attachment.attributes.uploaded &&
attachment.attributes.sourceFileChecksum === localChecksum
);
if (matching) {
Log.log(chalk`{dim - Skipped review attachment ${fileName}, already up to date}`);
return;
}
// Remove any stale attachments before uploading the new one. App Store
// Connect generally allows a single review attachment per review detail,
// so we replace whatever is currently there.
for (const attachment of existingAttachments) {
await logAsync(() => attachment.deleteAsync(), {
pending: `Deleting review attachment ${chalk.bold(attachment.attributes.fileName)}...`,
success: `Deleted review attachment ${chalk.bold(attachment.attributes.fileName)}`,
failure: `Failed deleting review attachment ${chalk.bold(attachment.attributes.fileName)}`,
});
}
await logAsync(() => reviewDetail.uploadAttachmentAsync(absolutePath), {
pending: `Uploading review attachment ${chalk.bold(fileName)}...`,
success: `Uploaded review attachment ${chalk.bold(fileName)}`,
failure: `Failed uploading review attachment ${chalk.bold(fileName)}`,
});
}
}
+6 -1
View File
@@ -128,5 +128,10 @@ export interface AppleReview {
demoPassword?: string;
demoRequired?: boolean;
notes?: string;
// attachment?: string;
/**
* Relative path (from project root) to a file shared with App Review.
* This is uploaded as an `appStoreReviewAttachment` and re-uploaded only when the
* file's checksum differs from the existing attachment.
*/
attachment?: string;
}