[codex] Refactor shared and SSH Effect services#3206
Open
juliusmarminge wants to merge 5 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
Contributor
|
🚀 Expo continuous deployment is ready!
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Needs human review Major refactor of error handling infrastructure across SSH and shared packages with behavioral changes to authentication failure detection and error message presentation. The scope and runtime impact warrant human review. You can customize Macroscope's approvability policy. Learn more. |
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
2 times, most recently
from
0b6d15f to
b04704c
Compare
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
2 times, most recently
from
7106351 to
a4af44e
Compare
github-actions
Bot
added
size:XXL
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
from
a4af44e to
91f873b
Compare
macroscopeapp
Bot
dismissed
their stale review
Dismissing prior approval to re-evaluate 91f873b
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
4 times, most recently
from
470e248 to
c20caa0
Compare
macroscopeapp
Bot
dismissed
their stale review
Dismissing prior approval to re-evaluate c20caa0
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
from
c20caa0 to
6999c3a
Compare
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
4 times, most recently
from
799b906 to
bd827ee
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: SSH cancel shows failed message
- Changed the type guard to narrow to SshPasswordPromptRequestError and updated ensureSshEnvironment to forward error.cause.message (the specific cancellation/timeout text) instead of error.message (the generic 'SSH authentication failed' wrapper message).
Or push these changes by commenting:
@cursor push cde610b458
Preview (cde610b458)
diff --git a/apps/desktop/src/ipc/methods/sshEnvironment.test.ts b/apps/desktop/src/ipc/methods/sshEnvironment.test.ts
--- a/apps/desktop/src/ipc/methods/sshEnvironment.test.ts
+++ b/apps/desktop/src/ipc/methods/sshEnvironment.test.ts
@@ -5,13 +5,18 @@
import * as Exit from "effect/Exit";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
-import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http";
+import * as Schema from "effect/Schema";
+import * as HttpClient from "effect/unstable/http/HttpClient";
+import * as HttpClientRequest from "effect/unstable/http/HttpClientRequest";
+import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse";
import {
DesktopSshEnvironmentRequestError,
fetchSshEnvironmentDescriptor,
} from "./sshEnvironment.ts";
+const isSshHttpBridgeError = Schema.is(SshHttpBridgeError);
+
function jsonResponse(request: HttpClientRequest.HttpClientRequest, body: unknown, status = 200) {
return HttpClientResponse.fromWeb(
request,
@@ -83,7 +88,7 @@
assert.instanceOf(error, DesktopSshEnvironmentRequestError);
assert.equal(error.operation, "fetch-environment-descriptor");
- assert.equal(error.cause instanceof SshHttpBridgeError, false);
+ assert.equal(isSshHttpBridgeError(error.cause), false);
}).pipe(Effect.provide(layer));
});
@@ -108,7 +113,7 @@
const error = failure.value;
assert.instanceOf(error, DesktopSshEnvironmentRequestError);
- assert.instanceOf(error.cause, SshHttpBridgeError);
+ assert.equal(isSshHttpBridgeError(error.cause), true);
assert.equal(requestCount, 0);
}).pipe(Effect.provide(layer));
});
diff --git a/apps/desktop/src/ipc/methods/sshEnvironment.ts b/apps/desktop/src/ipc/methods/sshEnvironment.ts
--- a/apps/desktop/src/ipc/methods/sshEnvironment.ts
+++ b/apps/desktop/src/ipc/methods/sshEnvironment.ts
@@ -50,14 +50,13 @@
const isEnvironmentOperationForbiddenError = Schema.is(EnvironmentOperationForbiddenError);
const isEnvironmentRequestInvalidError = Schema.is(EnvironmentRequestInvalidError);
const isEnvironmentScopeRequiredError = Schema.is(EnvironmentScopeRequiredError);
+const isSshHttpBridgeError = Schema.is(SshHttpBridgeError);
function readSshHttpStatus(cause: DesktopSshEnvironmentRequestCause): number | null {
- if (
- cause instanceof RemoteEnvironmentAuthUndeclaredStatusError ||
- cause instanceof SshHttpBridgeError
- ) {
+ if (cause instanceof RemoteEnvironmentAuthUndeclaredStatusError) {
return cause.status ?? null;
}
+ if (isSshHttpBridgeError(cause)) return null;
if (isEnvironmentRequestInvalidError(cause)) {
return 400;
}
@@ -131,7 +130,7 @@
DesktopSshEnvironment.isDesktopSshPasswordPromptCancellation(error)
? Effect.succeed({
type: DesktopSshPasswordPromptCancelledType,
- message: error.message,
+ message: error.cause instanceof Error ? error.cause.message : error.message,
})
: Effect.fail(error),
),
diff --git a/apps/desktop/src/ssh/DesktopSshEnvironment.test.ts b/apps/desktop/src/ssh/DesktopSshEnvironment.test.ts
--- a/apps/desktop/src/ssh/DesktopSshEnvironment.test.ts
+++ b/apps/desktop/src/ssh/DesktopSshEnvironment.test.ts
@@ -2,7 +2,7 @@
import * as NodeServices from "@effect/platform-node/NodeServices";
import { assert, describe, it } from "@effect/vitest";
import * as NetService from "@t3tools/shared/Net";
-import { SshPasswordPromptError } from "@t3tools/ssh/errors";
+import { SshPasswordPromptRequestError } from "@t3tools/ssh/errors";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
@@ -20,18 +20,18 @@
describe("sshEnvironment", () => {
it("treats password prompt timeouts as cancellable authentication prompts", () => {
- assert.equal(
- DesktopSshEnvironment.isDesktopSshPasswordPromptCancellation(
- new SshPasswordPromptError({
- message: "SSH authentication timed out for devbox.",
- cause: new DesktopSshPasswordPrompts.DesktopSshPromptTimedOutError({
- requestId: "prompt-1",
- destination: "devbox",
- }),
- }),
- ),
- true,
- );
+ const cause = new DesktopSshPasswordPrompts.DesktopSshPromptTimedOutError({
+ requestId: "prompt-1",
+ destination: "devbox",
+ });
+ const error = new SshPasswordPromptRequestError({
+ destination: "devbox",
+ cause,
+ });
+ assert.strictEqual(error.cause, cause);
+ assert.equal(error.message, "SSH authentication failed for devbox.");
+ assert.notInclude(error.message, cause.message);
+ assert.equal(DesktopSshEnvironment.isDesktopSshPasswordPromptCancellation(error), true);
});
it.effect("wires desktop host discovery through the ssh package runtime", () =>
diff --git a/apps/desktop/src/ssh/DesktopSshEnvironment.ts b/apps/desktop/src/ssh/DesktopSshEnvironment.ts
--- a/apps/desktop/src/ssh/DesktopSshEnvironment.ts
+++ b/apps/desktop/src/ssh/DesktopSshEnvironment.ts
@@ -4,32 +4,33 @@
DesktopSshEnvironmentTarget,
} from "@t3tools/contracts";
import * as NetService from "@t3tools/shared/Net";
-import {
- SshPasswordPrompt,
- type SshPasswordPromptShape,
- type SshPasswordRequest,
-} from "@t3tools/ssh/auth";
+import * as SshAuth from "@t3tools/ssh/auth";
import { discoverSshHosts } from "@t3tools/ssh/config";
import {
- SshCommandError,
- SshHostDiscoveryError,
- SshInvalidTargetError,
- SshLaunchError,
- SshPairingError,
+ type SshCommandError,
+ type SshHostDiscoveryError,
+ type SshInvalidTargetError,
+ type SshLaunchError,
+ type SshPairingError,
SshPasswordPromptError,
- SshReadinessError,
+ SshPasswordPromptRequestError,
+ type SshReadinessError,
} from "@t3tools/ssh/errors";
-import { SshEnvironmentManager, type RemoteT3RunnerOptions } from "@t3tools/ssh/tunnel";
+import * as SshTunnel from "@t3tools/ssh/tunnel";
import * as Context from "effect/Context";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
import * as Path from "effect/Path";
-import { HttpClient } from "effect/unstable/http";
-import { ChildProcessSpawner } from "effect/unstable/process";
+import * as Schema from "effect/Schema";
+import * as HttpClient from "effect/unstable/http/HttpClient";
+import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner";
import * as DesktopSshPasswordPrompts from "./DesktopSshPasswordPrompts.ts";
+const isSshPasswordPromptError = Schema.is(SshPasswordPromptError);
+const isSshPasswordPromptRequestError = Schema.is(SshPasswordPromptRequestError);
+
export type DesktopSshEnvironmentRuntimeServices =
| ChildProcessSpawner.ChildProcessSpawner
| FileSystem.FileSystem
@@ -52,27 +53,25 @@
| DesktopSshEnvironmentDiscoverError
| DesktopSshEnvironmentOperationError;
-export interface DesktopSshEnvironmentShape {
- readonly discoverHosts: (input?: {
- readonly homeDir?: string;
- }) => Effect.Effect<readonly DesktopDiscoveredSshHost[], DesktopSshEnvironmentDiscoverError>;
- readonly ensureEnvironment: (
- target: DesktopSshEnvironmentTarget,
- options?: { readonly issuePairingToken?: boolean },
- ) => Effect.Effect<DesktopSshEnvironmentBootstrap, DesktopSshEnvironmentOperationError>;
- readonly disconnectEnvironment: (
- target: DesktopSshEnvironmentTarget,
- ) => Effect.Effect<void, DesktopSshEnvironmentOperationError>;
-}
-
export class DesktopSshEnvironment extends Context.Service<
DesktopSshEnvironment,
- DesktopSshEnvironmentShape
+ {
+ readonly discoverHosts: (input?: {
+ readonly homeDir?: string;
+ }) => Effect.Effect<readonly DesktopDiscoveredSshHost[], DesktopSshEnvironmentDiscoverError>;
+ readonly ensureEnvironment: (
+ target: DesktopSshEnvironmentTarget,
+ options?: { readonly issuePairingToken?: boolean },
+ ) => Effect.Effect<DesktopSshEnvironmentBootstrap, DesktopSshEnvironmentOperationError>;
+ readonly disconnectEnvironment: (
+ target: DesktopSshEnvironmentTarget,
+ ) => Effect.Effect<void, DesktopSshEnvironmentOperationError>;
+ }
>()("@t3tools/desktop/ssh/DesktopSshEnvironment") {}
export interface DesktopSshEnvironmentLayerOptions {
readonly resolveCliPackageSpec?: () => string;
- readonly resolveCliRunner?: Effect.Effect<RemoteT3RunnerOptions>;
+ readonly resolveCliRunner?: Effect.Effect<SshTunnel.RemoteT3RunnerOptions>;
}
function discoverDesktopSshHostsEffect(input?: { readonly homeDir?: string }) {
@@ -81,34 +80,35 @@
export function isDesktopSshPasswordPromptCancellation(
error: unknown,
-): error is SshPasswordPromptError {
+): error is SshPasswordPromptRequestError {
return (
- error instanceof SshPasswordPromptError &&
+ isSshPasswordPromptError(error) &&
+ isSshPasswordPromptRequestError(error) &&
DesktopSshPasswordPrompts.isDesktopSshPasswordPromptCancellation(error.cause)
);
}
const makePasswordPrompt = (
- prompts: DesktopSshPasswordPrompts.DesktopSshPasswordPromptsShape,
-): SshPasswordPromptShape => ({
+ prompts: DesktopSshPasswordPrompts.DesktopSshPasswordPrompts["Service"],
+): SshAuth.SshPasswordPrompt["Service"] => ({
isAvailable: true,
- request: (request: SshPasswordRequest) =>
+ request: (request: SshAuth.SshPasswordRequest) =>
prompts.request(request).pipe(
Effect.mapError(
(cause) =>
- new SshPasswordPromptError({
- message: cause.message,
+ new SshPasswordPromptRequestError({
+ destination: request.destination,
cause,
}),
),
),
});
-const make = Effect.gen(function* () {
- const manager = yield* SshEnvironmentManager;
+export const make = Effect.gen(function* () {
+ const manager = yield* SshTunnel.SshEnvironmentManager;
const prompts = yield* DesktopSshPasswordPrompts.DesktopSshPasswordPrompts;
const runtimeContext = yield* Effect.context<DesktopSshEnvironmentRuntimeServices>();
- const passwordPrompt = SshPasswordPrompt.of(makePasswordPrompt(prompts));
+ const passwordPrompt = SshAuth.make(makePasswordPrompt(prompts));
return DesktopSshEnvironment.of({
discoverHosts: (input) =>
@@ -120,7 +120,7 @@
manager
.ensureEnvironment(target, ensureOptions)
.pipe(
- Effect.provideService(SshPasswordPrompt, passwordPrompt),
+ Effect.provideService(SshAuth.SshPasswordPrompt, passwordPrompt),
Effect.provide(runtimeContext),
Effect.withSpan("desktop.ssh.ensureEnvironment"),
),
@@ -128,7 +128,7 @@
manager
.disconnectEnvironment(target)
.pipe(
- Effect.provideService(SshPasswordPrompt, passwordPrompt),
+ Effect.provideService(SshAuth.SshPasswordPrompt, passwordPrompt),
Effect.provide(runtimeContext),
Effect.withSpan("desktop.ssh.disconnectEnvironment"),
),
@@ -138,7 +138,7 @@
export const layer = (options: DesktopSshEnvironmentLayerOptions = {}) =>
Layer.effect(DesktopSshEnvironment, make).pipe(
Layer.provide(
- SshEnvironmentManager.layer({
+ SshTunnel.layer({
...(options.resolveCliPackageSpec === undefined
? {}
: { resolveCliPackageSpec: options.resolveCliPackageSpec }),
diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts
--- a/apps/server/src/ws.ts
+++ b/apps/server/src/ws.ts
@@ -39,6 +39,7 @@
ProjectSearchEntriesError,
ProjectWriteFileError,
RelayClientInstallFailedError,
+ type RelayClientInstallFailureReason,
type RelayClientInstallProgressEvent,
OrchestrationReplayEventsError,
FilesystemBrowseError,
@@ -110,6 +111,36 @@
const isOrchestrationDispatchCommandError = Schema.is(OrchestrationDispatchCommandError);
const isWorkspacePathOutsideRootError = Schema.is(WorkspacePaths.WorkspacePathOutsideRootError);
+function relayClientInstallFailureReason(
+ error: RelayClient.RelayClientInstallError,
+): RelayClientInstallFailureReason {
+ switch (error._tag) {
+ case "RelayClientDownloadError":
+ case "RelayClientDownloadReadError":
+ return "download_failed";
+ case "RelayClientChecksumMismatchError":
+ return "invalid_checksum";
+ case "RelayClientInstallLockedError":
+ return "install_locked";
+ case "RelayClientOverrideMissingError":
+ return "override_missing";
+ case "RelayClientUnsupportedPlatformError":
+ return "unsupported_platform";
+ case "RelayClientChecksumVerificationError":
+ case "RelayClientExecutableValidationError":
+ return "validation_failed";
+ case "RelayClientDirectoryCreateError":
+ case "RelayClientInstallLockAcquireError":
+ case "RelayClientDownloadWriteError":
+ case "RelayClientArchiveExtractError":
+ case "RelayClientExecutablePermissionError":
+ case "RelayClientStageError":
+ case "RelayClientActivationError":
+ case "RelayClientInstallWriteError":
+ return "write_failed";
+ }
+}
+
const nowIso = Effect.map(DateTime.now, DateTime.formatIso);
function isThreadDetailEvent(event: OrchestrationEvent): event is Extract<
@@ -1142,11 +1173,11 @@
status,
}),
),
- Effect.catchTag("RelayClientInstallError", (error) =>
+ Effect.catchIf(RelayClient.isRelayClientInstallError, (error) =>
Queue.fail(
queue,
new RelayClientInstallFailedError({
- reason: error.reason,
+ reason: relayClientInstallFailureReason(error),
message: error.message,
}),
),
diff --git a/packages/shared/src/Net.test.ts b/packages/shared/src/Net.test.ts
--- a/packages/shared/src/Net.test.ts
+++ b/packages/shared/src/Net.test.ts
@@ -31,9 +31,7 @@
};
server.once("error", (cause) => {
- settle(
- Effect.fail(new NetService.NetError({ message: "Failed to open test server", cause })),
- );
+ settle(Effect.fail(new NetService.NetError({ host: host ?? "localhost", cause })));
});
if (host) {
diff --git a/packages/shared/src/Net.ts b/packages/shared/src/Net.ts
--- a/packages/shared/src/Net.ts
+++ b/packages/shared/src/Net.ts
@@ -1,15 +1,19 @@
import * as NodeNet from "node:net";
-import * as Data from "effect/Data";
+import * as Context from "effect/Context";
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
-import * as Context from "effect/Context";
import * as Predicate from "effect/Predicate";
+import * as Schema from "effect/Schema";
-export class NetError extends Data.TaggedError("NetError")<{
- readonly message: string;
- readonly cause?: unknown;
-}> {}
+export class NetError extends Schema.TaggedErrorClass<NetError>()("NetError", {
+ host: Schema.String,
+ cause: Schema.optional(Schema.Defect()),
+}) {
+ override get message(): string {
+ return `Failed to reserve loopback port on ${this.host}.`;
+ }
+}
const isErrnoExceptionWithCode = (
cause: unknown,
@@ -28,35 +32,34 @@
}
};
-export interface NetServiceShape {
- /**
- * Returns true when a TCP server can bind to {host, port}.
- */
- readonly canListenOnHost: (port: number, host: string) => Effect.Effect<boolean>;
+/**
+ * NetService - Service tag for startup networking helpers.
+ */
+export class NetService extends Context.Service<
+ NetService,
+ {
+ /**
+ * Returns true when a TCP server can bind to {host, port}.
+ */
+ readonly canListenOnHost: (port: number, host: string) => Effect.Effect<boolean>;
- /**
- * Checks loopback availability on both IPv4 and IPv6 localhost addresses.
- */
- readonly isPortAvailableOnLoopback: (port: number) => Effect.Effect<boolean>;
+ /**
+ * Checks loopback availability on both IPv4 and IPv6 localhost addresses.
+ */
+ readonly isPortAvailableOnLoopback: (port: number) => Effect.Effect<boolean>;
- /**
- * Reserve an ephemeral loopback port and release it immediately.
- */
- readonly reserveLoopbackPort: (host?: string) => Effect.Effect<number, NetError>;
+ /**
+ * Reserve an ephemeral loopback port and release it immediately.
+ */
+ readonly reserveLoopbackPort: (host?: string) => Effect.Effect<number, NetError>;
- /**
- * Resolve an available listening port, preferring the provided port first.
- */
- readonly findAvailablePort: (preferred: number) => Effect.Effect<number, NetError>;
-}
+ /**
+ * Resolve an available listening port, preferring the provided port first.
+ */
+ readonly findAvailablePort: (preferred: number) => Effect.Effect<number, NetError>;
+ }
+>()("@t3tools/shared/Net/NetService") {}
-/**
- * NetService - Service tag for startup networking helpers.
- */
-export class NetService extends Context.Service<NetService, NetServiceShape>()(
- "@t3tools/shared/Net/NetService",
-) {}
-
export const make = () => {
/**
* Returns true when a TCP server can bind to {host, port}.
@@ -160,7 +163,7 @@
};
probe.once("error", (cause) => {
- settle(Effect.fail(new NetError({ message: "Failed to reserve loopback port", cause })));
+ settle(Effect.fail(new NetError({ host, cause })));
});
probe.listen(0, host, () => {
@@ -171,7 +174,7 @@
settle(Effect.succeed(port));
return;
}
- settle(Effect.fail(new NetError({ message: "Failed to reserve loopback port" })));
+ settle(Effect.fail(new NetError({ host })));
});
});
@@ -180,7 +183,7 @@
});
});
- return {
+ return NetService.of({
canListenOnHost,
isPortAvailableOnLoopback,
reserveLoopbackPort,
@@ -191,7 +194,7 @@
}
return yield* reserveLoopbackPort();
}),
- } satisfies NetServiceShape;
+ });
};
export const layer = Layer.sync(NetService, make);
diff --git a/packages/shared/src/relayClient.test.ts b/packages/shared/src/relayClient.test.ts
--- a/packages/shared/src/relayClient.test.ts
+++ b/packages/shared/src/relayClient.test.ts
@@ -8,15 +8,14 @@
import * as Layer from "effect/Layer";
import * as Sink from "effect/Sink";
import * as Stream from "effect/Stream";
-import { HttpClient, HttpClientResponse } from "effect/unstable/http";
-import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
+import * as HttpClient from "effect/unstable/http/HttpClient";
+import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse";
+import * as ChildProcess from "effect/unstable/process/ChildProcess";
+import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner";
+
import { HostProcessArchitecture, HostProcessPlatform } from "./hostProcess.ts";
-import {
- RelayClientInstallError,
- CLOUDFLARED_VERSION,
- makeCloudflaredRelayClient,
-} from "./relayClient.ts";
+import * as RelayClient from "./relayClient.ts";
const hostRuntimeLayer = (env: Record<string, string> = {}) =>
Layer.mergeAll(
@@ -72,7 +71,7 @@
const overridePath = `${baseDir}/override-cloudflared`;
yield* fileSystem.writeFileString(overridePath, "override");
yield* fileSystem.chmod(overridePath, 0o755);
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
});
@@ -89,7 +88,7 @@
status: "available",
executablePath: overridePath,
source: "override",
- version: CLOUDFLARED_VERSION,
+ version: RelayClient.CLOUDFLARED_VERSION,
});
}).pipe(
Effect.scoped,
@@ -111,7 +110,7 @@
prefix: "t3-cloudflared-test-",
});
const bytes = new TextEncoder().encode("test-cloudflared-binary");
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
releaseAsset: {
url: "https://example.test/cloudflared",
@@ -128,12 +127,12 @@
}
}),
);
- const managedPath = `${baseDir}/tools/cloudflared/${CLOUDFLARED_VERSION}/linux-x64/cloudflared`;
+ const managedPath = `${baseDir}/tools/cloudflared/${RelayClient.CLOUDFLARED_VERSION}/linux-x64/cloudflared`;
expect(installed).toEqual({
status: "available",
executablePath: managedPath,
source: "managed",
- version: CLOUDFLARED_VERSION,
+ version: RelayClient.CLOUDFLARED_VERSION,
});
expect(new TextDecoder().decode(yield* fileSystem.readFile(managedPath))).toBe(
"test-cloudflared-binary",
@@ -167,7 +166,7 @@
const baseDir = yield* fileSystem.makeTempDirectoryScoped({
prefix: "t3-cloudflared-test-",
});
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
releaseAsset: {
url: "https://example.test/cloudflared",
@@ -177,8 +176,14 @@
});
const error = yield* manager.install.pipe(Effect.flip);
- expect(error).toBeInstanceOf(RelayClientInstallError);
- expect(error.reason).toBe("invalid_checksum");
+ expect(error).toBeInstanceOf(RelayClient.RelayClientChecksumMismatchError);
+ expect(error).toMatchObject({
+ expectedChecksum: Encoding.encodeHex(sha256(new TextEncoder().encode("expected"))),
+ actualChecksum: Encoding.encodeHex(sha256(new TextEncoder().encode("tampered"))),
+ });
+ expect(error.message).toBe(
+ "Downloaded relay client checksum did not match the pinned release.",
+ );
}).pipe(
Effect.scoped,
Effect.provide(
@@ -200,7 +205,7 @@
const baseDir = yield* fileSystem.makeTempDirectoryScoped({
prefix: "t3-cloudflared-test-",
});
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
releaseAsset: {
url: "https://example.test/cloudflared",
@@ -236,13 +241,13 @@
});
const binDir = `${baseDir}/bin`;
const executablePath = `${binDir}/cloudflared`;
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
});
expect(yield* manager.resolve).toEqual({
status: "missing",
- version: CLOUDFLARED_VERSION,
+ version: RelayClient.CLOUDFLARED_VERSION,
});
yield* fileSystem.makeDirectory(binDir);
@@ -254,7 +259,7 @@
status: "available",
executablePath,
source: "path",
- version: CLOUDFLARED_VERSION,
+ version: RelayClient.CLOUDFLARED_VERSION,
});
}).pipe(
Effect.scoped,
diff --git a/packages/shared/src/relayClient.ts b/packages/shared/src/relayClient.ts
--- a/packages/shared/src/relayClient.ts
+++ b/packages/shared/src/relayClient.ts
@@ -6,7 +6,6 @@
import * as Config from "effect/Config";
import * as Context from "effect/Context";
import * as Crypto from "effect/Crypto";
-import * as Data from "effect/Data";
import * as Effect from "effect/Effect";
import * as Encoding from "effect/Encoding";
import * as FileSystem from "effect/FileSystem";
@@ -14,9 +13,14 @@
import * as Option from "effect/Option";
import * as Path from "effect/Path";
import * as PlatformError from "effect/PlatformError";
+import * as Schema from "effect/Schema";
import * as Semaphore from "effect/Semaphore";
-import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http";
-import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
+import * as HttpClient from "effect/unstable/http/HttpClient";
+import * as HttpClientRequest from "effect/unstable/http/HttpClientRequest";
+import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse";
+import * as ChildProcess from "effect/unstable/process/ChildProcess";
+import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner";
+
import { HostProcessArchitecture, HostProcessPlatform } from "./hostProcess.ts";
export const CLOUDFLARED_VERSION = "2026.5.2";
@@ -44,24 +48,234 @@
export type AvailableRelayClient = Extract<RelayClientStatus, { readonly status: "available" }>;
-export class RelayClientInstallError extends Data.TaggedError("RelayClientInstallError")<{
- readonly reason:
- | "download_failed"
- | "invalid_checksum"
- | "install_locked"
- | "override_missing"
- | "unsupported_platform"
- | "validation_failed"
- | "write_failed";
- readonly message: string;
- readonly cause?: unknown;
-}> {}
+export class RelayClientDownloadError extends Schema.TaggedErrorClass<RelayClientDownloadError>()(
+ "RelayClientDownloadError",
+ {
+ url: Schema.String,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return "Could not download the relay client.";
+ }
+}
-class CloudflaredCommandError extends Data.TaggedError("CloudflaredCommandError")<{
- readonly command: string;
- readonly exitCode: number;
-}> {}
+export class RelayClientDownloadReadError extends Schema.TaggedErrorClass<RelayClientDownloadReadError>()(
+ "RelayClientDownloadReadError",
+ {
+ url: Schema.String,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return "Could not read the downloaded relay client binary.";
+ }
+}
+export class RelayClientChecksumMismatchError extends Schema.TaggedErrorClass<RelayClientChecksumMismatchError>()(
+ "RelayClientChecksumMismatchError",
+ {
+ expectedChecksum: Schema.String,
+ actualChecksum: Schema.String,
+ },
+) {
+ override get message(): string {
+ return "Downloaded relay client checksum did not match the pinned release.";
+ }
+}
+
+export class RelayClientInstallLockedError extends Schema.TaggedErrorClass<RelayClientInstallLockedError>()(
+ "RelayClientInstallLockedError",
+ {
+ lockPath: Schema.String,
+ },
+) {
+ override get message(): string {
+ return "Another relay client installation is still in progress.";
+ }
+}
+
+export class RelayClientOverrideMissingError extends Schema.TaggedErrorClass<RelayClientOverrideMissingError>()(
+ "RelayClientOverrideMissingError",
+ {
+ executablePath: Schema.String,
+ },
+) {
+ override get message(): string {
+ return `${CLOUDFLARED_PATH_ENV_NAME} does not point to an executable file.`;
+ }
+}
+
+export class RelayClientUnsupportedPlatformError extends Schema.TaggedErrorClass<RelayClientUnsupportedPlatformError>()(
+ "RelayClientUnsupportedPlatformError",
+ {
+ platform: Schema.String,
+ arch: Schema.String,
+ },
+) {
+ override get message(): string {
+ return `T3 Code does not provide a managed relay client binary for ${this.platform}-${this.arch}.`;
+ }
+}
+
+export class RelayClientChecksumVerificationError extends Schema.TaggedErrorClass<RelayClientChecksumVerificationError>()(
+ "RelayClientChecksumVerificationError",
+ {
+ url: Schema.String,
+ expectedChecksum: Schema.String,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return "Could not verify the downloaded relay client checksum.";
+ }
+}
+
+export class RelayClientExecutableValidationError extends Schema.TaggedErrorClass<RelayClientExecutableValidationError>()(
+ "RelayClientExecutableValidationError",
+ {
+ executablePath: Schema.String,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return "The downloaded relay client binary did not run.";
+ }
+}
+
+export class RelayClientDirectoryCreateError extends Schema.TaggedErrorClass<RelayClientDirectoryCreateError>()(
+ "RelayClientDirectoryCreateError",
+ {
+ directoryPath: Schema.String,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return "Could not create the relay client tool directory.";
+ }
+}
+
+export class RelayClientInstallLockAcquireError extends Schema.TaggedErrorClass<RelayClientInstallLockAcquireError>()(
+ "RelayClientInstallLockAcquireError",
+ {
+ lockPath: Schema.String,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return "Could not acquire the relay client installation lock.";
+ }
+}
+
+export class RelayClientDownloadWriteError extends Schema.TaggedErrorClass<RelayClientDownloadWriteError>()(
+ "RelayClientDownloadWriteError",
+ {
+ archivePath: Schema.String,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return "Could not write the relay client download.";
+ }
+}
+
+export class RelayClientArchiveExtractError extends Schema.TaggedErrorClass<RelayClientArchiveExtractError>()(
+ "RelayClientArchiveExtractError",
+ {
+ archivePath: Schema.String,
+ destinationDirectory: Schema.String,
+ cause: Schema.Defect(),
... diff truncated: showing 800 of 2838 linesYou can send follow-ups to the cloud agent here.
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
4 times, most recently
from
bb1d058 to
f003adb
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Prompt failures show generic message
- Updated SshPasswordPromptRequestError.message getter to forward the cause's message when the cause is an Error instance, restoring the pre-refactor behavior where specific error details (window unavailable, send errors, etc.) are surfaced instead of the generic 'SSH authentication failed' text.
Or push these changes by commenting:
@cursor push d3f717debe
Preview (d3f717debe)
diff --git a/packages/ssh/src/errors.ts b/packages/ssh/src/errors.ts
--- a/packages/ssh/src/errors.ts
+++ b/packages/ssh/src/errors.ts
@@ -363,6 +363,9 @@
},
) {
override get message(): string {
+ if (this.cause instanceof Error && this.cause.message) {
+ return this.cause.message;
+ }
return `SSH authentication failed for ${this.destination}.`;
}
}You can send follow-ups to the cloud agent here.
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
from
f003adb to
f105724
Compare
Contributor
There was a problem hiding this comment.
One convention issue found regarding error message derivation.
Posted via Macroscope — Effect Service Conventions
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
from
11044bd to
48d82f5
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Auth retry misses spawn failures
- Modified isSshAuthFailure to walk the cause chain so auth-related messages on nested causes (from SshCommandSpawnError, SshCommandExecutionError, SshTunnelSpawnError, SshTunnelMonitorError) are detected and trigger password retry.
Or push these changes by commenting:
@cursor push f7171b1734
Preview (f7171b1734)
diff --git a/packages/ssh/src/auth.test.ts b/packages/ssh/src/auth.test.ts
--- a/packages/ssh/src/auth.test.ts
+++ b/packages/ssh/src/auth.test.ts
@@ -30,6 +30,27 @@
}),
);
+ it.effect("detects ssh auth failures on cause chain", () =>
+ Effect.sync(() => {
+ const spawnError = new Error("Failed to spawn SSH command for host.", {
+ cause: new Error("Permission denied (publickey,password)."),
+ });
+ assert.equal(isSshAuthFailure(spawnError), true);
+
+ const nestedError = new Error("outer", {
+ cause: new Error("middle", {
+ cause: new Error("Authentication failed"),
+ }),
+ });
+ assert.equal(isSshAuthFailure(nestedError), true);
+
+ const noAuthError = new Error("Failed to spawn SSH command.", {
+ cause: new Error("ECONNREFUSED"),
+ });
+ assert.equal(isSshAuthFailure(noAuthError), false);
+ }),
+ );
+
it.effect("creates askpass env for cached password prompts", () =>
Effect.gen(function* () {
const fs = yield* FileSystem.FileSystem;
diff --git a/packages/ssh/src/auth.ts b/packages/ssh/src/auth.ts
--- a/packages/ssh/src/auth.ts
+++ b/packages/ssh/src/auth.ts
@@ -205,8 +205,7 @@
};
});
-export function isSshAuthFailure(error: unknown): boolean {
- const message = error instanceof Error ? error.message : String(error);
+function matchesAuthFailureMessage(message: string): boolean {
const normalized = message.toLowerCase();
return (
/permission denied \((?:publickey|password|keyboard-interactive|hostbased|gssapi-with-mic)[^)]*\)/u.test(
@@ -216,3 +215,22 @@
/too many authentication failures/u.test(normalized)
);
}
+
+export function isSshAuthFailure(error: unknown): boolean {
+ let current: unknown = error;
+ while (current != null) {
+ const message = current instanceof Error ? current.message : String(current);
+ if (matchesAuthFailureMessage(message)) {
+ return true;
+ }
+ if (typeof current !== "object") {
+ break;
+ }
+ const next = (current as Record<string, unknown>).cause;
+ if (next === undefined || next === current) {
+ break;
+ }
+ current = next;
+ }
+ return false;
+}You can send follow-ups to the cloud agent here.
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
from
3df226a to
896b7fc
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Cause chain triggers spurious SSH retries
- Added a guard in isSshAuthFailure to stop walking the Error.cause chain when encountering SshReadinessError variants, preventing their HTTP/probe causes from falsely matching SSH auth failure patterns.
Or push these changes by commenting:
@cursor push 1fb21d3d1f
Preview (1fb21d3d1f)
diff --git a/apps/desktop/src/ipc/methods/sshEnvironment.test.ts b/apps/desktop/src/ipc/methods/sshEnvironment.test.ts
--- a/apps/desktop/src/ipc/methods/sshEnvironment.test.ts
+++ b/apps/desktop/src/ipc/methods/sshEnvironment.test.ts
@@ -1,17 +1,23 @@
import { assert, describe, it } from "@effect/vitest";
-import { SshHttpBridgeError } from "@t3tools/ssh/errors";
+import { SshCommandSpawnError, SshHttpBridgeError } from "@t3tools/ssh/errors";
import * as Cause from "effect/Cause";
import * as Effect from "effect/Effect";
import * as Exit from "effect/Exit";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
-import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http";
+import * as Schema from "effect/Schema";
+import * as HttpClient from "effect/unstable/http/HttpClient";
+import * as HttpClientRequest from "effect/unstable/http/HttpClientRequest";
+import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse";
import {
DesktopSshEnvironmentRequestError,
fetchSshEnvironmentDescriptor,
+ toDesktopSshOperationPresentationError,
} from "./sshEnvironment.ts";
+const isSshHttpBridgeError = Schema.is(SshHttpBridgeError);
+
function jsonResponse(request: HttpClientRequest.HttpClientRequest, body: unknown, status = 200) {
return HttpClientResponse.fromWeb(
request,
@@ -34,6 +40,22 @@
}
describe("SSH environment IPC", () => {
+ it("presents legacy process causes without weakening structured errors", () => {
+ const cause = new Error("ssh executable was not found");
+ const structured = new SshCommandSpawnError({
+ command: ["ssh"],
+ exitCode: null,
+ stderr: "",
+ target: "devbox",
+ cause,
+ });
+
+ const presentation = toDesktopSshOperationPresentationError(structured);
+ assert.equal(structured.message, "Failed to spawn SSH command for devbox.");
+ assert.equal(presentation.message, cause.message);
+ assert.strictEqual(presentation.cause, structured);
+ });
+
it.effect("fetches and decodes the remote environment descriptor", () => {
const requestUrls: string[] = [];
const layer = makeHttpClientLayer((request) =>
@@ -83,7 +105,7 @@
assert.instanceOf(error, DesktopSshEnvironmentRequestError);
assert.equal(error.operation, "fetch-environment-descriptor");
- assert.equal(error.cause instanceof SshHttpBridgeError, false);
+ assert.equal(isSshHttpBridgeError(error.cause), false);
}).pipe(Effect.provide(layer));
});
@@ -108,7 +130,7 @@
const error = failure.value;
assert.instanceOf(error, DesktopSshEnvironmentRequestError);
- assert.instanceOf(error.cause, SshHttpBridgeError);
+ assert.equal(isSshHttpBridgeError(error.cause), true);
assert.equal(requestCount, 0);
}).pipe(Effect.provide(layer));
});
diff --git a/apps/desktop/src/ipc/methods/sshEnvironment.ts b/apps/desktop/src/ipc/methods/sshEnvironment.ts
--- a/apps/desktop/src/ipc/methods/sshEnvironment.ts
+++ b/apps/desktop/src/ipc/methods/sshEnvironment.ts
@@ -26,7 +26,13 @@
AuthSessionState,
AuthWebSocketTicketResult,
} from "@t3tools/contracts";
-import { SshHttpBridgeError } from "@t3tools/ssh/errors";
+import {
+ SshCommandExecutionError,
+ SshCommandSpawnError,
+ SshHttpBridgeError,
+ SshTunnelMonitorError,
+ SshTunnelSpawnError,
+} from "@t3tools/ssh/errors";
import { resolveLoopbackSshHttpBaseUrl } from "@t3tools/ssh/tunnel";
import * as Data from "effect/Data";
import * as Effect from "effect/Effect";
@@ -50,14 +56,30 @@
const isEnvironmentOperationForbiddenError = Schema.is(EnvironmentOperationForbiddenError);
const isEnvironmentRequestInvalidError = Schema.is(EnvironmentRequestInvalidError);
const isEnvironmentScopeRequiredError = Schema.is(EnvironmentScopeRequiredError);
+const isSshHttpBridgeError = Schema.is(SshHttpBridgeError);
+const isSshCausePresentationError = Schema.is(
+ Schema.Union([
+ SshCommandSpawnError,
+ SshCommandExecutionError,
+ SshTunnelSpawnError,
+ SshTunnelMonitorError,
+ ]),
+);
+export function toDesktopSshOperationPresentationError(
+ error: DesktopSshEnvironment.DesktopSshEnvironmentOperationError,
+): DesktopSshEnvironment.DesktopSshEnvironmentOperationError | Error {
+ if (isSshCausePresentationError(error) && error.cause instanceof Error) {
+ return new Error(error.cause.message, { cause: error });
+ }
+ return error;
+}
+
function readSshHttpStatus(cause: DesktopSshEnvironmentRequestCause): number | null {
- if (
- cause instanceof RemoteEnvironmentAuthUndeclaredStatusError ||
- cause instanceof SshHttpBridgeError
- ) {
+ if (cause instanceof RemoteEnvironmentAuthUndeclaredStatusError) {
return cause.status ?? null;
}
+ if (isSshHttpBridgeError(cause)) return null;
if (isEnvironmentRequestInvalidError(cause)) {
return 400;
}
@@ -127,11 +149,12 @@
}) {
const sshEnvironment = yield* DesktopSshEnvironment.DesktopSshEnvironment;
return yield* sshEnvironment.ensureEnvironment(target, options).pipe(
+ Effect.mapError(toDesktopSshOperationPresentationError),
Effect.catch((error) =>
DesktopSshEnvironment.isDesktopSshPasswordPromptCancellation(error)
? Effect.succeed({
type: DesktopSshPasswordPromptCancelledType,
- message: error.message,
+ message: error.cause.message,
})
: Effect.fail(error),
),
@@ -145,7 +168,9 @@
result: Schema.Void,
handler: Effect.fn("desktop.ipc.sshEnvironment.disconnectEnvironment")(function* (target) {
const sshEnvironment = yield* DesktopSshEnvironment.DesktopSshEnvironment;
- yield* sshEnvironment.disconnectEnvironment(target);
+ yield* sshEnvironment
+ .disconnectEnvironment(target)
+ .pipe(Effect.mapError(toDesktopSshOperationPresentationError));
}),
});
diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts
--- a/apps/desktop/src/main.ts
+++ b/apps/desktop/src/main.ts
@@ -1,7 +1,7 @@
import * as NodeHttpClient from "@effect/platform-node/NodeHttpClient";
import * as NodeRuntime from "@effect/platform-node/NodeRuntime";
import * as NodeServices from "@effect/platform-node/NodeServices";
-import { homedir } from "node:os";
+import * as NodeOS from "node:os";
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
@@ -60,7 +60,7 @@
const processArch = yield* HostProcessArchitecture;
return DesktopEnvironment.layer({
dirname: __dirname,
- homeDirectory: homedir(),
+ homeDirectory: NodeOS.homedir(),
platform,
processArch,
...metadata,
diff --git a/apps/desktop/src/ssh/DesktopSshEnvironment.test.ts b/apps/desktop/src/ssh/DesktopSshEnvironment.test.ts
--- a/apps/desktop/src/ssh/DesktopSshEnvironment.test.ts
+++ b/apps/desktop/src/ssh/DesktopSshEnvironment.test.ts
@@ -2,7 +2,6 @@
import * as NodeServices from "@effect/platform-node/NodeServices";
import { assert, describe, it } from "@effect/vitest";
import * as NetService from "@t3tools/shared/Net";
-import { SshPasswordPromptError } from "@t3tools/ssh/errors";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
@@ -34,18 +33,15 @@
});
it("treats password prompt timeouts as cancellable authentication prompts", () => {
- assert.equal(
- DesktopSshEnvironment.isDesktopSshPasswordPromptCancellation(
- new SshPasswordPromptError({
- message: "SSH authentication timed out for devbox.",
- cause: new DesktopSshPasswordPrompts.DesktopSshPromptTimedOutError({
- requestId: "prompt-1",
- destination: "devbox",
- }),
- }),
- ),
- true,
- );
+ const cause = new DesktopSshPasswordPrompts.DesktopSshPromptTimedOutError({
+ requestId: "prompt-1",
+ destination: "devbox",
+ });
+ const error = DesktopSshEnvironment.toSshPasswordPromptError(cause);
+ assert(DesktopSshEnvironment.isDesktopSshPasswordPromptCancellation(error));
+ assert.strictEqual(error.cause, cause);
+ assert.equal(error.message, "SSH authentication timed out for devbox.");
+ assert.equal(error.cause.message, "SSH authentication timed out for devbox.");
});
it.effect("wires desktop host discovery through the ssh package runtime", () =>
diff --git a/apps/desktop/src/ssh/DesktopSshEnvironment.ts b/apps/desktop/src/ssh/DesktopSshEnvironment.ts
--- a/apps/desktop/src/ssh/DesktopSshEnvironment.ts
+++ b/apps/desktop/src/ssh/DesktopSshEnvironment.ts
@@ -7,13 +7,19 @@
import * as SshAuth from "@t3tools/ssh/auth";
import { discoverSshHosts } from "@t3tools/ssh/config";
import {
- SshCommandError,
- SshHostDiscoveryError,
- SshInvalidTargetError,
- SshLaunchError,
- SshPairingError,
+ type SshCommandError,
+ type SshHostDiscoveryError,
+ type SshInvalidTargetError,
+ type SshLaunchError,
+ type SshPairingError,
+ SshPasswordPromptCancelledError,
SshPasswordPromptError,
- SshReadinessError,
+ SshPasswordPromptSecureRandomnessError,
+ SshPasswordPromptServiceStoppedError,
+ SshPasswordPromptTimedOutError,
+ SshPasswordPromptWindowClosedError,
+ SshPasswordPromptWindowUnavailableError,
+ type SshReadinessError,
} from "@t3tools/ssh/errors";
import * as SshTunnel from "@t3tools/ssh/tunnel";
import * as Context from "effect/Context";
@@ -21,11 +27,14 @@
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
import * as Path from "effect/Path";
+import * as Schema from "effect/Schema";
import * as HttpClient from "effect/unstable/http/HttpClient";
import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner";
import * as DesktopSshPasswordPrompts from "./DesktopSshPasswordPrompts.ts";
+const isSshPasswordPromptError = Schema.is(SshPasswordPromptError);
+
export type DesktopSshEnvironmentRuntimeServices =
| ChildProcessSpawner.ChildProcessSpawner
| FileSystem.FileSystem
@@ -69,15 +78,20 @@
readonly resolveCliRunner?: Effect.Effect<SshTunnel.RemoteT3RunnerOptions>;
}
+type DesktopSshPasswordPromptCancellationError = SshPasswordPromptError & {
+ readonly cause: DesktopSshPasswordPrompts.DesktopSshPasswordPromptCancellation;
+};
+
function discoverDesktopSshHostsEffect(input?: { readonly homeDir?: string }) {
return discoverSshHosts(input ?? {});
}
export function isDesktopSshPasswordPromptCancellation(
error: unknown,
-): error is SshPasswordPromptError {
+): error is DesktopSshPasswordPromptCancellationError {
return (
- error instanceof SshPasswordPromptError &&
+ isSshPasswordPromptError(error) &&
+ "cause" in error &&
DesktopSshPasswordPrompts.isDesktopSshPasswordPromptCancellation(error.cause)
);
}
@@ -89,31 +103,41 @@
export function toSshPasswordPromptError(
cause: DesktopSshPasswordPrompts.DesktopSshPasswordPromptRequestError,
): SshPasswordPromptError {
- let message: string;
switch (cause._tag) {
case "DesktopSshPromptRequestIdGenerationError":
- message = "Secure randomness is unavailable.";
- break;
+ return new SshPasswordPromptSecureRandomnessError({
+ destination: cause.destination,
+ cause,
+ });
case "DesktopSshPromptWindowUnavailableError":
case "DesktopSshPromptPresentationError":
- message = "T3 Code window is not available for SSH authentication.";
- break;
+ return new SshPasswordPromptWindowUnavailableError({
+ destination: cause.destination,
+ cause,
+ });
case "DesktopSshPromptTimedOutError":
- message = `SSH authentication timed out for ${cause.destination}.`;
- break;
+ return new SshPasswordPromptTimedOutError({
+ destination: cause.destination,
+ cause,
+ });
case "DesktopSshPromptCancelledError":
- message = `SSH authentication cancelled for ${cause.destination}.`;
- break;
+ return new SshPasswordPromptCancelledError({
+ destination: cause.destination,
+ cause,
+ });
case "DesktopSshPromptWindowClosedError":
- message = "SSH authentication was cancelled because the app window closed.";
- break;
+ return new SshPasswordPromptWindowClosedError({
+ destination: cause.destination,
+ cause,
+ });
case "DesktopSshPromptServiceStoppedError":
- message = "SSH password prompt service stopped.";
- break;
+ return new SshPasswordPromptServiceStoppedError({
+ destination: cause.destination,
+ cause,
+ });
default:
return unexpectedPasswordPromptError(cause);
}
- return new SshPasswordPromptError({ message, cause });
}
const makePasswordPrompt = (
@@ -128,7 +152,7 @@
const manager = yield* SshTunnel.SshEnvironmentManager;
const prompts = yield* DesktopSshPasswordPrompts.DesktopSshPasswordPrompts;
const runtimeContext = yield* Effect.context<DesktopSshEnvironmentRuntimeServices>();
- const passwordPrompt = SshAuth.SshPasswordPrompt.of(makePasswordPrompt(prompts));
+ const passwordPrompt = SshAuth.make(makePasswordPrompt(prompts));
return DesktopSshEnvironment.of({
discoverHosts: (input) =>
@@ -158,7 +182,7 @@
export const layer = (options: DesktopSshEnvironmentLayerOptions = {}) =>
Layer.effect(DesktopSshEnvironment, make).pipe(
Layer.provide(
- SshTunnel.SshEnvironmentManager.layer({
+ SshTunnel.layer({
...(options.resolveCliPackageSpec === undefined
? {}
: { resolveCliPackageSpec: options.resolveCliPackageSpec }),
diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts
--- a/apps/server/src/ws.ts
+++ b/apps/server/src/ws.ts
@@ -39,6 +39,7 @@
ProjectSearchEntriesError,
ProjectWriteFileError,
RelayClientInstallFailedError,
+ type RelayClientInstallFailureReason,
type RelayClientInstallProgressEvent,
OrchestrationReplayEventsError,
FilesystemBrowseError,
@@ -110,6 +111,36 @@
const isOrchestrationDispatchCommandError = Schema.is(OrchestrationDispatchCommandError);
const isWorkspacePathOutsideRootError = Schema.is(WorkspacePaths.WorkspacePathOutsideRootError);
+function relayClientInstallFailureReason(
+ error: RelayClient.RelayClientInstallError,
+): RelayClientInstallFailureReason {
+ switch (error._tag) {
+ case "RelayClientDownloadError":
+ case "RelayClientDownloadReadError":
+ return "download_failed";
+ case "RelayClientChecksumMismatchError":
+ return "invalid_checksum";
+ case "RelayClientInstallLockedError":
+ return "install_locked";
+ case "RelayClientOverrideMissingError":
+ return "override_missing";
+ case "RelayClientUnsupportedPlatformError":
+ return "unsupported_platform";
+ case "RelayClientChecksumVerificationError":
+ case "RelayClientExecutableValidationError":
+ return "validation_failed";
+ case "RelayClientDirectoryCreateError":
+ case "RelayClientInstallLockAcquireError":
+ case "RelayClientDownloadWriteError":
+ case "RelayClientArchiveExtractError":
+ case "RelayClientExecutablePermissionError":
+ case "RelayClientStageError":
+ case "RelayClientActivationError":
+ case "RelayClientInstallWriteError":
+ return "write_failed";
+ }
+}
+
const nowIso = Effect.map(DateTime.now, DateTime.formatIso);
function unexpectedCompatibilityError(error: never): never {
@@ -1212,11 +1243,11 @@
status,
}),
),
- Effect.catchTag("RelayClientInstallError", (error) =>
+ Effect.catchIf(RelayClient.isRelayClientInstallError, (error) =>
Queue.fail(
queue,
new RelayClientInstallFailedError({
- reason: error.reason,
+ reason: relayClientInstallFailureReason(error),
message: error.message,
}),
),
diff --git a/packages/shared/src/Net.test.ts b/packages/shared/src/Net.test.ts
--- a/packages/shared/src/Net.test.ts
+++ b/packages/shared/src/Net.test.ts
@@ -31,9 +31,7 @@
};
server.once("error", (cause) => {
- settle(
- Effect.fail(new NetService.NetError({ message: "Failed to open test server", cause })),
- );
+ settle(Effect.fail(new NetService.NetError({ host: host ?? "localhost", cause })));
});
if (host) {
@@ -47,6 +45,11 @@
it.layer(NetService.layer)("NetService", (it) => {
describe("Net helpers", () => {
+ it("preserves the loopback reservation error message", () => {
+ const error = new NetService.NetError({ host: "127.0.0.1" });
+ assert.equal(error.message, "Failed to reserve loopback port");
+ });
+
it.effect("reserveLoopbackPort returns a positive loopback port", () =>
Effect.gen(function* () {
const net = yield* NetService.NetService;
diff --git a/packages/shared/src/Net.ts b/packages/shared/src/Net.ts
--- a/packages/shared/src/Net.ts
+++ b/packages/shared/src/Net.ts
@@ -1,15 +1,19 @@
import * as NodeNet from "node:net";
-import * as Data from "effect/Data";
+import * as Context from "effect/Context";
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
-import * as Context from "effect/Context";
import * as Predicate from "effect/Predicate";
+import * as Schema from "effect/Schema";
-export class NetError extends Data.TaggedError("NetError")<{
- readonly message: string;
- readonly cause?: unknown;
-}> {}
+export class NetError extends Schema.TaggedErrorClass<NetError>()("NetError", {
+ host: Schema.String,
+ cause: Schema.optional(Schema.Defect()),
+}) {
+ override get message(): string {
+ return "Failed to reserve loopback port";
+ }
+}
const isErrnoExceptionWithCode = (
cause: unknown,
@@ -28,35 +32,34 @@
}
};
-export interface NetServiceShape {
- /**
- * Returns true when a TCP server can bind to {host, port}.
- */
- readonly canListenOnHost: (port: number, host: string) => Effect.Effect<boolean>;
+/**
+ * NetService - Service tag for startup networking helpers.
+ */
+export class NetService extends Context.Service<
+ NetService,
+ {
+ /**
+ * Returns true when a TCP server can bind to {host, port}.
+ */
+ readonly canListenOnHost: (port: number, host: string) => Effect.Effect<boolean>;
- /**
- * Checks loopback availability on both IPv4 and IPv6 localhost addresses.
- */
- readonly isPortAvailableOnLoopback: (port: number) => Effect.Effect<boolean>;
+ /**
+ * Checks loopback availability on both IPv4 and IPv6 localhost addresses.
+ */
+ readonly isPortAvailableOnLoopback: (port: number) => Effect.Effect<boolean>;
- /**
- * Reserve an ephemeral loopback port and release it immediately.
- */
- readonly reserveLoopbackPort: (host?: string) => Effect.Effect<number, NetError>;
+ /**
+ * Reserve an ephemeral loopback port and release it immediately.
+ */
+ readonly reserveLoopbackPort: (host?: string) => Effect.Effect<number, NetError>;
- /**
- * Resolve an available listening port, preferring the provided port first.
- */
- readonly findAvailablePort: (preferred: number) => Effect.Effect<number, NetError>;
-}
+ /**
+ * Resolve an available listening port, preferring the provided port first.
+ */
+ readonly findAvailablePort: (preferred: number) => Effect.Effect<number, NetError>;
+ }
+>()("@t3tools/shared/Net/NetService") {}
-/**
- * NetService - Service tag for startup networking helpers.
- */
-export class NetService extends Context.Service<NetService, NetServiceShape>()(
- "@t3tools/shared/Net/NetService",
-) {}
-
export const make = () => {
/**
* Returns true when a TCP server can bind to {host, port}.
@@ -160,7 +163,7 @@
};
probe.once("error", (cause) => {
- settle(Effect.fail(new NetError({ message: "Failed to reserve loopback port", cause })));
+ settle(Effect.fail(new NetError({ host, cause })));
});
probe.listen(0, host, () => {
@@ -171,7 +174,7 @@
settle(Effect.succeed(port));
return;
}
- settle(Effect.fail(new NetError({ message: "Failed to reserve loopback port" })));
+ settle(Effect.fail(new NetError({ host })));
});
});
@@ -180,7 +183,7 @@
});
});
- return {
+ return NetService.of({
canListenOnHost,
isPortAvailableOnLoopback,
reserveLoopbackPort,
@@ -191,7 +194,7 @@
}
return yield* reserveLoopbackPort();
}),
- } satisfies NetServiceShape;
+ });
};
export const layer = Layer.sync(NetService, make);
diff --git a/packages/shared/src/relayClient.test.ts b/packages/shared/src/relayClient.test.ts
--- a/packages/shared/src/relayClient.test.ts
+++ b/packages/shared/src/relayClient.test.ts
@@ -8,15 +8,14 @@
import * as Layer from "effect/Layer";
import * as Sink from "effect/Sink";
import * as Stream from "effect/Stream";
-import { HttpClient, HttpClientResponse } from "effect/unstable/http";
-import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
+import * as HttpClient from "effect/unstable/http/HttpClient";
+import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse";
+import * as ChildProcess from "effect/unstable/process/ChildProcess";
+import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner";
+
import { HostProcessArchitecture, HostProcessPlatform } from "./hostProcess.ts";
-import {
- RelayClientInstallError,
- CLOUDFLARED_VERSION,
- makeCloudflaredRelayClient,
-} from "./relayClient.ts";
+import * as RelayClient from "./relayClient.ts";
const hostRuntimeLayer = (env: Record<string, string> = {}) =>
Layer.mergeAll(
@@ -72,7 +71,7 @@
const overridePath = `${baseDir}/override-cloudflared`;
yield* fileSystem.writeFileString(overridePath, "override");
yield* fileSystem.chmod(overridePath, 0o755);
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
});
@@ -89,7 +88,7 @@
status: "available",
executablePath: overridePath,
source: "override",
- version: CLOUDFLARED_VERSION,
+ version: RelayClient.CLOUDFLARED_VERSION,
});
}).pipe(
Effect.scoped,
@@ -111,7 +110,7 @@
prefix: "t3-cloudflared-test-",
});
const bytes = new TextEncoder().encode("test-cloudflared-binary");
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
releaseAsset: {
url: "https://example.test/cloudflared",
@@ -128,12 +127,12 @@
}
}),
);
- const managedPath = `${baseDir}/tools/cloudflared/${CLOUDFLARED_VERSION}/linux-x64/cloudflared`;
+ const managedPath = `${baseDir}/tools/cloudflared/${RelayClient.CLOUDFLARED_VERSION}/linux-x64/cloudflared`;
expect(installed).toEqual({
status: "available",
executablePath: managedPath,
source: "managed",
- version: CLOUDFLARED_VERSION,
+ version: RelayClient.CLOUDFLARED_VERSION,
});
expect(new TextDecoder().decode(yield* fileSystem.readFile(managedPath))).toBe(
"test-cloudflared-binary",
@@ -167,7 +166,7 @@
const baseDir = yield* fileSystem.makeTempDirectoryScoped({
prefix: "t3-cloudflared-test-",
});
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
releaseAsset: {
url: "https://example.test/cloudflared",
@@ -177,8 +176,14 @@
});
const error = yield* manager.install.pipe(Effect.flip);
- expect(error).toBeInstanceOf(RelayClientInstallError);
- expect(error.reason).toBe("invalid_checksum");
+ expect(error).toBeInstanceOf(RelayClient.RelayClientChecksumMismatchError);
+ expect(error).toMatchObject({
+ expectedChecksum: Encoding.encodeHex(sha256(new TextEncoder().encode("expected"))),
+ actualChecksum: Encoding.encodeHex(sha256(new TextEncoder().encode("tampered"))),
+ });
+ expect(error.message).toBe(
+ "Downloaded relay client checksum did not match the pinned release.",
+ );
}).pipe(
Effect.scoped,
Effect.provide(
@@ -200,7 +205,7 @@
const baseDir = yield* fileSystem.makeTempDirectoryScoped({
prefix: "t3-cloudflared-test-",
});
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
releaseAsset: {
url: "https://example.test/cloudflared",
@@ -236,13 +241,13 @@
});
const binDir = `${baseDir}/bin`;
const executablePath = `${binDir}/cloudflared`;
- const manager = yield* makeCloudflaredRelayClient({
+ const manager = yield* RelayClient.makeCloudflaredRelayClient({
baseDir,
});
expect(yield* manager.resolve).toEqual({
status: "missing",
- version: CLOUDFLARED_VERSION,
+ version: RelayClient.CLOUDFLARED_VERSION,
});
yield* fileSystem.makeDirectory(binDir);
@@ -254,7 +259,7 @@
status: "available",
executablePath,
source: "path",
- version: CLOUDFLARED_VERSION,
+ version: RelayClient.CLOUDFLARED_VERSION,
});
}).pipe(
Effect.scoped,
diff --git a/packages/shared/src/relayClient.ts b/packages/shared/src/relayClient.ts
--- a/packages/shared/src/relayClient.ts
+++ b/packages/shared/src/relayClient.ts
@@ -6,7 +6,6 @@
import * as Config from "effect/Config";
import * as Context from "effect/Context";
import * as Crypto from "effect/Crypto";
-import * as Data from "effect/Data";
import * as Effect from "effect/Effect";
import * as Encoding from "effect/Encoding";
import * as FileSystem from "effect/FileSystem";
@@ -14,9 +13,14 @@
import * as Option from "effect/Option";
import * as Path from "effect/Path";
import * as PlatformError from "effect/PlatformError";
+import * as Schema from "effect/Schema";
import * as Semaphore from "effect/Semaphore";
-import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http";
-import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
+import * as HttpClient from "effect/unstable/http/HttpClient";
+import * as HttpClientRequest from "effect/unstable/http/HttpClientRequest";
+import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse";
+import * as ChildProcess from "effect/unstable/process/ChildProcess";
+import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner";
+
import { HostProcessArchitecture, HostProcessPlatform } from "./hostProcess.ts";
export const CLOUDFLARED_VERSION = "2026.5.2";
@@ -44,24 +48,234 @@
export type AvailableRelayClient = Extract<RelayClientStatus, { readonly status: "available" }>;
-export class RelayClientInstallError extends Data.TaggedError("RelayClientInstallError")<{
- readonly reason:
- | "download_failed"
- | "invalid_checksum"
- | "install_locked"
- | "override_missing"
- | "unsupported_platform"
- | "validation_failed"
- | "write_failed";
- readonly message: string;
- readonly cause?: unknown;
-}> {}
+export class RelayClientDownloadError extends Schema.TaggedErrorClass<RelayClientDownloadError>()(
+ "RelayClientDownloadError",
+ {
+ url: Schema.String,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return "Could not download the relay client.";
+ }
+}
-class CloudflaredCommandError extends Data.TaggedError("CloudflaredCommandError")<{
- readonly command: string;
- readonly exitCode: number;
-}> {}
+export class RelayClientDownloadReadError extends Schema.TaggedErrorClass<RelayClientDownloadReadError>()(
+ "RelayClientDownloadReadError",
+ {
+ url: Schema.String,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return "Could not read the downloaded relay client binary.";
+ }
+}
+export class RelayClientChecksumMismatchError extends Schema.TaggedErrorClass<RelayClientChecksumMismatchError>()(
+ "RelayClientChecksumMismatchError",
+ {
+ expectedChecksum: Schema.String,
+ actualChecksum: Schema.String,
+ },
+) {
+ override get message(): string {
+ return "Downloaded relay client checksum did not match the pinned release.";
+ }
+}
+
+export class RelayClientInstallLockedError extends Schema.TaggedErrorClass<RelayClientInstallLockedError>()(
+ "RelayClientInstallLockedError",
+ {
+ lockPath: Schema.String,
+ },
+) {
+ override get message(): string {
+ return "Another relay client installation is still in progress.";
+ }
+}
+
+export class RelayClientOverrideMissingError extends Schema.TaggedErrorClass<RelayClientOverrideMissingError>()(
+ "RelayClientOverrideMissingError",
+ {
+ executablePath: Schema.String,
+ },
+) {
+ override get message(): string {
... diff truncated: showing 800 of 3124 linesYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 896b7fc. Configure here.
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
2 times, most recently
from
b0851e0 to
3531338
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
codex/effect-service-shared-ssh
branch
from
3531338 to
8f51b24
Compare
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.
Summary
NetService,SshPasswordPrompt,SshEnvironmentManager, and the desktop SSH environment on inline service contracts andService["Service"]referencesmakefactories and canonicallayerconstructorsRelayClientabstract while preserving the implementation-specificmakeCloudflaredRelayClientandlayerCloudflaredexportsSchema.TaggedErrorClasserrors, with categorySchema.Unionexports and diagnostic attributes preservedReview fixes
makeCloudflaredRelayClientandlayerCloudflarednaming for the abstract relay client portScope
Orchestration and MCP modules are intentionally untouched. Existing suites changed only for service references and focused behavior/error-chain regressions; this PR adds no broad refactor-only test suite.
Validation
vp check(passes with 20 existing warnings in files outside this PR)vp run typecheckgit diff --check origin/main...HEADNote
Medium Risk
Broad error tag and shape changes across SSH, Net, and relay install paths can break callers that match old tags or
instanceofchecks; desktop/server boundaries add mapping to preserve some legacy behavior.Overview
Replaces broad
Data.TaggedErrortypes in shared Net, relay client install, and the SSH stack withSchema.TaggedErrorClassvariants that carry structured fields and stable computedmessagevalues, plus union exports andSchema.ishelpers for matching.SSH failures are split by stage (command spawn/exit/timeout, tunnel, launch, pairing, readiness, HTTP bridge, password prompts). Command/tunnel errors keep operational metadata on the structured error while desktop IPC can still show legacy process messages via
toDesktopSshOperationPresentationError. Password-prompt cancellation now readserror.cause.message, andisSshAuthFailureonly walks causes through specific SSH process wrappers so readiness/helper errors are not misclassified.Relay client install paths emit concrete error classes; server WS maps those tags back to existing
RelayClientInstallFailureReasonstrings for RPC events. Net loopback reservation failures use a fixed message withhost(and optionalcause) on the error.Effect services (
SshPasswordPrompt,SshEnvironmentManager, desktop SSH environment) move to inline service shapes with exportedmake/layerfactories and namespace imports at boundaries.Reviewed by Cursor Bugbot for commit 8f51b24. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Refactor SSH and shared Effect services to use discriminated union error types
packages/ssh/src/errors.ts,packages/shared/src/Net.ts, andpackages/shared/src/relayClient.tswithSchema.TaggedErrorClass-based discriminated unions, giving each failure mode its own class with structured fields and a standardizedmessagegetter.SshCommandSpawnError,SshTunnelExitError,SshReadinessTimeoutError) carrying target, exitCode, stderr, and timing context.isSshAuthFailurenow only traverses error causes through known SSH wrapper types and guards against cycles, preventing false positives from unrelated errors.toDesktopSshOperationPresentationErrorto surface legacy plain-Errormessages while preserving the structured error ascause, andreadSshHttpStatusnow returnsnullfor allSshHttpBridgeErrorvariants.makeWsRpcLayerswitches fromcatchTagtocatchIf(isRelayClientInstallError)and maps tags to canonicalRelayClientInstallFailureReasonstrings.SshCommandError,SshLaunchError) must now handle the new discriminated subtypes.Macroscope summarized 8f51b24.