refactor(types): readonly domain shapes with MutableDeep builders - #307
refactor(types): readonly domain shapes with MutableDeep builders#307YosefHayim wants to merge 1 commit into
Conversation
Why: Domain types should be immutable at the boundary; local builders need an explicit mutable draft type instead of weakening public shapes. What: Wrap domain types in Readonly / readonly arrays; add MutableDeep; PlannedAction hybrid (readonly description/destructive, mutable status/error); update consumers, Apple/Google adapters, and builders to use MutableDeep drafts. Impact: Stricter types across core; no intentional runtime behavior change (finding 1).
|
Skipping CodeAnt AI review — this PR changes more than 100 files, which usually means a migration, codemod, or vendored drop. Line-level review on diffs this large produces duplicate findings on the same rewrite pattern and drowns out anything that actually matters. If you still want a review, comment |
|
Important Review skippedToo many files! This PR contains 144 files, which is 44 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (144)
You can disable this status message by setting the 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 |
|
PR Summary by QodoRefactor: make domain types readonly and add MutableDeep draft builders
AI Description
Diagram
High-Level Assessment
Files changed (145)
|
Code Review by Qodo
1. pickSpecEntry uses function declaration
|
| * skipping the macOS resource-fork sibling. Returns the matching entry or null when absent. | ||
| */ | ||
| export function pickSpecEntry(entries: string[]): string | null { | ||
| export function pickSpecEntry(entries: readonly string[]): string | null { |
There was a problem hiding this comment.
1. pickspecentry uses function declaration 📘 Rule violation ⚙ Maintainability
pickSpecEntry is a module-level exported function declaration instead of a const arrow function. This violates the code-style rule requiring module-level functions to be declared as const arrow functions before first use.
Agent Prompt
## Issue description
The module-level exported function `pickSpecEntry` is declared using `export function ...`, but the style rule requires module-level functions to be declared as `export const ... = (...) => {}` (const arrow) before first use.
## Issue Context
This is in `src/apple/generated/specPatch.ts`, and the PR modified the `pickSpecEntry` signature.
## Fix Focus Areas
- src/apple/generated/specPatch.ts[56-65]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
19 issues found across 145 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/adopt.ts">
<violation number="1" location="src/core/types/adopt.ts:33">
P2: `EntitlementValue` is still partially mutable because array values remain `EntitlementValue[]`. That undermines the new readonly boundary model and allows post-read mutation of entitlement collections.</violation>
</file>
<file name="src/google/playClient.ts">
<violation number="1" location="src/google/playClient.ts:856">
P2: The Google request builder is now fed an entire readonly domain object via a JSON clone, but the clone is not a type-safe conversion to `Schema$Subscription`: `JSON.parse` returns arbitrary runtime data and the generic annotation only asserts the result's type. If the domain shape contains fields whose wire representation differs from Google's generated DTO (or unsupported nested fields), this sends them unvalidated. A dedicated mutable request builder that explicitly maps the supported fields would preserve the public readonly boundary without weakening the generated request type.</violation>
</file>
<file name="src/core/types/migrate.ts">
<violation number="1" location="src/core/types/migrate.ts:38">
P2: `Record<string, string>` entries remain mutable inside the `Readonly<>` wrapper — `Readonly` is shallow and only protects the `env` property from reassignment, not individual key mutations (`profile.env!["key"] = "value"` still compiles). The codebase already uses `Readonly<Record<string, string>>` elsewhere (e.g. fastlane.ts:294, playTracks.ts:43). Wrap in `Readonly<Record<string, string>>` to match the boundary-immutability goal.</violation>
<violation number="2" location="src/core/types/migrate.ts:69">
P2: `Record<string, EasBuildProfile>` and `Record<string, EasSubmitProfile>` entries are still mutable inside `Readonly<EasJson>` — `build` and `submit` can't be reassigned, but their entries (`easJson.build["profile"] = {...}`) still compile. Since `EasJson` is a boundary shape consumed by the pipeline and report modules, wrap the Records to enforce full immutability.</violation>
</file>
<file name="src/core/types/doctor.ts">
<violation number="1" location="src/core/types/doctor.ts:46">
P2: `DoctorAscApi` still returns a mutable capabilities array: `Readonly<{ capabilityType: string }>[]` freezes the row shape but not the collection, so a doctor consumer can mutate the API result with `.push()` or index assignment. Since this is explicitly documented as a read-only App Store Connect surface, the return type should be `readonly Readonly<{ ... }>[]`.</violation>
<violation number="2" location="src/core/types/doctor.ts:74">
P3: `DoctorContext` is the only domain type in this file that is not fully wrapped in `Readonly<>`, breaking the PR convention. Only `apps: readonly AppDescriptor[]` was made readonly; `config`, `platform`, `os`, `cwd`, `androidSdk`, and `shellLocale` stay mutable. Wrapping the whole type in `Readonly<{…}>` would be consistent with `DoctorCheck`, `DoctorReport`, `DoctorPlayApi`, and `DoctorAscApi`. Builders needing mutable access can use `MutableDeep<DoctorContext<…>>` per the PR strategy.</violation>
</file>
<file name="src/core/types/catalog.ts">
<violation number="1" location="src/core/types/catalog.ts:99">
P3: The readonly config wrappers leave the Google Play price maps mutable. `introPrices`, subscription override `prices`, and product override `prices` are still `Record<...>` values, so callers can mutate a config after it has crossed the public boundary; these maps should be `Readonly<Record<string, PlayPriceConfig>>`.</violation>
</file>
<file name="src/core/types/googlePlay.ts">
<violation number="1" location="src/core/types/googlePlay.ts:8">
P2: Several Google Play domain collections remain mutable despite the readonly refactor. `releaseNotes`, `countries`, and both `offerTags` fields use `Readonly<element>[]`, which still permits `push`/`splice` on arrays returned to callers; they should use `readonly Readonly<element>[]`.</violation>
</file>
<file name="src/core/listing/apply.ts">
<violation number="1" location="src/core/listing/apply.ts:139">
P3: The `applyDraft` API documentation is now separated from its declaration by `mergeAppleLocale`, so editor/TypeScript documentation associates the wrong description or leaves `applyDraft` undocumented. Keeping the helper's JSDoc with the helper and moving the `applyDraft` comment immediately above `export const applyDraft` preserves accurate API documentation.</violation>
</file>
<file name="src/core/snapshot/sources/appleListing.ts">
<violation number="1" location="src/core/snapshot/sources/appleListing.ts:52">
P3: `isJsonObject` (appleListing.ts) and `isJsonRecord` (playRestore.ts) are structurally identical type-narrowing helpers for `JsonValue` → `Readonly<{ [key: string]: JsonValue }>`. Since both are introduced in this batch, consider deduplicating to keep the codebase consistent. If they need to stay separate for architectural boundaries (Apple vs Play), that's fine — just a heads-up that they're byte-for-byte the same logic.</violation>
</file>
<file name="src/core/types/readiness.ts">
<violation number="1" location="src/core/types/readiness.ts:61">
P2: The readiness API is still exposing mutable result arrays even though this contract is documented as read-only: every `list*` method uses `Readonly<{ ... }>[]`, which makes each row readonly but still allows callers to call `.push`, `.splice`, or assign an index on the returned array. This leaves the public boundary weaker than the new readonly domain shapes and is inconsistent with `AdoptCatalogApi`/`SnapshotAscApi`, which use `readonly T[]`; the array modifier should be added to each list return type.</violation>
</file>
<file name="src/core/types/storeSurface.ts">
<violation number="1" location="src/core/types/storeSurface.ts:56">
P3: The readonly surface/config wrappers do not protect their nested keyed maps. `localizations`, `clips`, `releaseNotes`, and `ageRating` remain mutable `Record` values, allowing callers to edit a config through an index assignment despite the new readonly boundary; these should use `Readonly<Record<...>>`.</violation>
</file>
<file name="src/core/release/betaReview.ts">
<violation number="1" location="src/core/release/betaReview.ts:132">
P3: The apply helper unnecessarily widens `PlannedAction` so its fixed `description` and `destructive` fields become writable. Since this helper only updates `status` and `error`, keeping the parameter as `PlannedAction` preserves the hybrid contract and still accepts the value returned by `plan`.</violation>
</file>
<file name="src/core/store/gameCenter.ts">
<violation number="1" location="src/core/store/gameCenter.ts:381">
P2: The Game Center apply path can rewrite an action's description after it has been planned, which defeats `PlannedAction`'s new readonly/fixed-description contract. Keeping this parameter as `PlannedAction` and carrying the no-version-id explanation separately (or determining the final description before creating the action) would preserve the domain invariant instead of widening the action to `MutableDeep<PlannedAction>`.</violation>
</file>
<file name="src/core/store/reconcile.ts">
<violation number="1" location="src/core/store/reconcile.ts:8">
P2: Reconcile contexts now expose actions with mutable `description` and `destructive` fields, so callers can change the identity of an action after it has been planned and the readonly domain contract is no longer enforced. Keeping the context as `PlannedAction[]` preserves mutability for `status`/`error` while preventing edits to the fixed fields.</violation>
<violation number="2" location="src/core/store/reconcile.ts:51">
P2: The exported `plan` helper weakens the public `PlannedAction` contract by returning `MutableDeep<PlannedAction>`; callers can mutate `description` and `destructive` instead of only the apply-path fields. A narrow action handle that exposes just `status`/`error` (and a separate way to represent the exceptional Game Center message) would preserve the readonly domain boundary.</violation>
</file>
<file name="src/core/types/appleCatalog.ts">
<violation number="1" location="src/core/types/appleCatalog.ts:44">
P2: Consumers can still mutate the nested capability options array (`resource.settings?.[0].options?.push(...)`) because `Readonly<{ ... }>[]` makes each element readonly but leaves the array itself mutable. This weakens the readonly domain boundary; the array should be declared as `readonly Readonly<{ key: string }>[]` (and the same correction is needed for `screenshots` below).</violation>
<violation number="2" location="src/core/types/appleCatalog.ts:128">
P3: The new `Readonly` wrapper does not make the dynamic listing fields map immutable. A caller can still execute `listing.fields['description'] = '...'`, so the returned domain resource can be changed through a nested map; the map should be typed as `Readonly<Record<string, string>>` (and the other dynamic attribute maps in this file should follow the same pattern).</violation>
<violation number="3" location="src/core/types/appleCatalog.ts:236">
P2: The screenshot collection is still mutable: `Readonly<{ ... }>[]` protects the screenshot elements but not the array, so callers can append or remove feedback attachments from a returned domain resource. Using a `readonly ...[]` collection preserves the intended immutable boundary.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| | null | ||
| | EntitlementValue[] | ||
| | { | ||
| | Readonly<{ |
There was a problem hiding this comment.
P2: EntitlementValue is still partially mutable because array values remain EntitlementValue[]. That undermines the new readonly boundary model and allows post-read mutation of entitlement collections.
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>`EntitlementValue` is still partially mutable because array values remain `EntitlementValue[]`. That undermines the new readonly boundary model and allows post-read mutation of entitlement collections.</comment>
<file context>
@@ -30,104 +30,110 @@ export type EntitlementValue =
| null
| EntitlementValue[]
- | {
+ | Readonly<{
[key: string]: EntitlementValue;
- };
</file context>
| productId: subscription.productId, | ||
| 'regionsVersion.version': REGIONS_VERSION, | ||
| requestBody: { ...subscription, packageName }, | ||
| requestBody: mutableGoogleRequest<androidpublisher_v3.Schema$Subscription>({ |
There was a problem hiding this comment.
P2: The Google request builder is now fed an entire readonly domain object via a JSON clone, but the clone is not a type-safe conversion to Schema$Subscription: JSON.parse returns arbitrary runtime data and the generic annotation only asserts the result's type. If the domain shape contains fields whose wire representation differs from Google's generated DTO (or unsupported nested fields), this sends them unvalidated. A dedicated mutable request builder that explicitly maps the supported fields would preserve the public readonly boundary without weakening the generated request type.
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 856:
<comment>The Google request builder is now fed an entire readonly domain object via a JSON clone, but the clone is not a type-safe conversion to `Schema$Subscription`: `JSON.parse` returns arbitrary runtime data and the generic annotation only asserts the result's type. If the domain shape contains fields whose wire representation differs from Google's generated DTO (or unsupported nested fields), this sends them unvalidated. A dedicated mutable request builder that explicitly maps the supported fields would preserve the public readonly boundary without weakening the generated request type.</comment>
<file context>
@@ -840,7 +853,10 @@ export class GooglePlayClient {
productId: subscription.productId,
'regionsVersion.version': REGIONS_VERSION,
- requestBody: { ...subscription, packageName },
+ requestBody: mutableGoogleRequest<androidpublisher_v3.Schema$Subscription>({
+ ...subscription,
+ packageName,
</file context>
| * only one of them (or neither) still migrates cleanly; `cli` is optional. | ||
| */ | ||
| export type EasJson = { | ||
| export type EasJson = Readonly<{ |
There was a problem hiding this comment.
P2: Record<string, EasBuildProfile> and Record<string, EasSubmitProfile> entries are still mutable inside Readonly<EasJson> — build and submit can't be reassigned, but their entries (easJson.build["profile"] = {...}) still compile. Since EasJson is a boundary shape consumed by the pipeline and report modules, wrap the Records to enforce full immutability.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/migrate.ts, line 69:
<comment>`Record<string, EasBuildProfile>` and `Record<string, EasSubmitProfile>` entries are still mutable inside `Readonly<EasJson>` — `build` and `submit` can't be reassigned, but their entries (`easJson.build["profile"] = {...}`) still compile. Since `EasJson` is a boundary shape consumed by the pipeline and report modules, wrap the Records to enforce full immutability.</comment>
<file context>
@@ -8,130 +8,130 @@ export type MigrationSource = 'eas' | 'fastlane';
* only one of them (or neither) still migrates cleanly; `cli` is optional.
*/
-export type EasJson = {
+export type EasJson = Readonly<{
cli?: EasCli;
build: Record<string, EasBuildProfile>;
</file context>
| * `channel`/`distribution`/`developmentClient` become report notes, `env` keys seed `.env.example`. | ||
| */ | ||
| export type EasBuildProfile = { | ||
| export type EasBuildProfile = Readonly<{ |
There was a problem hiding this comment.
P2: Record<string, string> entries remain mutable inside the Readonly<> wrapper — Readonly is shallow and only protects the env property from reassignment, not individual key mutations (profile.env!["key"] = "value" still compiles). The codebase already uses Readonly<Record<string, string>> elsewhere (e.g. fastlane.ts:294, playTracks.ts:43). Wrap in Readonly<Record<string, string>> to match the boundary-immutability goal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/types/migrate.ts, line 38:
<comment>`Record<string, string>` entries remain mutable inside the `Readonly<>` wrapper — `Readonly` is shallow and only protects the `env` property from reassignment, not individual key mutations (`profile.env!["key"] = "value"` still compiles). The codebase already uses `Readonly<Record<string, string>>` elsewhere (e.g. fastlane.ts:294, playTracks.ts:43). Wrap in `Readonly<Record<string, string>>` to match the boundary-immutability goal.</comment>
<file context>
@@ -8,130 +8,130 @@ export type MigrationSource = 'eas' | 'fastlane';
* `channel`/`distribution`/`developmentClient` become report notes, `env` keys seed `.env.example`.
*/
-export type EasBuildProfile = {
+export type EasBuildProfile = Readonly<{
channel?: string;
distribution?: string;
</file context>
| }; | ||
| export type PlayTrackInfo = { track: string; releases: PlayRelease[] }; | ||
| export type PlayCountryAvailability = { | ||
| releaseNotes?: Readonly<{ language: string; text: string }>[]; |
There was a problem hiding this comment.
P2: Several Google Play domain collections remain mutable despite the readonly refactor. releaseNotes, countries, and both offerTags fields use Readonly<element>[], which still permits push/splice on arrays returned to callers; they should use readonly Readonly<element>[].
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>Several Google Play domain collections remain mutable despite the readonly refactor. `releaseNotes`, `countries`, and both `offerTags` fields use `Readonly<element>[]`, which still permits `push`/`splice` on arrays returned to callers; they should use `readonly Readonly<element>[]`.</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>
| * 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. */ |
There was a problem hiding this comment.
P3: The applyDraft API documentation is now separated from its declaration by mergeAppleLocale, so editor/TypeScript documentation associates the wrong description or leaves applyDraft undocumented. Keeping the helper's JSDoc with the helper and moving the applyDraft comment immediately above export const applyDraft preserves accurate API documentation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/listing/apply.ts, line 139:
<comment>The `applyDraft` API documentation is now separated from its declaration by `mergeAppleLocale`, so editor/TypeScript documentation associates the wrong description or leaves `applyDraft` undocumented. Keeping the helper's JSDoc with the helper and moving the `applyDraft` comment immediately above `export const applyDraft` preserves accurate API documentation.</comment>
<file context>
@@ -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,
</file context>
| .map(([locale, fields]) => toEntity(locale, fields)); | ||
| }); | ||
| /** Narrow a captured {@link JsonValue} to a plain object (rejecting arrays and null). */ | ||
| const isJsonObject = ( |
There was a problem hiding this comment.
P3: isJsonObject (appleListing.ts) and isJsonRecord (playRestore.ts) are structurally identical type-narrowing helpers for JsonValue → Readonly<{ [key: string]: JsonValue }>. Since both are introduced in this batch, consider deduplicating to keep the codebase consistent. If they need to stay separate for architectural boundaries (Apple vs Play), that's fine — just a heads-up that they're byte-for-byte the same logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/snapshot/sources/appleListing.ts, line 52:
<comment>`isJsonObject` (appleListing.ts) and `isJsonRecord` (playRestore.ts) are structurally identical type-narrowing helpers for `JsonValue` → `Readonly<{ [key: string]: JsonValue }>`. Since both are introduced in this batch, consider deduplicating to keep the codebase consistent. If they need to stay separate for architectural boundaries (Apple vs Play), that's fine — just a heads-up that they're byte-for-byte the same logic.</comment>
<file context>
@@ -48,8 +48,20 @@ const captureListing = (
.map(([locale, fields]) => toEntity(locale, fields));
});
+/** Narrow a captured {@link JsonValue} to a plain object (rejecting arrays and null). */
+const isJsonObject = (
+ capturedNode: JsonValue,
+): capturedNode is Readonly<{ [key: string]: JsonValue }> => {
</file context>
| }>; | ||
| /** One App Clip's card metadata. */ | ||
| export type AppClipConfig = { | ||
| export type AppClipConfig = Readonly<{ |
There was a problem hiding this comment.
P3: The readonly surface/config wrappers do not protect their nested keyed maps. localizations, clips, releaseNotes, and ageRating remain mutable Record values, allowing callers to edit a config through an index assignment despite the new readonly boundary; these should use Readonly<Record<...>>.
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 56:
<comment>The readonly surface/config wrappers do not protect their nested keyed maps. `localizations`, `clips`, `releaseNotes`, and `ageRating` remain mutable `Record` values, allowing callers to edit a config through an index assignment despite the new readonly boundary; these should use `Readonly<Record<...>>`.</comment>
<file context>
@@ -32,66 +32,66 @@ export type AchievementConfig = {
+}>;
/** One App Clip's card metadata. */
-export type AppClipConfig = {
+export type AppClipConfig = Readonly<{
action?: (typeof APP_CLIP_ACTIONS)[number];
localizations?: Record<string, AppClipLocalizationConfig>;
</file context>
| /** Apply one note write while retaining per-action failures in the reconciliation report. */ | ||
| const applyNote = ( | ||
| action: PlannedAction, | ||
| action: MutableDeep<PlannedAction>, |
There was a problem hiding this comment.
P3: The apply helper unnecessarily widens PlannedAction so its fixed description and destructive fields become writable. Since this helper only updates status and error, keeping the parameter as PlannedAction preserves the hybrid contract and still accepts the value returned by plan.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/release/betaReview.ts, line 132:
<comment>The apply helper unnecessarily widens `PlannedAction` so its fixed `description` and `destructive` fields become writable. Since this helper only updates `status` and `error`, keeping the parameter as `PlannedAction` preserves the hybrid contract and still accepts the value returned by `plan`.</comment>
<file context>
@@ -128,7 +129,7 @@ const selectBuild = (
/** Apply one note write while retaining per-action failures in the reconciliation report. */
const applyNote = (
- action: PlannedAction,
+ action: MutableDeep<PlannedAction>,
noteWrite: Effect.Effect<void, unknown>,
): Effect.Effect<void> =>
</file context>
| action: MutableDeep<PlannedAction>, | |
| action: PlannedAction, |
| * against desired config is a plain key-by-key comparison. | ||
| */ | ||
| export type ListingLocalization = { | ||
| export type ListingLocalization = Readonly<{ |
There was a problem hiding this comment.
P3: The new Readonly wrapper does not make the dynamic listing fields map immutable. A caller can still execute listing.fields['description'] = '...', so the returned domain resource can be changed through a nested map; the map should be typed as Readonly<Record<string, string>> (and the other dynamic attribute maps in this file should follow the same pattern).
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 128:
<comment>The new `Readonly` wrapper does not make the dynamic listing fields map immutable. A caller can still execute `listing.fields['description'] = '...'`, so the returned domain resource can be changed through a nested map; the map should be typed as `Readonly<Record<string, string>>` (and the other dynamic attribute maps in this file should follow the same pattern).</comment>
<file context>
@@ -74,184 +74,185 @@ export type SandboxTesterResource = {
* against desired config is a plain key-by-key comparison.
*/
-export type ListingLocalization = {
+export type ListingLocalization = Readonly<{
id: string;
locale: string;
</file context>
Summary
Readonly/readonlyarrays at the boundary.MutableDeepfor local builder drafts (Apple/Google adapters, pipeline, store reconcilers).PlannedActionstays hybrid: readonlydescription/destructive, mutablestatus/errorfor apply paths.Review focus
Finding 1: immutable domain shapes without weakening public types via assertions.
Note: Intentionally excludes the thin-CLI facade removals and asyncPool deletion so those can land (or fail) independently.
Test plan
MutableDeeponly at builder boundaries, not on public domain returnsSummary by cubic
Make domain types immutable at the boundary and introduce
MutableDeepfor safe, local builder drafts. Tightens many APIs to usereadonlyarrays/records;PlannedActionstays hybrid. No runtime behavior changes.Refactors
Readonlyand switched params toreadonlyarrays across core (build pipeline, credentials, planning, snapshot, store reconcilers).MutableDeepand updated Apple/Google adapters, pipeline, and tests to build drafts mutably, then return readonly results.PlannedActionwith readonlydescription/destructiveand mutablestatus/error.Schema.mutableusage and normalized helpers to acceptreadonlyinputs.Migration
MutableDeep<Foo>for local builders and returnFoo(readonly) at boundaries.readonly T[]; callers can still pass normal arrays, but don’t mutate inside.PlannedAction.status/error; treat other fields as readonly.Written for commit b8b86e3. Summary will update on new commits.