Skip to content

[bug] The runner's "don't touch an agent pinned to another machine" check is dead code — /v1/instances/my/instances strips config, so every machine attaches every agent #500

Description

@serge-ivo

The problem

pags up is supposed to leave an agent alone when the user has pinned it to a different machine.
It does not, on any machine, because the endpoint the runner polls does not return the pin.

isEligible reads it from the instance's config:

// packages/cli/src/commands/runner/membership.ts:32-38
const pin = inst.config?.runnerNode;
...
if (pin && pin !== thisNode && !alsoKnownAs.includes(pin)) return false;

and declares the field it expects at membership.ts:17:

config?: { runnerNode?: string | null } | null;

The discovery poll's only source is GET /v1/instances/my/instances (relay.ts:146-150), and
that route strips config before responding:

// workers/api/src/routes/instances.ts:304
const { config, instance_config, last_activity_at, ...rest } = r;

config is destructured out and never re-added; nothing puts runnerNode back on the response by
another name.

Measured (owner's account, GET /v1/instances/my/instances, 2026-08-11) — the keys returned
for an instance that IS pinned:

['agent_id', 'capabilities', 'category', 'created_at', 'description',
 'icon', 'icon_bg', 'id', 'lastActivityAt', 'name', 'slug', 'status']
has config? False   runnerNode anywhere in the object? False

So pin is undefined on every instance, on every machine, always. The third eligibility
condition — the only one that is not a copy of up.ts's startup filter — is dead code in
production.

Why it matters

The comment above it states the harm precisely, and it is the harm being taken:

membership.ts:22-27 — "an instance pinned to a different node belongs to that machine.
Attaching it here would either lose a race with the pinned machine or, worse, win one and
silently relocate the user's agent away from where they pinned it
."

Live, on the owner's account (GET /v1/terminals/nodes, 2026-08-11): Sergeys-Mac-mini.local
holds live relay sockets for all 18 of its instances, including iTerm2 Operator, kitty Operator, Terminal Operator and tmux Operator, which are pinned to RLs-MacBook-Air.local
(bound: true under the Air's machine group, which folds its Mac / RLs-MacBook-Air aliases).
Routing then resolves whichever socket happens to be live, which is not necessarily the machine
the user chose.

It also adds avoidable relay churn on every machine: each runner opens and holds sockets for the
entire account rather than its own share, which is the population that the 4409 problem in #497
then acts on.

The startup path has the same hole, differently

up.ts:73-85 filters on status === "active" and capabilities.runtime != null only — there is
no pin check at startup at all, by design ("Two conditions come straight from up.ts's startup
filter, so discovery and startup can never disagree", membership.ts:21). The intent was that
discovery would detach a wrongly-attached instance within 20s. With the pin invisible, neither
half runs.

Fix

Server-side, so every already-installed CLI is fixed with no release (the owner is on 0.4.45;
the shipped code already reads inst.config?.runnerNode): re-add a minimal config object to the
list response — config: { runnerNode } and nothing else, parsed with the existing
parseBoundRunnerNode. Do not return config wholesale: it is dropped deliberately because
it "may hold secrets/internal settings" (instances.ts:301-303), and that reason is still good.

Additive and optional, so no console change is required; nothing in store/console/src reads
config off this response today.

Rejected: adding a new top-level runnerNode field and teaching the CLI to read it. Cleaner
shape, but it needs a CLI release and leaves every deployed runner broken until users upgrade —
for zero behavioural gain over the shape the existing code already parses. If a nicer shape is
wanted later, ship both and drop the nested one after a version floor.

The test could not have caught this

// packages/cli/src/commands/runner/membership.test.ts:27-28
expect(isEligible(inst({ id: "a", config: { runnerNode: "other-box" } }), NODE)).toBe(false);
expect(isEligible(inst({ id: "a", config: { runnerNode: NODE } }), NODE)).toBe(true);

Both assertions pass and both feed the pure function an input shape the server has never sent.
Same blind spot as #438 — nothing verifies a parameter surviving the trip route → client.

Acceptance criteria

  • GET /v1/instances/my/instances returns config: { runnerNode } for a pinned instance and
    omits or nulls it otherwise; no other config key appears in the response.
  • A route-level test asserts the field is present for a pinned instance and that no other
    config key leaks — the assertion that makes the CLI's unit test mean something.
  • With two machines running pags up, an instance pinned to machine A has a live relay socket
    on A only; machine B detaches it within one discovery poll.

Regression risk

Related

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