Skip to content

refactor(google): readonly-domain-types (stack 3/12, re-split #307) #376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
YosefHayim wants to merge 1 commit into from
Closed

refactor(google): readonly-domain-types (stack 3/12, re-split #307) #376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 46 additions & 24 deletions src/google/playClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ import {
} from '@googleapis/androidpublisher';
import { Data, Effect, Option, Schema } from 'effect';
import type { ServiceAccount } from '../core/types/credentials.js';
import type { MutableDeep } from '../core/types/mutable.js';

/** Deep-clone a Launch boundary value into a mutable plain object for Google's generated client. */
const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => {

@cubic-dev-ai cubic-dev-ai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: mutableGoogleRequest takes requestShape: unknown and returns the caller-supplied generic type, effectively acting as an unchecked cast. Callers can pass any object shape and specify any return type (e.g. mutableGoogleRequest<androidpublisher_v3.Schema$Subscription>({...})), and TypeScript won't verify that the argument actually matches the requested schema. This removes compile-time safety on write paths for tracks/subscriptions/offers, allowing schema drift to compile and fail only at runtime. Consider typing the parameter as T (or using satisfies at call sites) so the compiler still verifies the input shape.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/google/playClient.ts, line 11:

<comment>mutableGoogleRequest takes `requestShape: unknown` and returns the caller-supplied generic type, effectively acting as an unchecked cast. Callers can pass any object shape and specify any return type (e.g. `mutableGoogleRequest<androidpublisher_v3.Schema$Subscription>({...})`), and TypeScript won't verify that the argument actually matches the requested schema. This removes compile-time safety on write paths for tracks/subscriptions/offers, allowing schema drift to compile and fail only at runtime. Consider typing the parameter as `T` (or using `satisfies` at call sites) so the compiler still verifies the input shape.</comment>

<file context>
@@ -5,6 +5,13 @@ import {
+import type { MutableDeep } from '../core/types/mutable.js';
+
+/** Deep-clone a Launch boundary value into a mutable plain object for Google's generated client. */
+const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => {
+  const clonedRequest: GoogleRequest = JSON.parse(JSON.stringify(requestShape));
+  return clonedRequest;
</file context>
Suggested change
const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => {
const mutableGoogleRequest = <GoogleRequest>(requestShape: GoogleRequest): MutableDeep<GoogleRequest> => {
const clonedRequest: MutableDeep<GoogleRequest> = JSON.parse(JSON.stringify(requestShape));
return clonedRequest;
};

const clonedRequest: GoogleRequest = JSON.parse(JSON.stringify(requestShape));

@cubic-dev-ai cubic-dev-ai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The new mutableGoogleRequest uses a JSON.parse(JSON.stringify(...)) round-trip to produce a mutable request body for Google's generated client. Today the inputs (normalized Play releases/subscriptions/offers) are plain JSON-safe data, so nothing breaks, but this clone is lossy and unguarded: it throws a SyntaxError if the input is ever undefined, and it silently drops undefined fields and cannot represent Date/BigInt/circular values. Since the only requirement is satisfying the mutable client types, a structuredClone (which preserves values without the JSON loss) is safer; if a plain cast is acceptable that avoids the clone cost entirely.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/google/playClient.ts, line 12:

<comment>The new `mutableGoogleRequest` uses a `JSON.parse(JSON.stringify(...))` round-trip to produce a mutable request body for Google's generated client. Today the inputs (normalized Play releases/subscriptions/offers) are plain JSON-safe data, so nothing breaks, but this clone is lossy and unguarded: it throws a `SyntaxError` if the input is ever `undefined`, and it silently drops `undefined` fields and cannot represent `Date`/`BigInt`/circular values. Since the only requirement is satisfying the mutable client types, a `structuredClone` (which preserves values without the JSON loss) is safer; if a plain cast is acceptable that avoids the clone cost entirely.</comment>

<file context>
@@ -5,6 +5,13 @@ import {
+
+/** Deep-clone a Launch boundary value into a mutable plain object for Google's generated client. */
+const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => {
+  const clonedRequest: GoogleRequest = JSON.parse(JSON.stringify(requestShape));
+  return clonedRequest;
+};
</file context>

return clonedRequest;
Comment on lines +10 to +13

@qodo-code-review qodo-code-review Bot Aug 7, 2026

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

1. Unchecked request cast 🐞 Bug ⚙ Maintainability

mutableGoogleRequest accepts unknown but returns the caller-selected generic type, so request
bodies are no longer type-checked against the googleapis schema types at the call site. This can
allow an incompatible request shape to compile and only fail at runtime in write paths
(tracks/subscriptions/offers).
Agent Prompt 
### Issue description
`mutableGoogleRequest` currently has the signature `<T>(requestShape: unknown) => T`, which turns it into an unchecked cast: callers can request any return type regardless of the actual argument shape.

### Issue Context
This helper is used to build `requestBody` objects for googleapis write calls (tracks/subscriptions/offers). Keeping the deep-clone behavior is fine, but we should preserve compiler checking that the provided object is actually assignable to the intended Google schema.

### Fix Focus Areas
- src/google/playClient.ts[10-13]
- src/google/playClient.ts[656-668]
- src/google/playClient.ts[883-896]
- src/google/playClient.ts[965-985]

### Suggested fix
1. Change the helper to be type-safe at the input:
   - e.g. `const mutableGoogleRequest = <T>(requestShape: T): MutableDeep<T> => structuredClone(requestShape) as MutableDeep<T>;`
   - (If `structuredClone` is not desired, keep JSON clone but still type the parameter as `T`, not `unknown`, and cast internally.)
2. At call sites, make the argument satisfy the Google schema type so TS checks it:
   - `requestBody: mutableGoogleRequest({ ... } satisfies androidpublisher_v3.Schema$Subscription)`
   - or assign the object to a `const body: androidpublisher_v3.Schema$Subscription = { ... }` before cloning.

This preserves the “mutable plain object” goal while preventing accidental schema drift from compiling silently.

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

Comment on lines +11 to +13

@qodo-code-review qodo-code-review Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

2. Brittle json deep clone 🐞 Bug ☼ Reliability

mutableGoogleRequest deep-clones via JSON.parse(JSON.stringify(...)), which will throw or change
values if non-JSON-safe data ever reaches a request body (e.g., circular references, BigInt, or
undefined array entries becoming null). That creates a sharp edge on the Google write paths that
now always route through this helper.
Agent Prompt 
### Issue description
The helper clones request bodies using JSON serialization, which is inherently lossy and can throw for certain runtime values.

### Issue Context
Even if current request bodies are intended to be JSON-shaped, this helper is now the standard pathway for several write operations; using a non-lossy clone reduces fragility if the shapes evolve.

### Fix Focus Areas
- src/google/playClient.ts[10-13]

### Suggested fix
Replace JSON cloning with `structuredClone` (Node >= 20) to avoid JSON lossy behavior:
- `const clonedRequest = structuredClone(requestShape) as MutableDeep<T>;`

If you must keep JSON cloning, consider adding an explicit error message when stringify/parse fails so failures are more diagnosable.

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

};
import type {
BasePlan,
InAppProductResource,
Expand Down Expand Up @@ -91,7 +98,7 @@ const normalizeReview = (
)?.developerComment;
let rating = 0;
if (typeof userComment?.starRating === 'number') rating = userComment.starRating;
const review: PlayReview = {
const review: MutableDeep<PlayReview> = {
reviewId: googleReview.reviewId,
rating,
answered: developerComment !== undefined,
Expand Down Expand Up @@ -128,7 +135,7 @@ const normalizeMoney = (money: androidpublisher_v3.Schema$Money | undefined): Pl
const normalizeTrackRelease = (
googleRelease: androidpublisher_v3.Schema$TrackRelease,
): PlayRelease => {
const release: PlayRelease = {};
const release: MutableDeep<PlayRelease> = {};
if (typeof googleRelease.name === 'string') release.name = googleRelease.name;
if (Array.isArray(googleRelease.versionCodes)) release.versionCodes = googleRelease.versionCodes;
if (typeof googleRelease.status === 'string') release.status = googleRelease.status;
Expand Down Expand Up @@ -231,7 +238,7 @@ const normalizeProductMoney = (
googlePrice: androidpublisher_v3.Schema$Price | undefined,
): PlayMoney | undefined => {
if (googlePrice === undefined) return;
const price: PlayMoney = {};
const price: MutableDeep<PlayMoney> = {};
if (typeof googlePrice.priceMicros === 'string') price.priceMicros = googlePrice.priceMicros;
if (typeof googlePrice.currency === 'string') price.currency = googlePrice.currency;
return price;
Expand All @@ -241,7 +248,7 @@ const normalizeInAppProduct = (
googleProduct: androidpublisher_v3.Schema$InAppProduct,
): InAppProductResource | undefined => {
if (typeof googleProduct.sku !== 'string') return;
const product: InAppProductResource = { sku: googleProduct.sku };
const product: MutableDeep<InAppProductResource> = { sku: googleProduct.sku };
if (typeof googleProduct.status === 'string') product.status = googleProduct.status;
if (typeof googleProduct.purchaseType === 'string') {
product.purchaseType = googleProduct.purchaseType;
Expand Down Expand Up @@ -285,19 +292,21 @@ const normalizeInAppProduct = (
/** Normalize one generated subscription base plan to the fields Launch reconciles. */
const normalizeBasePlan = (
googlePlan: androidpublisher_v3.Schema$BasePlan,
): BasePlan | undefined => {
): MutableDeep<BasePlan> | undefined => {
if (typeof googlePlan.basePlanId !== 'string') return;
const basePlan: BasePlan = { basePlanId: googlePlan.basePlanId };
const basePlan: MutableDeep<BasePlan> = { basePlanId: googlePlan.basePlanId };
if (typeof googlePlan.state === 'string') basePlan.state = googlePlan.state;
const billingPeriod = googlePlan.autoRenewingBasePlanType?.billingPeriodDuration;
if (typeof billingPeriod === 'string') {
basePlan.autoRenewingBasePlanType = { billingPeriodDuration: billingPeriod };
}
if (Array.isArray(googlePlan.regionalConfigs)) {
const regionalConfigs: RegionalBasePlanConfig[] = [];
const regionalConfigs: MutableDeep<RegionalBasePlanConfig>[] = [];
for (const googleRegion of googlePlan.regionalConfigs) {
if (typeof googleRegion.regionCode !== 'string') continue;
const regionalConfig: RegionalBasePlanConfig = { regionCode: googleRegion.regionCode };
const regionalConfig: MutableDeep<RegionalBasePlanConfig> = {
regionCode: googleRegion.regionCode,
};
if (typeof googleRegion.newSubscriberAvailability === 'boolean') {
regionalConfig.newSubscriberAvailability = googleRegion.newSubscriberAvailability;
}
Expand All @@ -316,25 +325,27 @@ const normalizeSubscription = (
googleSubscription: androidpublisher_v3.Schema$Subscription,
): SubscriptionResource | undefined => {
if (typeof googleSubscription.productId !== 'string') return;
const subscription: SubscriptionResource = { productId: googleSubscription.productId };
const subscription: MutableDeep<SubscriptionResource> = {
productId: googleSubscription.productId,
};
if (typeof googleSubscription.packageName === 'string') {
subscription.packageName = googleSubscription.packageName;
}
if (Array.isArray(googleSubscription.basePlans)) {
const basePlans: BasePlan[] = [];
const basePlans: MutableDeep<BasePlan>[] = [];
for (const googlePlan of googleSubscription.basePlans) {
const basePlan = normalizeBasePlan(googlePlan);
if (basePlan !== undefined) basePlans.push(basePlan);
}
subscription.basePlans = basePlans;
}
if (Array.isArray(googleSubscription.listings)) {
const listings: SubscriptionListing[] = [];
const listings: MutableDeep<SubscriptionListing>[] = [];
for (const googleListing of googleSubscription.listings) {
if (typeof googleListing.languageCode !== 'string') continue;
if (typeof googleListing.title !== 'string') continue;
if (typeof googleListing.description !== 'string') continue;
const listing: SubscriptionListing = {
const listing: MutableDeep<SubscriptionListing> = {
languageCode: googleListing.languageCode,
title: googleListing.title,
description: googleListing.description,
Expand All @@ -351,7 +362,7 @@ const normalizeSubscriptionOffer = (
googleOffer: androidpublisher_v3.Schema$SubscriptionOffer,
): SubscriptionOfferResource | undefined => {
if (typeof googleOffer.offerId !== 'string') return;
const offer: SubscriptionOfferResource = {
const offer: MutableDeep<SubscriptionOfferResource> = {
offerId: googleOffer.offerId,
phases: [],
regionalConfigs: [],
Expand All @@ -363,7 +374,7 @@ const normalizeSubscriptionOffer = (
if (Array.isArray(googleOffer.regionalConfigs)) {
for (const googleRegion of googleOffer.regionalConfigs) {
if (typeof googleRegion.regionCode !== 'string') continue;
const regionalConfig: RegionalSubscriptionOfferConfig = {
const regionalConfig: MutableDeep<RegionalSubscriptionOfferConfig> = {
regionCode: googleRegion.regionCode,
};
if (typeof googleRegion.newSubscriberAvailability === 'boolean') {
Expand All @@ -375,15 +386,17 @@ const normalizeSubscriptionOffer = (
if (Array.isArray(googleOffer.phases)) {
for (const googlePhase of googleOffer.phases) {
if (typeof googlePhase.recurrenceCount !== 'number') continue;
const phase: SubscriptionOfferPhase = {
const phase: MutableDeep<SubscriptionOfferPhase> = {
recurrenceCount: googlePhase.recurrenceCount,
regionalConfigs: [],
};
if (typeof googlePhase.duration === 'string') phase.duration = googlePhase.duration;
if (Array.isArray(googlePhase.regionalConfigs)) {
for (const googleRegion of googlePhase.regionalConfigs) {
if (typeof googleRegion.regionCode !== 'string') continue;
const regionalConfig: OfferPhaseRegionalConfig = { regionCode: googleRegion.regionCode };
const regionalConfig: MutableDeep<OfferPhaseRegionalConfig> = {
regionCode: googleRegion.regionCode,
};
if (googleRegion.price !== undefined)
regionalConfig.price = normalizeMoney(googleRegion.price);
if (googleRegion.free !== undefined) regionalConfig.free = {};
Expand Down Expand Up @@ -442,7 +455,7 @@ export const parseServiceAccount = (
Effect.map((serviceAccountKey) => {
let tokenUri = 'https://oauth2.googleapis.com/token';
if (serviceAccountKey.token_uri !== undefined) tokenUri = serviceAccountKey.token_uri;
const serviceAccount: ServiceAccount = {
const serviceAccount: MutableDeep<ServiceAccount> = {
clientEmail: serviceAccountKey.client_email,
privateKey: serviceAccountKey.private_key,
tokenUri,
Expand Down Expand Up @@ -651,7 +664,7 @@ export class GooglePlayClient {
packageName,
editId,
track,
requestBody: { track, releases: [...releases] },
requestBody: mutableGoogleRequest<androidpublisher_v3.Schema$Track>({ track, releases }),
}),
).pipe(Effect.asVoid),
);
Expand Down Expand Up @@ -704,7 +717,7 @@ export class GooglePlayClient {
countries.push({ countryCode: googleCountry.countryCode });
}
}
const availability: PlayCountryAvailability = { countries };
const availability: MutableDeep<PlayCountryAvailability> = { countries };
if (typeof countryAvailability.restOfWorld === 'boolean') {
availability.restOfWorld = countryAvailability.restOfWorld;
}
Expand Down Expand Up @@ -824,7 +837,7 @@ export class GooglePlayClient {
regions.sort((leftPrice, rightPrice) =>
leftPrice.regionCode.localeCompare(rightPrice.regionCode),
);
const convertedPrices: ConvertedPrices = { regions };
const convertedPrices: MutableDeep<ConvertedPrices> = { regions };
const fallbackPrice = priceConversion.convertedOtherRegionsPrice;
if (fallbackPrice !== undefined) {
convertedPrices.otherRegions = {
Expand Down Expand Up @@ -876,7 +889,10 @@ export class GooglePlayClient {
packageName,
productId: subscription.productId,
'regionsVersion.version': REGIONS_VERSION,
requestBody: { ...subscription, packageName },
requestBody: mutableGoogleRequest<androidpublisher_v3.Schema$Subscription>({
...subscription,
packageName,
}),
}),
).pipe(Effect.asVoid);
}
Expand All @@ -896,7 +912,10 @@ export class GooglePlayClient {
productId: subscription.productId,
updateMask,
'regionsVersion.version': REGIONS_VERSION,
requestBody: { ...subscription, packageName },
requestBody: mutableGoogleRequest<androidpublisher_v3.Schema$Subscription>({
...subscription,
packageName,
}),
}),
).pipe(Effect.asVoid);
}
Expand Down Expand Up @@ -958,7 +977,10 @@ export class GooglePlayClient {
basePlanId,
offerId: offer.offerId,
'regionsVersion.version': REGIONS_VERSION,
requestBody: { ...offer, packageName },
requestBody: mutableGoogleRequest<androidpublisher_v3.Schema$SubscriptionOffer>({
...offer,
packageName,
}),
}),
).pipe(Effect.asVoid);
}
Expand Down Expand Up @@ -1053,7 +1075,7 @@ export class GooglePlayClient {
if (typeof replyConfirmation.result?.replyText === 'string') {
storedReplyText = replyConfirmation.result.replyText;
}
const reply: PlayReplyResult = { replyText: storedReplyText };
const reply: MutableDeep<PlayReplyResult> = { replyText: storedReplyText };
const lastEdited = timestampToIso(replyConfirmation.result?.lastEdited);
if (lastEdited !== undefined) reply.lastEdited = lastEdited;
return reply;
Expand Down
3 changes: 2 additions & 1 deletion src/google/playReporting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
VitalsTimeline,
VitalsWindow,
} from '../core/types/vitals.js';
import type { MutableDeep } from '../core/types/mutable.js';
import { describePlayErrors, nonEmptyPageToken, serviceAccountJwtOptions } from './playClient.js';
/** Distinct from the Play Developer API scope - the reporting API rejects an `androidpublisher` token. */
const OAUTH_SCOPE = 'https://www.googleapis.com/auth/playdeveloperreporting';
Expand Down Expand Up @@ -272,7 +273,7 @@ export class PlayReportingClient {
for (const metricEntry of metricPage.rows) {
const date = dateTimeToIso(metricEntry.startTime);
if (date === undefined) continue;
const normalized: PlayVitalsRow = { metric, date };
const normalized: MutableDeep<PlayVitalsRow> = { metric, date };
const rate = metricNumber(metricEntry, metricSet.rate);
if (rate !== undefined) normalized.rate = rate;
const userPerceivedRate = metricNumber(metricEntry, metricSet.userPerceivedRate);
Expand Down
Loading