Skip to content

[bug] A wrong machine claim cannot be undone — nothing un-stamps machine_id, and a server-only un-claim would be re-stamped by the next pags up #467

Description

@serge-ivo

A machine claim is one-way: nothing in the product can take it back

#460 shipped the remedy for a hostname change — pags up asks on first run which unclaimed names are this machine, and pags machines claim <name> does the same for a box with no keyboard (a43eda0, released in @proagentstore/cli@0.4.45, e65cad0). Both are user-confirmed and both refuse a name another machine has proven. That is the right shape.

#460's own regression note flagged the gap and said the escape hatch had to be worked out before shipping the prompt: "A wrong claim is not self-correcting: claimMachineNames stamps rows, and nothing un-stamps them." It shipped without one. This issue works out the safety question that note left open.

Verified: there is no un-claim, anywhere

Grepping machine_id across workers/, it is written by exactly two statements:

  • routes/instances-runtime.ts:663-665 — the claim:
    UPDATE instance_runtime_nodes SET machine_id = ?1WHERE user_id = ?2AND (machine_id IS NULLOR machine_id ='') AND runner_node IN (…)
  • routes/instances-runtime.ts:93-95 — the register upsert:
    machine_id = COALESCE(excluded.machine_id, instance_runtime_nodes.machine_id)

