Skip to content

refactor(services): readonly-domain-types (stack 11/12, re-split #307) #384

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(services): readonly-domain-types (stack 11/12, re-split #307) #384

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
2 changes: 1 addition & 1 deletion src/core/services/appleCredentialsClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export type AppleCredentialsClient = Readonly<{
name: string,
bundleIdResourceId: string,
certificateId: string,
deviceIds: string[],
deviceIds: readonly string[],
profileType: 'IOS_APP_ADHOC' | 'TVOS_APP_ADHOC',
) => Effect.Effect<ProfileResource, unknown>;
readonly listBundleIdCapabilities: (
Expand Down
4 changes: 2 additions & 2 deletions src/core/services/artifactRetention.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import type { BuildArtifact, PruneOptions, PruneResult } from '../types/artifact

/** Artifact-index persistence and retention operations used by storage providers. */
export type ArtifactRetentionService = Readonly<{
readonly readIndex: (indexPath: string) => Effect.Effect<BuildArtifact[]>;
readonly readIndex: (indexPath: string) => Effect.Effect<readonly BuildArtifact[]>;
readonly writeIndex: (
artifactIndex: BuildArtifact[],
artifactIndex: readonly BuildArtifact[],
indexPath: string,
) => Effect.Effect<void, PlatformError>;
readonly prune: (
Comment on lines +13 to 18

@codeant-ai codeant-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.

Suggestion: The separate readIndex and writeIndex operations allow local storage uploads to perform an unsynchronized read-modify-write. Concurrent put effects can both read the same index and then overwrite each other, losing one artifact entry. Serialize index updates or provide an atomic append/update operation. [race condition]

Severity Level: Major ⚠️ 
- ⚠️ Concurrent local builds can lose history entries.
- ❌ `builds history` omits successfully stored artifacts.
- ⚠️ Retention decisions use incomplete artifact indexes.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/core/services/artifactRetention.ts
**Line:** 13:18
**Comment:**
	*Race Condition: The separate `readIndex` and `writeIndex` operations allow local storage uploads to perform an unsynchronized read-modify-write. Concurrent `put` effects can both read the same index and then overwrite each other, losing one artifact entry. Serialize index updates or provide an atomic append/update operation.

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
👍 | 👎

Expand Down
2 changes: 1 addition & 1 deletion src/core/services/localCredentialsStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type LocalCredentialsStoreService = Readonly<{
readonly loadAppleSigningAssets: (
keyId: string,
bundleId: string,
extensions?: string[],
extensions?: readonly string[],

@codeant-ai codeant-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.

Suggestion: The cache lookup now accepts extension bundle IDs, but the local credentials provider supplies only the statically configured app.iosExtensions. The build pipeline separately discovers extension targets from the generated Xcode project, so a project with an unconfigured/discovered extension can reuse signing assets without an extensionProfiles entry for that target; the generated export options then omits the extension profile and Xcode export fails. Pass the resolved extension set into credential resolution or make cache reuse validate all discovered targets before returning assets. [api mismatch]

Severity Level: Major ⚠️ 
- ❌ Local iOS builds can fail during signing export.
- ❌ Discovered extensions lack export profile mappings.
- ⚠️ Bare projects bypass static extension configuration.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/core/services/localCredentialsStore.ts
**Line:** 38:38
**Comment:**
	*Api Mismatch: The cache lookup now accepts extension bundle IDs, but the local credentials provider supplies only the statically configured `app.iosExtensions`. The build pipeline separately discovers extension targets from the generated Xcode project, so a project with an unconfigured/discovered extension can reuse signing assets without an `extensionProfiles` entry for that target; the generated export options then omits the extension profile and Xcode export fails. Pass the resolved extension set into credential resolution or make cache reuse validate all discovered targets before returning assets.

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
👍 | 👎

) => Effect.Effect<SigningAssets | null, unknown>;
readonly loadPlayServiceAccount: () => Effect.Effect<string | null, unknown>;
readonly loadAndroidKeystore: () => Effect.Effect<KeystoreAssets | null, unknown>;
Expand Down
4 changes: 2 additions & 2 deletions src/core/services/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ export type Logger = Readonly<{
readonly note: (message: string) => LogWrite;
readonly tip: (message: string) => LogWrite;
readonly notice: (lead: string, ...details: string[]) => LogWrite;
readonly box: (title: string, receiptLines: string[]) => LogWrite;
readonly shipped: (receiptLines: string[]) => LogWrite;
readonly box: (title: string, receiptLines: readonly string[]) => LogWrite;
readonly shipped: (receiptLines: readonly string[]) => LogWrite;
readonly line: (message: string) => LogWrite;
readonly gap: () => LogWrite;
}>;
Expand Down
8 changes: 6 additions & 2 deletions src/core/services/progress.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ const logStamp = (epochMilliseconds: number): string => {
* the full log on disk (falling back to the in-memory tail if it can't be read), since the real cause
* sometimes precedes the trailing lines.
*/
const reportFailure = (label: string, tail: string[], logFile: string) =>
const reportFailure = (label: string, tail: readonly string[], logFile: string) =>

@codeant-ai codeant-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.

Suggestion: The failure tail is populated from the raw child-process output, while build logs are only redacted when persisted. Reporting tail directly can therefore print API keys, bearer tokens, JWTs, or other secret-looking values to the terminal whenever a build fails. Redact each tail line before adding it to the failure report, especially when buildLog is active. [security]

Severity Level: Critical 🚨 
- ❌ Interactive failed builds can print raw credentials.
- ❌ Fastlane and Gradle build diagnostics may expose secrets.
- ⚠️ Persisted logs are redacted but terminal output is not.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/core/services/progress.ts
**Line:** 138:138
**Comment:**
	*Security: The failure tail is populated from the raw child-process output, while build logs are only redacted when persisted. Reporting `tail` directly can therefore print API keys, bearer tokens, JWTs, or other secret-looking values to the terminal whenever a build fails. Redact each tail line before adding it to the failure report, especially when `buildLog` is active.

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
👍 | 👎

@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.

P1: reportFailure prints the raw child-process tail directly to the terminal on build failure. Since build logs are only redacted when persisted to disk, any secret-looking values (API keys, bearer tokens, JWTs) present in the last lines of output will be echoed unredacted to the console. Consider redacting tail lines the same way persisted logs are redacted before including them in the failure report.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/core/services/progress.ts, line 138:

<comment>reportFailure prints the raw child-process tail directly to the terminal on build failure. Since build logs are only redacted when persisted to disk, any secret-looking values (API keys, bearer tokens, JWTs) present in the last lines of output will be echoed unredacted to the console. Consider redacting tail lines the same way persisted logs are redacted before including them in the failure report.</comment>

<file context>
@@ -135,7 +135,7 @@ const logStamp = (epochMilliseconds: number): string => {
  * sometimes precedes the trailing lines.
  */
-const reportFailure = (label: string, tail: string[], logFile: string) =>
+const reportFailure = (label: string, tail: readonly string[], logFile: string) =>
   Effect.gen(function* () {
     const lines = [
</file context>

Effect.gen(function* () {
const lines = [
`${label} failed. Last lines:`,
Expand All @@ -160,7 +160,11 @@ const reportFailure = (label: string, tail: string[], logFile: string) =>
* shows the live step from `parseStep` and a running clock; on failure the tail and log path are
* printed before the error propagates. In stream mode it is exactly {@link run} (inherited stdio).
*/
export const runWithProgress = (command: string, args: string[], options: ProgressRunOptions) =>
export const runWithProgress = (
command: string,
args: readonly string[],
options: ProgressRunOptions,
) =>
Effect.gen(function* () {
const { label, parseStep, ...progressCommandOptions } = options;
const commandOptions: {
Expand Down
2 changes: 1 addition & 1 deletion src/core/services/sandbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { SandboxTesterResource } from '../types/appleCatalog.js';
/** The exact slice of {@link AppStoreConnectClient} the sandbox domain depends on. */
export type AscSandboxApi = {
listSandboxTesters(): Effect.Effect<SandboxTesterResource[], unknown>;
clearSandboxTesterPurchaseHistory(testerIds: string[]): Effect.Effect<void, unknown>;
clearSandboxTesterPurchaseHistory(testerIds: readonly string[]): Effect.Effect<void, unknown>;
};
export type SandboxRequestFailure = Readonly<{
readonly _tag: 'SandboxRequestFailure';
Expand Down
2 changes: 1 addition & 1 deletion src/core/services/ssh.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export const rsyncUp = (
target: SshTarget,
localDir: string,
remoteDir: string,
excludes: string[],
excludes: readonly string[],
) => {
const sshCommand = ['ssh', ...sshFlags(target)].join(' ');
const args = ['-az', '--delete', '-e', sshCommand];
Expand Down
Loading