Skip to content

feat(runtime-host): own multi-account API-key onboarding - #3882

Merged
me2seeks merged 5 commits into
apache:mainfrom
me2seeks:feat/3852-runtime-host-multi-account-onboarding
Aug 29, 2026
Merged

feat(runtime-host): own multi-account API-key onboarding#3882
me2seeks merged 5 commits into
apache:mainfrom
me2seeks:feat/3852-runtime-host-multi-account-onboarding

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Before: Runtime Host onboarding without a Connection ID implicitly targeted the provider's canonical slug. Storage could persist multiple same-provider Connections, but CLI/TUI could only edit that canonical account and could not ask the Host to create another one.

After:

  • onboarding uses explicit create(providerType) and existing(connectionId) targets;
  • Storage allocates the candidate connectionId + slug at begin, freezes that identity in the ticket and v2 recovery journal, and revalidates it before credential/catalog mutation;
  • historical v1 onboarding journals remain recoverable by Connection identity;
  • Runtime Host serializes same-provider creates while keeping existing-account lanes keyed by exact Connection ID;
  • CLI/TUI projects every existing API-key account plus one add-account action per provider;
  • account/model labels remain globally unique, and choosing a same-provider account carries its exact Connection ID through verify and save;
  • a committed save remains successful when the follow-up catalog refresh fails, and retries retain the exact committed Connection identity.

Fixes#3869

Verification

  • Full workspace build and CLI typecheck passed on exact head 9dc56691e.
  • Focused CLI onboarding/TUI suites: 174/174 passed.
  • Full workspace test run: 3076 passed, 7 skipped, and 2 unrelated Runtime reasoning tests failed; both failures reproduce in unchanged files outside this PR.
  • Hosted test and windows_recovery both passed on exact head 9dc56691e.
  • Two rounds of independent exact-head review found no remaining P0-P3 issues after review remediation.
  • git diff --check passed.

Compatibility and scope

  • Rebasing preserved main's compatibility epoch 67 changes; this onboarding wire change advances the Runtime Host compatibility epoch from 67 to 68.
  • Existing Desktop-created Connections, including non-canonical slugs such as openai-2, remain editable by exact Connection ID.
  • OAuth entity binding, ScheduledTask identity, and Desktop onboarding convergence are intentionally outside this PR and remain tracked by [Tracking] Explicit multi-account provider connections across Runtime Host surfaces #3852 and their dedicated follow-ups.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex assisted with implementation, tests, review remediation, rebase conflict resolution, and verification. Commits contain a Generated-by: OpenAI Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Build, typecheck, affected suites, and diff checks pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@Astro-HanAstro-Han 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.

I reviewed this head and found blocking issues.

[P2] /model cannot disambiguate same-provider accounts

Storage writes name="OpenAI" for every new account; /model shows connectionName || slug only — two OpenAI accounts both render as OpenAI, user cannot tell which will be selected. Violates disambiguation requirement.

Fix: render name + slug together with regression test using real same-name onboarding.

Also Standards 3 hard (template, missing trailer, before/after).

Hosted test/windows_recovery: SUCCESS.

简体中文同名账号无法区分。

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@me2seeks
me2seeksforce-pushed the feat/3852-runtime-host-multi-account-onboarding branch from 92e3427 to e69f48eCompareAugust 26, 2026 08:16
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Addressed on head e69f48e5d:

  • /model now derives one visible label per Connection slug and appends the slug only when that final label is shared by multiple Connections;
  • two same-name catalog entities project as OpenAI · openai and OpenAI · openai-2, and selecting the second sends openai-2;
  • one Connection with multiple model rows remains OpenAI without a redundant slug;
  • name/fallback-slug collisions and the full model-change notice are covered;
  • the branch is rebased onto feat(runtime): bind new sessions to connection identity #3864 head 844ecff82, the single onboarding commit carries Generated-by: Codex, and the PR body now follows the repository template with Before/After and AI-use disclosure.

Validation: CLI typecheck/build; focused TUI 4/4; Runtime Host onboarding projection 6/6; Biome and diff-check. Hosted checks are rerunning for the new head.

@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Hosted freshness is complete for head e69f48e5d: both test and windows_recovery passed. The PR body has been updated to match the final check state.

@Astro-HanAstro-Han 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.

I reviewed this head and found blocking issues.

[P2] /model new-account path carries only connectionSlug, not connectionId

Multi-account selection sends only slug; if account A deleted and slug reused, server resolves by slug and silently binds to B.

Fix: carry connectionId through ModelChoice/driver/protocol.

[P2] Label dedup collision

modelChoiceConnectionLabels() groups by raw name then appends slug once; OpenAI/openai, OpenAI/openai-2, OpenAI · openai/relay can collide to OpenAI · openai duplicate. Fix: globally unique final labels with regression test.

Hosted test: SUCCESS and windows_recovery: SUCCESS, but code is NO-GO.

简体中文多账号标识与标签去重问题。

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@M4n5ter
M4n5terforce-pushed the feat/3852-runtime-host-multi-account-onboarding branch 3 times, most recently from 4d2db23 to b1ab9adCompareAugust 26, 2026 09:51
@me2seeks
me2seeksforce-pushed the feat/3852-runtime-host-multi-account-onboarding branch from b1ab9ad to 77ece33CompareAugust 26, 2026 10:17
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Addressed both review blockers in 77ece3349: /model now carries the exact connectionId + connectionSlug + model through the picker, driver, wire update, and CAS retry; account labels now use a globally unique deterministic allocator. Added regressions for same-name accounts, label collisions, deleted versus disabled resume, and identity preservation across retries.

@me2seeks
me2seeksforce-pushed the feat/3852-runtime-host-multi-account-onboarding branch 2 times, most recently from f1fa3f9 to a70bc67CompareAugust 26, 2026 12:45
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Addressed the hosted release-smoke failure on head a70bc6707.

The Runtime Host resolver already selected the exact Connection entity, but MakaRunContext.target dropped its connectionId; maka run and activation then created a Session with slug + model only, and the driver correctly rejected that as legacy state. Both create funnels now carry required connectionId + connectionSlug + model without weakening the legacy-null fail-closed rule.

A regression uses two same-provider accounts, selects account 2 by slug, runs through the real Runtime Host context and Session driver, and asserts the exact account-2 triplet on session.create. Two independent final reviews returned GO. Validation: CLI 500/500 before the patch-equivalent final restack, targeted real-funnel 3/3 after restack, typecheck, epoch guard, and the installed release-package controlled model-turn smoke.

Generated-by: Codex

@Astro-HanAstro-Han 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.

I reviewed this head and found the overall feature is not merge-ready.

Multi-account API-key onboarding (intent create/existing, Storage UUID/slug assignment, recovery intent, Host-committed identity) is necessary and bounded correctness shows no independent P0-P3; hosted test/windows_recovery SUCCESS.

[Feature/Global-design] NO-GO — composed sequence mismatch

This head (a70bc67) still carries the superseded 6-commit #3864 stack (actual parent 26bf00bb, body misstates f5ba1f2d) for 109 files / +4124 -668, and its last two commits are exactly the model-selection/CAS/run/activation work that now belongs to #3926 per #3852. The intended sequence is #3864 (04686a0) → #3926#3882 (onboarding-only). Until #3864 and #3926 land and #3882 is rebased/deduped to onboarding-only with corrected dependencies, this composed PR should not be merged.

简体中文当前组合与既定序列不一致,需等待前置 PR 落地后再重叠为 onboarding-only。

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@github-actionsgithub-actionsBot added the effort/XL Over 1000 readable lines label Aug 27, 2026
@me2seeks
me2seeksforce-pushed the feat/3852-runtime-host-multi-account-onboarding branch from a70bc67 to 19581e8CompareAugust 29, 2026 13:42

@M4n5terM4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I reviewed exact head 19581e83279577de74d1f70083e0159f5f334208 and found no P0/P1 issues and one P2 correctness issue, left inline. The Storage-owned candidate identity, exact existing-account targeting, transaction recovery, and concurrency guards otherwise held up under the affected tests.

Both exact-head hosted checks passed, but this branch currently conflicts with main. The conflict is also a protocol compatibility boundary: current main is epoch 67, while this head assigns epoch 66 to a different closed wire change. Please rebase, assign an epoch strictly newer than current main, and rerun the checks; I am leaving a COMMENT rather than an approval because the reviewed head cannot be merged as-is.


Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.

Comment threadpackages/cli/src/runtime-host-onboarding.ts
@me2seeks
me2seeksforce-pushed the feat/3852-runtime-host-multi-account-onboarding branch from 19581e8 to 9dc5669CompareAugust 29, 2026 15:33

@M4n5terM4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed exact head 9dc56691e01e72c445720b2d6da8bb255ed70d61. No P0-P3 issues remain.

The previous post-commit refresh finding is closed. The onboarding surface now keeps the Host-returned committed Connection identity separate from the follow-up catalog refresh. If refresh fails, the save remains successful with a restart warning, and retrying the same wizard target edits that exact Connection instead of allocating another account. A newly reselected Add Account row receives a fresh target identity, so a late save cannot rebind the user's new create intent; late projections also remain attempt-gated.

The rebase preserves current main and advances the incompatible onboarding protocol shape from epoch 67 to 68. The closed create | existing target still leaves Storage/Runtime Host as the only durable identity authority; the CLI only carries the returned exact identity.

Verification: both hosted checks are green on this exact head. Core, Storage, MCP, Runtime, Runtime Host, Eval, and CLI builds passed locally; 332/332 affected CLI/Host/Storage tests passed, along with the protocol epoch guard, Biome on all 20 changed files, ASF header audit, and diff checking. The sole review thread is resolved.


Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.

@me2seeks
me2seeks merged commit d7efbb4 into apache:mainAug 29, 2026
2 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XLOver 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(runtime-host): own explicit multi-account API-key onboarding targets

3 participants

@me2seeks@M4n5ter@Astro-Han