fix(machines): cancel stops a follow-up update there, only the release shown, one job per PC when ssh cannot resolve

changelog.d/pc-install-followups.md

@@ -0,0 +1,9 @@
+### Fixed
+- Cancelling an update of another PC asks that PC's own Ovis to stop that update only (by the id its
+  install request was given), while it still checks or builds,
+  and says it stopped only once that PC confirms it; otherwise (an older Ovis, or one already restarting)
+  it says the update may still finish there, never that it ended.
+- An install over SSH that finds an older Ovis there updates it only to the release and build shown
+  (this app's own), and cancelling that install stops its update there too.
+- One install or setup per PC holds even when ssh cannot resolve an address, and resolving addresses no
+  longer holds the server up.

scripts/conventions.test.ts

@@ -130,6 +130,7 @@ describe("demo transport", () => {
    */
   const DEMO_404: Record<string, string> = {
     "/api/account/enroll": "account linking: no screen of the app asks for it yet (the account screens come with their own demo answers)",
+    "/api/updates/cancel": "a connection server asks a PC's own Ovis to stop its update; the demo's browser never does",
     "/api/account/introduce": "account linking: no screen of the app asks for it yet (the account screens come with their own demo answers)",
     "/api/account/introduction": "account linking: no screen of the app asks for it yet (the account screens come with their own demo answers)",
     "/api/account/link": "account linking: no screen of the app asks for it yet (the account screens come with their own demo answers)",

scripts/relay-shell-browser-qa.ts

@@ -39,7 +39,7 @@ import { shellContentSecurityPolicy } from "../server/static.ts";
 import { REGIONAL_APPS } from "../shared/relay-regions.ts";
 import type { UpdateService } from "../server/update-api.ts";
 import { MIN_SERVER_VERSION } from "../shared/relay-mux.ts";
-import { noInstalledNotes, noUpdateNotes } from "../shared/update.ts";
+import { noInstalledNotes, noUpdateNotes, INSTALL_ID } from "../shared/update.ts";
 import type { UpdateRequest, UpdateStatus } from "../shared/update.ts";
 import { version as shellVersion } from "../package.json";
 import { exportPublicKey, generateKeyPair, openDaemonSecureChannel } from "../shared/relay-e2ee.ts";
@@ -1140,7 +1140,11 @@ export async function relayShellQa(tree: RelayServerTree): Promise<void> {
           assert.deepEqual(updates.requests, [], "nothing is installed before the tap");
           await page.getByRole("button", { name: "Update now" }).click();
           await bounded(updates.installed, "the PC's update service received the install");
-          assert.deepEqual(updates.requests, [{ command: "install", expected_revision: "b".repeat(40) }], "one install of the release on offer, no check first");
+          // one install of the release on offer, named by it, with the install_id its 202 gave (a cancel names that id)
+          assert.equal(updates.requests.length, 1, "one install of the release on offer, no check first");
+          const [asked] = updates.requests as { command?: string; expected_revision?: string; install_id?: string }[];
+          assert.deepEqual([asked?.command, asked?.expected_revision], ["install", "b".repeat(40)], "one install of the release on offer, no check first");
+          assert.match(asked?.install_id ?? "", INSTALL_ID, "the install carries the id its 202 gave");
           await page.getByText("Step 4 of 5: Building the app").waitFor();
           await shot19(page, "update");
           note(`pc-outdated (managed): WELCOME ${below} < ${MIN_SERVER_VERSION} -> update screen; Update now -> POST /api/updates/install over the relay -> fake service recorded ${JSON.stringify(updates.requests)}; progress from GET /api/updates`);

scripts/test-herdr.ts

@@ -24,6 +24,13 @@ process.env["TMPDIR"] = realpathSync(tmpdir());
 // herdr socket, in this process and the children tests spawn, whatever XDG_CONFIG_HOME a test sets later
 process.env["OVIS_TEST_USER_XDG_CONFIG_HOME"] ??= process.env["XDG_CONFIG_HOME"] ?? "";
 
+// under bun test, no test runs the account's ssh -G or reads its SSH config: addresses resolve as written, and a test that
+// needs another answer replaces the resolver itself (server/ssh-address.ts)
+if (process.env["NODE_ENV"] === "test") {
+  const { literalAddress, sshAddresses } = await import("../server/ssh-address.ts");
+  sshAddresses.resolve = async (target) => literalAddress(target);
+}
+
 export const TEST_SESSION = process.env["HERDR_TEST_SESSION"] || "ovis-test";
 
 /**

server/api.contract.test.ts

@@ -831,7 +831,9 @@ describe("update API", () => {
       status = { ...status, latest_revision: offered };
       const accepted = await install(JSON.stringify({ expected_revision: offered }));
       expect(accepted.status).toBe(202);
-      expect(requests).toEqual([{ command: "install", expected_revision: offered }]);
+      // with its own id, which only its 202 tells: a cancel must name it
+      const { install_id } = await accepted.json() as { install_id: string };
+      expect(requests).toEqual([{ command: "install", expected_revision: offered, install_id }]);
       // a release that moves after the 202 is told by the status, not by the request that finished
       status = { ...status, phase: "error", latest_revision: moved, error_code: "update_offer_changed", error: "A different release is on offer than the one asked for, so nothing was installed. Review it, then update again." };
       expect(await (await fetch(`${origin}/api/updates`)).json()).toMatchObject({ phase: "error", error_code: "update_offer_changed", latest_revision: moved });
@@ -842,17 +844,17 @@ describe("update API", () => {
       expect(await code(older)).toBe("update_restart_required");
       // a check stays bodyless
       expect((await fetch(`${origin}/api/updates/check`, { method: "POST", headers: { "x-herdr-update": "1" } })).status).toBe(202);
-      expect(requests).toEqual([{ command: "install", expected_revision: offered }, { command: "check" }]);
+      expect(requests).toEqual([{ command: "install", expected_revision: offered, install_id }, { command: "check" }]);
     } finally { managed.stop(); rmSync(managedState, { recursive: true, force: true }); }
   });
 
   it("keeps status and update actions behind the token gate", async () => {
     const protectedState = mkdtempSync(join(tmpdir(), "herdr-update-auth-"));
     const protectedServer = createServer({ port: 0, stateDir: protectedState, token: "test-update-token" });
     try {
-      for (const path of ["/api/updates", "/api/updates/notes", "/api/updates/installed", "/api/updates/check", "/api/updates/install"]) {
+      for (const path of ["/api/updates", "/api/updates/notes", "/api/updates/installed", "/api/updates/check", "/api/updates/install", "/api/updates/cancel"]) {
         const response = await fetch(`http://localhost:${protectedServer.port}${path}`, {
-          method: path === "/api/updates/check" || path === "/api/updates/install" ? "POST" : "GET", headers: { "x-herdr-update": "1" },
+          method: path === "/api/updates/check" || path === "/api/updates/install" || path === "/api/updates/cancel" ? "POST" : "GET", headers: { "x-herdr-update": "1" },
         });
         expect(response.status).toBe(401);
       }
@@ -889,8 +891,20 @@ describe("update API", () => {
       // the install still names the offer, as from the app's own controls
       expect((await call("/api/updates/install", `Bearer ${bridge}`, "{}")).status).toBe(400);
       expect(requests).toEqual([]);
-      expect((await call("/api/updates/install", `Bearer ${bridge}`, JSON.stringify({ expected_revision: offered }))).status).toBe(202);
-      expect(requests).toEqual([{ command: "install", expected_revision: offered }]);
+      const granted = await call("/api/updates/install", `Bearer ${bridge}`, JSON.stringify({ expected_revision: offered }));
+      expect(granted.status).toBe(202);
+      const { install_id } = await granted.json() as { install_id: string };
+      expect(requests).toEqual([{ command: "install", expected_revision: offered, install_id }]);
+      // and stop that install while it builds (POST /api/updates/cancel), once its supervisor can
+      status.phase = "building";
+      expect((await call("/api/updates/cancel", `Bearer ${bridge}`, JSON.stringify({ install_id }))).status).toBe(409);
+      status.install_cancellable = true;
+      // a cancel that names no install is refused; the one it names is passed on, for the supervisor to stop that one only
+      expect((await call("/api/updates/cancel", `Bearer ${bridge}`, "{}")).status).toBe(400);
+      expect((await call("/api/updates/cancel", `Bearer ${bridge}`, JSON.stringify({ install_id }))).status).toBe(202);
+      expect(requests.at(-1)).toEqual({ command: "cancel", install_id });
+      status.phase = "idle";
+      requests.splice(1);
       // a wrong bridge token is refused like any other, and the token opens no other update route
       expect((await call("/api/updates", `Bearer ${"c".repeat(64)}`)).status).toBe(401);
       for (const path of ["/api/updates/notes", "/api/updates/installed", "/api/updates/check", "/api/herdr/update"]) expect((await call(path, `Bearer ${bridge}`)).status).toBe(401);
@@ -900,7 +914,8 @@ describe("update API", () => {
       });
       const install = JSON.stringify({ expected_revision: offered });
       for (const [method, path, body] of [["GET", "/api/updates?anything=1"], ["HEAD", "/api/updates"], ["POST", "/api/updates", install], ["DELETE", "/api/updates"],
-        ["GET", "/api/updates/install"], ["PUT", "/api/updates/install", install], ["POST", "/api/updates/install?anything=1", install]] as const) {
+        ["GET", "/api/updates/install"], ["PUT", "/api/updates/install", install], ["POST", "/api/updates/install?anything=1", install],
+        ["GET", "/api/updates/cancel"], ["POST", "/api/updates/cancel?anything=1", ""]] as const) {
         expect({ method, path, status: (await send(method, path, body)).status }).toEqual({ method, path, status: 401 });
       }
       expect(requests).toHaveLength(1);

server/dispatch.ts

@@ -163,7 +163,7 @@ export async function handleRequest(request: Request, ctx: ServerContext): Promi
   // replaced (server/machines.ts ownOffer). An install still names the offered revision and the
   // custom header (update-api.ts), and whoever holds the token already drives every pane here. Only those two
   // requests, each by its own method and with no query: the token opens nothing else of these routes (#2054)
-  const bridgeUpdatePath = url.search === "" && (pathname === "/api/updates" ? request.method === "GET" : pathname === "/api/updates/install" && request.method === "POST");
+  const bridgeUpdatePath = url.search === "" && (pathname === "/api/updates" ? request.method === "GET" : (pathname === "/api/updates/install" || pathname === "/api/updates/cancel") && request.method === "POST");
   // the same server runs the move to herdr-core here for the person on its side (docs/runtime-move.md,
   // "On another PC"); each move route still asks for the app's own header and no query (updateRequestAllowed)
   const bridgeMovePath = RUNTIME_MOVE_ROUTE.test(pathname);
@@ -491,7 +491,7 @@ export async function handleRequest(request: Request, ctx: ServerContext): Promi
 
   // installing an update on this PC (Ovis or herdr; a check installs too where OVIS_AUTO_UPDATE=1) is the owner's, as changing its PCs is (machine-api.ts): from any device
   // he signed in or paired himself. The connection server's bridge token asks its own install by revision (bridgeUpdatePath)
-  if (request.method === "POST" && (pathname === "/api/updates/install" || pathname === "/api/updates/check" || pathname === "/api/herdr/update") && !ownerAccess(access) && !(bridgeAuthorized && bridgeUpdatePath)) {
+  if (request.method === "POST" && (pathname === "/api/updates/install" || pathname === "/api/updates/cancel" || pathname === "/api/updates/check" || pathname === "/api/herdr/update") && !ownerAccess(access) && !(bridgeAuthorized && bridgeUpdatePath)) {
     setRequestRoute(pathname);
     return jsonResponse({ error: { code: "forbidden", message: "Only the owner's own signed-in or paired devices can update this PC, from this app's own page." } }, 403);
   }

server/machine-api.ts

@@ -330,6 +330,8 @@ export async function handleMachineRequest(request: Request, manager: MachineMan
         setRequestRoute("/api/machines/setup");
         const body = await request.json() as SetupRequest;
         // a setup that updates what runs there is an update of that PC: the owner's only, as above
+        // the addresses its one-job-per-PC check compares, resolved first (ssh -G, off the event loop, 3 s at most)
+        await manager.resolveFor(body);
         return jsonResponse(manager.setup(body), 202);
       }
       if (parts.length === 2) {
@@ -362,6 +364,7 @@ export async function handleMachineRequest(request: Request, manager: MachineMan
         if (request.method !== "POST") return fail("method_not_allowed", "Use POST", 405);
         const body: unknown = await request.json().catch(() => null);
         if (!isJsonObject(body)) return fail("invalid_body", "Expected an SSH address, or the machine_id of a PC added here", 400);
+        await manager.resolveFor(body);
         try { return jsonResponse(manager.install(body as unknown as PcInstallRequest) satisfies PcInstallJob, 202); }
         catch (error) {
           if (error instanceof PcUpdateRefused) return fail("install_unavailable", error.message, 409);
@@ -413,6 +416,8 @@ export async function handleMachineRequest(request: Request, manager: MachineMan
       const body: unknown = text.trim() === "" ? {} : (() => { try { return JSON.parse(text); } catch { return null; } })();
       const shown = isJsonObject(body) ? body.revision : undefined;
       if (!isJsonObject(body) || shown !== undefined && typeof shown !== "string") return fail("invalid_body", "Expected nothing, or { revision } of the offer shown", 400);
+      // its address and those of the running jobs, resolved first, as for a setup
+      await manager.resolveFor({ machine_id: id });
       return jsonResponse(manager.updateBridge(id, false, shown as string | undefined), 202);
     }
     if (parts.length === 1 && request.method === "PATCH") {