Skip to content

refactor(build): readonly-domain-types (stack 6/12, re-split #307) - #379

Closed
YosefHayim wants to merge 1 commit into
refactor/types/readonly-stack-05-storefrom
refactor/types/readonly-stack-06-build
Closed

refactor(build): readonly-domain-types (stack 6/12, re-split #307)#379
YosefHayim wants to merge 1 commit into
refactor/types/readonly-stack-05-storefrom
refactor/types/readonly-stack-06-build

Conversation

@YosefHayim

@YosefHayim YosefHayim commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Stack 6/12 of re-split HOLD #307

Domain: build
Base: refactor/types/readonly-stack-05-store
Full green tip: refactor/foundation/readonly-types-full

Land stack in order. Intermediate PRs may not typecheck alone.


Summary by cubic

Make the build module readonly-first to prevent accidental mutation and tighten API contracts. No runtime behavior changes; this is a types-only refactor.

  • Refactors
    • Switched many params and returns to readonly, e.g. artifact index read/write, build history filters, diagnoses, env excludes, signing targets, and distribution history.
    • Introduced MutableDeep for values we intentionally build up (app, buildContext, Android release, allocate request); now use copies when extending objects/arrays.
    • Updated pipeline to construct mutable drafts from config (e.g. app, env, iOS entitlements/extensions) while keeping inputs readonly.
    • Adjusted selectors and prompts to accept readonly lists (selectApp, account picker, extension bundle IDs).

Written for commit a6be215. Summary will update on new commits.

Review in cubic

@changeset-bot

changeset-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a6be215

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codeant-ai

codeant-ai Bot commented Aug 7, 2026

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR a6be215 Aug 07, 2026 · 11:06 11:08

@codeant-ai

codeant-ai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai Bot added the size:M This PR changes 30-99 lines, ignoring generated files label Aug 7, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

refactor(build): adapt build pipeline to readonly domain types

✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Update build-domain helpers to accept readonly arrays and string lists.
• Introduce MutableDeep drafts where build flow must mutate app/context objects.
• Tighten artifact retention and remote allocation inputs to avoid accidental mutation.
Diagram

graph TD
  RT(("Readonly domain types")) --> MD(("MutableDeep")) --> BP["Build pipeline"] --> AR["Artifact retention"] --> UH["Update history"]
  BP --> RH["Remote pipeline"]

  RT --> AR
  RT --> UH
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Make pipeline steps pure (no mutation)
  • ➕ Eliminates need for MutableDeep and manual copying
  • ➕ Easier to reason about data flow; fewer hidden side effects
  • ➖ Bigger refactor across many pipeline steps; not ideal mid-stack migration
  • ➖ May require widespread signature changes/returning updated contexts everywhere
2. Introduce a shared deep-clone-to-mutable helper
  • ➕ Removes repetitive field-by-field copying (e.g., AppDescriptor drafts)
  • ➕ Centralizes cloning semantics and reduces maintenance burden
  • ➖ Deep cloning can be expensive/unclear for non-plain objects
  • ➖ May hide which fields are intentionally copied vs omitted

Recommendation: Given this is an intermediate stack PR, the current approach (widen inputs to readonly and locally create MutableDeep drafts only where mutation is required) is a pragmatic, low-risk migration step. After the stack lands, consider either (a) pushing mutation to explicit return values (pure steps) or (b) introducing a single well-defined clone-to-mutable utility to reduce manual copying in prepareBuild.

Files changed (11) +61 / -28

Refactor (11) +61 / -28
appleTargets.tsAccept readonly signing readiness list +3/-1

Accept readonly signing readiness list

• Changes multiTargetSigningWarnings to take a readonly TargetSigningReadiness[] to match readonly-domain-types. Behavior is unchanged; this documents and enforces non-mutating usage.

src/core/build/appleTargets.ts

artifactRetention.tsMake artifact index APIs readonly and use MutableDeep for policy drafts +11/-6

Make artifact index APIs readonly and use MutableDeep for policy drafts

• Updates artifact index read/write/prune helpers to accept/return readonly BuildArtifact arrays. Introduces a MutableDeep policyInput draft to allow conditional assignment of optional prune filters without fighting readonly types.

src/core/build/artifactRetention.ts

buildDiagnostics.tsFormat diagnoses from readonly input +1/-1

Format diagnoses from readonly input

• Updates formatDiagnoses to accept readonly BuildDiagnosis[] so callers can pass immutable diagnosis lists.

src/core/build/buildDiagnostics.ts

buildHistoryCommand.tsMake build-history helpers accept readonly stored builds +2/-2

Make build-history helpers accept readonly stored builds

• Updates filterBuilds and findBuild to take readonly BuildArtifact[] inputs while still returning new filtered results.

src/core/build/buildHistoryCommand.ts

pipeline.tsCreate mutable drafts for app/context under readonly domain types +26/-6

Create mutable drafts for app/context under readonly domain types

• Introduces a MutableDeep<AppDescriptor> draftApp and MutableDeep<ResolvedBuildContext> buildContext so later pipeline steps can safely mutate local copies (e.g., stamping version) without mutating readonly domain objects. Clones env and optional nested structures (entitlements, extensions, release notes) to avoid sharing references.

src/core/build/pipeline.ts

pipelineEnv.tsUpdate env selection/validation to accept readonly lists +3/-3

Update env selection/validation to accept readonly lists

• Changes selectApp to accept readonly AppDescriptor[] and updates envExclude/exclude parameters to readonly string[] for compatibility with readonly config types.

src/core/build/pipelineEnv.ts

pipelineSigning.tsUse readonly inputs for account and extension lists +6/-3

Use readonly inputs for account and extension lists

• Updates pickAccount to accept readonly AccountRecord[] and adjusts extension bundle-id handling to treat configured extensions as readonly. Ensures warning logic accepts readonly extension arrays.

src/core/build/pipelineSigning.ts

pipelineTypes.tsPreparedBuild now carries mutable drafts +3/-2

PreparedBuild now carries mutable drafts

• Updates PreparedBuild to use MutableDeep<AppDescriptor> and MutableDeep<ResolvedBuildContext>, reflecting that the pipeline mutates these during execution even if the source domain types are readonly.

src/core/build/pipelineTypes.ts

pipelineVersion.tsAllow version stamping on mutable app draft +3/-2

Allow version stamping on mutable app draft

• Updates applyChosenVersion and resolveMarketingVersion to accept MutableDeep<AppDescriptor> since they mutate app.version as part of stamping and reporting.

src/core/build/pipelineVersion.ts

remotePipeline.tsBuild AllocateRequest via MutableDeep draft +2/-1

Build AllocateRequest via MutableDeep draft

• Changes the host allocation request object to MutableDeep<AllocateRequest> to allow conditional assignment of aws/ssh fields under readonly domain typing.

src/core/build/remotePipeline.ts

updateHistory.tsWrite update history from readonly entries +1/-1

Write update history from readonly entries

• Updates writeHistory to accept a readonly UpdateHistoryEntry[] so callers can provide immutable history lists without copying.

src/core/distribution/updateHistory.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 11 files

Re-trigger cubic

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 76562dde-62df-4b5e-8e2a-fad2ef97e6d6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Readonly envExclude type mismatch 🐞 Bug ≡ Correctness
Description
resolveCommandEnv/validateResolvedEnv now accept readonly string[] but forward those values
into resolveEnv/missingKeys, which still require mutable string[], causing TypeScript
assignability errors at the call sites.
Code

src/core/build/pipelineEnv.ts[101]

+  envExclude?: readonly string[] | undefined;
Evidence
pipelineEnv now accepts readonly string[] and forwards it to env helpers that still declare
mutable string[] parameters, which is not assignable in TypeScript.

src/core/build/pipelineEnv.ts[96-123]
src/core/build/pipelineEnv.ts[133-141]
src/core/config/env.ts[46-55]
src/core/config/env.ts[64-79]
src/core/config/env.ts[133-142]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`src/core/build/pipelineEnv.ts` now types `envExclude`/`exclude` as `readonly string[]`, but the env helper APIs it calls still require mutable `string[]` (`resolveEnv` and `missingKeys`). This creates a TS assignability failure because `readonly string[]` is not assignable to `string[]`.

### Issue Context
The helper implementations appear to only iterate over the patterns and do not mutate them, so they can safely accept `readonly string[]`.

### Fix Focus Areas
- src/core/config/env.ts[46-55]
- src/core/config/env.ts[64-79]
- src/core/config/env.ts[133-142]
- src/core/build/pipelineEnv.ts[96-123]
- src/core/build/pipelineEnv.ts[133-141]

### Suggested fix
- Change `isEnvExcluded(name, patterns)` to accept `readonly string[]`.
- Change `missingKeys(..., excludedPatterns)` to accept `readonly string[]`.
- Change `ResolveEnvInput.envExclude` to `readonly string[] | undefined` and adjust any internal locals accordingly.
- Keep the pipeline call sites passing `config.envExclude` / `input.envExclude` directly (no copies needed).

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


2. Readonly extensions type mismatch 🐞 Bug ≡ Correctness
Description
warnUnreadySigningTargets now types extensions as readonly string[] but passes it to
gatherTargetSigningReadiness, which still expects mutable string[], causing a TypeScript
assignability error.
Code

src/core/build/pipelineSigning.ts[122]

+  extensions: readonly string[],
Evidence
warnUnreadySigningTargets now receives a readonly extensions array and forwards it to a callee
that still declares a mutable string[] parameter.

src/core/build/pipelineSigning.ts[118-133]
src/core/credentials/signingPreflight.ts[59-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`src/core/build/pipelineSigning.ts` changed `warnUnreadySigningTargets(..., extensions)` to `readonly string[]`, but it forwards `extensions` directly into `gatherTargetSigningReadiness`, whose signature still requires `string[]`. This is a TS assignability failure.

### Issue Context
`gatherTargetSigningReadiness` only maps over `extensions` and does not mutate it, so it can safely accept `readonly string[]`.

### Fix Focus Areas
- src/core/build/pipelineSigning.ts[118-133]
- src/core/credentials/signingPreflight.ts[59-69]

### Suggested fix
- Update `gatherTargetSigningReadiness` signature to `extensions: readonly string[]`.
- Ensure any internal uses (e.g. `extensions.map(...)`) remain valid.
- Avoid fixing this by copying at the call site unless the callee truly needs mutation.

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


Grey Divider

Context used
✅ Compliance rules (platform): 48 rules

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

cliEnv?: Record<string, string> | undefined;
includeLocal?: boolean | undefined;
envExclude?: string[] | undefined;
envExclude?: readonly string[] | undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Readonly envexclude type mismatch 🐞 Bug ≡ Correctness

resolveCommandEnv/validateResolvedEnv now accept readonly string[] but forward those values
into resolveEnv/missingKeys, which still require mutable string[], causing TypeScript
assignability errors at the call sites.
Agent Prompt
### Issue description
`src/core/build/pipelineEnv.ts` now types `envExclude`/`exclude` as `readonly string[]`, but the env helper APIs it calls still require mutable `string[]` (`resolveEnv` and `missingKeys`). This creates a TS assignability failure because `readonly string[]` is not assignable to `string[]`.

### Issue Context
The helper implementations appear to only iterate over the patterns and do not mutate them, so they can safely accept `readonly string[]`.

### Fix Focus Areas
- src/core/config/env.ts[46-55]
- src/core/config/env.ts[64-79]
- src/core/config/env.ts[133-142]
- src/core/build/pipelineEnv.ts[96-123]
- src/core/build/pipelineEnv.ts[133-141]

### Suggested fix
- Change `isEnvExcluded(name, patterns)` to accept `readonly string[]`.
- Change `missingKeys(..., excludedPatterns)` to accept `readonly string[]`.
- Change `ResolveEnvInput.envExclude` to `readonly string[] | undefined` and adjust any internal locals accordingly.
- Keep the pipeline call sites passing `config.envExclude` / `input.envExclude` directly (no copies needed).

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

app: AppDescriptor,
bundleId: string,
extensions: string[],
extensions: readonly string[],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Readonly extensions type mismatch 🐞 Bug ≡ Correctness

warnUnreadySigningTargets now types extensions as readonly string[] but passes it to
gatherTargetSigningReadiness, which still expects mutable string[], causing a TypeScript
assignability error.
Agent Prompt
### Issue description
`src/core/build/pipelineSigning.ts` changed `warnUnreadySigningTargets(..., extensions)` to `readonly string[]`, but it forwards `extensions` directly into `gatherTargetSigningReadiness`, whose signature still requires `string[]`. This is a TS assignability failure.

### Issue Context
`gatherTargetSigningReadiness` only maps over `extensions` and does not mutate it, so it can safely accept `readonly string[]`.

### Fix Focus Areas
- src/core/build/pipelineSigning.ts[118-133]
- src/core/credentials/signingPreflight.ts[59-69]

### Suggested fix
- Update `gatherTargetSigningReadiness` signature to `extensions: readonly string[]`.
- Ensure any internal uses (e.g. `extensions.map(...)`) remain valid.
- Avoid fixing this by copying at the call site unless the callee truly needs mutation.

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

@YosefHayim

Copy link
Copy Markdown
Owner Author

Superseded by land of tip stack #386 (same 12 domain commits).

@YosefHayim YosefHayim closed this Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant