Skip to content

feat(web): ask the cluster path for a runtime before finishing onboarding - #1541

Merged
frozenluo merged 3 commits into
mainfrom
feat/onboarding-cluster-runtime-step
Aug 26, 2026
Merged

feat(web): ask the cluster path for a runtime before finishing onboarding#1541
frozenluo merged 3 commits into
mainfrom
feat/onboarding-cluster-runtime-step

Conversation

@frozenluo

Copy link
Copy Markdown
Contributor

What

Picking Cluster at the onboarding fork used to finish the wizard right there. The fork is now a step on the way, and the pool path ends on its own Choose runtime step:

  • Both paths end on a last step of their own, so the fork always reads Continue (was Finish whenever the pool was preselected) and the step count is orgStepsBefore + (poolOffered ? 1 : 0) + 1.
  • New ClusterStep — the same RuntimeModelFields the daemon path uses, plus a line naming where the agent lands (Runs on the Kubernetes cluster · N nodes serving, wording matching DaemonsView/ClusterDetailView; AgentConnect Cloud on the managed install).
  • Runtime/model options come from one live pool member standing in for the pool, following editAgentCapabilitySource's rule: the placement names the SET, and a Pod is only a stand-in for what the cluster advertises.
  • Finish places the agent for real. A pool-born preset is already on the pool, so its runtime PATCH is the whole change; an unplaced preset is moved with { kind: 'pool' }. saveAgentSetup now takes an AgentPlacementTarget instead of a DaemonRow, and the daemon path names { kind: 'daemon', daemonId } itself.

Why

On the cluster path the org's preset kept whatever runtime it was born with, and an org created before the install had a pool member was left with no runtime and no placement at all — onboarding reported success and Home then said the agent was offline "until its daemon reconnects", for an org that never had a daemon.

