Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 40 additions & 8 deletions src/core/listing/apply.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { AndroidLocaleInfo, AppleLocaleInfo, StoreConfig } from '../store/storeConfig.js';
import type { DraftListing, ListingBrief, LocaleDraft } from '../types/listing.js';
import type { MutableDeep } from '../types/mutable.js';
/**
* App Store field limits, in characters. `keywords` is the limit on the *comma-joined* string (Apple
* counts the serialized field, and `storeConfig` joins with `", "`), not the count of keywords.
Expand All @@ -23,11 +24,11 @@ const clampText = (listingText: string, maxCharacters: number): string => {
return listingText.slice(0, maxCharacters).trimEnd();
};
/** The comma-joined serialization Apple counts against the 100-char keyword limit. */
export const serializeKeywords = (keywords: string[]): string => {
export const serializeKeywords = (keywords: readonly string[]): string => {
return keywords.join(', ');
};
/** Keep keywords from the front until adding the next would overflow the joined-string limit. */
const clampKeywords = (keywords: string[], maxCharacters: number): string[] => {
const clampKeywords = (keywords: readonly string[], maxCharacters: number): string[] => {
const keptKeywords: string[] = [];
for (const keyword of keywords) {
if (serializeKeywords([...keptKeywords, keyword]).length > maxCharacters) break;
Expand All @@ -47,7 +48,7 @@ export const clampDraft = (
warnings: string[];
} => {
const warningMessages: string[] = [];
const clampedDraft: DraftListing = {};
const clampedDraft: MutableDeep<DraftListing> = {};
/** Clamp one optional text field, recording a warning when it was over the limit. */
const fitText = (
listingText: string | undefined,
Expand Down Expand Up @@ -98,7 +99,7 @@ export const briefFor = (
currentListing: AppleLocaleInfo | undefined,
aboutOverride: string | undefined,
): ListingBrief => {
const listingBrief: ListingBrief = { locale: localeName, appName: displayName };
const listingBrief: MutableDeep<ListingBrief> = { locale: localeName, appName: displayName };
let aboutText = aboutOverride;
if (aboutText === undefined && currentListing !== undefined)
aboutText = currentListing.promotionalText;
Expand Down Expand Up @@ -135,6 +136,38 @@ export const deriveAndroidLocale = (listingDraft: DraftListing): AndroidLocaleIn
* fields (so untouched fields and other locales survive), per targeted platform. The App Store fields
* map 1:1; the Play fields are derived via {@link deriveAndroidLocale}. Returns a new config.
*/
/** Merge a draft over one locale's existing App Store listing, copying keywords into a mutable array. */
const mergeAppleLocale = (
existingLocale: AppleLocaleInfo | undefined,
listingDraft: DraftListing,
): AppleLocaleInfo => {
Comment on lines +140 to +143

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Manual locale merge drift 🐞 Bug ⚙ Maintainability

mergeAppleLocale manually copies each AppleLocaleInfo field, so any future additions to
AppleLocaleInfo that exist in persisted configs will be silently dropped when a locale is updated
via applyDraft. This is a forward-compatibility data-loss risk compared to the prior generic
spread merge behavior.
Agent Prompt
### Issue description
`mergeAppleLocale` enumerates `AppleLocaleInfo` fields one-by-one. If `AppleLocaleInfo` gains new optional fields later (and existing configs contain them), calling `applyDraft` for a locale will rebuild that locale object without the new fields, effectively deleting them.

### Issue Context
The old approach of spreading the existing locale preserved all enumerable runtime properties. The new approach requires manual synchronization with `AppleLocaleInfo`.

### Fix Focus Areas
- src/core/listing/apply.ts[139-169]

### Suggested fix
Refactor `mergeAppleLocale` to preserve all existing locale properties via a spread, and then only special-case `keywords` to ensure it is cloned into a new mutable array.

For example:
- Start with `const mergedLocale: MutableDeep<AppleLocaleInfo> = { ...(existingLocale ?? {}) }` (preserves any future fields).
- If `existingLocale?.keywords` exists, set `mergedLocale.keywords = [...existingLocale.keywords]`.
- Overlay the draft fields (`title`, `subtitle`, `description`, `promotionalText`, and `keywords`), cloning `keywords` from the draft when present.

This keeps the runtime “preserve unknown fields” behavior while still avoiding aliasing the keywords array.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

const mergedLocale: MutableDeep<AppleLocaleInfo> = {};
if (existingLocale !== undefined) {
if (existingLocale.title !== undefined) mergedLocale.title = existingLocale.title;
if (existingLocale.subtitle !== undefined) mergedLocale.subtitle = existingLocale.subtitle;
if (existingLocale.description !== undefined)
mergedLocale.description = existingLocale.description;
if (existingLocale.keywords !== undefined) mergedLocale.keywords = [...existingLocale.keywords];
if (existingLocale.releaseNotes !== undefined)
mergedLocale.releaseNotes = existingLocale.releaseNotes;
if (existingLocale.promotionalText !== undefined)
mergedLocale.promotionalText = existingLocale.promotionalText;
if (existingLocale.marketingUrl !== undefined)
mergedLocale.marketingUrl = existingLocale.marketingUrl;
if (existingLocale.supportUrl !== undefined)
mergedLocale.supportUrl = existingLocale.supportUrl;
if (existingLocale.privacyPolicyUrl !== undefined)
mergedLocale.privacyPolicyUrl = existingLocale.privacyPolicyUrl;
}
if (listingDraft.title !== undefined) mergedLocale.title = listingDraft.title;
if (listingDraft.subtitle !== undefined) mergedLocale.subtitle = listingDraft.subtitle;
if (listingDraft.description !== undefined) mergedLocale.description = listingDraft.description;
if (listingDraft.promotionalText !== undefined)
mergedLocale.promotionalText = listingDraft.promotionalText;
if (listingDraft.keywords !== undefined) mergedLocale.keywords = [...listingDraft.keywords];
return mergedLocale;
};

export const applyDraft = (
storeConfiguration: StoreConfig,
localeName: string,
Expand All @@ -148,12 +181,11 @@ export const applyDraft = (
if (listingTargets.ios) {
let appleListing = storeConfiguration.apple;
if (appleListing === undefined) appleListing = { info: {} };
const localeInfo: Record<string, AppleLocaleInfo> = { ...appleListing.info };
localeInfo[localeName] = mergeAppleLocale(appleListing.info[localeName], listingDraft);
updatedStoreConfiguration.apple = {
...appleListing,
info: {
...appleListing.info,
[localeName]: { ...appleListing.info[localeName], ...listingDraft },
},
info: localeInfo,
};
}
if (listingTargets.android) {
Expand Down
3 changes: 2 additions & 1 deletion src/core/listing/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Data, Effect, Redacted, Schema } from 'effect';
import { LaunchEnvironment, type LaunchEnvironmentService } from '../services/environment.js';
import type { DraftListing, ListingBrief, ListingGenerator } from '../types/listing.js';
import { APPLE_LIMITS, serializeKeywords } from './apply.js';
import type { MutableDeep } from '../types/mutable.js';

const GeneratedListingSchema = Schema.Struct({
title: Schema.optionalWith(Schema.String, { exact: true }),
Expand Down Expand Up @@ -138,7 +139,7 @@ export const parseDraftListing = (
stripJsonFence(completionText),
).pipe(
Effect.map((generatedListing) => {
const listingDraft: DraftListing = {};
const listingDraft: MutableDeep<DraftListing> = {};
const title = normalizeGeneratedText(generatedListing.title);
if (title !== undefined) listingDraft.title = title;
const subtitle = normalizeGeneratedText(generatedListing.subtitle);
Expand Down
2 changes: 1 addition & 1 deletion src/core/privacy/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const CollectedDataTypeSchema = Schema.Struct({
const USAGE_DESCRIPTION_RE =
/<key>(NS\w*UsageDescription)<\/key>\s*(?:<string>([^<]*)<\/string>|<string\s*\/>)/g;
/** De-duplicate while preserving first-seen order. */
const unique = (strings: string[]): string[] => {
const unique = (strings: readonly string[]): string[] => {
return [...new Set(strings)];
};
/** Collect every `<string>...</string>` inside an XML fragment. */
Expand Down
2 changes: 1 addition & 1 deletion src/core/privacy/reconcile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export const reconcilePrivacy = (app: string, surface: PrivacySurface): PrivacyF
*/
export const buildPrivacyReport = (
findings: PrivacyFinding[],
scanned: string[],
scanned: readonly string[],
): PrivacyReport => {
let exitCode: PrivacyReport['exitCode'] = READINESS_EXIT.ok;
if (findings.some((finding) => finding.severity === 'blocker')) exitCode = READINESS_EXIT.blocker;
Expand Down
3 changes: 2 additions & 1 deletion src/core/store/accessibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import { appRecordMissing, plan, type ReconcileContext } from './reconcile.js';
import { errorMessage } from '../services/errorMessage.js';
import type { PlannedAction } from '../types/reconcile.js';
import type { MutableDeep } from '../types/mutable.js';
import {
decodeStoreSurfaceConfig,
loadStoreSurfaceConfig,
Expand Down Expand Up @@ -126,7 +127,7 @@ const supportEquals = (
};
/** Expand omitted accessibility flags to `false`. */
const normalizeSupport = (support: AccessibilitySupport): AccessibilitySupport => {
const normalizedSupport: AccessibilitySupport = {};
const normalizedSupport: MutableDeep<AccessibilitySupport> = {};
for (const key of ACCESSIBILITY_SUPPORT_KEYS) normalizedSupport[key] = support[key] === true;
return normalizedSupport;
};
Expand Down
5 changes: 3 additions & 2 deletions src/core/store/appEvents.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Data, Effect } from 'effect';
import type { MutableDeep } from '../types/mutable.js';
import type {
AppEventLocalizationInput,
AppEventLocalizationResource,
Expand Down Expand Up @@ -191,7 +192,7 @@ export const createEvent = (
appEventsStore.getAppId(bundleId),
);
if (appId === null) return yield* Effect.fail(missingAppRecord(bundleId));
const eventAttributes: NewAppEvent = { referenceName };
const eventAttributes: MutableDeep<NewAppEvent> = { referenceName };
if (badge !== undefined) eventAttributes.badge = badge;
if (eventRequest.primaryLocale !== undefined) {
eventAttributes.primaryLocale = eventRequest.primaryLocale.trim();
Expand Down Expand Up @@ -224,7 +225,7 @@ export const localizeEvent = (
}),
);
}
const localizationAttributes: AppEventLocalizationInput = {};
const localizationAttributes: MutableDeep<AppEventLocalizationInput> = {};
if (localizationRequest.name !== undefined) {
localizationAttributes.name = localizationRequest.name;
}
Expand Down
10 changes: 5 additions & 5 deletions src/core/store/ascScreenshots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export type ScreenshotReconcileInput = {
};
/** Group members by a derived string key. */
const groupBy = <Member>(
members: Member[],
members: readonly Member[],
keyOf: (member: Member) => string,
): Map<string, Member[]> => {
const groups = new Map<string, Member[]>();
Expand Down Expand Up @@ -122,7 +122,7 @@ const reconcileAppScreenshots = (
api: ScreenshotsApi,
log: ActionLog,
appId: string,
screenshots: LocalScreenshot[],
screenshots: readonly LocalScreenshot[],
): Effect.Effect<void, unknown> =>
Effect.gen(function* () {
const versionId = yield* api.getEditableVersionId(appId);
Expand Down Expand Up @@ -178,7 +178,7 @@ const reconcileScreenshotSet = (
existingSet: ScreenshotSetResource | undefined,
displayType: string,
locale: string,
screenshots: LocalScreenshot[],
screenshots: readonly LocalScreenshot[],
): Effect.Effect<void, unknown> =>
Effect.gen(function* () {
const label = appleDisplayTypeLabel(displayType);
Expand Down Expand Up @@ -227,7 +227,7 @@ const reconcileSubscriptionReviewScreenshots = (
api: ScreenshotsApi,
log: ActionLog,
appId: string,
reviewScreenshots: SubscriptionReviewScreenshot[],
reviewScreenshots: readonly SubscriptionReviewScreenshot[],
): Effect.Effect<void, unknown> =>
Effect.gen(function* () {
const subscriptionIdByProduct = new Map<string, string>();
Expand Down Expand Up @@ -365,7 +365,7 @@ const reconcilePreviewSet = (
existingSet: PreviewSetResource | undefined,
previewType: string,
locale: string,
previews: LocalPreview[],
previews: readonly LocalPreview[],
): Effect.Effect<void, unknown> =>
Effect.gen(function* () {
const label = applePreviewTypeLabel(previewType);
Expand Down
4 changes: 2 additions & 2 deletions src/core/store/ascSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ export const reconcileApp = (
const catalogContext = emptyCatalogContext(api, input.dryRun, input.allowDestructive);
const appId = yield* requireAppStoreRecordId(api, input.bundleId);
yield* reconcileCapabilities(catalogContext, input.bundleId, input.capabilities);
let desiredInAppPurchases: InAppPurchaseConfig[] = [];
let desiredInAppPurchases: readonly InAppPurchaseConfig[] = [];
if (input.products.inAppPurchases !== undefined) {
desiredInAppPurchases = input.products.inAppPurchases;
}
Expand Down Expand Up @@ -372,7 +372,7 @@ const reconcileCapabilities = (
const reconcileInAppPurchases = (
catalogContext: CatalogReconcileContext,
appId: string,
desiredPurchases: InAppPurchaseConfig[],
desiredPurchases: readonly InAppPurchaseConfig[],
): Effect.Effect<void, unknown> =>
Effect.gen(function* () {
if (desiredPurchases.length === 0) return;
Expand Down
2 changes: 1 addition & 1 deletion src/core/store/availability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export type AvailabilityReconcileInput = {
dryRun: boolean;
};
/** Uppercase, trim, and de-duplicate a list of territory codes into a stable set. */
const normalizeTerritories = (territories: string[]): Set<string> => {
const normalizeTerritories = (territories: readonly string[]): Set<string> => {
return new Set(territories.map((code) => code.trim().toUpperCase()));
};
/** Sorted difference `a \ b` - codes in `a` not in `b`. */
Expand Down
11 changes: 6 additions & 5 deletions src/core/store/gameCenter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
GameCenterConfig,
LeaderboardConfig,
} from '../types/storeSurface.js';
import type { MutableDeep } from '../types/mutable.js';
import {
decodeStoreSurfaceConfig,
loadStoreSurfaceConfig,
Expand Down Expand Up @@ -199,7 +200,7 @@ const localizationLocale = (locale: string | undefined): string => {
* Connect) rather than failed.
*/
const applyLocalization = (
localizationAction: PlannedAction,
localizationAction: MutableDeep<PlannedAction>,
versionId: string | null,
vendorIdentifier: string,
writeLocalization: (confirmedVersionId: string) => Effect.Effect<void, unknown>,
Expand Down Expand Up @@ -287,7 +288,7 @@ const reconcileAchievements = (
reconcileContext: ReconcileContext,
api: AscGameCenterApi,
detail: NonNullable<EnsuredDetail>,
declaredAchievements: AchievementConfig[],
declaredAchievements: readonly AchievementConfig[],
): Effect.Effect<void, unknown> =>
Effect.gen(function* () {
let existingIdentifiers = new Set<string>();
Expand Down Expand Up @@ -333,7 +334,7 @@ const reconcileLeaderboards = (
reconcileContext: ReconcileContext,
api: AscGameCenterApi,
detail: NonNullable<EnsuredDetail>,
declaredLeaderboards: LeaderboardConfig[],
declaredLeaderboards: readonly LeaderboardConfig[],
): Effect.Effect<void, unknown> =>
Effect.gen(function* () {
let existingIdentifiers = new Set<string>();
Expand Down Expand Up @@ -395,9 +396,9 @@ export const reconcileGameCenter = (
);
return { bundleId: reconcileInput.bundleId, actions: reconcileContext.actions };
}
let achievements: AchievementConfig[] = [];
let achievements: readonly AchievementConfig[] = [];
if (gameCenterConfig.achievements !== undefined) achievements = gameCenterConfig.achievements;
let leaderboards: LeaderboardConfig[] = [];
let leaderboards: readonly LeaderboardConfig[] = [];
if (gameCenterConfig.leaderboards !== undefined) leaderboards = gameCenterConfig.leaderboards;
yield* reconcileAchievements(reconcileContext, api, detail, achievements);
yield* reconcileLeaderboards(reconcileContext, api, detail, leaderboards);
Expand Down
4 changes: 2 additions & 2 deletions src/core/store/offers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class FakeOffersApi implements AscOffersApi {
readonly createdWinBack: WinBackOfferCreate[] = [];
readonly createdPromoted: PromotedPurchaseCreate[] = [];
introCreateCount = 0;
reorderedTo: string[] | null = null;
reorderedTo: readonly string[] | null = null;
getAppId(): Effect.Effect<string | null> {
return Effect.succeed(this.appId);
}
Expand Down Expand Up @@ -115,7 +115,7 @@ class FakeOffersApi implements AscOffersApi {
visibleForAllUsers: input.visibleForAllUsers,
});
}
reorderPromotedPurchases(_appId: string, orderedIds: string[]): Effect.Effect<void> {
reorderPromotedPurchases(_appId: string, orderedIds: readonly string[]): Effect.Effect<void> {
this.reorderedTo = orderedIds;
return Effect.void;
}
Expand Down
Loading
Loading