refactor(types): readonly-domain-types (stack 1/12, re-split #307) - #374
Merged
Conversation
|
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:XL
PR Summary by Qodorefactor(types): make core domain type shapes readonly + add MutableDeep helper
AI Description
Diagram
High-Level Assessment
Files changed (27)
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| platform?: string; | ||
| actions: string[]; | ||
| }; | ||
| actions: readonly string[]; |
There was a problem hiding this comment.
Suggestion: The new readonly array type makes beta.actions.sort() invalid because sort() mutates its receiver. The existing migration test and any similar callers must copy the array before sorting, or this contract should remain mutable where callers are expected to sort in place. [type error]
Severity Level: Major ⚠️
- ❌ Fastlane migration tests fail TypeScript checking.
- ⚠️ Migration CI cannot validate lane parsing behavior.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/core/types/migrate.ts
**Line:** 122:122
**Comment:**
*Type Error: The new readonly array type makes `beta.actions.sort()` invalid because `sort()` mutates its receiver. The existing migration test and any similar callers must copy the array before sorting, or this contract should remain mutable where callers are expected to sort in place.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| readonly name: string; | ||
| put(artifact: BuildArtifact): Effect.Effect<StoredArtifact, unknown>; | ||
| list(): Effect.Effect<BuildArtifact[], unknown>; | ||
| list(): Effect.Effect<readonly BuildArtifact[], unknown>; |
There was a problem hiding this comment.
Suggestion: StorageProvider.list() now returns readonly BuildArtifact[], but existing callers pass its result directly to findBuild and filterBuilds, whose parameters are BuildArtifact[]. This makes those build-history and run-command call sites type-incompatible until they are updated to accept readonly arrays or copy the result. [api mismatch]
Severity Level: Critical 🚨
- ❌ Build-history typechecking fails in multiple commands.
- ❌ Run and resign artifact selection cannot compile.
- ⚠️ Release commands share the same incompatible history type.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/core/types/providers.ts
**Line:** 96:96
**Comment:**
*Api Mismatch: `StorageProvider.list()` now returns `readonly BuildArtifact[]`, but existing callers pass its result directly to `findBuild` and `filterBuilds`, whose parameters are `BuildArtifact[]`. This makes those build-history and run-command call sites type-incompatible until they are updated to accept readonly arrays or copy the result.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| export type SnapshotContext = Readonly<{ | ||
| config: LaunchConfig; | ||
| apps: AppDescriptor[]; | ||
| apps: readonly AppDescriptor[]; |
There was a problem hiding this comment.
Suggestion: SnapshotContext.apps and RestoreContext.apps are now readonly arrays, but the existing iosApps and androidApps helpers require mutable AppDescriptor[]. Snapshot sources pass these context arrays directly to those helpers, so capture and restore code becomes type-incompatible until the helpers accept readonly arrays or callers copy the arrays. [api mismatch]
Severity Level: Critical 🚨
- ❌ Apple snapshot capture sources fail typechecking.
- ❌ Google Play snapshot capture sources fail typechecking.
- ⚠️ Snapshot capture command compilation is blocked.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/core/types/snapshot.ts
**Line:** 168:168
**Comment:**
*Api Mismatch: `SnapshotContext.apps` and `RestoreContext.apps` are now readonly arrays, but the existing `iosApps` and `androidApps` helpers require mutable `AppDescriptor[]`. Snapshot sources pass these context arrays directly to those helpers, so capture and restore code becomes type-incompatible until the helpers accept readonly arrays or callers copy the arrays.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Code Review by Qodo
1. Mutating readonly actions
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+13
to
+16
| export type ReconcileReport = Readonly<{ | ||
| bundleId: string; | ||
| actions: PlannedAction[]; | ||
| }; | ||
| actions: readonly PlannedAction[]; | ||
| }>; |
There was a problem hiding this comment.
1. Mutating readonly actions 🐞 Bug ≡ Correctness
ReconcileReport.actions is now a readonly array, but reconcileJob still does report.actions.push(...), which is a TypeScript error and prevents the current code from compiling. This also violates the intended “immutable report” contract by mutating a report object after creation.
Agent Prompt
### Issue description
`ReconcileReport.actions` was changed to `readonly PlannedAction[]`, but `src/core/store/syncRun.ts` mutates that array with `push`. This becomes a TypeScript compilation error and breaks the intended immutable-domain boundary.
### Issue Context
`reconcileJob` currently computes a `ReconcileReport` via `reconcileApp(...)` and then appends asset actions by mutating `report.actions`.
### Fix Focus Areas
- src/core/types/reconcile.ts[12-16]
- src/core/store/syncRun.ts[157-193]
### Suggested fix
Prefer keeping `ReconcileReport` immutable:
- In `reconcileJob`, replace the in-place `push` with a new report object:
- `const assetActions = yield* reconcileAssetActions(...)`
- `const mergedReport: ReconcileReport = { ...report, actions: [...report.actions, ...assetActions] }`
- return `{ job, report: mergedReport }`
(Alternative: if you want an internal mutable builder, introduce a separate mutable “builder” type and only expose `ReconcileReport` at the boundary.)
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+42
to
+47
| export type CapabilitySetting = Readonly<{ | ||
| key: string; | ||
| options?: { | ||
| options?: Readonly<{ | ||
| key: string; | ||
| }[]; | ||
| }; | ||
| }>[]; | ||
| }>; |
There was a problem hiding this comment.
2. Readonly arrays still mutable 🐞 Bug ⚙ Maintainability
Some newly-Readonly domain types still declare nested arrays as mutable (Readonly<T>[]), so callers can still push/splice these arrays even though the containing object is Readonly. This undermines the readonly-domain guarantee and makes it easier to accidentally mutate shared data structures.
Agent Prompt
### Issue description
A few types were converted to `Readonly<...>` but still expose mutable arrays via `Readonly<Element>[]`. That means the property is non-reassignable, but the array contents can still be mutated.
### Issue Context
This PR otherwise consistently switches many array properties to `readonly X[]`, suggesting the intent is deep immutability for domain types.
### Fix Focus Areas
- src/core/types/appleCatalog.ts[42-53]
- src/core/types/appleCatalog.ts[234-241]
- src/core/types/googlePlay.ts[3-14]
### Suggested fix
Change these fields to readonly arrays, e.g.:
- `options?: readonly { key: string }[]` (or `readonly Readonly<{ key: string }>[]`)
- `screenshots: readonly { url: string; width?: number; height?: number }[]`
- `releaseNotes?: readonly { language: string; text: string }[]`
- `countries: readonly { countryCode: string }[]`
Keep the element type `Readonly<...>` only if you specifically want element objects to be non-writable too.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (27)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
12 issues found across 27 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/core/types/storeSurface.ts">
<violation number="1" location="src/core/types/storeSurface.ts:19">
P2: `NotifyConfig.events` remains mutable despite this readonly-domain conversion, allowing callers to mutate the notification policy through `push`, `splice`, or index assignment. Making this field a readonly array would keep the public config types consistent and prevent accidental policy mutation.</violation>
</file>
<file name="src/core/types/googlePlay.ts">
<violation number="1" location="src/core/types/googlePlay.ts:8">
P2: These types still let callers mutate nested Play data despite the new readonly-domain contract: `releaseNotes`, `countries`, `offerTags`, `prices`, and `listings` can be changed in place. Using `readonly ...[]` and `Readonly<Record<string, ...>>` for these fields would make the returned resources consistently immutable.</violation>
</file>
<file name="src/core/types/readiness.ts">
<violation number="1" location="src/core/types/readiness.ts:61">
P2: The read-only readiness API still exposes mutable result arrays, so a probe can `push`, `sort`, or `splice` a collection returned by the store layer despite the API's read-only boundary. Declaring each list result as `readonly Readonly<{ ... }>[]` would prevent accidental mutation and match the readonly collections elsewhere in this file.</violation>
</file>
<file name="src/core/types/insights.ts">
<violation number="1" location="src/core/types/insights.ts:26">
P2: Consumers can still mutate a returned insights report through `ratings.distribution[...]`, `ratings.sentiment[...]`, or `byStore[...]`, so the new readonly boundary is only partial. Using `Readonly<Record<...>>` for the summary maps and `Readonly<Partial<Record<...>>>` for `byStore` would keep mutation local to aggregation.</violation>
</file>
<file name="src/core/types/snapshot.ts">
<violation number="1" location="src/core/types/snapshot.ts:110">
P3: The ASC snapshot reader still exposes mutable result arrays even though this type is introducing the read-only API boundary. Using `readonly Readonly<{ ... }>[]` for all four list results would prevent consumers from accidentally changing captured collections.</violation>
</file>
<file name="src/core/types/adopt.ts">
<violation number="1" location="src/core/types/adopt.ts:33">
P3: Entitlement lists remain mutable despite this type being converted to a readonly domain shape; using `readonly EntitlementValue[]` for the recursive array arm would prevent adopters from mutating imported entitlement data through the type.</violation>
</file>
<file name="src/core/types/appleCatalog.ts">
<violation number="1" location="src/core/types/appleCatalog.ts:44">
P3: Capability settings still expose a mutable `options` array, so consumers can mutate a value that this readonly-domain type is intended to protect. Mark the array itself readonly, consistent with `settings` and the other readonly collections in this file.</violation>
<violation number="2" location="src/core/types/appleCatalog.ts:236">
P3: Screenshot-feedback resources still expose a mutable `screenshots` array, weakening the readonly domain contract and allowing callers to alter the collection. Make the array readonly as well as its element type.</violation>
</file>
<file name="src/core/types/doctor.ts">
<violation number="1" location="src/core/types/doctor.ts:55">
P2: The doctor ASC surface still exposes a mutable capability list despite being documented as read-only, so callers can alter the returned domain collection. Mark the result array readonly as well.</violation>
</file>
<file name="src/core/types/mutable.ts">
<violation number="1" location="src/core/types/mutable.ts:2">
P3: The function branch re-emits `Return` without recursing, so MutableDeep is not actually deep for function/method return types: a method typed `() => Readonly<Foo>` stays `() => Readonly<Foo>`, and the produced "mutable" copy still fails when a caller mutates the returned value. Consider `(...args: Args) => MutableDeep<Return>` (guarding against self-referential returns) so the deep-mutability contract holds.</violation>
<violation number="2" location="src/core/types/mutable.ts:6">
P2: The `object` branch is unbounded and will also match class instances such as Date, Map, Set, RegExp, Promise, and Buffer, reducing them to plain `{ -readonly [Key in keyof ...] }` structural types. That strips their actual type identity and breaks downstream code expecting e.g. a `Map`/`Set`/`Date` value once MutableDeep is applied to a shape containing one. Constrain the guard to plain records (e.g. `Type extends Record<string | number | symbol, unknown>` after excluding arrays/indexables) or explicitly preserve known built-ins.</violation>
</file>
<file name="src/core/types/providers.ts">
<violation number="1" location="src/core/types/providers.ts:123">
P3: The Submitter doc comment now reads `Readonly<{@link BuildCredentials}>` and `Readonly<{@link SubmitTarget}>` — an artifact of a blanket find/replace that wraps the JSDoc `@link` tags in `Readonly<...>`. The `{@link ...}` references no longer resolve cleanly and the docs will render the literal `Readonly<`/`>` noise. Restore the plain `{@link BuildCredentials}` and `{@link SubmitTarget}` tags; the `Readonly` wrapper doesn't belong inside a doc comment.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Comment on lines
+19
to
+23
| export type NotifyConfig = Readonly<{ | ||
| webhookUrl?: string; | ||
| command?: string; | ||
| events?: Array<'build' | 'submit' | 'review' | 'rollout'>; | ||
| }; | ||
| }>; |
There was a problem hiding this comment.
P2: NotifyConfig.events remains mutable despite this readonly-domain conversion, allowing callers to mutate the notification policy through push, splice, or index assignment. Making this field a readonly array would keep the public config types consistent and prevent accidental policy mutation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/storeSurface.ts, line 19:
<comment>`NotifyConfig.events` remains mutable despite this readonly-domain conversion, allowing callers to mutate the notification policy through `push`, `splice`, or index assignment. Making this field a readonly array would keep the public config types consistent and prevent accidental policy mutation.</comment>
<file context>
@@ -7,22 +7,22 @@ import type {
+}>;
/** Transition notifications under `LaunchConfig.notify`. */
-export type NotifyConfig = {
+export type NotifyConfig = Readonly<{
webhookUrl?: string;
command?: string;
</file context>
Suggested change
| export type NotifyConfig = Readonly<{ | |
| webhookUrl?: string; | |
| command?: string; | |
| events?: Array<'build' | 'submit' | 'review' | 'rollout'>; | |
| }; | |
| }>; | |
| export type NotifyConfig = Readonly<{ | |
| webhookUrl?: string; | |
| command?: string; | |
| events?: readonly ('build' | 'submit' | 'review' | 'rollout')[]; | |
| }>; |
| @@ -1,77 +1,77 @@ | |||
| import type { PlayMoneyUnits } from './playPricing.js'; | |||
There was a problem hiding this comment.
P2: These types still let callers mutate nested Play data despite the new readonly-domain contract: releaseNotes, countries, offerTags, prices, and listings can be changed in place. Using readonly ...[] and Readonly<Record<string, ...>> for these fields would make the returned resources consistently immutable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/googlePlay.ts, line 8:
<comment>These types still let callers mutate nested Play data despite the new readonly-domain contract: `releaseNotes`, `countries`, `offerTags`, `prices`, and `listings` can be changed in place. Using `readonly ...[]` and `Readonly<Record<string, ...>>` for these fields would make the returned resources consistently immutable.</comment>
<file context>
@@ -1,77 +1,77 @@
-};
-export type PlayTrackInfo = { track: string; releases: PlayRelease[] };
-export type PlayCountryAvailability = {
+ releaseNotes?: Readonly<{ language: string; text: string }>[];
+}>;
+export type PlayTrackInfo = Readonly<{ track: string; releases: readonly PlayRelease[] }>;
</file context>
| Readonly<{ | ||
| id: string; | ||
| }[], | ||
| }>[], |
There was a problem hiding this comment.
P2: The read-only readiness API still exposes mutable result arrays, so a probe can push, sort, or splice a collection returned by the store layer despite the API's read-only boundary. Declaring each list result as readonly Readonly<{ ... }>[] would prevent accidental mutation and match the readonly collections elsewhere in this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/readiness.ts, line 61:
<comment>The read-only readiness API still exposes mutable result arrays, so a probe can `push`, `sort`, or `splice` a collection returned by the store layer despite the API's read-only boundary. Declaring each list result as `readonly Readonly<{ ... }>[]` would prevent accidental mutation and match the readonly collections elsewhere in this file.</comment>
<file context>
@@ -40,177 +40,177 @@ export type ProbeCheckResult = Effect.Effect<ProbeResult, unknown, ReadinessProb
+ Readonly<{
id: string;
- }[],
+ }>[],
unknown
>;
</file context>
| * set so callers never divide by zero or branch on emptiness mid-render. | ||
| */ | ||
| export type RatingSummary = { | ||
| export type RatingSummary = Readonly<{ |
There was a problem hiding this comment.
P2: Consumers can still mutate a returned insights report through ratings.distribution[...], ratings.sentiment[...], or byStore[...], so the new readonly boundary is only partial. Using Readonly<Record<...>> for the summary maps and Readonly<Partial<Record<...>>> for byStore would keep mutation local to aggregation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/insights.ts, line 26:
<comment>Consumers can still mutate a returned insights report through `ratings.distribution[...]`, `ratings.sentiment[...]`, or `byStore[...]`, so the new readonly boundary is only partial. Using `Readonly<Record<...>>` for the summary maps and `Readonly<Partial<Record<...>>>` for `byStore` would keep mutation local to aggregation.</comment>
<file context>
@@ -12,47 +12,47 @@ export type Sentiment = 'positive' | 'neutral' | 'negative';
* set so callers never divide by zero or branch on emptiness mid-render.
*/
-export type RatingSummary = {
+export type RatingSummary = Readonly<{
total: number;
average: number;
</file context>
Comment on lines
+55
to
+60
| listBundleIdCapabilities(bundleIdResourceId: string): Effect.Effect< | ||
| Readonly<{ | ||
| capabilityType: string; | ||
| }>[], | ||
| unknown | ||
| >; |
There was a problem hiding this comment.
P2: The doctor ASC surface still exposes a mutable capability list despite being documented as read-only, so callers can alter the returned domain collection. Mark the result array readonly as well.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/doctor.ts, line 55:
<comment>The doctor ASC surface still exposes a mutable capability list despite being documented as read-only, so callers can alter the returned domain collection. Mark the result array readonly as well.</comment>
<file context>
@@ -20,48 +20,49 @@ export type DoctorStatus = 'ok' | 'fail' | 'info';
+ }> | null,
+ unknown
+ >;
+ listBundleIdCapabilities(bundleIdResourceId: string): Effect.Effect<
+ Readonly<{
+ capabilityType: string;
</file context>
Suggested change
| listBundleIdCapabilities(bundleIdResourceId: string): Effect.Effect< | |
| Readonly<{ | |
| capabilityType: string; | |
| }>[], | |
| unknown | |
| >; | |
| listBundleIdCapabilities(bundleIdResourceId: string): Effect.Effect< | |
| readonly Readonly<{ | |
| capabilityType: string; | |
| }>[], | |
| unknown | |
| >; |
| | null | ||
| | EntitlementValue[] | ||
| | { | ||
| | Readonly<{ |
There was a problem hiding this comment.
P3: Entitlement lists remain mutable despite this type being converted to a readonly domain shape; using readonly EntitlementValue[] for the recursive array arm would prevent adopters from mutating imported entitlement data through the type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/adopt.ts, line 33:
<comment>Entitlement lists remain mutable despite this type being converted to a readonly domain shape; using `readonly EntitlementValue[]` for the recursive array arm would prevent adopters from mutating imported entitlement data through the type.</comment>
<file context>
@@ -30,104 +30,110 @@ export type EntitlementValue =
| null
| EntitlementValue[]
- | {
+ | Readonly<{
[key: string]: EntitlementValue;
- };
</file context>
| }; | ||
| export type BetaFeedbackScreenshotSubmissionResource = BetaFeedbackSubmissionResource & | ||
| Readonly<{ | ||
| screenshots: Readonly<{ |
There was a problem hiding this comment.
P3: Screenshot-feedback resources still expose a mutable screenshots array, weakening the readonly domain contract and allowing callers to alter the collection. Make the array readonly as well as its element type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/appleCatalog.ts, line 236:
<comment>Screenshot-feedback resources still expose a mutable `screenshots` array, weakening the readonly domain contract and allowing callers to alter the collection. Make the array readonly as well as its element type.</comment>
<file context>
@@ -74,184 +74,185 @@ export type SandboxTesterResource = {
-};
+export type BetaFeedbackScreenshotSubmissionResource = BetaFeedbackSubmissionResource &
+ Readonly<{
+ screenshots: Readonly<{
+ url: string;
+ width?: number;
</file context>
Suggested change
| screenshots: Readonly<{ | |
| screenshots: readonly Readonly<{ |
Comment on lines
+44
to
+46
| options?: Readonly<{ | ||
| key: string; | ||
| }[]; | ||
| }; | ||
| }>[]; |
There was a problem hiding this comment.
P3: Capability settings still expose a mutable options array, so consumers can mutate a value that this readonly-domain type is intended to protect. Mark the array itself readonly, consistent with settings and the other readonly collections in this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/appleCatalog.ts, line 44:
<comment>Capability settings still expose a mutable `options` array, so consumers can mutate a value that this readonly-domain type is intended to protect. Mark the array itself readonly, consistent with `settings` and the other readonly collections in this file.</comment>
<file context>
@@ -39,33 +39,33 @@ export type ProfileResource = {
+export type CapabilitySetting = Readonly<{
key: string;
- options?: {
+ options?: Readonly<{
key: string;
- }[];
</file context>
Suggested change
| options?: Readonly<{ | |
| key: string; | |
| }[]; | |
| }; | |
| }>[]; | |
| options?: readonly Readonly<{ | |
| key: string; | |
| }>[]; |
| @@ -0,0 +1,7 @@ | |||
| export type MutableDeep<Type> = Type extends (...args: infer Args) => infer Return | |||
| ? (...args: Args) => Return | |||
There was a problem hiding this comment.
P3: The function branch re-emits Return without recursing, so MutableDeep is not actually deep for function/method return types: a method typed () => Readonly<Foo> stays () => Readonly<Foo>, and the produced "mutable" copy still fails when a caller mutates the returned value. Consider (...args: Args) => MutableDeep<Return> (guarding against self-referential returns) so the deep-mutability contract holds.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/mutable.ts, line 2:
<comment>The function branch re-emits `Return` without recursing, so MutableDeep is not actually deep for function/method return types: a method typed `() => Readonly<Foo>` stays `() => Readonly<Foo>`, and the produced "mutable" copy still fails when a caller mutates the returned value. Consider `(...args: Args) => MutableDeep<Return>` (guarding against self-referential returns) so the deep-mutability contract holds.</comment>
<file context>
@@ -0,0 +1,7 @@
+export type MutableDeep<Type> = Type extends (...args: infer Args) => infer Return
+ ? (...args: Args) => Return
+ : Type extends readonly (infer Item)[]
+ ? MutableDeep<Item>[]
</file context>
Comment on lines
+123
to
+124
| * submits to a Play track via fastlane `supply`. Each narrows Readonly<{@link BuildCredentials}> to its platform | ||
| * and maps the neutral Readonly<{@link SubmitTarget}> onto its store's concept (Android also reads |
There was a problem hiding this comment.
P3: The Submitter doc comment now reads Readonly<{@link BuildCredentials}> and Readonly<{@link SubmitTarget}> — an artifact of a blanket find/replace that wraps the JSDoc @link tags in Readonly<...>. The {@link ...} references no longer resolve cleanly and the docs will render the literal Readonly</> noise. Restore the plain {@link BuildCredentials} and {@link SubmitTarget} tags; the Readonly wrapper doesn't belong inside a doc comment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/providers.ts, line 123:
<comment>The Submitter doc comment now reads `Readonly<{@link BuildCredentials}>` and `Readonly<{@link SubmitTarget}>` — an artifact of a blanket find/replace that wraps the JSDoc `@link` tags in `Readonly<...>`. The `{@link ...}` references no longer resolve cleanly and the docs will render the literal `Readonly<`/`>` noise. Restore the plain `{@link BuildCredentials}` and `{@link SubmitTarget}` tags; the `Readonly` wrapper doesn't belong inside a doc comment.</comment>
<file context>
@@ -120,19 +120,19 @@ export type StorageProviderResolver = Readonly<{
* `app-store-connect` submits to TestFlight/App Store via fastlane `pilot`/`deliver`; `google-play`
- * submits to a Play track via fastlane `supply`. Each narrows {@link BuildCredentials} to its platform
- * and maps the neutral {@link SubmitTarget} onto its store's concept (Android also reads
+ * submits to a Play track via fastlane `supply`. Each narrows Readonly<{@link BuildCredentials}> to its platform
+ * and maps the neutral Readonly<{@link SubmitTarget}> onto its store's concept (Android also reads
* `buildContext.android`).
</file context>
Suggested change
| * submits to a Play track via fastlane `supply`. Each narrows Readonly<{@link BuildCredentials}> to its platform | |
| * and maps the neutral Readonly<{@link SubmitTarget}> onto its store's concept (Android also reads | |
| * submits to a Play track via fastlane `supply`. Each narrows {@link BuildCredentials} to its platform | |
| * and maps the neutral {@link SubmitTarget} onto its store's concept (Android also reads |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack 1/12 of re-split HOLD #307
Domain:
typesBase:
mainFull green tip:
refactor/foundation/readonly-types-fullLand stack in order. Intermediate PRs may not typecheck alone.
Summary by cubic
Make core domain types readonly to enforce immutability and prevent accidental mutations. Adds a
MutableDeeputility for rare, intentional mutation; no runtime behavior changes.Refactors
Readonly<...>and arrays inreadonly T[]across core types (catalog, store surfaces, plans, providers, artifacts, listings, credentials, dashboards, readiness, reconcile, etc.).PlannedAction.descriptionandPlannedAction.destructivereadonly.MutableDeep<Type>helper (src/core/types/mutable.ts) to opt into deep mutability when needed.& {}).Migration
MutableDeep<T>or copy into a mutable local.readonlyarrays and avoid mutating methods.Written for commit a2c2a7d. Summary will update on new commits.