Notes for the reviewer

  • Finish stays enabled when no member is serving. A pool-born preset finishes fine (PATCH only); an unplaced one gets the CP's own refusal inline (no daemon in the target member set is ready) and the org is not marked onboarded. Happy to gate the button instead if you'd rather not surface that.
  • Verified end to end against a local Control Plane with a pool member: fork → Choose runtime (runtime/model from the member's profiles) → Finish persists runtime/model with the placement left on the pool set; the unplaced-preset variant exercises the move and surfaces the CP refusal.
  • pnpm --filter @agentconnect.md/web test (2047 tests), typecheck, and eslint all clean. The pool-fork suite was rewritten for the new step and gained cases for the move, the patch-only pool-born preset, and the no-members line.

🤖 Generated with Claude Code

…ding

Picking Cluster at the fork used to finish onboarding right there, leaving the
org's preset agent with whatever runtime it was born with — and nothing at all
on an install whose pool had no member when the org was created. The fork is now
a step on the way: both paths end on their own last step, and the pool path gets
the same Runtime/Model pickers the daemon path has, sourced from what a live pool
member advertises, plus a line naming where the agent lands.

Finish places the agent for real: a pool-born preset only needs its runtime
patched, while an unplaced one is moved onto the pool. `saveAgentSetup` therefore
takes an `AgentPlacementTarget` instead of a `DaemonRow`, so the daemon path
names its own target the same way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fast review — logic checks out.

  • total step math and the ClusterStep/DaemonStep fork both correctly derive from poolOffered; backFrom/onBack wiring is consistent between the two.
  • saveAgentSetup's AgentPlacementTarget | null refactor is a clean, mechanical widening from DaemonRow; call sites ({kind:'pool'}, {kind:'daemon', daemonId}, null) all match.
  • Pool-born vs unplaced preset branching via isPoolPlacementKind(builtinAgent?.placementKind) matches the described behavior and the new tests (patches a pool-born preset without moving it / places the unplaced built-in agent on the pool).
  • Minor, non-blocking: that isPoolPlacementKind call omits setId/orgSetIds (unlike most other call sites), so a preset already parked on a named group, not the pool, would be misread as "already placed" and skip the move. Onboarding runs once early, before groups typically exist, so this is low-risk — just flagging for awareness.
  • Tests cover the new step, the "no serving member" copy, and both placement branches; nothing looks missing.

No blocking issues.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for one onboarding correctness issue. The new cluster step can complete onboarding with a pool-born preset rewritten from the deployment-configured runtime/model while the serving member's runtime facts are still settling. Please preserve the preset's current pair or wait for advertised pool capabilities before enabling the mutating Finish path. The provider-neutral { kind: 'pool' } placement, patch-before-move ordering, step flow, and pending-daemon cleanup otherwise look consistent with the surrounding design.

Verification: I confirmed the trusted merge commit has exactly the stated base and PR-head parents, and git diff --check passes. Focused web tests, typecheck, and ESLint could not reach project code in this runner: the default pnpm SQLite cache is unwritable, and the task-scoped writable Corepack cache could not download pinned pnpm because registry DNS returned EAI_AGAIN, including on the approved network retry.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

Comment thread packages/web/src/components/console/views/OnboardingView.tsx
A serving pool member can advertise no runtime profiles yet, and the shared
pickers then fall back to the static runtime list — whose first entry is not the
pool runtime a deployment configures. Because the pool branch skips the move
that would have refused it, Finish wrote `claude-acp` with no model over a
pool-born preset and marked the org onboarded.

With nothing advertised there is now nothing to pick and nothing to write: the
step says so and Finish is completion only. When the cluster names the preset's
own runtime but has not listed its models yet, the existing pin is kept instead
of cleared.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fast re-review of 337903a9 — this revision fixes the blocking finding from the prior pass (silent fallback-runtime overwrite of a pool preset while the cluster is mid-probe).

The fix in ClusterStep (OnboardingView.tsx:550-557):

  • advertised = (source?.runtimeModels.length ?? 0) > 0 — forces runtime = '' when the serving member hasn't reported any profiles yet, instead of falling through to FALLBACK_RUNTIME_IDS.
  • saveAgentSetup already no-ops on empty runtime (if (!builtinAgent || !runtime) return true), so Finish completes onboarding without touching updateAgent/moveAgent in that state — verified by the new "writes nothing while the cluster has not advertised its runtimes" test.
  • The model pin-preservation line correctly keeps initial.model only when staying on the same runtime and the runtime's models haven't landed yet (runtime === initial?.runtime && !rm.selectedModel), covered by "keeps the preset model pin when the advertised runtime has no models yet".
  • User-facing copy explicitly explains the deferred state rather than silently doing nothing, and the Finish button's disabled condition was correctly simplified since runtime is now always safe to pass.

Traced both new test cases against the logic by hand — they match. No new blocking issues found.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for one remaining onboarding correctness issue. The follow-up correctly preserves an already pool-placed preset when runtime facts are absent, but the same completion-only path also applies to an unplaced preset. With no pool member, or while the first serving member has not advertised profiles, Finish now skips both the runtime PATCH and { kind: 'pool' } move and marks the organization onboarded—the exact no-runtime/no-placement state this PR is intended to eliminate. Please distinguish the already-on-pool case from an unplaced preset and keep the latter from completing until it can be placed.

I inspected the exact 337903a9bffedf729b2e54b32673470f2027b20a revision through read-only GitHub API data. It is one commit directly atop the previously reviewed 361957fe12e56337db8a6941e5038791c25db503 revision and changes only OnboardingView.tsx plus its focused tests. No trusted local checkout was available, so local execution was skipped.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

Comment thread packages/web/src/components/console/views/OnboardingView.tsx Outdated
…ishing

Completing with nothing written is honest only for a preset already on the pool.
An UNPLACED one took the same path: Finish skipped both the runtime PATCH and
the `{ kind: 'pool' }` move and marked the org onboarded with no runtime and no
placement — the state this step exists to remove.

The step now knows whether the preset is already there. If it is not, Finish
stays disabled while the cluster advertises nothing, the copy says what is being
waited on and offers the Daemon path, and the step polls the fleet every 3s (as
the daemon step does while it waits) so a member that finishes probing lands
here without a reload.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fast re-review of 2e5a1898 — this closes the gap the other reviewer found in 337903a9 (an unplaced preset could finish onboarding with no runtime/placement during a mid-probe cluster window, recreating the PR's motivating bug).

The fix:

  • placed = !builtinAgent || isPoolPlacementKind(builtinAgent.placementKind) (OnboardingView.tsx:273) — true only when there's nothing to place or the preset is already on the pool.
  • canFinish = advertised || placed gates the Finish button — an unplaced preset can no longer complete while nothing's advertised; a no-op-safe preset (already pool-placed, or no agent at all) still can, preserving the prior fix's behavior.
  • New polling (clusterWaiting effect, mirrors the existing daemon-connect poll pattern) refreshes daemons every 3s while waiting, so the button unblocks itself once the member reports profiles — no manual reload needed.
  • Distinct copy for the two !advertised cases: "leaves runtime as-is" when already placed vs. "waiting to report runtimes … pick Daemon instead" when blocked, with an escape hatch back to the daemon path.

Traced all four placed/advertised combinations by hand against saveAgentSetup's guard and the button's disabled state — they're all correct, and the new test ('keeps Finish disabled for an unplaced preset while the cluster advertises nothing') matches. No new blocking issues.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. This revision correctly separates completion-only pool presets from presets that still require placement: when no runtime profiles are advertised, an unplaced preset now has Finish disabled, receives actionable waiting copy, and polls the fleet until profiles arrive; a preset already on the pool can still complete without rewriting its configured runtime/model. The focused regression exercises the previously failing unplaced branch, and the change preserves the advertised-runtime PATCH plus { kind: 'pool' } move path once capabilities are available.

Verification: the local checkout is the exact trusted 2e5a189815c3e80306d563c2da29a63d10343985 head, directly atop 337903a9bffedf729b2e54b32673470f2027b20a; the follow-up changes only OnboardingView.tsx and its focused test, and git diff --check passes. The focused test command could not reach project code in this runner because the default pnpm SQLite cache is unwritable; a task-scoped writable Corepack cache then could not download pinned pnpm because registry DNS returned EAI_AGAIN, including on the approved network retry, and no repository-local test binaries were present.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

@frozenluo
frozenluo merged commit 9d29245 into main Aug 26, 2026
12 checks passed
@frozenluo
frozenluo deleted the feat/onboarding-cluster-runtime-step branch August 26, 2026 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant