mirror of
https://github.com/agentscope-ai/ReMe.git
synced 2026-11-15 09:11:24 +00:00
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.
This commit is contained in:
parent
f14741cad9
commit
fc4d92aa59
8 changed files with 257 additions and 19 deletions
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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<ServiceRecordRef | undefined> => {
|
||||
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,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue