Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

Commit c0e8e64

Browse files
committed
PLN-324: Guard onboarding races
- Cancel stale managed onboarding continuations before manual API key, settings, or saved-config changes can be overwritten - Catch activate-time onboarding rejections outside the process-level unhandled rejection handler - Add cancellation-aware repo seeding plus focused regression tests Testing: pnpm -C apps/desktop exec tsx --test test/app-lifecycle.test.ts test/managed-onboarding-run.test.ts test/managed-onboarding.test.ts test/seed-repos-config.test.ts; pnpm -C apps/desktop typecheck; pnpm -C apps/desktop lint; pnpm -C apps/desktop test; git diff --check Risks: Low; managed onboarding now prefers explicit user action and activation failures are logged without exiting.
1 parent 756f001 commit c0e8e64

8 files changed

Lines changed: 247 additions & 9 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* Handles Electron lifecycle callbacks that dispatch async Desktop work.
3+
*
4+
* Electron does not await event listener promises. Keeping the catch inside the
5+
* lifecycle helper prevents expected activation-time failures from becoming
6+
* process-level unhandled rejections.
7+
*/
8+
export async function handleActivateEvent(deps: {
9+
handleActivate: () => Promise<void>;
10+
log: (message: string) => void;
11+
}): Promise<void> {
12+
try {
13+
await deps.handleActivate();
14+
} catch (error) {
15+
const message =
16+
error instanceof Error
17+
? `${error.message}${error.stack ? `\n${error.stack}` : ""}`
18+
: String(error);
19+
deps.log(`activate handling failed: ${message}`);
20+
}
21+
}

apps/desktop/src/main/app.ts

