From e4b2e4458cb2c15c1c41005bdc68faed55daa9f5 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 12 Nov 2026 22:29:53 +0000 Subject: [PATCH 1/2] test(server): failed agent-device daemon stop is reported Turning agent device access off ran 'agent-device daemon stop' and ignored a nonzero exit or timeout, so stopAgent reported success while the daemon kept running. This test fails on main. Refs pingdotgg/t3code#35639 AI-assisted; verified with the tests and tools listed above. --- .../server/src/device/LocalDeviceHost.test.ts | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/apps/server/src/device/LocalDeviceHost.test.ts b/apps/server/src/device/LocalDeviceHost.test.ts index e133b4f9db36c..97a4dd549eb93 100644 --- a/apps/server/src/device/LocalDeviceHost.test.ts +++ b/apps/server/src/device/LocalDeviceHost.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "@effect/vitest"; import * as NodePath from "@effect/platform-node/NodePath"; import * as HostProcess from "@t3tools/shared/HostProcess"; 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 * as Path from "effect/Path"; @@ -41,6 +42,7 @@ const agentFixture = Effect.fn("agentFixture")(function* ( ) => Effect.Effect, avdOutput: ProcessRunner.ProcessRunOutput = processOutput(0), previousDaemon?: typeof LocalDeviceHost.__testing.AgentDeviceDaemonFile.Type, + daemonStopOutput: ProcessRunner.ProcessRunOutput = processOutput(0), ) { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; @@ -100,9 +102,9 @@ const agentFixture = Effect.fn("agentFixture")(function* ( Effect.provideService(ProcessRunner.ProcessRunner, { run: (input) => { runs.push(input); - return input.args.includes("devices") - ? startup(stateDir) - : Effect.succeed(input.args.includes("-list-avds") ? avdOutput : processOutput(0)); + if (input.args.includes("devices")) return startup(stateDir); + if (input.args.includes("daemon")) return Effect.succeed(daemonStopOutput); + return Effect.succeed(input.args.includes("-list-avds") ? avdOutput : processOutput(0)); }, }), Effect.provideService( @@ -338,6 +340,29 @@ describe("AgentDevice startup", () => { }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); + // regression: #35639 turning agent access off reported success while the daemon kept running + it.effect("reports an agent-device daemon that fails to stop", () => + Effect.gen(function* () { + for (const failedStop of [ + processOutput(1, "", "stop failed"), + processOutput(0, "", "", true), + ]) { + const { host } = yield* agentFixture( + () => Effect.die("A session that never readies agent tools must not start the daemon"), + processOutput(0), + { + httpPort: 12345, + token: "previous-token", + pid: process.pid, + version: AGENT_DEVICE_VERSION, + }, + failedStop, + ); + expect(Exit.isFailure(yield* Effect.exit(host.stopAgent))).toBe(true); + } + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + it.effect("drops a dead daemon's record without running agent-device when the host stops", () => Effect.gen(function* () { // Above the Linux and macOS PID limits, so never a live process. From 0e1e6b63cd22e16b98480dbd4ce1b1201908b333 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 12 Nov 2026 22:38:08 +0000 Subject: [PATCH 2/2] fix(server): turning agent device access off reports a daemon that fails to stop Stopping the agent-device daemon ignored a failed or timed-out `daemon stop`, and a live recorded daemon whose CLI was gone, so stopAgent always succeeded and agent access was saved as off while the daemon kept running. The SSH host also ignored the remote stop result. stopAgent now fails in those cases, and configure stops agent tools before saving the setting, so a failed stop leaves access on and the error reaches Settings. Host shutdown still ignores the failure. Fixes pingdotgg/t3code#35639 AI-assisted; verified with the tests and tools listed above. --- apps/server/src/device/DeviceHost.ts | 2 +- apps/server/src/device/DeviceService.test.ts | 35 ++++++++++++++++++-- apps/server/src/device/DeviceService.ts | 21 ++++++++++-- apps/server/src/device/LocalDeviceHost.ts | 27 ++++++++++++--- apps/server/src/device/SshDeviceHost.ts | 6 ++-- apps/server/src/device/sshDeviceScript.ts | 3 +- 6 files changed, 81 insertions(+), 13 deletions(-) diff --git a/apps/server/src/device/DeviceHost.ts b/apps/server/src/device/DeviceHost.ts index eafa3b6319de3..5e35458d62efc 100644 --- a/apps/server/src/device/DeviceHost.ts +++ b/apps/server/src/device/DeviceHost.ts @@ -206,7 +206,7 @@ export class DeviceHost extends Context.Service< DeviceHostError | DeviceHostTimeoutError | AgentDeviceStartError | NodeRuntimeUnavailableError >; readonly current: Effect.Effect; - readonly stopAgent: Effect.Effect; + readonly stopAgent: Effect.Effect; /** Stops helpers. Devices themselves keep running; the user owns those. */ readonly stop: Effect.Effect; } diff --git a/apps/server/src/device/DeviceService.test.ts b/apps/server/src/device/DeviceService.test.ts index f2e0bcdf0c725..db29485a16ce4 100644 --- a/apps/server/src/device/DeviceService.test.ts +++ b/apps/server/src/device/DeviceService.test.ts @@ -99,6 +99,7 @@ const fixture = Effect.fn("fixture")(function* ( readonly routesStreams?: boolean; readonly onClose?: Effect.Effect<"ok" | "missing" | "reject" | "http_error" | "malformed">; readonly onPlatformAvailability?: Effect.Effect; + readonly onStopAgent?: Effect.Effect; } = {}, ) { const settings = yield* Ref.make(DEFAULT_SERVER_SETTINGS); @@ -292,7 +293,7 @@ const fixture = Effect.fn("fixture")(function* ( ), stopAgent: Effect.sync(() => { agentStops.push("stop"); - }), + }).pipe(Effect.andThen(options.onStopAgent ?? Effect.void)), stop: Effect.sync(() => { starts.push("stop"); }), @@ -632,7 +633,28 @@ describe("device setup consent", () => { it.effect("installs agent support only after the separate agent permission", () => Effect.gen(function* () { - const { service, agentStarts, agentStops, settings } = yield* fixture(); + const stopFails = yield* Ref.make(false); + const { service, agentStarts, agentStops, settings } = yield* fixture( + Effect.void, + undefined, + false, + undefined, + false, + undefined, + { + onStopAgent: Effect.flatMap(Ref.get(stopFails), (fails) => + fails + ? Effect.fail( + new DeviceHost.DeviceHostError({ + hostId: LOCAL_DEVICE_HOST_ID, + step: "stopping agent tools", + cause: new Error("stop failed"), + }), + ) + : Effect.void, + ), + }, + ); yield* service.configure({ enabled: true }); expect(agentStarts).toEqual([]); expect(yield* service.agentReadinessIfSupported()).toBeNull(); @@ -642,8 +664,15 @@ describe("device setup consent", () => { expect((yield* Ref.get(settings)).enableAgentDeviceAccess).toBe(true); expect((yield* service.state).agentAccessEnabled).toBe(true); + yield* Ref.set(stopFails, true); + const failed = yield* Effect.flip(service.configure({ agentAccessEnabled: false })); + expect(failed._tag).toBe("DeviceHostUnavailableError"); + expect((yield* Ref.get(settings)).enableAgentDeviceAccess).toBe(true); + expect((yield* service.state).agentAccessEnabled).toBe(true); + yield* Ref.set(stopFails, false); + yield* service.configure({ agentAccessEnabled: false, onboardingCompleted: true }); - expect(agentStops).toEqual(["stop"]); + expect(agentStops).toEqual(["stop", "stop"]); expect((yield* service.state).onboardingCompleted).toBe(true); expect((yield* Ref.get(settings)).deviceOnboardingCompleted).toBe(true); }).pipe(Effect.scoped), diff --git a/apps/server/src/device/DeviceService.ts b/apps/server/src/device/DeviceService.ts index b365a6b9fe5c1..0b8f322a3f260 100644 --- a/apps/server/src/device/DeviceService.ts +++ b/apps/server/src/device/DeviceService.ts @@ -702,6 +702,25 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function* const nextAgentAccess = input.agentAccessEnabled ?? currentSettings.agentAccessEnabled; yield* lifecycleLock.withPermit( Effect.gen(function* () { + // Stop first so a daemon that keeps running leaves agent access on, not falsely off. + if (nextEnabled && input.agentAccessEnabled === false) { + yield* Effect.forEach( + hosts.values(), + (host) => + host.stopAgent.pipe( + Effect.mapError( + (error) => + new DeviceHostUnavailableError({ + hostId: host.id, + hostLabel: host.label, + reason: `Device support failed during ${error.step}.`, + cause: error, + }), + ), + ), + { discard: true }, + ); + } yield* settings .updateSettings({ ...(input.enabled === undefined ? {} : { enableDeviceSupport: input.enabled }), @@ -724,8 +743,6 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function* ); if (!nextEnabled) { yield* Effect.forEach(hosts.values(), (host) => host.stop, { discard: true }); - } else if (input.agentAccessEnabled === false) { - yield* Effect.forEach(hosts.values(), (host) => host.stopAgent, { discard: true }); } yield* publish((state) => ({ ...state, diff --git a/apps/server/src/device/LocalDeviceHost.ts b/apps/server/src/device/LocalDeviceHost.ts index c5097509dcade..a068df36a2137 100644 --- a/apps/server/src/device/LocalDeviceHost.ts +++ b/apps/server/src/device/LocalDeviceHost.ts @@ -642,11 +642,19 @@ export const make = Effect.fn("LocalDeviceHost.make")(function* () { ...(Option.isSome(recorded) && recorded.value.version ? [recorded.value.version] : []), AGENT_DEVICE_VERSION, ]); - if (!entryPath) return; + if (!entryPath) { + // A live recorded daemon outlives its deleted CLI, so nothing here can stop it. + if (Option.isNone(recorded) || !recorded.value.pid) return; + return yield* new DeviceHost.DeviceHostError({ + hostId, + step: "stopping agent tools", + cause: new Error("agent-device is not installed, so its running daemon cannot be stopped"), + }); + } const nodePath = yield* resolveNodeExecutable("Local device support", hostEnvironment).pipe( Effect.provideService(HostProcess.Platform, hostPlatform), ); - yield* runner.run({ + const result = yield* runner.run({ command: nodePath, args: [ entryPath, @@ -659,10 +667,21 @@ export const make = Effect.fn("LocalDeviceHost.make")(function* () { timeout: Duration.seconds(10), timeoutBehavior: "timedOutResult", }); + if (result.timedOut || result.code !== 0) { + return yield* new DeviceHost.DeviceHostError({ + hostId, + step: "stopping agent tools", + cause: result, + }); + } }).pipe( Effect.provideService(FileSystem.FileSystem, fs), Effect.provideService(Path.Path, path), - Effect.ignore, + Effect.mapError((cause) => + cause._tag === "DeviceHostError" + ? cause + : new DeviceHost.DeviceHostError({ hostId, step: "stopping agent tools", cause }), + ), ); const ensureHubReady = Effect.fn("LocalDeviceHost.ensureHubReady")(function* ( @@ -872,7 +891,7 @@ export const make = Effect.fn("LocalDeviceHost.make")(function* () { const running = yield* Ref.getAndSet(runningRef, null); yield* stopHub(running?.hub); yield* fs.remove(hubStatePath(), { force: true }).pipe(Effect.ignore); - yield* stopAgentDeviceDaemon; + yield* stopAgentDeviceDaemon.pipe(Effect.ignore); }), ); diff --git a/apps/server/src/device/SshDeviceHost.ts b/apps/server/src/device/SshDeviceHost.ts index d769a85ddb080..9a4ecc98af0b2 100644 --- a/apps/server/src/device/SshDeviceHost.ts +++ b/apps/server/src/device/SshDeviceHost.ts @@ -530,7 +530,7 @@ export const make = Effect.fn("SshDeviceHost.make")(function* ( connectionScope = null; if (previousScope) yield* Scope.close(previousScope, Exit.void); if (!enabled) yield* provide(bootstrap(config, owner, "stop-agent")); - return yield* connect().pipe( + const connected = connect().pipe( Effect.onError(() => Effect.gen(function* () { const failedScope = connectionScope; @@ -541,6 +541,8 @@ export const make = Effect.fn("SshDeviceHost.make")(function* ( }), ), ); + // The daemon is already stopped; a failed hub reconnect is retried by the next ensureReady. + return yield* enabled ? connected : connected.pipe(Effect.orElseSucceed(() => null)); }), ); yield* Effect.addFinalizer(() => stop); @@ -580,7 +582,7 @@ export const make = Effect.fn("SshDeviceHost.make")(function* ( ), ), ), - stopAgent: changeAgent(false).pipe(Effect.asVoid, Effect.ignore), + stopAgent: changeAgent(false).pipe(Effect.asVoid), stop, platformAvailability, } satisfies DeviceHost.DeviceHost["Service"]; diff --git a/apps/server/src/device/sshDeviceScript.ts b/apps/server/src/device/sshDeviceScript.ts index 2e3e895293fa3..a5714232ba5ec 100644 --- a/apps/server/src/device/sshDeviceScript.ts +++ b/apps/server/src/device/sshDeviceScript.ts @@ -164,7 +164,8 @@ async function install(name, version, entry) { stopHub(hub); fs.rmSync(hubFile, { force: true }); } - stopAgent(); + const hadDaemon = Boolean(read(daemonFile)); + if (!stopAgent() && hadDaemon && mode === 'stop-agent') throw Error('Could not stop the agent-device daemon in ' + state); return; } // A start without agent tools means the server holds no claims, so a daemon still here is