Repository navigation
feat(hub,cli,web): fleet runner version governance (skew, self-upgrade, soft-fail reopen) - #1108
Conversation
Deploying hapi with
|
| Latest commit: |
9956836
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a2ec4fdc.hapi-bqd.pages.dev |
| Branch Preview URL: | https://fix-hub-runner-version-gover.hapi-bqd.pages.dev |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Guard self-upgrade by the runner-self-upgrade capability —
upgradeMachineRunnercallsrunner-self-upgradefor any machine missing the required Cursor capability, but runners old enough to be skewed often do not have this newly-added RPC registered. The automatic attempt then fails every cooldown and the UI Upgrade button returns a 502 instead of offering a viable path. Evidence:hub/src/sync/syncEngine.ts:587.
Suggested fix:import { MACHINE_CAPABILITIES } from '@hapi/protocol/runnerCapabilities' const capabilities = machine.metadata?.capabilities ?? [] if (!capabilities.includes(MACHINE_CAPABILITIES.RunnerSelfUpgrade)) { return { type: 'error', message: 'Runner does not support self-upgrade; upgrade the CLI manually and restart the runner', code: 'upgrade_unavailable', } }
- [Major] Catch missing
bunso npm fallback can run —installFromNpmintends to trybun add -gand then fall back tonpm install -g, butrunCommand('bun', ...)can throw before returning whenbunis not on PATH. That makes npm-channel fleet upgrades fail on npm/npx installs that have npm available but not bun. Evidence:cli/src/upgrade/selfUpgrade.ts:64.
Suggested fix:async function runCommand(command: string, args: string[]): Promise<{ ok: boolean; output: string }> { try { const proc = Bun.spawn([command, ...args], { stdout: 'pipe', stderr: 'pipe', env: process.env, }) const [stdout, stderr, exitCode] = await Promise.all([ new Response(proc.stdout).text(), new Response(proc.stderr).text(), proc.exited, ]) return { ok: exitCode === 0, output: `${stdout}\n${stderr}`.trim() } } catch (error) { return { ok: false, output: error instanceof Error ? error.message : String(error) } } }
Questions
- None.
Summary
- Review mode: initial
- The new skew soft-fail path is reasonable, but fleet upgrade currently fails for the exact older runners that lack the new self-upgrade RPC, and npm fallback does not actually fall back if
bunis missing.
Testing
- Not run (automation; static review only).
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Preserve runner argv during self-upgrade relaunch —
scheduleRunnerRelaunchalways starts the replacement asrunner start-sync, so a runner originally started with--workspace-root ...comes back unscoped after Upgrade. That drops the workspace-root restrictions documented for browse/spawn and regresses the same handoff invariant the runner heartbeat already preserves viastartedWithArgv. Evidence:cli/src/upgrade/selfUpgrade.ts:159.
Suggested fix:import { readRunnerState } from '@/persistence' async function scheduleRunnerRelaunch(cliExecutable?: string): Promise<void> { const state = await readRunnerState() const args = Array.isArray(state?.startedWithArgv) && state.startedWithArgv[0] === 'runner' ? state.startedWithArgv : ['runner', 'start-sync'] const env = { ...process.env, HAPI_RUNNER_HANDOFF_FROM_PID: String(process.pid), } const child = cliExecutable ? spawn(cliExecutable, args, { detached: true, stdio: 'ignore', env }) : spawnHappyCLI(args, { detached: true, stdio: 'ignore', env }) child.unref() setTimeout(() => process.exit(0), 250) } await scheduleRunnerRelaunch(installedExecutable)
- [Major] Do not publish a
0.0.0npm upgrade offer — when the hub is not running from a monorepo and the caller does not passtargetVersion,resolveUpgradeOfferfalls back to0.0.0. The real callers added in this PR omittargetVersion, so documented npm installs can ask runners to install@twsxtd/hapi@0.0.0, making every fleet-upgrade attempt fail. Evidence:hub/src/upgrade/resolveUpgradeOffer.ts:81.
Suggested fix:// cli/src/commands/hub.ts import packageJson from '../../package.json' const hub = await startHub({ args: context.commandArgs, cliVersion: packageJson.version, }) // hub/src/startHub.ts export interface StartHubOptions { args?: string[] cliVersion?: string } const resolveCurrentUpgradeOffer = () => resolveUpgradeOffer({ hubPackageRoot: defaultHubPackageRoot(), execPath: process.execPath, targetVersion: options.cliVersion, })
Questions
- None.
Summary
- Review mode: follow-up after new commits
- The two previous bot findings were addressed, but the current self-upgrade path still has two Major regressions: replacement runners lose their original launch flags, and packaged hubs can emit an unusable npm target version.
Testing
- Not run (automation; static review only).
git diff --check origin/main...HEADpassed.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Do not skip self-upgrade when capabilities are still behind — the runner returns
already-currentas soon aslocalVersion === offer.targetVersion, but the hub starts fleet upgrade because the machine is missingoffer.targetCapabilities. Same-version source/soup builds, or any missed version bump, will be reported as successfully handled while the runner remains skewed; the hub also treatsalready-currentas success, so the banner/auto-upgrade can churn without fixing the missing RPC. Evidence:cli/src/upgrade/selfUpgrade.ts:37, related handlinghub/src/sync/syncEngine.ts:601.
Suggested fix:import { CURRENT_MACHINE_CAPABILITIES } from "@hapi/protocol/runnerCapabilities" function hasTargetCapabilities(offer: HubUpgradeOffer, localCapabilities: readonly string[]): boolean { const local = new Set(localCapabilities) return offer.targetCapabilities.every((cap) => local.has(cap)) } export function shouldApplyUpgradeOffer( offer: HubUpgradeOffer, localVersion: string, localCapabilities: readonly string[] = CURRENT_MACHINE_CAPABILITIES, ): ApplyDecision { // existing channel validation... if (localVersion === offer.targetVersion && hasTargetCapabilities(offer, localCapabilities)) { return { apply: false, reason: "already-current" } } return { apply: true, reason: "upgrade" } }
Questions
- None.
Summary
- Review mode: follow-up after new commits
- The previous bot findings are addressed. One remaining Major issue: self-upgrade is version-only, so a runner can stay capability-skewed even after the hub asks it to upgrade.
Testing
- Not run (automation; static review only).
HAPI Bot
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
- Review mode: follow-up after new commits
- Previous HAPI Bot finding is addressed in the latest head. Full current diff reviewed; no new high-confidence issues found. Residual risk: mechanical validation could not be completed in this runner because
bunis unavailable.
Testing
git diff --check b74a11ecc39451b1ae92cbfc7fb98744227a204a...368a364266e6c860c70ef33ce03a6c162020ed42passed.bun typechecknot run:buncommand not found.
HAPI Bot
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
Full current diff reviewed; no high-confidence issues found. Residual risk: runner self-upgrade install/relaunch paths were reviewed statically but not exercised in this runner.
Testing
git diff --check origin/main...HEADpassed.- Not run:
bun typecheck/bun run test(bunnot found in runner).
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Version-drifted runners can disappear from the upgrade UI — the hub now treats pure version drift and missing target capabilities as upgrade-required, but the banner only checks
isMachineCapabilitySkewed, whose required set is justcursor-chat-store-status. A runner that already has that capability but is behind on version or lacksrunner-self-upgradecan hit the auto-upgrade path, fail withupgrade_unavailable, and still never show the banner orUPDATE REQUIREDaffordance. Evidenceweb/src/components/RunnerVersionSkewBanner.tsx:27,hub/src/sync/syncEngine.ts:482,shared/src/upgradeChannel.ts:106.
Suggested fix:Wireimport { machineTrailsUpgradeOffer, type HubUpgradeOffer } from '@hapi/protocol/upgradeChannel' export function listSkewedMachines(machines: Machine[], offer: HubUpgradeOffer | null): Machine[] { return machines.filter((machine) => ( machine.active && ( isMachineCapabilitySkewed(machine.metadata?.capabilities) || (offer ? machineTrailsUpgradeOffer(offer, machine.metadata?.happyCliVersion, machine.metadata?.capabilities) : false) ) )) }
offerfrom/api/upgrade/offer, and use the same predicate for the machine selector’sUPDATE REQUIREDlabel.
Questions
- None.
Summary
Review mode: follow-up after new commits
Found one Major issue: frontend skew detection does not match the backend fleet-upgrade predicate, so some out-of-date runners can lose the visible recovery path.
Testing
git diff --check origin/main...HEADpassed.- Not run:
bun typecheck/bun run test(review automation; PR code not executed).
HAPI Bot
There was a problem hiding this comment.
Findings
-
[Major] Fleet policy updates fail from standalone web origins — the new policy endpoint is
PUT, and the web client calls it with JSON, but the hub CORS middleware still allows onlyGET, POST, PATCH, DELETE, OPTIONS. For documented standalone hosting, the browser preflight forPUT /api/upgrade/policyis rejected before auth, so users cannot change the fleet-upgrade policy from the remote web app. Evidencehub/src/web/routes/upgrade.ts:31, contexthub/src/web/server.ts:234.
Suggested fix:const corsMiddleware = cors({ origin: corsOriginOption, allowMethods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], allowHeaders: ['authorization', 'content-type'] })
-
[Minor] Machine selector still uses the old skew predicate — the banner now uses
machineTrailsUpgradeOfferplus the policy, but the new-session selector labelsUPDATE REQUIREDonly for missing required capabilities and ignoressilent. Pure version drift will be selectable without the label, whilesilentcan still show an alert label for capability drift. Evidenceweb/src/components/NewSession/MachineSelector.tsx:17.
Suggested fix:import { machineTrailsUpgradeOffer, type FleetUpgradePolicy, type HubUpgradeOffer } from '@hapi/protocol/upgradeChannel' function getMachineOptionLabel( machine: Machine, offer: HubUpgradeOffer | null, policy: FleetUpgradePolicy, ): string { const skew = policy !== 'silent' && offer && machine.active && machineTrailsUpgradeOffer(offer, machine.metadata?.happyCliVersion, machine.metadata?.capabilities) ? ' · UPDATE REQUIRED' : '' return `${title}${platform}${version}${skew}` }
Summary
Review mode: follow-up after new commits
The prior banner predicate issue is fixed for RunnerVersionSkewBanner, but the same mismatch remains in MachineSelector. The new fleet policy route also needs CORS coverage for the documented standalone-web deployment path.
Testing
- Not run (automation; PR code not executed).
HAPI Bot
There was a problem hiding this comment.
Findings
-
[Major] Fleet policy updates fail from standalone web origins — the new policy endpoint is
PUT, and the web client sends JSON to it, but the hub CORS middleware still allows onlyGET, POST, PATCH, DELETE, OPTIONS. For documented standalone hosting, the browser preflight forPUT /api/upgrade/policyis rejected before auth, so users cannot change the fleet-upgrade policy from the remote web app. Evidencehub/src/web/routes/upgrade.ts:31, contexthub/src/web/server.ts:234.
Suggested fix:const corsMiddleware = cors({ origin: corsOriginOption, allowMethods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], allowHeaders: ['authorization', 'content-type'] })
-
[Minor] Machine selector still uses the old skew predicate — the banner and auto-upgrade path now use
machineTrailsUpgradeOfferplus the fleet policy, but the new-session selector labelsUPDATE REQUIREDonly for missing required capabilities and ignoressilent. Pure version drift will be selectable without the label, whilesilentcan still show an alert label for capability drift. Evidenceweb/src/components/NewSession/MachineSelector.tsx:17.
Suggested fix:import { machineTrailsUpgradeOffer, type FleetUpgradePolicy, type HubUpgradeOffer } from '@hapi/protocol/upgradeChannel' function getMachineOptionLabel( machine: Machine, offer: HubUpgradeOffer | null, policy: FleetUpgradePolicy, ): string { const skew = policy !== 'silent' && offer && machine.active && machineTrailsUpgradeOffer(offer, machine.metadata?.happyCliVersion, machine.metadata?.capabilities) ? ' · UPDATE REQUIRED' : '' return `${title}${platform}${version}${skew}` }
Summary
Review mode: follow-up after new commits
The latest follow-up moved runner-management settings copy, but these two issues remain on current head. The prior banner predicate issue is fixed for RunnerVersionSkewBanner; the mismatch remains in MachineSelector.
Testing
- Not run (automation; PR code not executed).
HAPI Bot
There was a problem hiding this comment.
Findings
-
[Major] Self-upgrade exits before runner cleanup —
scheduleRunnerRelaunchschedulesprocess.exit(0)after 250 ms, while the caller delaysrequestShutdownby 500 ms, so the runner bypasses the normal cleanup path that updates state, stops the control server, and releases/removes runner state/lock files. A failed replacement can leave the machine offline or stuck behind stale handoff state. Evidencecli/src/upgrade/selfUpgrade.ts:194, contextcli/src/api/apiMachine.ts:380.
Suggested fix:async function scheduleRunnerRelaunch(cliExecutable?: string): Promise<void> { // spawn replacement as today child.unref() } await scheduleRunnerRelaunch(installedExecutable) if (options.requestShutdown) { options.requestShutdown() } else { setTimeout(() => process.exit(0), 500) }
-
[Major] Fleet policy updates fail from standalone web origins — the new policy endpoint is
PUT, and the web client sends JSON to it, but the hub CORS middleware still allows onlyGET, POST, PATCH, DELETE, OPTIONS. For documented standalone hosting, the browser preflight forPUT /api/upgrade/policyis rejected before auth, so users cannot change the fleet-upgrade policy from the remote web app. Evidencehub/src/web/routes/upgrade.ts:31, contexthub/src/web/server.ts:234.
Suggested fix:const corsMiddleware = cors({ origin: corsOriginOption, allowMethods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], allowHeaders: ['authorization', 'content-type'] })
-
[Major] Machine re-registration can keep stale capabilities —
mergeMachineRegistrationMetadatapreserves every old metadata field when the incoming runner omits it. If a runner that does not advertisecapabilitiesreconnects or downgrades against a row that previously had them, the hub can keep believingrunner-self-upgrade/cursor-chat-store-statusare present and then hide skew or route RPCs to handlers that are not registered. Evidenceshared/src/machineRegistration.ts:87.
Suggested fix:export function mergeMachineRegistrationMetadata(existing: unknown, incoming: unknown): Record<string, unknown> { const current = asRecord(existing) ?? {} const next = asRecord(incoming) ?? {} const merged: Record<string, unknown> = { ...next } if (next.displayName === undefined && current.displayName !== undefined) { merged.displayName = current.displayName } return merged }
-
[Minor] Machine selector still uses the old skew predicate — the banner and auto-upgrade path now use
machineTrailsUpgradeOfferplus the fleet policy, but the new-session selector labelsUPDATE REQUIREDonly for missing required capabilities and ignoressilent. Pure version drift will be selectable without the label, whilesilentcan still show an alert label for capability drift. Evidenceweb/src/components/NewSession/MachineSelector.tsx:17.
Suggested fix:const skew = policy !== 'silent' && offer && machine.active && machineTrailsUpgradeOffer(offer, machine.metadata?.happyCliVersion, machine.metadata?.capabilities) ? ' · UPDATE REQUIRED' : ''
Questions
- None.
Summary
- Review mode: follow-up after new commits
- The latest follow-up changes default fleet policy and moves runner-management settings, but the previous CORS and selector issues remain. I also found a self-upgrade shutdown race and a stale machine metadata merge problem in the current full diff.
Testing
- Not run (review automation; PR code not executed).
HAPI Bot
There was a problem hiding this comment.
Findings
-
[Major] Self-upgrade exits before runner cleanup —
scheduleRunnerRelaunchstill schedulesprocess.exit(0)after 250 ms, while the only caller delaysrequestShutdownby 500 ms. That bypasses the runner's normal shutdown path, so the old process can exit before state/lock cleanup and beforerequestShutdowngets a chance to run. Evidencecli/src/upgrade/selfUpgrade.ts:212, contextcli/src/api/apiMachine.ts:438.
Suggested fix:async function scheduleRunnerRelaunch(cliExecutable?: string): Promise<void> { // spawn replacement as today child.unref() } await scheduleRunnerRelaunch(installedExecutable) if (options.requestShutdown) { options.requestShutdown() } else { setTimeout(() => process.exit(0), 500) }
-
[Major] Fleet policy updates fail from standalone web origins — the new policy endpoint is
PUT, and the web client sends JSON to it, but the hub CORS middleware still allows onlyGET, POST, PATCH, DELETE, OPTIONS. In the documented standalone web setup, browser preflight forPUT /api/upgrade/policyfails before auth, so operators cannot change this setting from the remote web app. Evidencehub/src/web/routes/upgrade.ts:31, contexthub/src/web/server.ts:234.
Suggested fix:const corsMiddleware = cors({ origin: corsOriginOption, allowMethods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], allowHeaders: ['authorization', 'content-type'] })
-
[Major] Machine re-registration can preserve stale capabilities — the refresh path starts the merge from stored metadata, and
machineRegistrationNeedsRefreshignores cases where the incoming runner omitscapabilities. A runner that reconnects/downgrades without advertising capabilities can keep oldrunner-self-upgrade/cursor-chat-store-statusmetadata, causing the hub to hide skew or attempt RPCs the runner does not actually register. Evidenceshared/src/machineRegistration.ts:87.
Suggested fix:if (!capabilitiesEqual(current.capabilities, next.capabilities)) { return true } export function mergeMachineRegistrationMetadata(existing: unknown, incoming: unknown): Record<string, unknown> { const current = asRecord(existing) ?? {} const next = asRecord(incoming) ?? {} const merged: Record<string, unknown> = { ...next } if (next.displayName === undefined && current.displayName !== undefined) { merged.displayName = current.displayName } return merged }
-
[Minor] Machine selector still uses the old skew predicate — the banner and upgrade flow use
machineTrailsUpgradeOfferplus the fleet policy, but the new-session selector labelsUPDATE REQUIREDonly for missing required capabilities. Pure version drift is selectable without the label, whilesilentpolicy can still show the label for capability drift. Evidenceweb/src/components/NewSession/MachineSelector.tsx:17.
Suggested fix:const skew = policy !== 'silent' && offer && machine.active && machineTrailsUpgradeOffer(offer, machine.metadata?.happyCliVersion, machine.metadata?.capabilities) ? ' · UPDATE REQUIRED' : ''
Questions
- None.
Summary
- Review mode: follow-up after new commits
- The Windows artifact follow-up changes do not resolve the earlier shutdown, CORS, stale capability merge, or selector-label issues. I did not find additional high-confidence findings beyond those current-head problems.
Testing
- Not run (review automation; PR code not executed).
HAPI Bot
Ops call for tiann#1108 dogfood: hapi-runner-from-active ignores Upgrade binaries and defaults HAPI_DISABLE_VERSION_HANDOFF=1. Rebuild auto-cleans web/dist.prev churn; restart heals stale switch + lists effective WORKING. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
-
[Major] Self-upgrade exits before runner cleanup —
scheduleRunnerRelaunchstill schedulesprocess.exit(0)after 250 ms, while the only caller delaysrequestShutdownby 500 ms. That bypasses the runner's normal shutdown path, so the old process can exit before state/lock cleanup and beforerequestShutdowngets a chance to run. Evidencecli/src/upgrade/selfUpgrade.ts:212, contextcli/src/api/apiMachine.ts:380.
Suggested fix:async function scheduleRunnerRelaunch(cliExecutable?: string): Promise<void> { // spawn replacement as today child.unref() } await scheduleRunnerRelaunch(installedExecutable) if (options.requestShutdown) { options.requestShutdown() } else { setTimeout(() => process.exit(0), 500) }
-
[Major] Fleet policy updates fail from standalone web origins — the new policy endpoint is
PUT, and the web client sends JSON to it, but the hub CORS middleware still allows onlyGET, POST, PATCH, DELETE, OPTIONS. In the documented standalone web setup, browser preflight forPUT /api/upgrade/policyfails before auth, so operators cannot change this setting from the remote web app. Evidencehub/src/web/routes/upgrade.ts:31, contexthub/src/web/server.ts:234.
Suggested fix:const corsMiddleware = cors({ origin: corsOriginOption, allowMethods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], allowHeaders: ['authorization', 'content-type'] })
-
[Major] Machine re-registration can preserve stale capabilities — the refresh path only compares capabilities when the incoming metadata includes them, then the merge starts from stored metadata. A runner that reconnects/downgrades without advertising capabilities can keep old
runner-self-upgrade/cursor-chat-store-statusmetadata, causing the hub to hide skew or attempt RPCs the runner does not actually register. Evidenceshared/src/machineRegistration.ts:87.
Suggested fix:if (!capabilitiesEqual(current.capabilities, next.capabilities)) { return true } export function mergeMachineRegistrationMetadata(existing: unknown, incoming: unknown): Record<string, unknown> { const current = asRecord(existing) ?? {} const next = asRecord(incoming) ?? {} const merged: Record<string, unknown> = { ...next } if (next.displayName === undefined && current.displayName !== undefined) { merged.displayName = current.displayName } return merged }
-
[Minor] Machine selector still uses the old skew predicate — the banner and upgrade flow use
machineTrailsUpgradeOfferplus the fleet policy, but the new-session selector labelsUPDATE REQUIREDonly for missing required capabilities. Pure version drift is selectable without the label, whilesilentpolicy can still show the label for capability drift. Evidenceweb/src/components/NewSession/MachineSelector.tsx:17.
Suggested fix:const skew = policy !== 'silent' && offer && machine.active && machineTrailsUpgradeOffer(offer, machine.metadata?.happyCliVersion, machine.metadata?.capabilities) ? ' · UPDATE REQUIRED' : ''
Questions
- None.
Summary
Review mode: follow-up after new commits
The latest follow-up adds versionHandoffDisabled handling and banner filtering, but the current head still contains the shutdown, CORS, stale capability merge, and selector-label issues above. I did not find additional high-confidence findings beyond these current-head problems.
Testing
- Not run (review automation; PR code not executed).
HAPI Bot
Review debt cleared — tip
|
| Theme | Status | Evidence |
|---|---|---|
1. scheduleRunnerRelaunch hard process.exit(0) @250ms races requestShutdown @500ms |
Fixed | Spawn only in scheduleRunnerRelaunch; applyRunnerSelfUpgrade calls requestShutdown and only hard-exits (500ms) when no shutdown hook is provided |
2. CORS omits PUT for /api/upgrade/policy |
Fixed | hub/src/web/server.ts allowMethods now includes PUT |
3. machineRegistration preserves stale capabilities |
Fixed | Merge starts from registering payload; omitted capabilities → []. needsRefresh always compares capabilities (omit = empty) |
4. Banner predicate narrower than machineTrailsUpgradeOffer |
Already fixed in tip before this commit (d56577879 / earlier) — banner uses machineTrailsUpgradeOffer + versionHandoffDisabled exclude. Bot thread was stale vs tip. |
|
| Minor: MachineSelector capability-only skew | Fixed | Same offer/policy/handoff gates as banner (machineNeedsUpdateLabel) |
Tests: machineRegistration, fleet upgrade, banner, MachineSelector unit; bun typecheck green.
Resolved all open review threads on this PR (were 17). Please re-run Codex if needed — expect Majors to clear.
There was a problem hiding this comment.
Findings
- No issues found at >=80% confidence in the current diff.
Questions
- None.
Summary
Review mode: follow-up after new commits
The follow-up changes address the prior bot findings I checked: graceful runner shutdown, CORS for PUT /api/upgrade/policy, stale capability clearing on re-registration, and selector labeling via the offer/policy predicate. Residual risk is the broad fleet-upgrade surface across CLI, hub, shared contracts, and web UI.
Testing
- Not run (review automation; PR code not executed).
HAPI Bot
Rebase onto upstream/main (Release 0.23.4)Meta fleet: was DIRTY/CONFLICTING (~47 behind). Rebased onto
Still do not merge — wait on @tiann. Bot may re-review this tip. |
5114dc3 to
43b3e82
Compare
There was a problem hiding this comment.
Findings
- No issues found at >=80% confidence in the current diff.
Questions
- None.
Summary
Review mode: follow-up after new commits
Reviewed the full PR diff against main plus the prior HAPI Bot review context. The follow-up head still looks consistent across runner capability advertisement, hub upgrade offer/policy routing, self-upgrade gating, Cursor reopen soft-fail behavior, and the web banner/settings wiring. Residual risk is the broad fleet-upgrade path because review automation did not execute PR code.
Testing
- Not run (review automation; PR code not executed).
HAPI Bot
Rebase onto upstream/main againWas DIRTY/CONFLICTING (~27 behind). Rebased onto
Still do not merge — wait on @tiann. |
43b3e82 to
27d6a54
Compare
There was a problem hiding this comment.
Findings
- [Major] Terminal session registration can mask a stale runner —
getOrCreateMachinenow refreshes capability/version metadata for every registration, but local terminal bootstraps also hit/cli/machineswithrunnerState: nullwhile using the same machine id (cli/src/agent/sessionFactory.ts:167). If a newer local CLI starts a session while an older runner is still online, this path can replace the active runner row with the newer CLI'shappyCliVersion/capabilities, somachineTrailsUpgradeOfferstops reporting skew and the banner/auto-upgrade path disappears even though the live runner socket is still old (hub/src/sync/syncEngine.ts:493). Evidence:hub/src/store/machines.ts:74.
Suggested fix:const isRunnerRegistration = runnerState !== null && runnerState !== undefined if (isRunnerRegistration && machineRegistrationNeedsRefresh(stored.metadata, metadata)) { const merged = mergeMachineRegistrationMetadata(stored.metadata, metadata) // existing updateMachineMetadata path } if (!isRunnerRegistration) { return stored }
Questions
- None.
Summary
Review mode: follow-up after new commits
Found one Major issue in the latest full PR diff. The new runner-governance metadata needs to be owned by runner registration/heartbeat only; terminal session bootstrap should not be able to rewrite the active runner identity row. Residual risk remains in the broad self-upgrade/artifact path because review automation did not execute PR code.
Testing
- Not run (review automation; PR code not executed).
HAPI Bot
Follow-up: gate machine identity refresh on runner registrationAddresses Codex Major on
Still wait on @tiann — do not merge. |
ac16b35 to
a74be5b
Compare
|
Addressed the latest Codex Major (
Prior Major (terminal registration masking) remains fixed since |
There was a problem hiding this comment.
Findings
- [Major] npm self-upgrade relaunches the old CLI — the npm branch installs the target package, but it leaves
installedExecutableundefined, soscheduleRunnerRelaunch()falls back tospawnHappyCLI(). In compiled mode that helper resolves the currently running executable (process.argv[0]/process.execPath) and propagates it viaHAPI_CLI_EXECUTABLE, so the RPC can reportstartedand stop the runner while the replacement process is still the old generation. Evidencecli/src/upgrade/selfUpgrade.ts:244, related contextcli/src/utils/spawnHappyCLI.ts:97.
Suggested fix:async function installFromNpm(offer: HubUpgradeOffer): Promise<string> { // existing bun/npm install logic return process.platform === 'win32' ? 'hapi.exe' : 'hapi' } if (options.offer.channel === 'npm') { installedExecutable = await installFromNpm(options.offer) }
Questions
- None.
Summary
Review mode: follow-up after new commits
Found one Major issue in the latest full PR diff. The prior bot finding about terminal registration masking stale runner metadata is not repeated; the current code gates that refresh to runner registration.
Testing
- Not run (review automation; PR code not executed).
HAPI Bot
|
Addressed Codex Major (npm self-upgrade relaunches old CLI) in tip:
Tests: |
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
- [Major] Windows npm self-upgrade can relaunch a
.cmdshim without shell handling.resolvePostNpmInstallExecutable()can returnhapi.cmd, butscheduleRunnerRelaunch()spawns that path directly. This repo already usesshell: truefor Windows command shims elsewhere; without it, the npm-channel upgrade path will fail on Windows.cli/src/upgrade/selfUpgrade.ts:110-119,227-232
Suggested fix:const needsShell = process.platform === 'win32' && /\.(cmd|bat)$/i.test(cliExecutable) const child = spawn(cliExecutable, args, { detached: true, stdio: 'ignore', env, shell: needsShell, windowsHide: needsShell, })
- [Major]
versionHandoffDisabledhosts are filtered out of the skew banner entirely, so the new Restart escape hatch never appears for the exact machines that need it. The backend still treatsrestartMachineRunner()as the escape hatch for handoff-disabled/soup hosts, butlistSkewedMachines()drops them before render, andMachineSelectorapplies the same filter.web/src/components/RunnerVersionSkewBanner.tsx:50-56,web/src/components/NewSession/MachineSelector.tsx:27-30
Suggested fix:return machines.filter((machine) => machine.active && machineTrailsUpgradeOffer(offer, machine.metadata?.happyCliVersion, machine.metadata?.capabilities) ) // then disable only the Upgrade action when versionHandoffDisabled is true
Summary
Review mode: follow-up after new commits. I found 2 regressions in the new upgrade flow. Residual risk: no Windows runtime smoke test in the diff, and no UI test covering the handoff-disabled restart path.
Testing
Not run (automation)
HAPI Bot
|
Addressed the two latest Codex Majors:
|
4a1b6d4 to
0d9eaf0
Compare
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No high-confidence issues found in the full latest diff. The follow-up head rebases the previously reviewed fleet-governance stack onto the current base; the modified integration points were re-checked along with the complete PR diff. Residual risk remains in real cross-platform supervisor handoffs and on-demand hub-artifact compilation/download behavior, which were reviewed statically but not exercised locally in this automation run.
Testing
- Local tests: Not run (automation; contributor code was not executed per the security constraint).
git diff --checkagainst the PR base: passed.- GitHub checks at posting time: both
Test / testjobs passed; Cloudflare Pages passed.
HAPI Bot
Maintained release on top of official v0.27.2 (upstream main 00b6e44). Carries: provider profiles tiann#1091, notifications tiann#987, searchable share tiann#986, Codex agent budget tiann#847, fleet runner version governance tiann#1108, session jobs tiann#1424, steer tiann#906, project groups tiann#955, and the maintained distribution overlay. Drops tiann#1092 (workspace management) per operator decision. Includes the desktop launcher packaging (tiann#769) gated behind the maintained release flow.
There was a problem hiding this comment.
Findings
-
[Minor] Coalesce upgrades per machine before invoking the runner - under
auto,maybeFleetUpgradeMachine()starts this method asynchronously, while the still-visible banner and another browser can call the same public method concurrently. Both requests independently prepare an artifact and sendrunner-self-upgrade; the CLI's process-local gate rejects the second as "Runner upgrade already in progress", which the hub converts intoupgrade_failedand may toast as a real fleet failure. Evidence:hub/src/sync/syncEngine.ts:1045Suggested fix:
const existing = this.fleetUpgradeInFlight.get(machineId) if (existing) return await existing const task = this.upgradeMachineRunnerUnlocked(machineId, namespace) this.fleetUpgradeInFlight.set(machineId, task) try { return await task } finally { if (this.fleetUpgradeInFlight.get(machineId) === task) { this.fleetUpgradeInFlight.delete(machineId) } }
Questions
- None.
Summary
Review mode: follow-up after new commits
One Minor issue found in the full latest diff. Without hub-side per-machine coalescing, overlapping automatic and manual upgrade requests can produce duplicate work and false failure reporting. Residual risk remains in real cross-platform supervisor handoffs and artifact installation paths, which were reviewed statically only.
Testing
- Not run (automation; contributor code was not executed per the security constraint).
git diff --checkagainst the PR base: passed.- GitHub checks at posting time: both
testjobs passed; Cloudflare Pages passed. - Add a regression test that overlaps an auto-triggered attempt with a manual
upgradeMachineRunner()call and asserts one artifact preparation/RPC plus a shared result.
HAPI Bot
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The prior Minor finding is resolved at the current head: concurrent automatic and manual runner upgrades now share one per-machine in-flight operation, with guarded cleanup after settlement.
Residual risk remains in cross-platform runner handoff, supervisor restart, and artifact installation behavior, which was reviewed statically only. The PR test plan also leaves operator dogfood incomplete.
Testing
- Not run (automation; contributor code was not executed per the security constraint).
git diff --checkagainst the PR base: passed.- GitHub checks at posting time: both
testjobs passed; Cloudflare Pages passed; this review job was still in progress. - Coverage gap: the added coalescing regression overlaps two direct
upgradeMachineRunner()calls; an end-to-end auto-heartbeat plus manual banner overlap remains untested.
HAPI Bot
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The new head addresses the ghost-RPC state by re-registering machine handlers on keepalive and refusing self-upgrade until the handler is present in the live registry.
Residual risk remains in cross-platform runner handoff, supervisor restart, and artifact build/install behavior, which was reviewed statically only. The PR test plan also leaves operator dogfood incomplete.
Testing
- Not run (automation; contributor code was not executed per the security constraint).
git diff --checkagainst the PR base: passed.- GitHub checks at posting time: both
testjobs passed; Cloudflare Pages passed; this review job was still in progress. - Coverage gap: no direct integration test exercises
ApiMachineClientkeepalive re-registration through the hub beforemachine-alive; the added tests cover the live-registry gate, not the end-to-end healing path.
HAPI Bot
Ghost RpcRegistry vs advertised caps, chicken/egg first restart, npm vs artifact channels, soup landmines, and kill-criteria before claiming smooth fleet deploy. Cross-link from reenroll runbook. Co-authored-by: Cursor <cursoragent@cursor.com>
Hub↔runner protocol drift was reported as missing Cursor chat data when cursor-chat-store-status was unregistered. Soft-fail reopen on probe errors, advertise required machine capabilities, surface an unmissable upgrade banner, and stop-runner when a newer CLI binary is already on disk. Fixes #1084 Co-authored-by: Cursor <cursoragent@cursor.com>
Compact the out-of-date banner (minimize + 1h snooze + per-host Restart) so it no longer blocks the session list. Auto stop-runner on skew stays opt-in via HAPI_AUTO_UPGRADE_RUNNERS / autoUpgradeRunners (default off). Co-authored-by: Cursor <cursoragent@cursor.com>
QuotaExceededError from setItem aborted minimize before React state updated, leaving the banner stuck over the session list. Persist to memory when storage fails; only enable Restart when a newer CLI is already on disk; clarify opt-in is stop-runner only, not package push. Co-authored-by: Cursor <cursoragent@cursor.com>
…tart CLI version handoff already reloads the runner when the on-disk binary mtime changes. Hub-driven stop-runner on skew duplicated that. Keep the skew banner and manual Restart only as a stuck/disabled-handoff escape. Co-authored-by: Cursor <cursoragent@cursor.com>
fee9084 to
bf12041
Compare
There was a problem hiding this comment.
Findings
- [Major] Ordinary terminal sessions can publish runner-only capabilities, hiding an actually outdated runner. Evidence:
cli/src/agent/sessionFactory.ts:59. - [Major] The new Restart action only sends
stop-runner, so an unsupervised runner can remain offline. Evidence:hub/src/sync/rpcGateway.ts:283.
Questions
- None.
Summary
Review mode: follow-up after new commits
Two major issues found in the latest full PR diff. The previous HAPI Bot review was used only as context because its reviewed commit belongs to the earlier mirror history.
Testing
- Not run (automation; contributor code was not executed per the security constraint).
git diff --checkagainst the PR base: passed.- GitHub checks at posting time: both
testjobs passed; Cloudflare Pages passed; this review job was still in progress. - Coverage gaps: no test proves terminal bootstrap cannot advertise machine-RPC capabilities for an older live runner; no integration test proves Restart produces a replacement PID and keeps the machine online.
HAPI Bot
Address #1108 bot Majors on the thin tip: terminal/lazy bootstraps no longer merge CURRENT_MACHINE_CAPABILITIES into the machine row (only asRunner registration does). Banner Restart refuses unsupervised hosts so stop-runner cannot leave a detached laptop offline; supervised runners advertise supervisedRestart via HAPI_RUNNER_SUPERVISED=1. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
- [Major] Runner governance metadata is sticky across rollback/reconfiguration — newly advertised
capabilities, mtimes, andsupervisedRestartare merged over the stored machine row; absent values never clear. An older or newly unsupervised runner can therefore remain recorded as current/supervised, hiding skew and potentially making a later Restart stop an unsupervised runner. Evidence:cli/src/agent/sessionFactory.ts:72, related merge contexthub/src/store/machines.ts:47.
Suggested fix:const runnerOwnedKeys = ['capabilities', 'startedCliMtimeMs', 'installedCliMtimeMs', 'supervisedRestart'] as const const mergeBase = runnerState == null ? stored.metadata : omitKeys(stored.metadata, runnerOwnedKeys) const merged = mergeMachineMetadata(mergeBase, metadata) // Current runners should also send supervisedRestart: process.env.HAPI_RUNNER_SUPERVISED === '1'
- [Minor] Canonical supervisor setups never set the new supervision marker — the installation text promises a per-host Restart escape hatch, but runtime requires
HAPI_RUNNER_SUPERVISED=1; that variable is not present in the systemd/pm2/launchd examples or CLI configuration docs. Evidence:docs/guide/installation.md:331; configuration: Not found in repo/docs.
Suggested fix:[Service] Environment=HAPI_RUNNER_SUPERVISED=1
HAPI_RUNNER_SUPERVISED=1 pm2 start "hapi runner start-sync" --name hapi-runner - [Minor] The machine selector adds a hard-coded English upgrade status, so the Chinese UI renders
UPDATE REQUIRED. Evidence:web/src/components/NewSession/MachineSelector.tsx:19.
Suggested fix:const skew = machine.active && isMachineCapabilitySkewed(machine.metadata?.capabilities) ? ` · ${updateRequiredLabel}` : '' // Pass t('runner.skew.updateRequired') and add en/zh-CN locale entries.
Questions
- None.
Summary
Review mode: follow-up after new commits
The two previous Major findings were rechecked against the full latest diff. The terminal-bootstrap capability issue is addressed. The unsupervised-stop guard is present, but persisted runner metadata can still become stale and bypass that guard after rollback or supervision changes.
Testing
- Not run (automation; contributor code was not executed per the security constraint).
git diff --checkagainst the PR base: passed.- GitHub checks at review time: Cloudflare Pages passed; one
testjob passed; onetestjob remained in progress; this review job remained in progress. - Coverage gap: no store/integration test re-registers the same machine from supervised/current to unsupervised or legacy metadata and verifies runner-owned governance fields are cleared.
HAPI Bot
… label Omit-means-clear on runner registration so rollback cannot leave supervisedRestart/capabilities sticky; always advertise boolean supervisedRestart from asRunner. Document HAPI_RUNNER_SUPERVISED=1 and localize MachineSelector UPDATE REQUIRED. Co-authored-by: Cursor <cursoragent@cursor.com>
Review threads addressed — tip
|
| Thread | Fix |
|---|---|
| [Major] Sticky runner governance metadata | Runner registration clears omitted ads (clearOmittedRunnerAds); asRunner always sends supervisedRestart boolean |
| [Minor] SUPERVISED missing from docs | systemd / pm2 / launchd examples + installation.md pointer |
| [Minor] MachineSelector English hard-code | runner.skew.updateRequired en + zh-CN |
Thin gate: 35 files / 6 commits vs upstream/main. Still do not merge — wait on @tiann + bot re-review.
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits
No issues found at the current head. The prior sticky runner-governance metadata, supervisor documentation, and machine-selector localization findings are resolved. Residual risk remains around the real supervisor cold-relaunch/reconnect path; coverage in this PR mocks the stop RPC rather than exercising systemd/pm2/launchd end to end.
Testing
- Not run (automation; contributor code was not executed per the security constraint).
- git diff --check against the PR base: passed.
- GitHub checks at review time: both test jobs and Cloudflare Pages passed.
- Coverage gap: no end-to-end supervised runner restart/handoff test; operator dogfood remains unchecked in the PR test plan.
HAPI Bot
Absorb tiann#1108 runner ads and tiann#1480 steer. Keep tag/proof gates before metadata refresh; runner registration still sends asRunner mtimes. Co-authored-by: Cursor <cursoragent@cursor.com>
Gate A after upstream merge of fix/hub-runner-version-governance (squash 1cd4d11). Manifest DROPPED; Gate A' retro filed. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com> tiann#1108 exit reflection: chip cache lags live classify until :00 hapi-meta-daily.
Incident plan: latching 🛑 needs_operator chip (no peer ping) plus pinned fat upgrade SHAs for estate soup. Playback pending confirm. Co-authored-by: Cursor <cursoragent@cursor.com>
The driver/fleet-runner-upgrade comment cited tiann#1108, so mw_manifest_pr_layer_active kept Gate A dirty after thin merge cleanup. Reword to #122-only scope. Co-authored-by: Cursor <cursoragent@cursor.com>
Adopt upstream machine capability redesign (tiann#1108): MachineMetadata.capabilities is now the runner's RPC capability id array (consumed by isMachineCapabilitySkewed and cliBinaryUpdatedOnDisk in RunnerVersionSkewBanner/MachineSelector); fork's OMP flag moves out of capabilities to a top-level ompAvailable field. Update all fork consumers: sessionFactory, run.ts, apiMachine, machineCache, omp-host-integration routes, NewSession, OmpProviderSettingsRow, and their tests. Keep both sides' additive changes: upstream share-transfer/search imports, isTranscriptEcho, fleet-version mtime/supervisedRestart metadata, restart-runner route, NotifySummaryText/SpeakSummaryButton, SteerQueuedMessageResponse and fork's multi-user settings routes, cc-switch providers route, ompInputMode, ProbeAgentSkills response, formatUsageSnapshotLabel, session-summary-in-chat flag, scratchlist drawer props, queue Steer button, and fork CI (session-scroll spec + integration env). Locales: keep fork wording per prior resolution precedent (2af3340, 3d8d695). Co-Authored-By: Mouriya-Emma <85676458+Mouriya-Emma@users.noreply.github.com>
…e, soft-fail reopen) (tiann#1108) * fix(hub): govern runner capabilities so Cursor reopen soft-fails on skew Hub↔runner protocol drift was reported as missing Cursor chat data when cursor-chat-store-status was unregistered. Soft-fail reopen on probe errors, advertise required machine capabilities, surface an unmissable upgrade banner, and stop-runner when a newer CLI binary is already on disk. Fixes tiann#1084 Co-authored-by: Cursor <cursoragent@cursor.com> * fix(web,hub): make runner skew banner dismissible; gate auto-upgrade Compact the out-of-date banner (minimize + 1h snooze + per-host Restart) so it no longer blocks the session list. Auto stop-runner on skew stays opt-in via HAPI_AUTO_UPGRADE_RUNNERS / autoUpgradeRunners (default off). Co-authored-by: Cursor <cursoragent@cursor.com> * fix(web): tolerate full sessionStorage on skew banner minimize QuotaExceededError from setItem aborted minimize before React state updated, leaving the banner stuck over the session list. Persist to memory when storage fails; only enable Restart when a newer CLI is already on disk; clarify opt-in is stop-runner only, not package push. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(hub): drop redundant autoUpgradeRunners; runners already self-restart CLI version handoff already reloads the runner when the on-disk binary mtime changes. Hub-driven stop-runner on skew duplicated that. Keep the skew banner and manual Restart only as a stuck/disabled-handoff escape. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(cli,hub,web): runner-only caps ads; gate Restart on supervisor Address tiann#1108 bot Majors on the thin tip: terminal/lazy bootstraps no longer merge CURRENT_MACHINE_CAPABILITIES into the machine row (only asRunner registration does). Banner Restart refuses unsupervised hosts so stop-runner cannot leave a detached laptop offline; supervised runners advertise supervisedRestart via HAPI_RUNNER_SUPERVISED=1. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(hub,cli,web): clear sticky runner ads; docs SUPERVISED; i18n skew label Omit-means-clear on runner registration so rollback cannot leave supervisedRestart/capabilities sticky; always advertise boolean supervisedRestart from asRunner. Document HAPI_RUNNER_SUPERVISED=1 and localize MachineSelector UPDATE REQUIRED. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Debian <heavygee@oos-linux.in.lockhouse> Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
After #1037, Cursor reopen hard-failed when the hub was newer than a remote runner: missing
cursor-chat-store-statuswas treated as deleted chat data even whenstore.dbstill existed.This PR makes hub↔runner generation governance explicit:
onDisk: falsestill blocks.installedCliMtimeMs≠startedCliMtimeMs), hub may callstop-runnerso systemd/handoff loads the new generation; otherwise the banner stays.Test plan
bun typecheckbun run test(shared capability registry, soft-fail reopen, runner ensure, web reopen gate, skew banner)Runner out of date on proxmox+ upgrade/restart instructions)Issues
Fixes #1084
Upgrade notes
Fleet self-upgrade is not zero-touch on first generation or right after a hub restart.
Chicken / egg
machine-alive) while the hubRpcRegistryis empty — every machine RPC then fails withRPC handler not registered.type: "success"(HTTP 200 alone is not enough)upgrade_unavailable— not toastingupgrade_failedwhen caps are advertised but the live RPC is missingcapabilitiesempty / ancient CLI) still need a manual install; the hub cannot inventrunner-self-upgrade.Guide:
docs/guide/deployment.md→ Fleet upgrade after a hub update.This tip (
fee9084b2)runner-self-upgraderegistration before fleet upgrade (advertised-but-not-live →upgrade_unavailable).rpc-registeron every keepalive so a stable hub heals ghost registries without a full reconnect.rpc-register.Kill-criteria before claiming smooth fleet deploy
After hub is on this generation and each remote runner has restarted once onto it (or keepalive heal is live):
upgrade_unavailablewhile healing — neverupgrade_failedfrom advertised-but-not-live caps.POST .../upgrade-runnereither starts apply or returnsupgrade_unavailablewith restart guidance./cli/upgrade/cli-artifactmust return a real binary (not SPA HTML).Cross-session upgrade / restart nudges
Any "restart the runner", "run
hapi upgrade", or remat instruction sent from one HAPI session to another must use peer delivery (ping_peer/sentFrom: peer) so the target UI shows an@sessionchip (verified when a session capability is present, or@name/@idwith ⚠ when unattributed). Do not inject those instructions as ordinary user-composer text on the target — operators will treat that as their own keystrokes. Unverified (⚠) chips are claims only: do not auto-execute shell/upgrade actions from them. Peer provenance does not replace live RPC registration after a hub bounce; empty machine RPC registries still need the keepalive/rpc-registerheal in this PR.Refuse to greenlight
RpcRegistry