No statement anywhere sets it back to NULL. The CLI has machines list and machines claim and nothing else (packages/cli/src/commands/machines.ts:77-80). DELETE /v1/terminals/nodes/:node (#393) deletes registration rows; it does not un-merge, and terminals.ts:387-395 resolves its own target throughmachineNamesFor, i.e. through the merge.

What a wrong claim actually costs

  1. Routing.lib/runner-client.ts:92,120 reads machine_id off these rows to build aliasNodesFor, and getBoundRunnerConn will route a pin onto a proven alias. A wrong merge therefore runs an agent's coding session on a machine the owner did not choose, in a different checkout — the exact outcome lib/machine-identity.ts:28-34 says the authoritative pin exists to prevent. The merge is the one thing that can make the pin lie.
  2. The picker.routes/instances.ts:441-443 and routes/terminals.ts:174-183 fold by machine_id, so two genuinely different laptops collapse into one "Runs on" entry and the owner can no longer express the distinction, let alone fix it.
  3. Forget.terminals.ts:387-395 resolves DELETE /nodes/:node through machineNamesFor(target, rows), so once merged, forgetting one laptop sweeps the other one's registrations too. diagnoseForget blocks on connected / pinned / open sessions, but it computes those blockers over the merged name set, so the guard inherits the wrong premise rather than catching it.

Verified: it partially self-heals, which narrows the ticket

COALESCE(excluded.machine_id, …) means the true machine overwrites a wrongly-claimed row the next time it registers that (instance_id, runner_node) with its own id. So a wrong claim heals for exactly the rows the real machine still registers, and only when its CLI is ≥ MACHINE_ID_MIN_CLI (0.4.40, machine-identity.ts:248) — an older runner sends no id, COALESCE keeps the wrong one, and the two machines stay merged indefinitely. It never heals for instances the real machine no longer serves, or for a machine that is gone.

So the missing hatch is not "always broken"; it is "broken exactly where the user cannot fix it by turning the other laptop on".

The safety question, answered

Who may un-claim, and does it need the CLI's on-the-machine evidence?

Claiming and un-claiming are not symmetric, and this is the part that decides the design. A claim asserts an identity, which is why #460 refuses every inferred rule — a wrong assertion silently redirects work. An un-claim only removes a proof, and the state it removes it to is machine_id IS NULL, which machine-identity.ts:36-37 already designates as the fail-closed default: "Unknown identity fails closed — never a heal we cannot prove." An un-claim can therefore never send work to a machine the owner did not choose; the worst it can do is stop a pin resolving, which diagnoseAttachment's pinned-machine-offline branch explains on screen with the remedy ("set Runs on to …"). The owner may un-claim from any surface; it does not need to be run on the machine to be safe.

Can it be raced? Yes — and this is the constraint that shapes the fix.buildRuntimeRegistrationBody sends machineNames: machine.names on every register (packages/cli/src/commands/runner/http.ts:51-60, relay.ts:32-47), the register route calls claimMachineNames on every register (routes/instances.ts:376-381), and loadMachineIdentity rewrites machine.json on every start (packages/cli/src/machine.ts:168-183). A server-side-only un-claim NULLs the rows and the claiming machine re-stamps them on its next pags up — within one restart, silently. A server-only un-claim does not work.

What happens to sessions bound to the node?coding_sessions.runner_node holds the name. A session that is only reachable because of the merge would be orphaned mid-run. The vocabulary for this already exists: mirror diagnoseForget (routes/terminals.ts:309-322) — refuse while an active/suspended session or a live pin resolves only through the name being un-claimed, and say which.

What to do — cheapest first

1. pags machines unclaim <name> — both halves in one process. Remove the name from machine.json (withClaimedNames's inverse) and call the new route below, from the same command. Same process, so there is no window in which one half is done and the other is not. Also append the name to the existing declined list (MachineIdentity.declined, machine.ts:155-158) so #460's first-run prompt does not offer it straight back.

2. DELETE /v1/terminals/nodes/:node/claimUPDATE instance_runtime_nodes SET machine_id = NULL WHERE user_id = ?1 AND runner_node = ?2 AND machine_id = ?3, where ?3 is the caller's asserted machine id. Scoping the write to the id being removed means the request can only ever un-say something this machine said. Refuse with diagnoseForget's blocker vocabulary when an open session or a pin resolves only through it.

3. Ship them together. (2) alone is undone by the next register (see the race above), so a console button on its own would be a control that appears to work and silently reverts — worse than no control.

Alternatives considered and rejected

  • A console-only "these are not the same machine". Rejected as the first move for the race, not for safety: machine.json still carries the name and re-claims it on the next pags up. Reasonable as a follow-up after (1) exists, if it also writes a server-side refusal.
  • A server-side denied-claims table so a console un-claim sticks. Rejected for now: it is a migration plus a new precedence rule inside claimMachineNames, for a case the CLI already covers — the wrong claim was made on a machine the user has, because that is the only place a claim can be made.
  • Auto-un-claim when two ids "look" like different machines. Rejected for the mirror of [enhancement] Recovering from a hostname change means hand-editing machine.json — the CLI should ask, on first run, which of the account's unclaimed names are this machine #460's reasoning: any automatic rule that can un-merge can un-merge a correct merge, and a correct merge is what is holding six stranded pins together.
  • Make claimMachineNames an upsert that re-asserts over a non-NULL row. Firmly rejected — the machine_id IS NULL predicate is one of the three things instances-runtime.ts:646-652 names as what keeps the claim safe.
  • Rely on the COALESCE self-heal and document it. Rejected: it does not cover an instance the real machine no longer registers, and it silently does not cover a machine on a CLI older than 0.4.40.

Acceptance criteria

  • After pags machines claim X on machine A, pags machines unclaim X on A leaves every instance_runtime_nodes row for runner_node = X with machine_id IS NULL, and X is gone from machine.json's names.
  • Restarting pags up on A does not re-claim X (it is in declined), and the [enhancement] Recovering from a hostname change means hand-editing machine.json — the CLI should ask, on first run, which of the account's unclaimed names are this machine #460 first-run prompt does not offer it again.
  • GET /v1/terminals/nodes stops folding X into A; X appears as its own entry again.
  • Un-claiming a name that another machine's id holds changes nothing and says so (the request may only remove the caller's own id).
  • Un-claiming is refused, with the blocking agents/sessions named, while an active/suspended coding session or a pin resolves only through that name.
  • An agent that was routing through the alias reports pinned-machine-offline afterwards — an explained state, not a silent failure.

Regression risk

Files: workers/api/src/routes/instances-runtime.ts:640-668,93-95 · workers/api/src/routes/instances.ts:336-381,441-443 · workers/api/src/routes/terminals.ts:174-183,289-322,380-400 · workers/api/src/lib/machine-identity.ts:26-37,193-236,248 · workers/api/src/lib/runner-client.ts:92,120 · packages/cli/src/commands/machines.ts · packages/cli/src/machine.ts:150-183 · packages/cli/src/machine-claim.ts · packages/cli/src/commands/runner/http.ts:47-61, relay.ts:32-47. Follows #460, whose regression note asked for exactly this; #393, #379.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions