Skip to content

[bug] A pinned-to-an-offline-machine agent is told to run pags up --force — describeFacts drops the two fields the pinned-machine diagnosis needs, so the branch is dead on every surface but one #461

Description

@serge-ivo

The one diagnosis that names a pin is unreachable from the surface that needs it — so a pinned agent is told to run pags up --force

diagnoseAttachment has a pinned-machine-offline branch that says the right thing. describeFacts,
which is how three call sites reach it, does not pass the two fields that branch depends on. So every
one of those call sites falls through to machine-online-agent-detached and prescribes
pags up --force — the remedy the pinned case's own code comment says must not be offered.

Measured, production, 2026-08-08 13:2x UTC

One instance, 12ebf1f0-… ("Coder Home"), pinned to a machine that is switched off:

GET /v1/instances/12ebf1f0-…/runner-node
runnerNode: "Sergeys-Mac-mini.local"
nodesDetail: Mac connected=true nodeOnline=true
Sergeys-Mac-mini.local connected=false nodeOnline=false
resolvedNode: null

Two surfaces, same instance, same second:

GET /v1/instances/12ebf1f0-…/runtime/status ← pin-aware
runtime.status "online" runnerNode "Mac" runnerVersion 0.4.44 health.ok true
GET /v1/instances/12ebf1f0-…/coding/repos ← pin-blind
recheck.reason "The machine is online but this agent isn't attached — another runner on it may
already hold this agent. Try: `pags up --force`"

Both sentences are produced by the same function, from the same facts, about the same instance. The
second is wrong in both halves: nothing else holds this agent, and --force cannot fix it. pags up
is already running on Mac with a live socket for this very instance. What the user needs to do is
change one dropdown.

Where it is

lib/runtime-attachment.ts:38-52 — the diagnosis takes the pin:

pinnedNode?: string|null;
liveNodeExcludedByPin?: string|null;

and :63-73 uses it, before the heartbeat check, with the reasoning already written down:

// BEFORE the heartbeat check, deliberately (#380). … a pin onto a dead machine reads as "fresh"// and diagnoses `machine-online-agent-detached`, prescribing `pags up --force` for a machine that// is switched off.

That comment describes, exactly, the output measured above.

lib/runner-availability.ts:110-116 — the adapter that drops them:

exportfunctiondescribeFacts(facts: RuntimeFacts): AttachmentDiagnosis{returndiagnoseAttachment({hasRuntimeRow: facts.hasRuntimeRow,relayConnected: facts.relayConnected,lastSeenAt: facts.lastSeenAt,});}

RuntimeFacts (lib/instance-connectivity.ts:12-20) carries no pin at all, so the adapter has nothing
to forward even if it wanted to. Grepping the whole tree, pinnedNode is supplied by exactly one
caller — routes/instances.ts:793-806, the /runtime/status route — which is why that surface gets it
right and no other does.

The three call sites that inherit the wrong answer

  1. routes/coding-repos.ts:176 (noRunnerReason) → the recheck.reason above. This is the one that
    matters most, because it is the sentence [bug] A repo has been marked broken for five days by a dropped WebSocket — a transport failure is stored as the repo's state, and no verdict is ever re-taken #440 added specifically so a stuck repo would explain
    itself
    , on the very instance [bug] A repo has been marked broken for five days by a dropped WebSocket — a transport failure is stored as the repo's state, and no verdict is ever re-taken #440 was filed about.
  2. lib/runner-availability.ts:161 (pauseMessage) → what a Workflow announces into the owner's
    thread when it pauses for an unreachable runner. A pinned instance is told to --force a machine
    that is off, for up to 30 minutes of waiting.
  3. lib/runner-availability.ts:175 → the same, on the terminal path.

Two decisions composing into the bug

Neither half is wrong on its own, which is why it shipped. diagnoseAttachment was given the pin
because #380 proved it was needed. describeFacts was written as a thin adapter over RuntimeFacts,
which is a connectivity record and has no business knowing about instance config. The defect is that
the second is the only path the first is reached by, outside one route — so the branch exists, is
correct, is tested, and is dead everywhere it is needed.

What to do — cheapest first

  1. Carry the pin on the facts. Add pinnedNode and liveNodeExcludedByPin to RuntimeFacts and
    populate them in runtimeConnectivity (which already reads the instance row to resolve routing, so
    the pin is in hand), then forward both in describeFacts. One field each, and all three call sites
    are fixed at once because they all go through it.
  2. Make the reason assertable.routes/coding-repos.test.ts:377 currently asserts only
    expect(String(body.recheck.reason)).toMatch(/\S/) — that the reason is non-empty. That is why a
    wrong reason shipped green. Assert the pinned case names the pinned machine and the live one, and
    does not contain --force.
  3. Pin the invariant where it can be stated once: no diagnosis whose state is
    pinned-machine-offline may carry a remedy, and no diagnosis produced for an instance with a live
    socket on a pin-excluded node may be machine-online-agent-detached.

Alternatives considered and rejected

Acceptance criteria

  • With an instance pinned to an offline machine while another machine holds a live socket for it,
    GET /coding/repos returns a recheck.reason that names both machines and tells the user to
    change "Runs on" — and does not contain pags up --force.
  • The same holds for a Workflow pause announcement on that instance.
  • A test asserts the content of the pinned-case reason, not merely that it is non-empty.
  • The unpinned machine-online-agent-detached case still says pags up --force, unchanged.

Regression risk

  • describeFacts feeds user-facing copy on three surfaces; changing what it returns changes three
    messages. The existing runtime-attachment.test.ts covers the branch logic and should catch a
    mis-ordering, but the adapter is what is untested — add a case there specifically.
  • Populating the pin in runtimeConnectivity adds an instance-config read to a function used in a
    batched path (MAX_RELAY_PROBES, subordinate_status). Resolve it from the row already being read
    rather than adding a query per instance.

Files: workers/api/src/lib/runner-availability.ts:110-116,161,175,
workers/api/src/lib/instance-connectivity.ts:12-20, workers/api/src/lib/runtime-attachment.ts:38-73,
workers/api/src/routes/coding-repos.ts:176-195, workers/api/src/routes/instances.ts:793-806,
workers/api/src/routes/coding-repos.test.ts:377.
Found while auditing #440, whose fourth item this defeats. Related: #379, #380, #237.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions