feat(coding-agent)!: adopt pi's ModelRuntime architecture for full model-provider parity - #2064
Merged
Conversation
…el refresh Match upstream pi's invariant: once fresh OAuth tokens are persisted, the catalog-refresh outcome must not fail the login or touch the stored credential. Previously, a timed-out (aborted) or partially failed model-catalog refresh after a successful /login threw 'Model refresh aborted after OAuth login' and rolled the fresh tokens back to the previous credential. Because providers rotate refresh tokens, the rollback could re-install a server-side-invalidated credential, permanently stranding the user with 'invalid_grant' on every send while every re-login was rolled back again. Machines with slow routes to catalog endpoints hit this deterministically. - rpc-provider-auth: persist the credential, run the refresh best-effort, and drop the rollback and aborted/error classification (a thrown refresh still propagates but no longer unpersists tokens) - interactive-auth-login: drop the provider-error and aborted throws from the post-login refresh; models fall back to the cached snapshot - update RPC and interactive regression tests to lock in credential survival; add an aborted-refresh interactive case Assistant-model: Claude Fable 5
Replace the fork-era registry engine with pi's provider-owned runtime and thin extension facade. Remove legacy OAuth registration and layered models configuration while preserving Atomic path, auth, RPC, and isolated-engine compatibility. Assistant-model: Claude Opus 5
Persist the full RPC credential union through ModelRuntime and refresh the catalog before returning. Restore Atomic-specific retry, compaction, footer, OAuth cancellation, migration, and usage-accounting regression coverage removed during the runtime parity port. Assistant-model: Claude Opus 5
Restore isolated RPC model refresh credential reload, explicit force and timeout forwarding, auth-storage transactional behavior, and SDK/RPC regression coverage while documenting pi parity changes. Assistant-model: Claude Opus 5
Assistant-model: Claude Opus 5
Reload frontend credential state after isolated login and logout so auth selectors reflect stored credentials. Transport and honor OAuth login labels and callback-server metadata, restoring reachable manual-code input. Restore credential reload regression coverage, isolate RPC persistence tests from ambient auth, and document the AuthStorage and ModelRegistry API reductions. Assistant-model: Claude Opus 5
Restore callback-server and login-label metadata for builtin OAuth providers so manual redirect login remains reachable. Keep logout and isolated auth status aligned with stored credentials and engine-published availability, and restore deterministic RPC prompt-lock coverage. Assistant-model: Claude Opus 5
Correct the models.json migration docs, preserve refresh option coverage, label logout credentials by stored type, and honor truthy offline flags. Assistant-model: Claude Opus 5
Fix the tautological restored-model provider guard and restore the positive registered-provider regression coverage. Preserve readStoredCredential's non-throwing malformed-file contract and split ModelRuntime snapshot helpers for file-length headroom. Correct models.json migration documentation and project-scoped configuration coverage. Assistant-model: Claude Opus 5
Route session restoration through the restored-model resolver and narrow unknown-model reconstruction to supported dynamic and configured providers. Restore discriminating SDK and retired-model regression coverage. Use the extension-facing model registry for workflow catalogs, guard the advisory Anthropic auth check from stale credential failures, and restore the full RPC OAuth login coverage including correlated callback dispatch. Assistant-model: Claude Opus 5
Show the Anthropic subscription warning once from stored OAuth status without triggering refresh-capable auth lookup, while keeping lookup failures non-fatal. Label login status from the stored credential type and restore regression assertions for both behaviors. Document the removed CreateAgentSessionOptions authStorage and modelRegistry overrides. Assistant-model: Claude Opus 5
Keep thrown post-login catalog refresh failures visible without rolling back persisted OAuth credentials, while preserving successful handling of returned provider errors and aborted results. Restore the PR #2059 regression assertion and propagate interactive refresh failures. Disable network catalog refresh during --api-key startup, migrate shipped SDK examples and documentation to ModelRuntime, and restore session-service startup timing spans. Assistant-model: Claude Opus 5
The migrated selector diverged from pi: it dropped pi's selector-owned 15s refresh timeout, routed failures through the status line with 'showing available models' wording, rendered the snapshot through an async promise chain, and passed allowNetwork although the runtime owns offline gating. Port pi's implementation verbatim: synchronous snapshot render, selector-owned timeout with 'Model refresh timed out; showing cached models.', failures on the error surface, muted status color, and a signal-only refresh call. Tests updated to pi semantics plus a fake- timer timeout case and a runtime-owned network-gating assertion. Assistant-model: Claude Fable 5
The migration left the bundled llama.cpp extension on Atomic's legacy ProviderConfig registration with a hardcoded 'local' API key, so the new ModelRuntime treated the provider as configured for everyone: every /model refresh attempted http://127.0.0.1:8080 and unconfigured users saw 'Could not refresh llama.cpp; showing cached models.' Port pi's provider.ts verbatim: native Provider registration with provider-owned auth whose check/resolve return undefined until LLAMA_BASE_URL or a stored login configures a server, so refresh skips the provider entirely when unconfigured. Includes pi's /login flow (server URL + optional API key). Ports pi's llama-extension test suite (one assertion adapted to Atomic's unified pending-registration list), rewrites the root parity tests against the native surface with a new dormant-until-configured regression, and corrects the changelog's refresh-timeout entry to pi's real semantics (selector-owned 15s timeout). Assistant-model: Claude Fable 5
|
Too many files changed for review. ( Bypass the limit by tagging |
All five conflicts resolve to the branch side: the ModelRuntime migration supersedes the #2059 versions of these files while keeping its credential-survival invariant and changelog entry. Assistant-model: Claude Fable 5
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
flora131
pushed a commit
that referenced
this pull request
Jul 29, 2026
…rewrite #2064 rewrote these assertions and dropped the platform-aware handling the previous versions had, so all three fail on Windows only: - footer-width: `formatCwdForFooter` joins with `sep`, so the expectation is `~\project` on Windows. The rewrite hardcoded `~/project` (was: `` `~${sep}project` ``). - sdk-session-manager: the session file separator check hardcoded `/` (was: `` `${expectedSessionDir}${sep}` ``). - sdk-session-manager: the system prompt always renders cwd with POSIX separators (`system-prompt.ts` applies `.replace(/\\/g, "/")`), so the Windows expectation needs the same normalization (was: `` sessionCwd.replaceAll("\\", "/") ``). Restore all three. Production code is unchanged and was correct throughout. This is what exhausted the Windows job budget: the three failures triggered the bounded retry, and two full ~170s suite runs pushed the job past its 15m cap, where GitHub reported the kill as `cancelled`. Assistant-model: Claude Opus 5
flora131
added a commit
that referenced
this pull request
Jul 29, 2026
The Windows `test` job has been killed at its 15m `timeout-minutes` budget on every run since #2064, always inside the coding-agent vitest step. The step log was empty for the entire 10m before the kill, so the cancellation carried no evidence about which test stalled. The cause of the blackout is `run-flaky-test-suite.ts`: it collected the whole child stdout/stderr with `new Response(child.stdout).text()` and only wrote it after the child exited. A child killed by the job timeout never exits normally, so every collected line was discarded. Tee both streams as they arrive instead. The retry, deterministic-file, and diagnostics-artifact contracts are unchanged; a timed-out attempt now leaves its progress in the step log. Also raise the Windows budget to 30m temporarily so the suite can run to completion and report its real failures. Restored once the stall is fixed. Assistant-model: Claude Opus 5
flora131
added a commit
that referenced
this pull request
Jul 29, 2026
…rewrite #2064 rewrote these assertions and dropped the platform-aware handling the previous versions had, so all three fail on Windows only: - footer-width: `formatCwdForFooter` joins with `sep`, so the expectation is `~\project` on Windows. The rewrite hardcoded `~/project` (was: `` `~${sep}project` ``). - sdk-session-manager: the session file separator check hardcoded `/` (was: `` `${expectedSessionDir}${sep}` ``). - sdk-session-manager: the system prompt always renders cwd with POSIX separators (`system-prompt.ts` applies `.replace(/\\/g, "/")`), so the Windows expectation needs the same normalization (was: `` sessionCwd.replaceAll("\\", "/") ``). Restore all three. Production code is unchanged and was correct throughout. This is what exhausted the Windows job budget: the three failures triggered the bounded retry, and two full ~170s suite runs pushed the job past its 15m cap, where GitHub reported the kill as `cancelled`. Assistant-model: Claude Opus 5
flora131
added a commit
that referenced
this pull request
Jul 29, 2026
All 374 Windows test files now pass in 191s, but the process then sits for ~412s until the job cap kills it. The last green Windows run (pre-#2064) exited 0.8s after printing its summary, so the exit hang is new and is what still exhausts the budget. Add vitest's hanging-process reporter to name the handle. Removed once the leak is fixed. Assistant-model: Claude Opus 5
flora131
added a commit
that referenced
this pull request
Jul 29, 2026
…le (#2066) * ci: stream test-suite output so timed-out Windows jobs stay diagnosable The Windows `test` job has been killed at its 15m `timeout-minutes` budget on every run since #2064, always inside the coding-agent vitest step. The step log was empty for the entire 10m before the kill, so the cancellation carried no evidence about which test stalled. The cause of the blackout is `run-flaky-test-suite.ts`: it collected the whole child stdout/stderr with `new Response(child.stdout).text()` and only wrote it after the child exited. A child killed by the job timeout never exits normally, so every collected line was discarded. Tee both streams as they arrive instead. The retry, deterministic-file, and diagnostics-artifact contracts are unchanged; a timed-out attempt now leaves its progress in the step log. Also raise the Windows budget to 30m temporarily so the suite can run to completion and report its real failures. Restored once the stall is fixed. Assistant-model: Claude Opus 5 * ci: keep the Windows budget at its contracted 15m The deterministic CI contract pins the Windows timeout at 15m, so the temporary diagnostic bump failed 'Deterministic CI and release contracts' before the vitest step could run. Streamed output alone now carries the evidence: the killed step reports progress up to the kill point. Assistant-model: Claude Opus 5 * fix(test): restore Windows path handling dropped by the ModelRuntime rewrite #2064 rewrote these assertions and dropped the platform-aware handling the previous versions had, so all three fail on Windows only: - footer-width: `formatCwdForFooter` joins with `sep`, so the expectation is `~\project` on Windows. The rewrite hardcoded `~/project` (was: `` `~${sep}project` ``). - sdk-session-manager: the session file separator check hardcoded `/` (was: `` `${expectedSessionDir}${sep}` ``). - sdk-session-manager: the system prompt always renders cwd with POSIX separators (`system-prompt.ts` applies `.replace(/\\/g, "/")`), so the Windows expectation needs the same normalization (was: `` sessionCwd.replaceAll("\\", "/") ``). Restore all three. Production code is unchanged and was correct throughout. This is what exhausted the Windows job budget: the three failures triggered the bounded retry, and two full ~170s suite runs pushed the job past its 15m cap, where GitHub reported the kill as `cancelled`. Assistant-model: Claude Opus 5 * fix(test): make two Windows-only suite failures portable With the separator fixes in place the Windows suite ran far enough to expose two more failures, both from tests that assume POSIX process and filesystem behavior: - bash-session-metadata: the detached-async-job test leaves a shell running with the temp dir as its cwd. Windows locks a directory in that state, so `afterEach` cleanup failed with EBUSY and failed the test. Retry the removal and ignore a residual failure; reclaiming an OS temp dir is never the assertion. - sdk-session-manager: the cwd check compared `pwd` output through `realpathSync`. On Windows the bash tool resolves to Git Bash, which reports MSYS paths (`/tmp/...`) that Node resolves against the current drive, so the lstat failed with ENOENT on `C:\tmp\...`. Prove the working directory by reading a marker file only resolvable from sessionCwd, which tests the same contract without depending on how a shell spells a path. Assistant-model: Claude Opus 5 * test: temporarily report hanging handles on the Windows suite All 374 Windows test files now pass in 191s, but the process then sits for ~412s until the job cap kills it. The last green Windows run (pre-#2064) exited 0.8s after printing its summary, so the exit hang is new and is what still exhausts the budget. Add vitest's hanging-process reporter to name the handle. Removed once the leak is fixed. Assistant-model: Claude Opus 5 * test: dump the surviving process tree at Windows suite teardown The Windows suite now passes all 374 files in ~200s, then sits until the job cap kills it. vitest's hanging-process reporter printed nothing, because the suite runs under `bun --bun` where that detection does not apply. Swap it for a global teardown that dumps the live process tree on Windows, to name whatever still holds the event loop open. The failure path exits in 0.2s while the success path hangs, which points at a surviving child rather than a reporting bug. Removed once the leak is fixed. Assistant-model: Claude Opus 5 * test: probe surviving handles after the Windows suite finishes The teardown process-tree dump showed no surviving bash/sleep/git children and no leftover vitest workers, so the handle that blocks exit lives inside vitest's own process rather than in spawned test children. Replace the one-shot dump with unref'd probes at +15s and +45s that report process.getActiveResourcesInfo(). Unref'd timers fire only while the loop is still alive, so a healthy run exits before they run and pays nothing. Assistant-model: Claude Opus 5 * test: name the Windows survivors holding the suite's stdio pipes * test: widen the Windows teardown probe to every recent process * fix(test): stop the Windows suite from launching a real browser The interactive OAuth callback test drives a real LoginDialogComponent through onAuth, and showAuthUrl calls openBrowser. On Windows that spawns a detached `rundll32 url.dll,FileProtocolHandler https://corp.invalid/login`, which inherits the test process's stdout/stderr write handles. The CI runner has no browser to hand off to, so rundll32 never exits: the vitest process, `bun run`, and every descendant terminate, but the step's pipes never reach EOF and the job hangs until its 15-minute budget expires. CI evidence (run 30495631917, Windows job): all 2887 tests pass, the child exits code=0 at 22:25:38, and a Win32_Process table taken every 30s after that shows exactly one non-infrastructure survivor, pid 2508 `rundll32 url.dll,FileProtocolHandler https://corp.invalid/login`, started 22:24:23 with a dead parent, still alive when the job is cancelled. 5ef323e added this test, which is why the bisect lands there. Mock the launcher exactly as pi's login-dialog suites do, and assert the dialog still hands the URL to it so the launch path stays covered without spawning a process. Assistant-model: Claude Opus 5 * test: remove the Windows teardown diagnostic scaffolding The probes named the leaking process, so drop them: the vitest globalSetup that dumped the surviving process tree, its config entry, and the process table the flaky-suite wrapper printed after the child exited. The streamed tee stays, since a timed-out attempt still needs a self-describing log. Assistant-model: Claude Opus 5
sina85
added a commit
to sina85/atomic
that referenced
this pull request
Aug 3, 2026
Bring the node-card model/thinking/fast + durable thinkingLevel persistence up to date with upstream's tabs reformat, the ModelRuntime refactor (bastani-inc#2064), and the reshaped graph node cards (bastani-inc#2140). Re-applied thinkingLevel across the durable serialize/restore path, integrated the model row into the reshaped card, and preserved the replay model-identity fix. NODE_H stays 6 for the model row; updated the bastani-inc#2100 graph-perf composed-row expectation to match the taller card.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces Atomic's fork-era
ModelRegistryengine with upstream pi's model-provider architecture, ported from pi @027a5847(coding-agent 0.82.1). Stacked on #2059 (its credential-survival invariant is now guaranteed structurally by pi'slogin()shape rather than by Atomic-specific glue).Changes
ModelRuntime(snapshot-based availability,RuntimeCredentials,ModelConfig,provider-composer, pi'sremote-catalog-provider) is now the engine;ModelRegistryshrinks to pi's thin synchronous extension facade. 48 call-site files (AgentSession, SDK, RPC, interactive, isolated-engine) consume the runtime directly..pi+.atomicmodels.json merging, custom API-key login surfaces beyond pi's provider contract,AuthStoragesync compatibility API (now pi's asyncCredentialStore), andModelRegistry/CreateAgentSessionOptionssurface not present in pi. See changelogBreaking Changesfor the migration guide.modelRefreshTimeoutMsbounds only runtime creation; the/modelselector owns its 15 s timeout, renders cached models immediately, and reports timeout/errors with pi's exact strings and surfaces.Could not refresh llama.cpp; showing cached models.— the provider now stays dormant (no refresh attempt) untilLLAMA_BASE_URLor a stored login configures a server;/loginprompts for server URL + optional key like pi.llama-extension.test.tsported (one assertion adapted to Atomic's unified pending-registration list); root parity tests rewritten against the native surface incl. a dormant-until-configured regression; selector tests updated to pi semantics with a fake-timer timeout case; fix(auth): never roll back OAuth credentials on failed post-login model refresh #2059 credential-survival regressions preserved.Breaking Changes
Documented in
packages/coding-agent/CHANGELOG.md[Unreleased]→Breaking Changes(OAuth bridge removal, single-fileModelConfig, asyncAuthStorage, facade-onlyModelRegistry,modelRuntimesession option).Verification
bun run typecheck,bun run lint,bun run check:file-length— cleanbunx vitest --run(packages/coding-agent): 2881 passed / 0 failed (369 files)bun run test:unit(root, incl. workflows/subagents): 4407 passed / 0 failedregisterOAuthProvider/registerLegacyOAuthProvider/getLegacyOAuthProvider/refreshLegacyOAuthNotes
Produced by a bounded goal-workflow run (10 orchestrator turns with reviewer gates) plus two follow-up parity fixes found in post-run audit: the selector refresh port and the llama.cpp provider port.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.