diff --git a/docs/hub.md b/docs/hub.md index 336b6600c..de1385321 100644 --- a/docs/hub.md +++ b/docs/hub.md @@ -95,11 +95,11 @@ Hub authentication rejection or close code `4403` permanently revokes the local daemon deletes its credential, stops reconnecting, and retains only the relationship ID, Hub origin, scopes, and a sanitized reason for status reporting. -`paseo hub disconnect` disables socket reconnect before requesting remote revocation. If the Hub is -offline, the daemon persists `disconnecting` and retries revocation across daemon restarts without -opening a Hub socket. This also covers an enrollment whose request may have succeeded but whose -response was lost. `--force` removes local authority immediately and warns that remote revocation may -still be pending. +`paseo hub disconnect` disables socket reconnect and execution authority before making one bounded +remote revocation request. The daemon then removes the local relationship whether the request +succeeds or fails. A failed request returns a warning that server-side revocation may remain pending. +`--force` skips the remote request. Legacy persisted `disconnecting` records are removed on startup; +the daemon does not retry revocation in the background. `paseo hub logout` removes only the active human CLI credential and preserves credentials for other origins. Interactive logout inspects and optionally disconnects a same-origin daemon before deleting the login; a failed requested disconnect preserves the login. JSON and noninteractive logout never prompt or disconnect implicitly. diff --git a/packages/cli/src/commands/hub/commands.test.ts b/packages/cli/src/commands/hub/commands.test.ts index 2815982a2..a68f1ab2d 100644 --- a/packages/cli/src/commands/hub/commands.test.ts +++ b/packages/cli/src/commands/hub/commands.test.ts @@ -344,7 +344,6 @@ describe("Hub commands", () => { daemon.beforeDisconnect = () => { assert.equal(credentials.active()?.credential, "human-secret"); }; - const result = await runHubLogout( { json: true, disconnectDaemon: true, force: true }, { @@ -359,9 +358,31 @@ describe("Hub commands", () => { assert.equal(daemon.disconnects, 1); assert.deepEqual(daemon.disconnectForces, [true]); assert.equal(result.data.daemonDisconnected, true); + assert.equal(result.data.warning, undefined); }); - it("disconnect reports the current Hub and preserves it in the final result", async () => { + it("logout reports a daemon disconnect warning", async () => { + const credentials = new MemoryCredentials(); + credentials.save({ origin: "https://hub.test", credential: "human-secret" }); + const daemon = new FakeDaemon("https://hub.test"); + daemon.disconnectWarning = "Hub could not be reached; cleanup may remain pending."; + + const result = await runHubLogout( + { json: true, disconnectDaemon: true }, + { + credentials, + daemon: new FakeDaemonConnection(daemon), + isInteractive: () => false, + confirmDisconnect: async () => false, + reporter: quietReporter, + }, + ); + + assert.equal(result.data.daemonDisconnected, true); + assert.equal(result.data.warning, "Hub could not be reached; cleanup may remain pending."); + }); + + it("disconnect reports clean local status without a ghost Hub origin", async () => { const daemon = new FakeDaemon("https://hub.test"); const progress: string[] = []; @@ -374,7 +395,7 @@ describe("Hub commands", () => { ); assert.deepEqual(progress, ["Disconnecting this daemon from https://hub.test"]); - assert.equal(result.data[0]?.hub, "https://hub.test"); + assert.equal(result.data[0]?.hub, null); }); it("preserves login when an accepted daemon disconnect fails", async () => { @@ -448,6 +469,7 @@ class FakeDaemon implements HubDaemonClient { disconnects = 0; readonly disconnectForces: boolean[] = []; disconnectError: Error | null = null; + disconnectWarning: string | null = null; beforeDisconnect: (() => void) | null = null; constructor(private readonly origin: string) {} @@ -466,7 +488,10 @@ class FakeDaemon implements HubDaemonClient { this.disconnectForces.push(force); this.beforeDisconnect?.(); if (this.disconnectError !== null) throw this.disconnectError; - return { status: hubStatus("not_connected", null) }; + return { + status: hubStatus("not_connected", null), + ...(this.disconnectWarning ? { warning: this.disconnectWarning } : {}), + }; } async close() {} diff --git a/packages/cli/src/commands/hub/disconnect.ts b/packages/cli/src/commands/hub/disconnect.ts index 5cbe0e429..867201376 100644 --- a/packages/cli/src/commands/hub/disconnect.ts +++ b/packages/cli/src/commands/hub/disconnect.ts @@ -31,7 +31,7 @@ export function runHubDisconnect( ); } const response = await client.disconnectHub(options.force ?? false); - return hubStatusResult(response.status, response.warning, current.hubOrigin); + return hubStatusResult(response.status, response.warning); }); } @@ -42,7 +42,7 @@ export function addHubDisconnectCommand( addJsonAndDaemonHostOptions( parent .command("disconnect") - .option("--force", "Remove local authority even if the Hub is offline"), + .option("--force", "Remove local authority without notifying the Hub"), ).action( withOutput(async (...args) => { const options = args.at(-2) as HubDisconnectOptions; diff --git a/packages/cli/src/commands/hub/logout.ts b/packages/cli/src/commands/hub/logout.ts index e4ea72137..28f083702 100644 --- a/packages/cli/src/commands/hub/logout.ts +++ b/packages/cli/src/commands/hub/logout.ts @@ -11,6 +11,7 @@ interface HubLogoutResult { origin: string | null; status: "logged_out" | "not_logged_in"; daemonDisconnected: boolean; + warning?: string; } const schema: OutputSchema = { @@ -19,6 +20,7 @@ const schema: OutputSchema = { { header: "HUB", field: "origin" }, { header: "STATUS", field: "status" }, { header: "DAEMON DISCONNECTED", field: "daemonDisconnected" }, + { header: "WARNING", field: "warning" }, ], }; @@ -70,11 +72,16 @@ export async function runHubLogout( options, `Disconnecting this daemon from ${active.origin}`, ); - await withHubDaemon(dependencies.daemon, options.host, async (daemon) => { - await daemon.disconnectHub(options.force ?? false); + const disconnect = await withHubDaemon(dependencies.daemon, options.host, async (daemon) => { + return daemon.disconnectHub(options.force ?? false); }); dependencies.credentials.logoutActive(); - return logoutResult({ origin: active.origin, status: "logged_out", daemonDisconnected: true }); + return logoutResult({ + origin: active.origin, + status: "logged_out", + daemonDisconnected: disconnect.status.state === "not_connected", + ...(disconnect.warning ? { warning: disconnect.warning } : {}), + }); } export function addHubLogoutCommand(parent: Command, dependencies: HubLogoutDependencies): void { @@ -83,7 +90,7 @@ export function addHubLogoutCommand(parent: Command, dependencies: HubLogoutDepe .command("logout") .description("Remove the active stored Hub CLI login") .option("--disconnect-daemon", "Also disconnect a daemon related to the same Hub") - .option("--force", "Remove daemon authority even if Hub is offline"), + .option("--force", "Remove daemon authority without notifying the Hub"), ).action( withOutput(async (...args) => { const options = args.at(-2) as HubLogoutOptions; diff --git a/packages/server/src/server/hub/relationship-controller.test.ts b/packages/server/src/server/hub/relationship-controller.test.ts index c7ffbd833..9f83a5a6a 100644 --- a/packages/server/src/server/hub/relationship-controller.test.ts +++ b/packages/server/src/server/hub/relationship-controller.test.ts @@ -344,16 +344,18 @@ describe("Hub relationship", () => { const credential = relationship.relationshipFile()?.credential?.secret; relationship.loseEnrollmentResponse(); await connecting.result; - relationship.failRevocations(2); + relationship.failRevocations(1); const disconnected = await relationship.disconnect(); - expect(disconnected.state).toBe("disconnecting"); - expect(relationship.relationshipFile()?.state).toBe("disconnecting"); + const reconnected = await relationship.beginConnect("fresh-token").result; - await relationship.restartDaemon(); - await relationship.retry(); - - expect(relationship.revocationAttempts()).toBe(3); + expect(disconnected).toMatchObject({ + state: "not_connected", + hub: null, + warning: expect.stringContaining("server-side revocation may remain pending"), + }); + expect(reconnected.state).toBe("connecting"); + expect(relationship.revocationAttempts()).toBe(1); expect(relationship.latestRevocation()).toEqual( expect.objectContaining({ daemonId: enrollment.daemonId, @@ -361,7 +363,10 @@ describe("Hub relationship", () => { credential, }), ); - expect(relationship.relationshipFile()).toBeNull(); + expect(relationship.pendingRelationshipRetries()).toBe(0); + expect(relationship.loggableValues(disconnected)).toContain( + "Failed to notify Hub before removing local relationship", + ); }); test("disconnect revokes only after an in-flight enrollment settles", async () => { @@ -371,7 +376,8 @@ describe("Hub relationship", () => { const enrollment = await relationship.enrollmentBegins(); const disconnecting = relationship.beginDisconnect(); - await relationship.relationshipStateBecomes("disconnecting"); + await relationship.connectionStateBecomes("disconnecting"); + expect(relationship.relationshipFile()?.state).toBe("disconnecting"); expect(relationship.revocationAttempts()).toBe(0); relationship.completeEnrollment(); @@ -385,6 +391,40 @@ describe("Hub relationship", () => { expect(relationship.relationshipFile()).toBeNull(); }); + test("disconnect persists terminal intent only while remote notification is in flight", async () => { + relationship = await HubRelationshipHarness.start(); + await relationship.beginConnect().result; + relationship.holdRevocation(); + + const disconnecting = relationship.beginDisconnect(); + await relationship.relationshipStateBecomes("disconnecting"); + + expect(await relationship.status()).toMatchObject({ state: "disconnecting" }); + expect(relationship.pendingRelationshipRetries()).toBe(0); + + relationship.completeRevocation(); + await expect(disconnecting.result).resolves.toMatchObject({ state: "not_connected" }); + expect(relationship.relationshipFile()).toBeNull(); + }); + + test("force disconnect does not wait for or notify an in-flight enrollment", async () => { + relationship = await HubRelationshipHarness.start(); + relationship.holdEnrollment(); + const connecting = relationship.beginConnect("one-time-token"); + await relationship.enrollmentBegins(); + + const disconnected = await relationship.disconnect(true); + + expect(disconnected).toMatchObject({ state: "not_connected", hub: null }); + expect(relationship.revocationAttempts()).toBe(0); + expect(relationship.relationshipFile()).toBeNull(); + + relationship.completeEnrollment(); + await connecting.result; + expect(relationship.socketAttempts()).toBe(0); + expect(await relationship.status()).toMatchObject({ state: "not_connected", hub: null }); + }); + test("daemon restart reconnects the same durable relationship", async () => { relationship = await HubRelationshipHarness.start(); await relationship.beginConnect().result; @@ -729,48 +769,73 @@ describe("Hub relationship", () => { expect(loggable).not.toContain(enrollment.idempotencyKey); }); - test("offline disconnect retries across runtime and restart without opening a socket", async () => { + test("offline disconnect removes local authority after one notify attempt", async () => { relationship = await HubRelationshipHarness.start(); await relationship.beginConnect().result; relationship.connectLatestSocket(); relationship.failRevocations(3); - const disconnecting = await relationship.disconnect(); - await relationship.retry(); + const disconnected = await relationship.disconnect(); await relationship.restartDaemon(); - await relationship.retry(); - expect(disconnecting.state).toBe("disconnecting"); - expect(relationship.revocationAttempts()).toBe(4); + expect(disconnected).toMatchObject({ + state: "not_connected", + hub: null, + warning: expect.stringContaining("server-side revocation may remain pending"), + }); + expect(await relationship.status()).toMatchObject({ state: "not_connected", hub: null }); + expect(relationship.revocationAttempts()).toBe(1); expect(relationship.socketAttempts()).toBe(1); + expect(relationship.pendingRelationshipRetries()).toBe(0); expect(relationship.relationshipFile()).toBeNull(); }); - test("successful disconnect clears a transient revocation error", async () => { + test("successful disconnect notifies the Hub and returns clean local status", async () => { relationship = await HubRelationshipHarness.start(); await relationship.beginConnect().result; - relationship.failRevocations(1); - const disconnecting = await relationship.disconnect(); - await relationship.retry(); + const disconnected = await relationship.disconnect(); - expect(disconnecting).toMatchObject({ state: "disconnecting", error: expect.any(String) }); + expect(disconnected).toMatchObject({ state: "not_connected", hub: null, error: null }); + expect(disconnected).not.toHaveProperty("warning"); + expect(relationship.revocationAttempts()).toBe(1); expect(await relationship.status()).toMatchObject({ state: "not_connected", error: null }); }); - test("force disconnect removes local authority and reports the remote warning", async () => { + test("force disconnect removes local authority without notifying the Hub", async () => { relationship = await HubRelationshipHarness.start(); await relationship.beginConnect().result; - relationship.failRevocations(1); - await relationship.disconnect(); const forced = await relationship.disconnect(true); - expect(forced.state).toBe("not_connected"); - expect(forced.warning).toContain("remote revocation"); + expect(forced).toMatchObject({ state: "not_connected", hub: null }); + expect(forced).not.toHaveProperty("warning"); + expect(relationship.revocationAttempts()).toBe(0); + expect(relationship.loggableValues(forced)).not.toContain( + "Failed to notify Hub before removing local relationship", + ); expect(relationship.relationshipFile()).toBeNull(); }); + test("startup removes a legacy persisted disconnecting relationship", async () => { + relationship = await HubRelationshipHarness.start(); + await relationship.beginConnect().result; + const active = relationship.relationshipFile(); + await relationship.corruptRelationshipFile( + JSON.stringify({ ...active, state: "disconnecting" }), + ); + + await relationship.startStoppedDaemon(); + + expect(await relationship.status()).toMatchObject({ state: "not_connected", hub: null }); + expect(relationship.relationshipFile()).toBeNull(); + expect(relationship.revocationAttempts()).toBe(0); + expect(relationship.pendingRelationshipRetries()).toBe(0); + expect(relationship.loggableValues(await relationship.status())).toContain( + "Removed legacy disconnecting Hub relationship during startup", + ); + }); + test("disconnect leaves no intent artifacts and shutdown closes the owned agent", async () => { relationship = await HubRelationshipHarness.start(); await relationship.beginConnect().result; diff --git a/packages/server/src/server/hub/relationship-controller.ts b/packages/server/src/server/hub/relationship-controller.ts index 2c213aae6..8ce6f8050 100644 --- a/packages/server/src/server/hub/relationship-controller.ts +++ b/packages/server/src/server/hub/relationship-controller.ts @@ -197,11 +197,19 @@ export class HubRelationshipController implements HubRelationshipManagement { this.clock = options.clock ?? systemClock; this.retryPolicy = options.retryPolicy ?? new BoundedExponentialHubRetryPolicy(); this.record = this.load(); + if (this.record?.state === "disconnecting") { + // COMPAT(hubUnilateralDisconnect): added in v0.4.0, remove after 2027-02-13 once legacy records have aged out. + this.options.logger.warn( + { daemonId: this.record.relationship.daemonId }, + "Removed legacy disconnecting Hub relationship during startup", + ); + rmSync(this.filePath, { force: true }); + this.record = null; + } if (this.record?.state === "revoked") { this.state = "revoked"; this.lastError = this.record.reason ?? null; - } else if (this.record?.state === "disconnecting") this.state = "disconnecting"; - else if (this.record) this.state = "connecting"; + } else if (this.record) this.state = "connecting"; } async start(): Promise { @@ -218,7 +226,6 @@ export class HubRelationshipController implements HubRelationshipManagement { ); } } - if (this.record?.state === "disconnecting") await this.tryRevocation(this.record); } async stop(): Promise { @@ -291,14 +298,6 @@ export class HubRelationshipController implements HubRelationshipManagement { await pendingCreateCleanup; return { status: this.status() }; } - if (input.force) { - this.remove(); - await pendingCreateCleanup; - return { - status: this.status(), - warning: "Local Hub credential removed; remote revocation may remain pending.", - }; - } const disconnecting: DisconnectingRecord = { version: 1, state: "disconnecting", @@ -309,12 +308,29 @@ export class HubRelationshipController implements HubRelationshipManagement { this.persist(disconnecting); this.record = disconnecting; this.state = "disconnecting"; - if (waitForEnrollment) { + if (waitForEnrollment && !input.force) { await Promise.all(this.inFlightEnrollments); } - await this.tryRevocation(disconnecting); + let warning: string | undefined; + if (!input.force) { + try { + await this.options.remote.revoke({ + daemonId: disconnecting.relationship.daemonId, + hubOrigin: disconnecting.relationship.hubOrigin, + credential: disconnecting.credential.secret, + }); + } catch (error) { + this.options.logger.warn( + { err: error, daemonId: disconnecting.relationship.daemonId }, + "Failed to notify Hub before removing local relationship", + ); + warning = + "Hub could not be reached; local relationship removed, but server-side revocation may remain pending."; + } + } + this.remove(); await pendingCreateCleanup; - return { status: this.status() }; + return { status: this.status(), ...(warning ? { warning } : {}) }; } private async tryEnrollment(pending: PendingRecord, enrollmentGeneration: number): Promise { @@ -457,24 +473,6 @@ export class HubRelationshipController implements HubRelationshipManagement { }); } - private async tryRevocation(record: DisconnectingRecord): Promise { - const generation = this.generation; - try { - await this.options.remote.revoke({ - daemonId: record.relationship.daemonId, - hubOrigin: record.relationship.hubOrigin, - credential: record.credential.secret, - }); - if (generation !== this.generation) return; - this.remove(); - } catch (error) { - if (generation !== this.generation) return; - this.lastError = error instanceof Error ? error.message : String(error); - this.state = "disconnecting"; - this.schedule(() => void this.tryRevocation(record)); - } - } - private schedule(task: () => void): void { this.retry?.cancel(); const generation = this.generation; diff --git a/packages/server/src/server/hub/test-utils/relationship-harness.ts b/packages/server/src/server/hub/test-utils/relationship-harness.ts index a74ebe068..ac3408670 100644 --- a/packages/server/src/server/hub/test-utils/relationship-harness.ts +++ b/packages/server/src/server/hub/test-utils/relationship-harness.ts @@ -284,6 +284,7 @@ class InMemoryHubRelationships implements HubRelationshipRemote { private enrollmentRejection: 401 | 403 | null = null; private enrollmentScopes = ["hub.execution.*"]; private revokeFailures = 0; + private revocationGate: Deferred | null = null; private readonly relationships = new Set(); readonly enrollmentSnapshots: RelationshipInvocationSnapshot[] = []; readonly socketSnapshots: RelationshipInvocationSnapshot[] = []; @@ -353,8 +354,19 @@ class InMemoryHubRelationships implements HubRelationshipRemote { this.revokeFailures = count; } + holdRevocation(): void { + this.revocationGate = deferred(); + } + + completeRevocation(): void { + if (!this.revocationGate) throw new Error("No revocation is waiting"); + this.revocationGate.resolve(); + this.revocationGate = null; + } + async revoke(input: HubRevocation): Promise { this.revocations.push({ ...input }); + await this.revocationGate?.promise; if (this.revokeFailures > 0) { this.revokeFailures--; throw new Error("Hub is offline"); @@ -510,6 +522,14 @@ export class HubRelationshipHarness { } } + async connectionStateBecomes(expected: string): Promise { + for (let attempt = 0; attempt < 100; attempt++) { + if ((await this.status()).state === expected) return; + await new Promise((resolve) => setTimeout(resolve, 10)); + } + throw new Error(`Hub connection state did not become ${expected}`); + } + async manageRelationshipFromExternalSocket(): Promise { const address = Object.values(networkInterfaces()) .flat() @@ -596,6 +616,14 @@ export class HubRelationshipHarness { this.remote.failRevocations(count); } + holdRevocation(): void { + this.remote.holdRevocation(); + } + + completeRevocation(): void { + this.remote.completeRevocation(); + } + failProviderPromptStart(prompt = "Create through the Hub"): void { this.promptsToFail.add(prompt); }