From 75158fb7a016480232835af26cc501414a6d9302 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 20 Jun 2026 03:06:09 -0700 Subject: [PATCH 1/3] refactor(client-runtime): structure VCS action errors Co-authored-by: codex --- apps/web/src/state/sourceControlActions.ts | 56 ++++++-- .../src/state/vcsAction.test.ts | 116 +++++++++++++++ .../client-runtime/src/state/vcsAction.ts | 135 +++++++++++++----- 3 files changed, 259 insertions(+), 48 deletions(-) diff --git a/apps/web/src/state/sourceControlActions.ts b/apps/web/src/state/sourceControlActions.ts index 3f532739f25..297ae5717df 100644 --- a/apps/web/src/state/sourceControlActions.ts +++ b/apps/web/src/state/sourceControlActions.ts @@ -126,12 +126,6 @@ function resolveScope(scope: SourceControlActionScope) { }; } -function unavailableResult(message: string) { - return AsyncResult.failure( - Cause.fail(new VcsActionUnavailableError({ message })), - ); -} - export function useSourceControlActionRunning( scope: SourceControlActionScope, kinds: ReadonlyArray, @@ -149,7 +143,15 @@ export function useVcsInitAction(scope: SourceControlActionScope) { const action = useCallback(async () => { const target = resolveScope(scope); if (target === null) { - return unavailableResult("Git init is unavailable."); + return AsyncResult.failure( + Cause.fail( + new VcsActionUnavailableError({ + operation: "init", + environmentId: scope.environmentId, + cwd: scope.cwd, + }), + ), + ); } return init({ environmentId: target.environmentId, @@ -172,7 +174,15 @@ export function useVcsPullAction(scope: SourceControlActionScope) { const action = useCallback(async () => { const target = resolveScope(scope); if (target === null) { - return unavailableResult("Git pull is unavailable."); + return AsyncResult.failure( + Cause.fail( + new VcsActionUnavailableError({ + operation: "pull", + environmentId: scope.environmentId, + cwd: scope.cwd, + }), + ), + ); } return pull({ environmentId: target.environmentId, @@ -211,7 +221,15 @@ export function useGitStackedAction(scope: SourceControlActionScope) { onProgress?: (event: GitActionProgressEvent) => void; }) => { if (resolveScope(scope) === null) { - return unavailableResult("Git action is unavailable."); + return AsyncResult.failure( + Cause.fail( + new VcsActionUnavailableError({ + operation: "run_change_request", + environmentId: scope.environmentId, + cwd: scope.cwd, + }), + ), + ); } return runStackedAction({ actionId: input.actionId, @@ -257,7 +275,15 @@ export function useSourceControlPublishRepositoryAction(scope: SourceControlActi }) => { const target = resolveScope(scope); if (target === null) { - return unavailableResult("Repository publishing is unavailable."); + return AsyncResult.failure( + Cause.fail( + new VcsActionUnavailableError({ + operation: "publish_repository", + environmentId: scope.environmentId, + cwd: scope.cwd, + }), + ), + ); } return publishRepository({ environmentId: target.environmentId, @@ -286,7 +312,15 @@ export function usePreparePullRequestThreadAction(scope: SourceControlActionScop async (input: { reference: string; mode: "local" | "worktree"; threadId?: ThreadId }) => { const target = resolveScope(scope); if (target === null) { - return unavailableResult("Pull request thread preparation is unavailable."); + return AsyncResult.failure( + Cause.fail( + new VcsActionUnavailableError({ + operation: "prepare_pull_request_thread", + environmentId: scope.environmentId, + cwd: scope.cwd, + }), + ), + ); } return preparePullRequestThread({ environmentId: target.environmentId, diff --git a/packages/client-runtime/src/state/vcsAction.test.ts b/packages/client-runtime/src/state/vcsAction.test.ts index b2ac9507319..47bee5b0f27 100644 --- a/packages/client-runtime/src/state/vcsAction.test.ts +++ b/packages/client-runtime/src/state/vcsAction.test.ts @@ -7,6 +7,7 @@ import { describe, expect, it } from "@effect/vitest"; import * as Cause from "effect/Cause"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import { AsyncResult, Atom, AtomRegistry } from "effect/unstable/reactivity"; @@ -21,12 +22,16 @@ import { EMPTY_VCS_ACTION_STATE, getVcsActionTargetKey, normalizeVcsActionProgressEvent, + VcsActionMissingTerminalEventError, + VcsActionRemoteFailureError, + VcsActionUnavailableError, } from "./vcsAction.ts"; const actionId = "action-123"; const action = "commit_push" as const; const cwd = "/repo"; const environmentId = EnvironmentId.make("environment-1"); +const isVcsActionUnavailableError = Schema.is(VcsActionUnavailableError); const result: GitRunStackedActionResult = { action, branch: { @@ -254,6 +259,7 @@ describe("vcsActionState", () => { target, transportActionId, actionId, + action, onProgress: (event) => Effect.sync(() => { observed.push(event); @@ -266,6 +272,84 @@ describe("vcsActionState", () => { }), ); + it.effect("retains remote failure context in a distinct error", () => + Effect.gen(function* () { + const target = { environmentId, cwd }; + const transportActionId = createVcsActionTransportId(target, actionId); + const error = yield* consumeVcsActionProgress( + Stream.fromIterable([ + { + actionId: transportActionId, + action, + cwd, + kind: "action_failed", + phase: "push", + message: "The remote rejected the push.", + }, + ]), + { + target, + transportActionId, + actionId, + action, + onProgress: () => Effect.void, + }, + ).pipe(Effect.flip); + + expect(error).toBeInstanceOf(VcsActionRemoteFailureError); + expect(error).toMatchObject({ + actionId, + transportActionId, + action, + environmentId, + cwd, + phase: "push", + detail: "The remote rejected the push.", + }); + expect(error.message).toBe( + "Source control action 'commit_push' failed during push: The remote rejected the push.", + ); + }), + ); + + it.effect("reports a missing terminal event as a protocol failure", () => + Effect.gen(function* () { + const target = { environmentId, cwd }; + const transportActionId = createVcsActionTransportId(target, actionId); + const error = yield* consumeVcsActionProgress( + Stream.fromIterable([ + { + actionId: transportActionId, + action, + cwd, + kind: "phase_started", + phase: "commit", + label: "Committing...", + }, + ]), + { + target, + transportActionId, + actionId, + action, + onProgress: () => Effect.void, + }, + ).pipe(Effect.flip); + + expect(error).toBeInstanceOf(VcsActionMissingTerminalEventError); + expect(error).toMatchObject({ + actionId, + transportActionId, + action, + environmentId, + cwd, + }); + expect(error.message).toBe( + "Source control action 'commit_push' ended without a terminal result.", + ); + }), + ); + it("keys mutation ownership by environment and cwd", () => { const runtime = Atom.runtime(Layer.empty) as unknown as Atom.AtomRuntime< EnvironmentRegistry, @@ -286,6 +370,38 @@ describe("vcsActionState", () => { registry.dispose(); }); + it("retains the incomplete target and operation when tracking is unavailable", async () => { + const runtime = Atom.runtime(Layer.empty) as unknown as Atom.AtomRuntime< + EnvironmentRegistry, + never + >; + const manager = createVcsActionManager(runtime); + const registry = AtomRegistry.make(); + const result = await manager.track( + registry, + { environmentId, cwd: null }, + { operation: "pull", label: "Pulling latest changes" }, + async () => AsyncResult.success(undefined), + ); + + expect(AsyncResult.isFailure(result)).toBe(true); + if (AsyncResult.isFailure(result)) { + const error = Cause.squash(result.cause); + expect(error).toBeInstanceOf(VcsActionUnavailableError); + if (!isVcsActionUnavailableError(error)) { + throw error; + } + expect(error).toMatchObject({ + operation: "pull", + environmentId, + cwd: null, + }); + expect(error.message).toBe("Source control operation 'pull' is unavailable."); + } + + registry.dispose(); + }); + it("tracks finite mutations without letting an older completion clear newer state", async () => { const runtime = Atom.runtime(Layer.empty) as unknown as Atom.AtomRuntime< EnvironmentRegistry, diff --git a/packages/client-runtime/src/state/vcsAction.ts b/packages/client-runtime/src/state/vcsAction.ts index b06f5ac65bc..a88171aba36 100644 --- a/packages/client-runtime/src/state/vcsAction.ts +++ b/packages/client-runtime/src/state/vcsAction.ts @@ -1,10 +1,11 @@ import { EnvironmentId, type EnvironmentId as EnvironmentIdType, + GitActionProgressPhase, type GitActionProgressEvent, type GitRunStackedActionInput, type GitRunStackedActionResult, - type GitStackedAction, + GitStackedAction, WS_METHODS, } from "@t3tools/contracts"; import * as Cause from "effect/Cause"; @@ -24,16 +25,18 @@ import { } from "./runtime.ts"; import { vcsCommandScheduler } from "./vcsCommandScheduler.ts"; -export type VcsActionOperation = - | "refresh_status" - | "run_change_request" - | "pull" - | "switch_ref" - | "create_ref" - | "create_worktree" - | "init" - | "publish_repository" - | "prepare_pull_request_thread"; +export const VcsActionOperation = Schema.Literals([ + "refresh_status", + "run_change_request", + "pull", + "switch_ref", + "create_ref", + "create_worktree", + "init", + "publish_repository", + "prepare_pull_request_thread", +]); +export type VcsActionOperation = typeof VcsActionOperation.Type; export interface VcsActionState { readonly isRunning: boolean; @@ -77,16 +80,54 @@ export interface RunVcsStackedActionInput { export class VcsActionUnavailableError extends Schema.TaggedErrorClass()( "VcsActionUnavailableError", { - message: Schema.String, + operation: VcsActionOperation, + environmentId: Schema.NullOr(EnvironmentId), + cwd: Schema.NullOr(Schema.String), }, -) {} +) { + override get message(): string { + return `Source control operation '${this.operation.replaceAll("_", " ")}' is unavailable.`; + } +} -export class VcsActionExecutionError extends Schema.TaggedErrorClass()( - "VcsActionExecutionError", +export class VcsActionRemoteFailureError extends Schema.TaggedErrorClass()( + "VcsActionRemoteFailureError", { - message: Schema.String, + actionId: Schema.String, + transportActionId: Schema.String, + action: GitStackedAction, + environmentId: EnvironmentId, + cwd: Schema.String, + phase: Schema.NullOr(GitActionProgressPhase), + detail: Schema.String, }, -) {} +) { + override get message(): string { + const phase = this.phase === null ? "execution" : this.phase; + return `Source control action '${this.action}' failed during ${phase}: ${this.detail}`; + } +} + +export class VcsActionMissingTerminalEventError extends Schema.TaggedErrorClass()( + "VcsActionMissingTerminalEventError", + { + actionId: Schema.String, + transportActionId: Schema.String, + action: GitStackedAction, + environmentId: EnvironmentId, + cwd: Schema.String, + }, +) { + override get message(): string { + return `Source control action '${this.action}' ended without a terminal result.`; + } +} + +export const VcsActionExecutionError = Schema.Union([ + VcsActionRemoteFailureError, + VcsActionMissingTerminalEventError, +]); +export type VcsActionExecutionError = typeof VcsActionExecutionError.Type; export const EMPTY_VCS_ACTION_STATE = Object.freeze({ isRunning: false, @@ -200,6 +241,7 @@ export function consumeVcsActionProgress( readonly target: ResolvedVcsActionTarget; readonly transportActionId: string; readonly actionId: string; + readonly action: GitStackedAction; readonly onProgress: (event: GitActionProgressEvent) => Effect.Effect; }, ): Effect.Effect { @@ -226,15 +268,25 @@ export function consumeVcsActionProgress( return Effect.succeed(terminalEvent.result); } if (terminalEvent?.kind === "action_failed") { - return Effect.fail( - new VcsActionExecutionError({ - message: terminalEvent.message, + return Effect.fail( + new VcsActionRemoteFailureError({ + actionId: input.actionId, + transportActionId: input.transportActionId, + action: terminalEvent.action, + environmentId: input.target.environmentId, + cwd: input.target.cwd, + phase: terminalEvent.phase, + detail: terminalEvent.message, }), ); } - return Effect.fail( - new VcsActionExecutionError({ - message: "Source control action ended without a result.", + return Effect.fail( + new VcsActionMissingTerminalEventError({ + actionId: input.actionId, + transportActionId: input.transportActionId, + action: input.action, + environmentId: input.target.environmentId, + cwd: input.target.cwd, }), ); }), @@ -339,18 +391,25 @@ export function applyVcsActionProgressEvent( export function createVcsActionManager( runtime: Atom.AtomRuntime, ) { - const unavailableTargetKey = "vcs-action-target:unavailable"; const runStackedActionCommands = new Map< string, AtomCommand >(); - const getRunStackedActionCommand = (key: string) => { - const existing = runStackedActionCommands.get(key); + const getRunStackedActionCommand = (requestedTarget: VcsActionTarget) => { + const targetKey = getVcsActionTargetKey(requestedTarget); + const commandKey = + targetKey ?? + JSON.stringify([ + "vcs-action-target:unavailable", + requestedTarget.environmentId, + requestedTarget.cwd, + ]); + const existing = runStackedActionCommands.get(commandKey); if (existing !== undefined) { return existing; } - const target = key === unavailableTargetKey ? null : parseVcsActionTargetKey(key); - const stateAtom = target === null ? EMPTY_VCS_ACTION_ATOM : vcsActionStateAtom(key); + const target = targetKey === null ? null : parseVcsActionTargetKey(targetKey); + const stateAtom = targetKey === null ? EMPTY_VCS_ACTION_ATOM : vcsActionStateAtom(targetKey); const command = createRuntimeCommand< EnvironmentRegistry | R, E, @@ -358,14 +417,16 @@ export function createVcsActionManager( GitRunStackedActionResult, unknown >(runtime, { - label: `vcs-action:run-stacked:${key}`, + label: `vcs-action:run-stacked:${commandKey}`, scheduler: vcsCommandScheduler, - concurrency: { mode: "serial", key: () => key }, + concurrency: { mode: "serial", key: () => commandKey }, execute: (input: RunVcsStackedActionInput, registry) => { if (target === null) { return Effect.fail( new VcsActionUnavailableError({ - message: "Source control action is unavailable.", + operation: "run_change_request", + environmentId: requestedTarget.environmentId, + cwd: requestedTarget.cwd, }), ); } @@ -396,6 +457,7 @@ export function createVcsActionManager( target, transportActionId, actionId: input.actionId, + action: input.action, onProgress: (event) => Effect.sync(() => { const current = registry.get(stateAtom); @@ -427,7 +489,7 @@ export function createVcsActionManager( ); }, }); - runStackedActionCommands.set(key, command); + runStackedActionCommands.set(commandKey, command); return command; }; @@ -446,10 +508,7 @@ export function createVcsActionManager( return { stateAtom: getVcsActionStateAtom, - runStackedAction: (target: VcsActionTarget) => { - const key = getVcsActionTargetKey(target); - return getRunStackedActionCommand(key ?? unavailableTargetKey); - }, + runStackedAction: (target: VcsActionTarget) => getRunStackedActionCommand(target), track: async ( registry: AtomRegistry.AtomRegistry, target: VcsActionTarget, @@ -461,7 +520,9 @@ export function createVcsActionManager( return AsyncResult.failure( Cause.fail( new VcsActionUnavailableError({ - message: "Source control action is unavailable.", + operation: input.operation, + environmentId: target.environmentId, + cwd: target.cwd, }), ), ); From 8fadd1b9cd6958efa49f51532d5a7bed08af6766 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 20 Jun 2026 06:52:17 -0700 Subject: [PATCH 2/3] refactor(client-runtime): structure VCS target key errors Co-authored-by: codex --- .../src/state/vcsAction.test.ts | 22 +++++++++++++++ .../client-runtime/src/state/vcsAction.ts | 28 +++++++++++++++---- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/client-runtime/src/state/vcsAction.test.ts b/packages/client-runtime/src/state/vcsAction.test.ts index 47bee5b0f27..b47595a7a0c 100644 --- a/packages/client-runtime/src/state/vcsAction.test.ts +++ b/packages/client-runtime/src/state/vcsAction.test.ts @@ -22,8 +22,10 @@ import { EMPTY_VCS_ACTION_STATE, getVcsActionTargetKey, normalizeVcsActionProgressEvent, + parseVcsActionTargetKey, VcsActionMissingTerminalEventError, VcsActionRemoteFailureError, + VcsActionTargetKeyParseError, VcsActionUnavailableError, } from "./vcsAction.ts"; @@ -62,6 +64,26 @@ function progress(event: T): T { } describe("vcsActionState", () => { + it("preserves malformed target keys and their native cause", () => { + const key = "not-json"; + let error: unknown; + + try { + parseVcsActionTargetKey(key); + } catch (cause) { + error = cause; + } + + expect(error).toBeInstanceOf(VcsActionTargetKeyParseError); + expect(error).toMatchObject({ key, cause: expect.any(SyntaxError) }); + }); + + it("rejects invalid target key shapes", () => { + const key = JSON.stringify([environmentId]); + + expect(() => parseVcsActionTargetKey(key)).toThrowError(VcsActionTargetKeyParseError); + }); + it("projects phase and hook progress without owning the async operation", () => { const initial = beginVcsActionState({ operation: "run_change_request", diff --git a/packages/client-runtime/src/state/vcsAction.ts b/packages/client-runtime/src/state/vcsAction.ts index a88171aba36..9fa17cf58ff 100644 --- a/packages/client-runtime/src/state/vcsAction.ts +++ b/packages/client-runtime/src/state/vcsAction.ts @@ -123,6 +123,18 @@ export class VcsActionMissingTerminalEventError extends Schema.TaggedErrorClass< } } +export class VcsActionTargetKeyParseError extends Schema.TaggedErrorClass()( + "VcsActionTargetKeyParseError", + { + key: Schema.String, + cause: Schema.Defect(), + }, +) { + override get message(): string { + return `Invalid source control action target key: ${JSON.stringify(this.key)}.`; + } +} + export const VcsActionExecutionError = Schema.Union([ VcsActionRemoteFailureError, VcsActionMissingTerminalEventError, @@ -145,6 +157,9 @@ export const EMPTY_VCS_ACTION_STATE = Object.freeze({ const nowMs = (): number => DateTime.toEpochMillis(DateTime.nowUnsafe()); let nextLocalActionId = 0; +const decodeVcsActionTargetKey = Schema.decodeUnknownSync( + Schema.Tuple([EnvironmentId, Schema.String]), +); export const vcsActionStateAtom = Atom.family((key: string) => { return Atom.make(EMPTY_VCS_ACTION_STATE).pipe( @@ -165,12 +180,13 @@ export function getVcsActionTargetKey(target: VcsActionTarget): string | null { return JSON.stringify([target.environmentId, target.cwd]); } -function parseVcsActionTargetKey(key: string): ResolvedVcsActionTarget { - const [environmentId, cwd] = JSON.parse(key) as [string, string]; - return { - environmentId: EnvironmentId.make(environmentId), - cwd, - }; +export function parseVcsActionTargetKey(key: string): ResolvedVcsActionTarget { + try { + const [environmentId, cwd] = decodeVcsActionTargetKey(JSON.parse(key)); + return { environmentId, cwd }; + } catch (cause) { + throw new VcsActionTargetKeyParseError({ key, cause }); + } } export function getVcsActionStateAtom(target: VcsActionTarget) { From fa416780644f64bf59facd665dd064f3c849bcb5 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 20 Jun 2026 10:02:01 -0700 Subject: [PATCH 3/3] fix(client-runtime): sanitize VCS failure diagnostics Co-authored-by: codex --- .../src/state/vcsAction.test.ts | 21 +++++++++++-------- .../client-runtime/src/state/vcsAction.ts | 12 +++++------ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/client-runtime/src/state/vcsAction.test.ts b/packages/client-runtime/src/state/vcsAction.test.ts index b47595a7a0c..7e618535ad8 100644 --- a/packages/client-runtime/src/state/vcsAction.test.ts +++ b/packages/client-runtime/src/state/vcsAction.test.ts @@ -64,8 +64,8 @@ function progress(event: T): T { } describe("vcsActionState", () => { - it("preserves malformed target keys and their native cause", () => { - const key = "not-json"; + it("preserves malformed target key diagnostics and the native cause without copying the key", () => { + const key = "not-json-with-credential=do-not-log"; let error: unknown; try { @@ -75,7 +75,9 @@ describe("vcsActionState", () => { } expect(error).toBeInstanceOf(VcsActionTargetKeyParseError); - expect(error).toMatchObject({ key, cause: expect.any(SyntaxError) }); + expect(error).toMatchObject({ keyLength: key.length, cause: expect.any(SyntaxError) }); + expect(error).not.toHaveProperty("key"); + expect((error as Error).message).not.toContain(key); }); it("rejects invalid target key shapes", () => { @@ -294,10 +296,11 @@ describe("vcsActionState", () => { }), ); - it.effect("retains remote failure context in a distinct error", () => + it.effect("retains structural remote failure context without copying the remote payload", () => Effect.gen(function* () { const target = { environmentId, cwd }; const transportActionId = createVcsActionTransportId(target, actionId); + const remoteMessage = "The remote rejected the push with credential=do-not-log."; const error = yield* consumeVcsActionProgress( Stream.fromIterable([ { @@ -306,7 +309,7 @@ describe("vcsActionState", () => { cwd, kind: "action_failed", phase: "push", - message: "The remote rejected the push.", + message: remoteMessage, }, ]), { @@ -326,11 +329,11 @@ describe("vcsActionState", () => { environmentId, cwd, phase: "push", - detail: "The remote rejected the push.", + remoteMessageLength: remoteMessage.length, }); - expect(error.message).toBe( - "Source control action 'commit_push' failed during push: The remote rejected the push.", - ); + expect(error).not.toHaveProperty("detail"); + expect(error.message).toBe("Source control action 'commit_push' failed during push."); + expect(error.message).not.toContain(remoteMessage); }), ); diff --git a/packages/client-runtime/src/state/vcsAction.ts b/packages/client-runtime/src/state/vcsAction.ts index 9fa17cf58ff..8ae3219a243 100644 --- a/packages/client-runtime/src/state/vcsAction.ts +++ b/packages/client-runtime/src/state/vcsAction.ts @@ -99,12 +99,12 @@ export class VcsActionRemoteFailureError extends Schema.TaggedErrorClass()( "VcsActionTargetKeyParseError", { - key: Schema.String, + keyLength: Schema.Number, cause: Schema.Defect(), }, ) { override get message(): string { - return `Invalid source control action target key: ${JSON.stringify(this.key)}.`; + return `Invalid source control action target key (${this.keyLength} characters).`; } } @@ -185,7 +185,7 @@ export function parseVcsActionTargetKey(key: string): ResolvedVcsActionTarget { const [environmentId, cwd] = decodeVcsActionTargetKey(JSON.parse(key)); return { environmentId, cwd }; } catch (cause) { - throw new VcsActionTargetKeyParseError({ key, cause }); + throw new VcsActionTargetKeyParseError({ keyLength: key.length, cause }); } } @@ -292,7 +292,7 @@ export function consumeVcsActionProgress( environmentId: input.target.environmentId, cwd: input.target.cwd, phase: terminalEvent.phase, - detail: terminalEvent.message, + remoteMessageLength: terminalEvent.message.length, }), ); }