Skip to content

[bug] #461's fix did not reach the second adapter — classifySubordinateConnectivity drops the pin, so subordinate_status, start_work and the chat tool still prescribe pags up --force for a pinned agent #468

Description

@serge-ivo

#461 fixed the adapter it found. There is a second adapter, and it still drops the pin.

#461 (07ec7af) carried pinnedNode / liveNodeExcludedByPin on RuntimeFacts and forwarded them in describeFacts, which fixed the three surfaces that reach the diagnosis through it. Its body states: "Grepping the whole tree, pinnedNode is supplied by exactly one caller — routes/instances.ts:793-806."

That is true of pinnedNode as a named argument. It is not true of diagnoseAttachment's callers. There are three construction sites, not two:

workers/api/src/lib/runner-availability.ts:123-131   describeFacts            ← fixed by #461 (5 of 5 inputs)
workers/api/src/routes/instances.ts:823-829          /runtime/status          ← was always right (5 of 5)
workers/api/src/lib/subordinate-connectivity.ts:78-83 classifySubordinateConnectivity ← still 3 of 5

subordinate-connectivity.ts:78-83:

const d = diagnoseAttachment({
    hasRuntimeRow: input.hasRuntimeRow,
    relayConnected: input.relayConnected,
    lastSeenAt,
    now: input.now,
});

No pinnedNode, no liveNodeExcludedByPin — and classifySubordinateConnectivity's own input type (:46-57) has no field for them, so its three call sites have nothing to pass even though every one of them is holding a RuntimeFacts that now carries both.

The three surfaces that still say the wrong thing

All three build the input by enumerating fields off a RuntimeFacts produced by runtimeConnectivity / runtimeConnectivityMany — the reader #461 taught to resolve the pin — and all three stop one field short:

  1. lib/connectors/supervision.ts:349-357subordinate_status, the MCP tool a supervisor reads. facts comes from runtimeConnectivityMany at :329. A pinned-to-an-offline-machine subordinate is reported as machine-online-agent-detached with remedy: "pags up --force", and subordinate-connectivity.ts:97 renders it as The machine is online but this agent isn't attached — another runner on it may already hold this agent. (machine: <the machine that IS up>). A model relays that to the human verbatim — which is what this file's own header (:7-15) exists to prevent: "Asked to delegate a typecheck, the Coder Lead … told the user to run pags up — which was already running."
  2. lib/loop-drivers.ts:163-171 — the coding driver's refusal, i.e. what start_work and the Loop button answer with. facts is runtimeConnectivity(...) at :163, and the comment immediately above it says "Connectivity FIRST, and from the same resolver delegation itself uses — so the refusal names the real blocker." For a pinned instance it names the wrong blocker and prescribes --force.
  3. lib/coding-session-open.ts:372-381ensureSessionForChat, the chat-tool path. Same facts, same enumeration, same wrong sentence.

noSessionMessage (lib/coding-session-lifecycle.ts:88-93) then appends Run \pags up --force` on .becauseconnectivity.remedyis non-null — the one branch its comment says *"is the ONE branch allowed to prescribepags up`"*.

The mechanism, and why it survived a fix aimed straight at it

diagnoseAttachment's input set has grown twice — 3 fields (#237) → 5 (#380) — and each growth broke whichever caller was not updated. #461 recognised this and wrote the invariant into runner-availability.ts:120-121:

"Forward everything, and let the diagnosis decide. A field added to RuntimeFacts for the diagnosis's benefit and not passed here is the same bug again."

The rule is right and it is written on the wrong function. It is scoped to describeFacts, while the property that actually matters is no caller of diagnoseAttachment may enumerate its inputs by hand. Three of the four paths do exactly that, and the compiler cannot object because every added field is optional — it has to be, so an old row can decline to answer.

Not verified end-to-end

I have not reproduced the wrong subordinate_status string against production in this session — it needs an instance pinned to a machine that is off while another holds a live socket. #461 measured exactly that state on 12ebf1f0-… ("Coder Home") on 2026-08-08 and its production capture is the evidence that the input state occurs. What is verified here is the code path: three sites, RuntimeFacts in hand, two fields dropped, machine-online-agent-detached the only reachable outcome for a fresh-heartbeat pinned instance.

What to do — cheapest first

1. Add the two fields to classifySubordinateConnectivity's input and forward them. Four lines. Fixes all three surfaces at once, exactly as #461's one-field-each fix did for its three.

2. Stop enumerating. Spread. RuntimeFacts's field names are already identical to classifySubordinateConnectivity's (hasRuntimeRow, relayConnected, node, runnerVersion, lastSeenAt, plus the two new ones), so each call site becomes classifySubordinateConnectivity({ requiresRunner, ...facts }) and a sixth field added later flows through without another ticket. supervision.ts needs an exported EMPTY facts value for its f ?? … case (instance-connectivity.ts:100-108 already has one, unexported).

3. Assert the invariant where it can be stated once. subordinate-connectivity.test.ts has no pinned case; coding-session-lifecycle.test.ts:6-20,118 builds its fixtures through this function without one. Add: a pinned-machine-offline input yields state: "pinned-machine-offline", remedy: null, and a message containing neither --force nor pags up — the same three assertions runner-availability.test.ts:152-157 added for the other adapter.

4. Leave routes/instances.ts:823-829 as a direct call, and say why. This is the part of this ticket that started as "the route duplicates describeFacts", and the duplication is real but is not the defect. The route computes its inputs from data it already holds — getLiveRuntime for liveness, parseBoundRunnerNode(instance.config) for the pin, liveNodeIgnoringPin only when pinned — so routing it through describeFacts(await runtimeConnectivity(...)) would add a second relay probe and two D1 reads to duplicate work it has just done, and would swap runtime.last_seen_at (the row it is describing) for the freshest row across both tables, which is a different fact. What it should gain is a comment naming itself as a construction site and the same test as (3), so the next field addition has three places to look and a failing test if it misses one.

Alternatives considered and rejected

Acceptance criteria

  • For an instance pinned to an offline machine while another machine holds a live socket for it, subordinate_status reports state: "pinned-machine-offline", no remedy, and a message naming both machines.
  • start_work / the Loop button on that instance refuse with the same diagnosis, and the refusal contains no pags up --force.
  • The chat-tool path (ensureSessionForChat) matches.
  • The unpinned machine-online-agent-detached case is unchanged and still says pags up --force on all three.
  • A test asserts the pinned content, not merely that the message is non-empty.

Regression risk

  • These three surfaces are the refusal text for delegation, start_work, and the supervisor's roster; changing the diagnosis changes three user-visible sentences and one state value that a model branches on. canWork is false in both the old and the new state, so no delegation decision changes — only the explanation. Worth stating in the change, because it is what makes this safe to ship without touching the routing tests.
  • coding-session-lifecycle.test.ts builds connected / offline / detached fixtures from this function; adding optional fields cannot change them, but the spread in (2) will change the shape passed at three call sites — typecheck covers it.

Files: workers/api/src/lib/subordinate-connectivity.ts:46-57,78-83 · workers/api/src/lib/connectors/supervision.ts:326-360 · workers/api/src/lib/loop-drivers.ts:158-172 · workers/api/src/lib/coding-session-open.ts:366-386 · workers/api/src/lib/coding-session-lifecycle.ts:82-99 · workers/api/src/lib/instance-connectivity.ts:12-40,100-108,121-160 · workers/api/src/lib/runner-availability.ts:110-131 · workers/api/src/lib/runtime-attachment.ts:45-100 · workers/api/src/routes/instances.ts:812-830. Follows #461; the branch itself is #380, the surface is #259.

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