Lines changed: 92 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ import {
9090
type TrustedDesktopConfigResult,
9191
withSingleManagedOnboardingRetry,
9292
} from "./managed-onboarding.js";
93+
import {
94+
ManagedOnboardingRunTracker,
95+
type ManagedOnboardingRunToken,
96+
} from "./managed-onboarding-run.js";
9397
import {
9498
getCanonicalOnboardingHandoffPath,
9599
isCanonicalOnboardingHandoffPath,
@@ -146,6 +150,7 @@ export class DesktopApplication {
146150
private bootReadyForOnboarding = false;
147151
private processingOnboardingHandoff = false;
148152
private readonly queuedOpenFileHandoffs = new OnboardingHandoffQueue();
153+
private readonly managedOnboardingRuns = new ManagedOnboardingRunTracker();
149154
private managedOnboardingState: ManagedOnboardingState = { status: "idle" };
150155
private readonly queueStatsTelemetryDebounce: QueueStatsDebounce =
151156
createQueueStatsDebounce(
@@ -560,6 +565,7 @@ export class DesktopApplication {
560565
private async handleLoadedOnboardingHandoff(
561566
payload: PendingOnboardingHandoff,
562567
): Promise<void> {
568+
const run = this.managedOnboardingRuns.begin();
563569
this.managedOnboardingState = {
564570
status: "awaiting-origin-confirmation",
565571
webAppOrigin: payload.webAppOrigin,
@@ -569,6 +575,9 @@ export class DesktopApplication {
569575
this.showWindow();
570576

571577
const confirmed = await this.confirmManagedOnboardingOrigin(payload);
578+
if (this.shouldStopManagedOnboardingRun(run, "origin confirmation")) {
579+
return;
580+
}
572581
if (!confirmed) {
573582
this.setManagedOnboardingFailure(
574583
"origin_confirmation_dismissed",
@@ -578,7 +587,7 @@ export class DesktopApplication {
578587
return;
579588
}
580589

581-
await this.runManagedOnboardingProvisioning(payload);
590+
await this.runManagedOnboardingProvisioning(payload, run);
582591
}
583592

584593
private async confirmManagedOnboardingOrigin(
@@ -599,7 +608,11 @@ export class DesktopApplication {
599608

600609
private async runManagedOnboardingProvisioning(
601610
payload: PendingOnboardingHandoff,
611+
run: ManagedOnboardingRunToken,
602612
): Promise<void> {
613+
if (this.shouldStopManagedOnboardingRun(run, "provisioning start")) {
614+
return;
615+
}
603616
this.managedOnboardingState = {
604617
status: "provisioning",
605618
webAppOrigin: payload.webAppOrigin,
@@ -612,8 +625,12 @@ export class DesktopApplication {
612625
fetchTrustedDesktopConfig({ webAppOrigin: payload.webAppOrigin }),
613626
shouldRetry: isRetryableTrustedConfigFailure,
614627
delayMs: MANAGED_ONBOARDING_RETRY_DELAY_MS,
615-
isCancelled: () => this.shuttingDown,
628+
isCancelled: () =>
629+
this.managedOnboardingRuns.isCancelled(run, this.shuttingDown),
616630
});
631+
if (this.shouldStopManagedOnboardingRun(run, "trusted config result")) {
632+
return;
633+
}
617634
if (trustedConfig.kind !== "ok") {
618635
this.setManagedOnboardingFailure(
619636
trustedConfig.reason,
@@ -623,6 +640,9 @@ export class DesktopApplication {
623640
return;
624641
}
625642

643+
if (this.shouldStopManagedOnboardingRun(run, "claim start")) {
644+
return;
645+
}
626646
this.managedOnboardingState = {
627647
status: "provisioning",
628648
webAppOrigin: payload.webAppOrigin,
@@ -643,9 +663,13 @@ export class DesktopApplication {
643663
}),
644664
shouldRetry: isRetryableBootstrapClaimFailure,
645665
delayMs: MANAGED_ONBOARDING_RETRY_DELAY_MS,
646-
isCancelled: () => this.shuttingDown,
666+
isCancelled: () =>
667+
this.managedOnboardingRuns.isCancelled(run, this.shuttingDown),
647668
});
648669

670+
if (this.shouldStopManagedOnboardingRun(run, "claim result")) {
671+
return;
672+
}
649673
if (claimResult.kind === "manual_fallback") {
650674
this.setManagedOnboardingFailure(
651675
claimResult.reason,
@@ -663,6 +687,9 @@ export class DesktopApplication {
663687
return;
664688
}
665689

690+
if (this.shouldStopManagedOnboardingRun(run, "managed key persistence")) {
691+
return;
692+
}
666693
this.apiKeyStore.setApiKey(claimResult.apiKey, "DESKTOP_MANAGED");
667694
const sandboxBaseDirectory = normalizeScopePath(
668695
payload.sandboxBaseDirectory,
@@ -685,7 +712,16 @@ export class DesktopApplication {
685712
});
686713

687714
if (safeSandboxBaseDirectory) {
688-
await seedReposConfig(safeSandboxBaseDirectory);
715+
if (this.shouldStopManagedOnboardingRun(run, "repo config seeding")) {
716+
return;
717+
}
718+
await seedReposConfig(safeSandboxBaseDirectory, {
719+
isCancelled: () =>
720+
this.managedOnboardingRuns.isCancelled(run, this.shuttingDown),
721+
});
722+
if (this.shouldStopManagedOnboardingRun(run, "completion state update")) {
723+
return;
724+
}
689725
this.managedOnboardingState = {
690726
status: "idle",
691727
webAppOrigin: payload.webAppOrigin,
@@ -706,6 +742,33 @@ export class DesktopApplication {
706742
this.showWindow();
707743
}
708744

745+
private shouldStopManagedOnboardingRun(
746+
run: ManagedOnboardingRunToken,
747+
stage: string,
748+
): boolean {
749+
if (!this.managedOnboardingRuns.isCancelled(run, this.shuttingDown)) {
750+
return false;
751+
}
752+
gatewayLog.debug(
753+
"managed-onboarding",
754+
`Skipping stale managed onboarding continuation at ${stage}.`,
755+
);
756+
return true;
757+
}
758+
759+
private cancelManagedOnboardingForUserChange(reason: string): void {
760+
this.managedOnboardingRuns.cancel();
761+
if (this.managedOnboardingState.status === "idle") {
762+
return;
763+
}
764+
gatewayLog.debug(
765+
"managed-onboarding",
766+
`Canceled automated onboarding because ${reason}.`,
767+
);
768+
this.managedOnboardingState = { status: "idle" };
769+
this.notifyOnboardingStateChanged();
770+
}
771+
709772
private setManagedOnboardingFailure(
710773
reason: string,
711774
message: string,
@@ -1326,6 +1389,21 @@ export class DesktopApplication {
13261389
);
13271390
}
13281391

1392+
const updatesOnboardingState = (
1393+
[
1394+
"sandboxBaseDirectory",
1395+
"onboardingCompleted",
1396+
"relayOrigin",
1397+
"apiOrigin",
1398+
"webAppOrigin",
1399+
] as const
1400+
).some((key) => key in partial);
1401+
if (updatesOnboardingState) {
1402+
this.cancelManagedOnboardingForUserChange(
1403+
"settings were updated manually",
1404+
);
1405+
}
1406+
13291407
const updated = this.settingsStore.update(
13301408
nextPartial as Partial<DesktopSettings>,
13311409
);
@@ -1500,11 +1578,13 @@ export class DesktopApplication {
15001578
if (!trimmed.startsWith("sk_live_")) {
15011579
throw new Error("API key must start with sk_live_");
15021580
}
1581+
this.cancelManagedOnboardingForUserChange("a manual API key was set");
15031582
this.apiKeyStore.setApiKey(trimmed);
15041583
this.restartCloudSocket();
15051584
return this.apiKeyStore.getStatus();
15061585
});
15071586
ipcMain.handle("desktop:clear-api-key", () => {
1587+
this.cancelManagedOnboardingForUserChange("the API key was cleared");
15081588
this.apiKeyStore.clearApiKey();
15091589
this.restartCloudSocket();
15101590
return this.apiKeyStore.getStatus();
@@ -1571,7 +1651,6 @@ export class DesktopApplication {
15711651
if (!trimmedApiKey.startsWith("sk_live_")) {
15721652
throw new Error("API key must start with sk_live_");
15731653
}
1574-
this.apiKeyStore.setApiKey(trimmedApiKey, "USER_CREATED");
15751654
} else {
15761655
const onboardingAttemptId =
15771656
typeof payload.onboardingAttemptId === "string"
@@ -1582,6 +1661,10 @@ export class DesktopApplication {
15821661
}
15831662
}
15841663

1664+
this.cancelManagedOnboardingForUserChange("manual onboarding completed");
1665+
if (trimmedApiKey) {
1666+
this.apiKeyStore.setApiKey(trimmedApiKey, "USER_CREATED");
1667+
}
15851668
this.settingsStore.update({
15861669
...(relayOrigin !== undefined ? { relayOrigin } : {}),
15871670
...(apiOrigin !== undefined ? { apiOrigin } : {}),
@@ -1735,6 +1818,9 @@ export class DesktopApplication {
17351818
}
17361819
const { wasActive } = this.settingsStore.deleteConfig(id);
17371820
if (wasActive) {
1821+
this.cancelManagedOnboardingForUserChange(
1822+
"the active saved config was deleted",
1823+
);
17381824
this.settingsStore.setRelayOrigin(DEFAULT_DESKTOP_SETTINGS.relayOrigin);
17391825
this.settingsStore.setApiOrigin(DEFAULT_DESKTOP_SETTINGS.apiOrigin);
17401826
this.settingsStore.setWebAppOrigin(DEFAULT_DESKTOP_SETTINGS.webAppOrigin);
@@ -1764,6 +1850,7 @@ export class DesktopApplication {
17641850
if (!safeStorage.isEncryptionAvailable()) {
17651851
throw new Error("safeStorage is not available -- cannot apply config");
17661852
}
1853+
this.cancelManagedOnboardingForUserChange("a saved config was applied");
17671854
const appliedConfig = this.settingsStore.applyConfig(id);
17681855
const profileKey = this.apiKeyStore.getProfileKeyRecord(id);
17691856
if (profileKey) {

apps/desktop/src/main/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { app, nativeTheme } from "electron";
22
import { DesktopApplication } from "./app.js";
3+
import { handleActivateEvent } from "./app-lifecycle.js";
34
import { handleUncaughtException, handleUnhandledRejection } from "./error-handlers.js";
45
import { gatewayLog } from "./gateway-logger.js";
56

@@ -31,7 +32,10 @@ app.on("ready", () => {
3132
});
3233

3334
app.on("activate", () => {
34-
void desktopApplication.handleActivate();
35+
void handleActivateEvent({
36+
handleActivate: () => desktopApplication.handleActivate(),
37+
log: (message) => gatewayLog.warn("activate", message),
38+
});
3539
});
3640

3741
let quitPromise: Promise<void> | null = null;
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Tracks the active managed-onboarding continuation so newer user action can
3+
* cancel stale async work before it writes credentials or settings.
4+
*/
5+
export type ManagedOnboardingRunToken = {
6+
readonly id: number;
7+
};
8+
9+
/**
10+
* Small, framework-free run coordinator for Desktop managed onboarding.
11+
*
12+
* A token is current only until a newer managed run starts or a manual action
13+
* cancels automated onboarding. Callers should check the token after awaited
14+
* work and immediately before durable side effects.
15+
*/
16+
export class ManagedOnboardingRunTracker {
17+
private currentRunId = 0;
18+
19+
begin(): ManagedOnboardingRunToken {
20+
this.currentRunId += 1;
21+
return { id: this.currentRunId };
22+
}
23+
24+
cancel(): void {
25+
this.currentRunId += 1;
26+
}
27+
28+
isCurrent(token: ManagedOnboardingRunToken): boolean {
29+
return token.id === this.currentRunId;
30+
}
31+
32+
isCancelled(
33+
token: ManagedOnboardingRunToken,
34+
externallyCancelled = false,
35+
): boolean {
36+
return externallyCancelled || !this.isCurrent(token);
37+
}
38+
}

apps/desktop/src/main/seed-repos-config.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,31 @@ import { loadReposConfig, saveReposConfig } from "../server/operations/repos-con
1212
* Repos are added explicitly by the user via POST /api/gateway/repos — this
1313
* function never auto-discovers repos from the filesystem.
1414
*
15-
* Best-effort — logs errors but never throws.
15+
* Best-effort — logs errors but never throws. When provided, `isCancelled`
16+
* lets long-running callers avoid writing stale repo defaults after the user
17+
* has switched to another onboarding/settings path.
1618
*/
17-
export async function seedReposConfig(rawSandboxBaseDirectory: string): Promise<void> {
19+
export async function seedReposConfig(
20+
rawSandboxBaseDirectory: string,
21+
options: { isCancelled?: () => boolean } = {},
22+
): Promise<void> {
1823
try {
24+
if (options.isCancelled?.()) {
25+
return;
26+
}
1927
const sandboxBaseDirectory = normalizeScopePath(rawSandboxBaseDirectory);
2028
if (!sandboxBaseDirectory) {
2129
return;
2230
}
31+
if (options.isCancelled?.()) {
32+
return;
33+
}
2334

2435
const symphonyDir = computeSymphonyDir(sandboxBaseDirectory);
2536
const configDir = path.join(symphonyDir, "config");
37+
if (options.isCancelled?.()) {
38+
return;
39+
}
2640
mkdirSync(configDir, { recursive: true });
2741

2842
// Ensure worktreeParentDir + worktreeParentDirConfirmed are both set.
@@ -37,6 +51,9 @@ export async function seedReposConfig(rawSandboxBaseDirectory: string): Promise<
3751
// set confirmed only.
3852
// Single load → mutate in-memory → single save.
3953
const config = await loadReposConfig(configDir);
54+
if (options.isCancelled?.()) {
55+
return;
56+
}
4057
let dirty = false;
4158

4259
const existingDir = config.settings.worktreeParentDir;
@@ -58,7 +75,7 @@ export async function seedReposConfig(rawSandboxBaseDirectory: string): Promise<
5875
dirty = true;
5976
}
6077

61-
if (dirty) {
78+
if (dirty && !options.isCancelled?.()) {
6279
await saveReposConfig(config, configDir);
6380
}
6481
} catch (err) {
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import assert from "node:assert/strict";
2+
import { test } from "node:test";
3+
import { handleActivateEvent } from "../src/main/app-lifecycle.js";
4+
5+
test("activate handler logs rejected async work without rethrowing", async () => {
6+
const logs: string[] = [];
7+
8+
await assert.doesNotReject(
9+
handleActivateEvent({
10+
handleActivate: async () => {
11+
throw new Error("handoff read failed");
12+
},
13+
log: (message) => logs.push(message),
14+
}),
15+
);
16+
17+
assert.equal(logs.length, 1);
18+
assert.match(logs[0], /activate handling failed: handoff read failed/);
19+
});
20+
21+
test("activate handler does not log successful activation", async () => {
22+
const logs: string[] = [];
23+
24+
await handleActivateEvent({
25+
handleActivate: async () => {},
26+
log: (message) => logs.push(message),
27+
});
28+
29+
assert.deepEqual(logs, []);
30+
});

0 commit comments

Comments
 (0)