Skip to content

Commit 26e6b3f

Browse files
juliusmarmingecodex
andcommitted
Address connect review feedback
Co-authored-by: codex <[email protected]>
1 parent d088589 commit 26e6b3f

11 files changed

Lines changed: 283 additions & 52 deletions

‎apps/server/src/cli/connect.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@ import * as Exit from "effect/Exit";
88
import * as Logger from "effect/Logger";
99
import * as Option from "effect/Option";
1010
import * as References from "effect/References";
11+
import * as Terminal from "effect/Terminal";
1112

1213
import {
1314
acquireRelayClientForLink,
1415
formatHeadlessAuthorizationPrompt,
1516
formatRelayClientReady,
1617
headlessSessionConfig,
1718
isPublishAgentActivityEnabledValue,
19+
recoverBootServiceOffer,
1820
reportCloudDisconnectResults,
1921
} from "./connect.ts";
2022

@@ -54,6 +56,13 @@ it.effect("detects headless operation from individual SSH config values", () =>
5456
}),
5557
);
5658

59+
it.effect("treats cancelling optional background setup as a successful skip", () =>
60+
Effect.gen(function* () {
61+
const result = yield* recoverBootServiceOffer(Effect.fail(new Terminal.QuitError({})));
62+
assert.isFalse(result);
63+
}),
64+
);
65+
5766
it.effect("does not install the relay client when the user declines the managed download", () =>
5867
Effect.gen(function* () {
5968
let installCalls = 0;

‎apps/server/src/cli/connect.ts‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ const authorizeCli = Effect.fn("cloud.cli.authorize")(function* (options: {
9898
if (!useOutOfBandOAuth) {
9999
const authorization = yield* tokens.get;
100100
if (authorization._tag === "Authorized") {
101-
return null;
101+
return authorization.token.identity ?? null;
102102
}
103103
yield* Console.log("\nHeadless mode enabled. A new authorization link is ready below.");
104104
}
@@ -112,7 +112,7 @@ const authorizeCli = Effect.fn("cloud.cli.authorize")(function* (options: {
112112
),
113113
);
114114
if (Option.isSome(existing)) {
115-
return null;
115+
return existing.value.identity ?? null;
116116
}
117117
const { token, identity } = yield* CliTokenManager.outOfBandOAuthLogin(
118118
promptForOutOfBandOAuthCode,
@@ -712,6 +712,21 @@ const offerBootService = Effect.gen(function* () {
712712
return true;
713713
});
714714

715+
export const recoverBootServiceOffer = <R>(
716+
offer: Effect.Effect<boolean, BootService.BootServiceError | Terminal.QuitError, R>,
717+
) =>
718+
offer.pipe(
719+
Effect.catchTags({
720+
QuitError: () => Effect.succeed(false),
721+
BootServiceUnsupportedError: (error) =>
722+
Console.log(`Skipping background setup: ${error.message}`).pipe(Effect.as(false)),
723+
BootServiceCommandError: (error) =>
724+
Console.warn(`Background setup did not finish: ${error.message}`).pipe(Effect.as(false)),
725+
BootServiceInstallError: (error) =>
726+
Console.warn(`Background setup did not finish: ${error.message}`).pipe(Effect.as(false)),
727+
}),
728+
);
729+
715730
export const connectCommand = Command.make("connect", {
716731
...projectLocationFlags,
717732
headless: headlessFlag,
@@ -733,20 +748,7 @@ export const connectCommand = Command.make("connect", {
733748

734749
// Connect itself already succeeded; a boot-service failure must not
735750
// fail the command, just tell the user what happened and move on.
736-
const background = yield* offerBootService.pipe(
737-
Effect.catchTags({
738-
BootServiceUnsupportedError: (error) =>
739-
Console.log(`Skipping background setup: ${error.message}`).pipe(Effect.as(false)),
740-
BootServiceCommandError: (error) =>
741-
Console.warn(`Background setup did not finish: ${error.message}`).pipe(
742-
Effect.as(false),
743-
),
744-
BootServiceInstallError: (error) =>
745-
Console.warn(`Background setup did not finish: ${error.message}`).pipe(
746-
Effect.as(false),
747-
),
748-
}),
749-
);
751+
const background = yield* recoverBootServiceOffer(offerBootService);
750752
yield* Console.log(
751753
background
752754
? "\n✓ Background service ready\n\nT3 Code will stay reachable after you log out."

‎apps/server/src/cloud/CliTokenManager.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ it.layer(NodeServices.layer)("CliTokenManager.outOfBandOAuthLogin", (it) => {
185185

186186
assert.equal(token.accessToken, "access-token-1");
187187
assert.equal(token.refreshToken, "refresh-token-1");
188+
assert.equal(token.identity, "[email protected]");
188189
// The id_token's email claim is surfaced so connect can show the account.
189190
assert.equal(identity, "[email protected]");
190191

‎apps/server/src/cloud/CliTokenManager.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ const PersistedToken = Schema.Struct({
120120
accessToken: Schema.String,
121121
refreshToken: Schema.String,
122122
expiresAtEpochMs: Schema.Number,
123+
identity: Schema.optional(Schema.String),
123124
});
124125
export type PersistedToken = typeof PersistedToken.Type;
125126

@@ -251,13 +252,15 @@ const exchangeToken = Effect.fn("cloud.cli_token.exchange")(function* (
251252
Effect.flatMap(HttpClientResponse.schemaBodyJson(OAuthTokenResponse)),
252253
);
253254
const now = yield* Clock.currentTimeMillis;
255+
const identity = idTokenIdentity(response.id_token);
254256
return {
255257
token: {
256258
accessToken: response.access_token,
257259
refreshToken: response.refresh_token ?? params.refresh_token ?? "",
258260
expiresAtEpochMs: now + response.expires_in * 1_000,
261+
...(identity === null ? {} : { identity }),
259262
} satisfies PersistedToken,
260-
identity: idTokenIdentity(response.id_token),
263+
identity,
261264
};
262265
});
263266

@@ -357,7 +360,9 @@ export const make = Effect.gen(function* () {
357360
refresh_token: token.refreshToken,
358361
client_id: metadata.clientId,
359362
});
360-
return refreshed;
363+
return refreshed.identity === undefined && token.identity !== undefined
364+
? { ...refreshed, identity: token.identity }
365+
: refreshed;
361366
});
362367

363368
const login = Effect.fn("cloud.cli_token.login")(function* () {

‎apps/server/src/cloud/bootService.test.ts‎

Lines changed: 124 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ interface RecordedCommand {
2727

2828
const makeRecordingRunnerLayer = (
2929
commands: Array<RecordedCommand>,
30-
options?: { readonly failCommand?: string },
30+
options?: {
31+
readonly failCommand?: string;
32+
readonly failWhen?: (command: string, args: ReadonlyArray<string>) => boolean;
33+
},
3134
) =>
3235
Layer.succeed(
3336
ProcessRunner.ProcessRunner,
@@ -36,7 +39,9 @@ const makeRecordingRunnerLayer = (
3639
Effect.sync(() => {
3740
assert.isUndefined(input.env);
3841
commands.push({ command: input.command, args: input.args });
39-
const failed = input.command === options?.failCommand;
42+
const failed =
43+
input.command === options?.failCommand ||
44+
options?.failWhen?.(input.command, input.args) === true;
4045
return {
4146
stdout: "",
4247
stderr: failed ? `${input.command} exploded` : "",
@@ -152,6 +157,11 @@ it("flags package-manager cache entry points as ephemeral", () => {
152157
BootService.isEphemeralCacheEntry("/home/theo/.bun/install/cache/[email protected]/dist/bin.mjs"),
153158
);
154159
assert.isFalse(BootService.isEphemeralCacheEntry("/usr/local/lib/node_modules/t3/dist/bin.mjs"));
160+
assert.isFalse(
161+
BootService.isEphemeralCacheEntry(
162+
"/home/theo/dev/pnpm/dlx-tools/t3/node_modules/t3/dist/bin.mjs",
163+
),
164+
);
155165
assert.isFalse(
156166
BootService.isEphemeralCacheEntry(
157167
"/home/theo/.t3/runtime/versions/0.0.27/node_modules/t3/dist/bin.mjs",
@@ -234,6 +244,29 @@ it.layer(NodeServices.layer)("BootService", (it) => {
234244
}),
235245
);
236246

247+
it.effect("reinstalls a pinned runtime when its entry point is missing", () =>
248+
Effect.gen(function* () {
249+
const { dirs, fs, path } = yield* makeTestContext();
250+
const commands: Array<RecordedCommand> = [];
251+
const service = yield* BootService.make({
252+
baseDir: dirs.baseDir,
253+
logsDir: dirs.logsDir,
254+
cliVersion: "0.0.27",
255+
host: makeHost("/home/theo/.npm/_npx/abc/node_modules/t3/dist/bin.mjs"),
256+
}).pipe(Effect.provide(makeRecordingRunnerLayer(commands)), provideHostRefs(dirs.home));
257+
258+
const plan = yield* service.install;
259+
yield* fs.makeDirectory(path.dirname(plan.t3EntryPath), { recursive: true });
260+
yield* fs.writeFileString(plan.t3EntryPath, "#!/usr/bin/env node\n");
261+
yield* fs.remove(plan.t3EntryPath);
262+
commands.length = 0;
263+
264+
yield* service.install;
265+
266+
assert.isTrue(commands.some(({ command }) => command === "npm"));
267+
}),
268+
);
269+
237270
it.effect("reads executable metadata from host process references", () =>
238271
Effect.gen(function* () {
239272
const { dirs } = yield* makeTestContext();
@@ -376,6 +409,95 @@ it.layer(NodeServices.layer)("BootService", (it) => {
376409
);
377410
const status = yield* service.status;
378411
assert.isFalse(status.installed);
412+
assert.isTrue(
413+
commands.some(
414+
({ command, args }) =>
415+
command === "systemctl" && args.join(" ") === "--user disable --now t3code.service",
416+
),
417+
);
418+
}),
419+
);
420+
421+
it.effect("restores the previous unit when a repair cannot activate", () =>
422+
Effect.gen(function* () {
423+
const { dirs, fs, path } = yield* makeTestContext();
424+
const initialCommands: Array<RecordedCommand> = [];
425+
const initialService = yield* BootService.make({
426+
baseDir: dirs.baseDir,
427+
logsDir: dirs.logsDir,
428+
cliVersion: "0.0.27",
429+
host: makeHost(dirs.stableEntry),
430+
}).pipe(
431+
Effect.provide(makeRecordingRunnerLayer(initialCommands)),
432+
provideHostRefs(dirs.home),
433+
);
434+
yield* initialService.install;
435+
436+
const unitPath = path.join(dirs.home, ".config", "systemd", "user", "t3code.service");
437+
const previousUnit = yield* fs.readFileString(unitPath);
438+
const replacementEntry = path.join(dirs.home, "replacement-bin.mjs");
439+
yield* fs.writeFileString(replacementEntry, "#!/usr/bin/env node\n");
440+
const repairCommands: Array<RecordedCommand> = [];
441+
const repairService = yield* BootService.make({
442+
baseDir: dirs.baseDir,
443+
logsDir: dirs.logsDir,
444+
cliVersion: "0.0.28",
445+
host: makeHost(replacementEntry),
446+
}).pipe(
447+
Effect.provide(makeRecordingRunnerLayer(repairCommands, { failCommand: "loginctl" })),
448+
provideHostRefs(dirs.home),
449+
);
450+
451+
const error = yield* repairService.install.pipe(Effect.flip);
452+
453+
assert.isTrue(isCommandError(error));
454+
assert.equal(yield* fs.readFileString(unitPath), previousUnit);
455+
assert.isTrue(
456+
repairCommands.some(
457+
({ command, args }) =>
458+
command === "systemctl" && args.join(" ") === "--user restart t3code.service",
459+
),
460+
);
461+
}),
462+
);
463+
464+
it.effect("keeps the unit when stopping it during uninstall fails", () =>
465+
Effect.gen(function* () {
466+
const { dirs, fs, path } = yield* makeTestContext();
467+
const installCommands: Array<RecordedCommand> = [];
468+
const installedService = yield* BootService.make({
469+
baseDir: dirs.baseDir,
470+
logsDir: dirs.logsDir,
471+
cliVersion: "0.0.27",
472+
host: makeHost(dirs.stableEntry),
473+
}).pipe(
474+
Effect.provide(makeRecordingRunnerLayer(installCommands)),
475+
provideHostRefs(dirs.home),
476+
);
477+
yield* installedService.install;
478+
479+
const uninstallCommands: Array<RecordedCommand> = [];
480+
const failingService = yield* BootService.make({
481+
baseDir: dirs.baseDir,
482+
logsDir: dirs.logsDir,
483+
cliVersion: "0.0.27",
484+
host: makeHost(dirs.stableEntry),
485+
}).pipe(
486+
Effect.provide(
487+
makeRecordingRunnerLayer(uninstallCommands, {
488+
failWhen: (command, args) =>
489+
command === "systemctl" && args.includes("disable") && args.includes("--now"),
490+
}),
491+
),
492+
provideHostRefs(dirs.home),
493+
);
494+
495+
const error = yield* failingService.uninstall.pipe(Effect.flip);
496+
497+
assert.isTrue(isCommandError(error));
498+
assert.isTrue(
499+
yield* fs.exists(path.join(dirs.home, ".config", "systemd", "user", "t3code.service")),
500+
);
379501
}),
380502
);
381503

0 commit comments

Comments
 (0)