From fc4d92aa593b0afc3dc5f6f9dc07da2177c55ccc Mon Sep 17 00:00:00 2001 From: LiangshouX Date: Tue, 3 Nov 2026 13:23:25 +0800 Subject: [PATCH] fix(dsh): keep the recovery for an unverified service record A recorded process that holds the port but whose identity cannot be read left the tab with no action at all: no Stop (the process is not ours to signal), no Start (a conflict disables it) and no Forget (nothing had refused yet, so no record was ever named). The record now travels with the status, so the card can offer "Forget this record", and a start names the same record as an unverified one. The process is still never signalled: an unreadable identity is no proof. A conflict caused by an unrelated process stays a plain conflict, with no record to name. --- .../dsh/src/environment/client/rpc.ts | 1 + .../dsh/src/environment/client/service.tsx | 18 +++++ .../dsh/src/environment/host/manager.ts | 46 +++++++----- .../dsh/src/environment/host/service.ts | 38 +++++++++- integrations/dsh/src/environment/types.ts | 10 +++ integrations/dsh/tests/env-manager.test.mjs | 71 +++++++++++++++++++ integrations/dsh/tests/env-service.test.mjs | 40 +++++++++++ integrations/dsh/tests/env-tab.test.mjs | 52 ++++++++++++++ 8 files changed, 257 insertions(+), 19 deletions(-) diff --git a/integrations/dsh/src/environment/client/rpc.ts b/integrations/dsh/src/environment/client/rpc.ts index 7b011de5..832d7535 100644 --- a/integrations/dsh/src/environment/client/rpc.ts +++ b/integrations/dsh/src/environment/client/rpc.ts @@ -249,6 +249,7 @@ export function parseServiceState(value: unknown): ServiceState { optionalPidField(body, "pid", what); optionalStringField(body, "version", what); optionalStringField(body, "startedAt", what); + optionalRecordField(body, "record", what); return value as unknown as ServiceState; } diff --git a/integrations/dsh/src/environment/client/service.tsx b/integrations/dsh/src/environment/client/service.tsx index eb0a322d..9b19c2c1 100644 --- a/integrations/dsh/src/environment/client/service.tsx +++ b/integrations/dsh/src/environment/client/service.tsx @@ -220,6 +220,24 @@ export function useService( ? undefined : current, ); + // A record the Host could not verify is the only way out of a port it + // cannot clear, and the conflict card offers no other action: the + // status names it, so the recovery is offered without waiting for a + // refused start the card does not allow. + const unverified = next.record; + if (unverified) + setForgetTarget((current) => + current && + current.endpoint === unverified.endpoint && + current.pid === unverified.pid + ? current + : { + endpoint: unverified.endpoint, + ...(unverified.pid === undefined + ? {} + : { pid: unverified.pid }), + }, + ); // A start this tab requested maps "stopped with a live pid" (the Host's // "launch still booting" signal) to `starting`; a plain status read // keeps the Host's phase, so a leftover process never locks the card. diff --git a/integrations/dsh/src/environment/host/manager.ts b/integrations/dsh/src/environment/host/manager.ts index e3e2d75f..7e1e2e0f 100644 --- a/integrations/dsh/src/environment/host/manager.ts +++ b/integrations/dsh/src/environment/host/manager.ts @@ -567,28 +567,42 @@ export class ReMeEnvManager { // conflict card offers no way out, so it is replaced here instead of // leaving the user with a process to kill by hand. const leftover = await this.leftoverService(endpointValue); - if (!leftover) + if (leftover) { + const replaced = await stopService(this.serviceIO, { + statePath: this.serviceStatePath, + endpoint: leftover.endpoint, + ownership: "owned", + }); + if (!replaced.stopped) + return this.refused( + logPath, + now, + "previous-not-stopped", + `A previous launch could not be stopped: ${ + replaced.reason ?? "unknown reason" + }`, + { record: replaced.record }, + ); + } else { + // The port may be held by a recorded process whose identity cannot + // be read. It must never be signalled, but the record has to be + // named: it is the only way out of a port the tab cannot clear. + const unverified = current.record; + if (unverified) + return this.refused( + logPath, + now, + "unverified-record", + `The recorded service for ${unverified.endpoint} (pid ${unverified.pid}) could not be verified; leaving it untouched`, + { pid: unverified.pid, record: unverified }, + ); return this.refused( logPath, now, "conflict", "Another process is already listening on this endpoint", ); - const replaced = await stopService(this.serviceIO, { - statePath: this.serviceStatePath, - endpoint: leftover.endpoint, - ownership: "owned", - }); - if (!replaced.stopped) - return this.refused( - logPath, - now, - "previous-not-stopped", - `A previous launch could not be stopped: ${ - replaced.reason ?? "unknown reason" - }`, - { record: replaced.record }, - ); + } } else { const previous = await stopService(this.serviceIO, { statePath: this.serviceStatePath, diff --git a/integrations/dsh/src/environment/host/service.ts b/integrations/dsh/src/environment/host/service.ts index d8788971..511a6bd5 100644 --- a/integrations/dsh/src/environment/host/service.ts +++ b/integrations/dsh/src/environment/host/service.ts @@ -14,7 +14,7 @@ import { basename, dirname } from "node:path"; import { safeError } from "../../endpoint.js"; import { ReMeClient } from "../../reme/client.js"; import type { ReMeHealthResult } from "../../reme/types.js"; -import { serviceEndpointKey } from "../endpoint-key.js"; +import { serviceEndpointKey, serviceOriginKey } from "../endpoint-key.js"; import { parseLocalEndpoint } from "../endpoint-rules.js"; import type { HealthErrorCode, @@ -369,8 +369,36 @@ export async function serviceStatus( recordForEndpoint && recordAlive ? serviceOwnership(io, persisted, options.endpoint) : "foreign"; + /** + * The record behind the status, when the Host cannot prove that the process + * is the one the record describes. + * + * An unreadable identity is no proof, so the process is never signalled — + * but the record is what the tab has to drop to get the port back, and it + * has to travel with the status: the conflict card cannot offer a recovery + * that only a refused start would name. + * + * A record for another path on the same origin describes the same local + * service, so it is related too. + */ + const unverified = async ( + owner: ServiceOwnership, + ): Promise => { + if (!persisted || !recordAlive) return undefined; + if (owner === "unknown") return serviceRecordRef(persisted); + if (recordForEndpoint) return undefined; + if ( + serviceOriginKey(persisted.endpoint) !== + serviceOriginKey(options.endpoint) + ) + return undefined; + const other = await serviceOwnership(io, persisted, persisted.endpoint); + return other === "unknown" ? serviceRecordRef(persisted) : undefined; + }; if (health.ok) { - const managed = (await ownership()) === "owned"; + const owner = await ownership(); + const managed = owner === "owned"; + const record = await unverified(owner); return { phase: "running", managed, @@ -380,6 +408,7 @@ export async function serviceStatus( : {}), ...(health.version ? { version: health.version } : {}), ...(managed && persisted ? { startedAt: persisted.startedAt } : {}), + ...(record ? { record } : {}), logPath: options.logPath, }; } @@ -393,7 +422,9 @@ export async function serviceStatus( if (portOpen) { // Our own service can occupy the port without being healthy (still // booting, or a rejected token); identify it so the tab can stop it. - const managed = (await ownership()) === "owned"; + const owner = await ownership(); + const managed = owner === "owned"; + const record = await unverified(owner); return { phase: "conflict", managed, @@ -401,6 +432,7 @@ export async function serviceStatus( ...(managed && persisted?.pid !== undefined ? { pid: persisted.pid } : {}), + ...(record ? { record } : {}), logPath: options.logPath, }; } diff --git a/integrations/dsh/src/environment/types.ts b/integrations/dsh/src/environment/types.ts index 4c5f53c9..a4e1e299 100644 --- a/integrations/dsh/src/environment/types.ts +++ b/integrations/dsh/src/environment/types.ts @@ -141,6 +141,16 @@ export interface ServiceState { pid?: number; version?: string; startedAt?: string; + /** + * The record that describes the process behind this status, when the Host + * could not prove that the process is the one the record describes. + * + * The process is then never signalled — an unreadable identity is no proof + * — but the record is what the tab has to drop to get the port back, so it + * travels with the status instead of waiting for a refusal that a disabled + * button can never trigger. + */ + record?: ServiceRecordRef; logPath: string; } diff --git a/integrations/dsh/tests/env-manager.test.mjs b/integrations/dsh/tests/env-manager.test.mjs index 53f26d5f..823979fe 100644 --- a/integrations/dsh/tests/env-manager.test.mjs +++ b/integrations/dsh/tests/env-manager.test.mjs @@ -671,6 +671,77 @@ test("replaces a leftover launch whose record cannot be re-keyed", async (t) => assert.equal(readServiceState(statePath)?.pid, 5001); }); +test("a conflict held by an unverified record names that record", async (t) => { + // The recorded process holds the port, but its identity cannot be read: it + // must never be signalled. The refusal still names the record, because the + // record is the only way out of a port the tab cannot clear. + const { manager, calls, directory, io } = managerIn(t); + manager.serviceStatus = ReMeEnvManager.prototype.serviceStatus; + const statePath = join(directory, "reme-memory", "service.json"); + writeServiceState(statePath, { + pid: 6000, + endpoint: "http://127.0.0.1:2334", + workspaceDir: directory, + pythonPath: "D:\\env\\python.exe", + command: "reme start", + startedAt: "2026-10-27T00:00:00.000Z", + }); + // The process is alive, but its command line cannot be read. + io.processCommandLine = async () => ""; + io.fetchHealth = async () => ({ ok: false, version: "", error: "boom" }); + io.portOpen = async () => true; + + const status = await manager.serviceStatus("http://127.0.0.1:2334"); + assert.equal(status.phase, "conflict"); + assert.equal(status.managed, false); + assert.deepEqual(status.record, { + endpoint: "http://127.0.0.1:2334", + pid: 6000, + }); + + const result = await manager.serviceStart( + "http://127.0.0.1:2334", + "D:\\env\\python.exe", + join(directory, "workspace"), + ); + assert.equal(result.started, false); + assert.equal(result.code, "unverified-record"); + assert.deepEqual(result.record, { + endpoint: "http://127.0.0.1:2334", + pid: 6000, + }); + assert.deepEqual( + calls.killTree, + [], + "an unverified process is never signalled", + ); + assert.equal(calls.spawn.length, 0); +}); + +test("a conflict held by an unrelated process stays a plain conflict", async (t) => { + // No record at all: the port belongs to something else, and there is no + // record the tab could forget. + const { manager, calls, directory, io } = managerIn(t); + manager.serviceStatus = ReMeEnvManager.prototype.serviceStatus; + io.fetchHealth = async () => ({ ok: false, version: "", error: "boom" }); + io.portOpen = async () => true; + + const status = await manager.serviceStatus("http://127.0.0.1:2334"); + assert.equal(status.phase, "conflict"); + assert.equal(status.record, undefined); + + const result = await manager.serviceStart( + "http://127.0.0.1:2334", + "D:\\env\\python.exe", + join(directory, "workspace"), + ); + assert.equal(result.started, false); + assert.equal(result.code, "conflict"); + assert.equal(result.record, undefined, "there is no record to name"); + assert.deepEqual(calls.killTree, []); + assert.equal(calls.spawn.length, 0); +}); + test("a prefixed request never stops the healthy service it cannot replace", async (t) => { // A prefixed endpoint can never be launched: the launch sets only host and // port, so the request is refused later anyway. Replacing the same-origin diff --git a/integrations/dsh/tests/env-service.test.mjs b/integrations/dsh/tests/env-service.test.mjs index 7ea86260..66bb4ed3 100644 --- a/integrations/dsh/tests/env-service.test.mjs +++ b/integrations/dsh/tests/env-service.test.mjs @@ -233,6 +233,46 @@ test("a conflict still identifies a provably owned service", async (t) => { assert.equal(status.pid, 777); }); +test("an unreadable identity names the record the status cannot clear", async (t) => { + // The process is alive, but its command line cannot be read: ownership stays + // unproven, so the process is never signalled. The record is nevertheless + // the only way out of a port the tab cannot clear, so it travels with the + // status instead of waiting for a refusal the card cannot trigger. + const { directory, statePath } = tempState(t, "reme-unverified-"); + record(statePath); + const { io } = fakeIO({ + portOpen: async () => true, + processCommandLine: async () => "", + }); + const status = await serviceStatus(io, { + endpoint: ENDPOINT, + statePath, + logPath: join(directory, "service.log"), + }); + assert.equal(status.phase, "conflict"); + assert.equal(status.managed, false, "an unproven identity is not ours"); + assert.equal(status.pid, undefined, "and no pid is claimed either"); + assert.deepEqual(status.record, { endpoint: ENDPOINT, pid: 777 }); +}); + +test("a record that names no backend stays out of the status", async (t) => { + // A record for another endpoint says nothing about this one, so it is not + // offered as a recovery here. + const { directory, statePath } = tempState(t, "reme-other-record-"); + record(statePath, { endpoint: "http://127.0.0.1:2333" }); + const { io } = fakeIO({ + portOpen: async () => true, + processCommandLine: async () => "", + }); + const status = await serviceStatus(io, { + endpoint: ENDPOINT, + statePath, + logPath: join(directory, "service.log"), + }); + assert.equal(status.phase, "conflict"); + assert.equal(status.record, undefined); +}); + test("a recycled pid that is no longer the service is not managed", async (t) => { const { statePath } = tempState(t, "reme-pid-"); record(statePath); diff --git a/integrations/dsh/tests/env-tab.test.mjs b/integrations/dsh/tests/env-tab.test.mjs index 15140dd4..ec3b74f7 100644 --- a/integrations/dsh/tests/env-tab.test.mjs +++ b/integrations/dsh/tests/env-tab.test.mjs @@ -1794,6 +1794,58 @@ test("Forget targets the record the Host named, not the endpoint on screen", asy assert.equal(forget.input.expectPid, 6000, "the record is pinned by its pid"); }); +test("a conflict whose record cannot be verified offers forgetting it", async (t) => { + // The port is held by a recorded process whose identity cannot be read. The + // card offers no Stop (the process is not ours to signal) and no Start (a + // conflict disables it), so the record the Host names with the status is + // the only way out. + const CARD = "http://127.0.0.1:2334"; + const conflict = { + phase: "conflict", + managed: false, + endpoint: CARD, + record: { endpoint: CARD, pid: 6000 }, + logPath: "C:\\Users\\test\\.reme\\service.log", + }; + const { renderer, calls, restore } = await mount( + { endpoint: CARD }, + { + responses: { + "remeEnv/serviceStatus": conflict, + "remeEnv/serviceForget": { forgotten: true, pid: 6000 }, + }, + }, + ); + t.after(restore); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + assert.ok( + buttonByText(renderer, envZh.forgetRecord), + "the record-only way out is offered without a refused start", + ); + // The confirmation names the record and the process it describes. + await act(async () => { + buttonByText(renderer, envZh.forgetRecord).props.onClick(); + }); + const confirming = texts(renderer.toJSON()).join(" "); + assert.ok(confirming.includes(CARD), "the confirmation names the record"); + assert.ok(confirming.includes("6000"), "the confirmation names the pid"); + await act(async () => { + buttonByText(renderer, envZh.forgetRecord).props.onClick(); + }); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + const forget = calls.rpc.find( + (call) => call.method === "remeEnv/serviceForget", + ); + assert.ok(forget, "forgetting reaches the Host"); + assert.equal(forget.input.endpoint, CARD); + assert.equal(forget.input.expectPid, 6000, "the record is pinned by its pid"); +}); + test("a record that changed is not forgotten on a second click", async (t) => { const stopped = { phase: "stopped",