fix(pi): advertise runner capabilities at registration and on connect - #1386
Conversation
The hub only persists registration-time runner state for brand-new machines, and the socket heartbeat replays only what the hub already persisted. A runner upgraded in place (e.g. to 0.27.0, which adds piExistingSessionResume) never gets its new capabilities observed: the stale runner_state stays in the hub DB and Pi resume fails with "Pi resume requires an upgraded runner". - shared: RUNNER_CAPABILITIES single source of truth - runner: advertise capabilities again on every socket connect, so a reconnected runner self-heals without a hub-side change - hub: merge registration-time capabilities into an existing machine's runner_state, leaving live fields (status/pid/startedAt) socket-owned
There was a problem hiding this comment.
Findings
- [Minor] Registration skips capability backfill when metadata also changes; see inline comment at
hub/src/store/machines.ts:105.
Summary
Review mode: initial
One Minor correctness issue found in the existing-machine registration path. No additional high-confidence correctness, security, data-loss, performance, or maintainability findings.
Testing
Not run (automation). Static review of the added tests found no combined metadata-and-capability registration case; GitHub test check is pending.
HAPI Bot
| } | ||
| return row | ||
| } | ||
| const mergedRunnerState = mergeRunnerCapabilities(stored.runnerState, runnerState) |
There was a problem hiding this comment.
[MINOR] Merge capabilities when metadata changes too
The metadata branch above returns before this new branch whenever registration changes metadata. An upgraded runner necessarily changes happyCliVersion (cli/src/agent/sessionFactory.ts:48), so its first upgraded registration does not backfill piExistingSessionResume; the hub can still return resume_failed until the asynchronous socket state update completes. Keep the refreshed row and continue through both merges, then add a test where metadata and capabilities change in the same call.
Suggested fix:
let current = stored
if (mergedMetadata !== undefined) {
// Run the existing metadata UPDATE.
current = getMachine(db, id) ?? throwMissingMachine()
}
const mergedRunnerState = mergeRunnerCapabilities(current.runnerState, runnerState)
if (mergedRunnerState !== undefined) {
// Run the existing runner_state UPDATE.
current = getMachine(db, id) ?? throwMissingMachine()
}
return currentThere was a problem hiding this comment.
Fixed in 18ac442. getOrCreateMachine now keeps the refreshed row and runs both merges in the same call, so the capabilities backfill is no longer skipped when registration also changes metadata (e.g. happyCliVersion on upgrade). Added a test covering metadata and capabilities changing together (hub/src/store/machines.test.ts, runner capabilities backfill > merges capabilities even when metadata also changes in the same call). All 9 machines tests and hub typecheck pass.
The existing-machine registration path returned early after the metadata merge, skipping the capabilities backfill whenever registration changed metadata too. An upgraded runner necessarily changes happyCliVersion, so its first upgraded registration missed the backfill and Pi resume could still fail until the async socket state update landed. Merge both fields in the same call and return the latest row; add a test covering metadata and capabilities changing together.
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits
The previous capability/metadata early-return finding is resolved at hub/src/store/machines.ts:104. No additional high-confidence correctness, security, data-loss, performance, or maintainability issues were found in the full latest diff. Residual risk: CI was still pending at review time.
Testing
- Not run (automation). Static review only; the GitHub
testcheck was pending. Added coverage exercises registration backfill, unchanged or absent capabilities, live-field preservation, combined metadata/capability changes, and socket reconnect advertisement.
HAPI Bot
Summary
Fixes
Pi resume requires an upgraded runner(HTTP 500, coderesume_failed) after upgrading a runner to a version that advertises new capabilities.Root cause
The capability handshake between runner and hub is broken for existing machines:
piExistingSessionResume: trueonly in therunnerStateit sends with the initial HTTP registration (POST /machines).hub/src/store/machines.tsgetOrCreateMachinemerges registration-timemetadatafor machines that already exist, but discards the incomingrunnerStateentirely — so a machine registered by an older runner never picks up new capabilities on upgrade.cli/src/api/apiMachine.ts) replays{ status, pid, httpPort, startedAt }on every connect, without capabilities, and the hub persists that as the authoritativerunner_state— overwriting whatever was stored and keepingcapabilitiesempty forever.Result:
hub/src/sync/syncEngine.tsseestargetMachine.runnerState?.capabilities?.piExistingSessionResume !== trueand refuses to resume an imported native Pi session, even though the runner binary is current.Changes
RUNNER_CAPABILITIESconstant as the single source of truth for the capability set.cli/src/runner/run.ts,cli/src/api/apiMachine.ts): advertiseRUNNER_CAPABILITIESagain on every socket connect, so a reconnected/upgraded runner self-heals without any hub-side change.hub/src/store/machines.ts): merge registration-time capabilities into an existing machine'srunner_state, leaving live fields (status,pid,startedAt, ...) socket-owned. Without this, the registration refresh never observes upgrades for machines that already exist.Tests
hub/src/store/machines.test.ts: +3 cases — capabilities merged into an existing machine's runner state; live runner-state fields kept socket-owned on registration; no write when capabilities are unchanged or absent.cli/src/api/apiMachine.test.ts: +1 case — socket connect advertisespiExistingSessionResumealongside the running state.bun run typecheckand the touched test suites pass.