refactor(oauth): make the login platform layer provider-neutral - #34
Merged
Conversation
Model listing, capability derivation and the on-disk config shape lived in a provider-specific module while every other login path imported them from there. Move them into open-platform.ts as PlatformModelInfo / PlatformConfigShape and collapse the two duplicate copies of capabilitiesForModel and toModelInfo into one; the survivor parses supported_reasoning_efforts so no field is lost. Drop the managed-subscription provider along with it: the device-code OAuth flow, its token storage and manager, the managed usage and feedback endpoints, the agent-core OAuth service and REST routes, the SDK auth facade, and the managed model provider. Nothing configured them any more once model listing became provider-neutral. Logging in is now: API key, models.dev catalog provider, or OpenAI Codex OAuth. 'Is the user logged in' becomes PythinkerHarness.isAuthenticated(), one predicate over configured providers with a usable credential, shared by the VS Code extension and the ACP adapter. /feedback opens the issue tracker.
…onfig Nothing writes an oauth ref now that logins end in an api key: OpenAI Codex stores its access token as the provider apiKey with the refresh token under `source`, and every other path is a plain key. Remove the field, the schema, its TOML round-trip, the ProviderManager auth resolver it fed, and the mutual-exclusion rule that only existed to keep it apart from static keys. The web client loses the device-code login dialog and the /oauth REST calls behind it; logging in there opens the provider manager, which adds API-key and catalog providers.
The platform picker still offered 'Kimi (OAuth)' and the VS Code welcome screen still said 'Sign in with Kimi' with a device-code URL panel, but the handlers behind both were gone — picking that option did nothing and the URL panel could never fill in. Drop the option, the loginUrl event nothing emits any more, and the managed-provider login gate that could no longer fire; the VS Code welcome screen now describes what login actually does.
… cleanup Review found the VS Code sign-out looping removeProvider over every configured provider — that deletes hand-written config.toml entries and can stop halfway. Only providers a login created carry a `source`, so scope the removal to those and write it once through replaceConfig. Also: the web settings dialog still printed a hardcoded managed provider id and offered a sign-out whose handler was gone; the web /login command opened the provider panel without loading it; and the SDK config smoke fixture paired a Moonshot provider with a model it does not serve. Add the OpenAI Codex refresh regression the deleted managed-scope tests used to cover.
…r OAuth-backed The readiness probe used to have two branches: an api key, or a managed OAuth token. Only the api-key branch is left, and OpenAI Codex satisfies it only because it stores its access token as the provider apiKey. Nothing said so, so a future change to that storage would silently send every Codex user to AuthTokenMissingError on their first turn.
The Codex refresh case only asserted the stale default was gone, not what replaced it; it re-points at the model it just fetched, so assert that. The VS Code refresh-failure case never checked the refresh was attempted. The mobile sheet's account button now names the panel it opens, matching the desktop dialog.
…d them The ACP adapter tests stubbed harness.auth.status; the adapter now asks harness.isAuthenticated, so every stub returned undefined and the auth gate surfaced as -32603 Internal error instead of -32000 auth_required. Replace the two status payloads in the shared helper with the predicate itself. Drop the managed_provider expectation and the managed OAuth refresh route case from the server e2e suites, both of which cover endpoints that no longer exist.
|
Important Review skippedToo many files! This PR contains 167 files, which is 17 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to Pro+ to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (167)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
commit: |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…ed them auth-gate, session-load and session-resume imported AUTHED/UNAUTHED but still hand-rolled `isAuthenticated`, leaving the imports dead. auth-gate was already half-converted (the inline harness uses UNAUTHED), so this finishes what 1456fb2 started and matches the 18 sibling test files.
Member
Author
|
CodeRabbit skipped this PR on size (167 files > 150), so the review ran through the local CodeRabbit CLI instead: Basis The three |
Merged
elkaix
added a commit
that referenced
this pull request
Aug 7, 2026
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @pythoughts/pythinker-code@0.11.0 ### Minor Changes - [#32](#32) [`a504a82`](a504a82) - Rename the ACP authentication method to reflect that login is multi-provider: it now reads "Log in with a provider" and explains that the provider is chosen in a terminal. Clients matching the previous wording will need updating. - [#32](#32) [`a504a82`](a504a82) - Let a Dynamic Workflow require structured output from its subagents. Passing `output_schema` makes each subagent return a validated object instead of free text, and a subagent that cannot satisfy the schema is reported separately from one that failed outright. - [#32](#32) [`a504a82`](a504a82) - `pythinker login` now opens a provider picker instead of going straight to one provider, and accepts `--provider <id|name>` to skip it. The VS Code extension's sign-in offers the same providers, and both surfaces present the same thinking-effort levels for a given model. - [#34](#34) [`42da384`](42da384) - Make the login platform layer provider-neutral. Model listing, capability derivation and the on-disk config shape are now one set of types shared by every login path, instead of living in a provider-specific module that other providers imported from; the duplicate copies of the capability derivation and the model-info parser are collapsed into one. Logging in is an API key, a models.dev catalog provider, or OpenAI Codex OAuth. "Is the user logged in" is now a single predicate over configured providers with a usable credential, shared by the CLI, the VS Code extension and the ACP adapter. `/feedback` opens the issue tracker. - [#32](#32) [`a504a82`](a504a82) - Give every Dynamic Workflow run an id and stamp it on the subagent events it produces, so a client can tell which run a given subagent belongs to when several are in flight. - [#32](#32) [`a504a82`](a504a82) - Add two ways to rein in Dynamic Workflow fan-out: `disableWorkflows` turns the tool off entirely, and `workflowSizeGuideline` sets an advisory ceiling that is mentioned to the model and warned about, on every surface, when a run exceeds it. Both are settable in config or by environment variable. - [#32](#32) [`a504a82`](a504a82) - Bound subagent fan-out with hard caps: 128 subagents per call, 200 per session, and a nesting depth of 3. Nesting was previously unbounded, so a workflow that spawned workflows could grow without limit; past depth 3 the call now fails instead. - [#32](#32) [`a504a82`](a504a82) - Replace the Dynamic Workflow progress bar with the two things it can actually know: how many tool calls each agent has made, and how long it has been silent. The old bar pinned every tool-using agent at 75% until it finished, so an agent working hard and one wedged for ten minutes looked identical. A row that goes quiet now turns amber, then red. ### Patch Changes - [#32](#32) [`a504a82`](a504a82) - Survive two malformed inputs that used to end a run. A catalog entry that is not an object is now dropped when the catalog is read, instead of reaching the provider picker and throwing past the bundled-catalog fallback that was meant to save the login. A non-finite subagent concurrency limit now falls back to the default: `NaN` passed every clamp, and each free-slot test against it was false, so the batch launched nothing and never finished. - [#32](#32) [`a504a82`](a504a82) - Offer a model's declared thinking-effort levels when signing in to OpenAI Codex. The picker previously fell back to low / medium / high regardless of what the model supports, disagreeing with the effort list recorded in the config it then wrote. - [#32](#32) [`a504a82`](a504a82) - Accept a provider's plain id for `--provider` at login, so a catalog provider no longer has to be named by its full display name, and stop a cancelled OpenAI Codex sign-in from holding the process open for the rest of its two-minute callback timeout. In the editor extension, signing in now shows one cancellable progress notification, a repeated sign-in joins the one already running instead of opening a second set of prompts, and a completed sign-in is no longer reported as failed when the status refresh behind it fails. - [#32](#32) [`a504a82`](a504a82) - Save the thinking-effort level picked during login. Only an on/off flag was stored, so choosing low, medium, or xhigh reopened the session at high, and an OpenAI Codex login reopened at the model's maximum effort regardless of the choice. - [#32](#32) [`a504a82`](a504a82) - Write the thinking effort picked at login to disk. The apply step recorded the level, but the patch that saved the result listed everything except it, so an API-key login still reopened at the default effort. Choosing `off` now also clears a level a previous login left behind, which a patch that only merges could not do by omitting the key. - [#32](#32) [`a504a82`](a504a82) - Keep the configured provider signed in when a login is abandoned. Backing out at the model picker, or a failure while fetching the model list, no longer clears the existing credentials, and dismissing the provider picker returns to the sign-in screen instead of reporting a failed login. - [#32](#32) [`a504a82`](a504a82) - Refuse a device authorization whose verification URL is not HTTPS. Every surface hands that URL to the host's "open externally" API, so a provider answering with `file:`, `javascript:`, or an installed application's own scheme had the agent launch it. The check runs where the response is parsed, so the terminal, the TUI, and the editor extension are all covered. - [#32](#32) [`a504a82`](a504a82) - Finish handling blank Dynamic Workflow items. A run that dropped one reported its results after a note explaining the drop, which made the whole result parse as unsupported and rendered a successful run as failed; the note now follows the results. A blank entry also no longer leaves a row queued forever with the header stuck below its total, and no longer pushes a full item list over the subagent cap and back into whole-call rejection. - [#32](#32) [`a504a82`](a504a82) - Ignore empty entries in a Dynamic Workflow's item list instead of rejecting the call. A trailing empty item used to fail argument validation, which discarded the whole workflow before any subagent started and forced the agent to send every prompt again. The dropped count is now reported with the results, and the launch panel counts only the subagents that will actually run. - [#32](#32) [`a504a82`](a504a82) - Stop a Dynamic Workflow row that has not started from reading as stalled. A queued row measured its silence from the launch of the whole run, so a long queue turned every waiting row amber and then red while nothing was wrong. A queued row now shows the same placeholder a finished one does, and a suspended row keeps its count without the alarm colours, because only a running row can stall. - [#32](#32) [`a504a82`](a504a82) - Keep a Dynamic Workflow subagent's output schema when a provider rate limit forces its turn to be retried. The retried turn lost the schema, so the subagent answered in prose and the workflow reported it as completed rather than as a schema failure. - [#32](#32) [`a504a82`](a504a82) - Show a Dynamic Workflow's running rows with a spinning grey dot, so a working agent reads as motion rather than as a static dot the eye cannot tell from a finished one, and shimmer the Orchestrating label in periwinkle instead of grey. - [#32](#32) [`a504a82`](a504a82) - Show the whole large-workflow warning in the editor extension. The line was truncated to the panel width, so in a narrow side panel the reader saw the opening words and no reason. ## @pythoughts/pythinker-code-sdk@0.12.0 ### Minor Changes - [#34](#34) [`42da384`](42da384) - Make the login platform layer provider-neutral. Model listing, capability derivation and the on-disk config shape are now one set of types shared by every login path, instead of living in a provider-specific module that other providers imported from; the duplicate copies of the capability derivation and the model-info parser are collapsed into one. Logging in is an API key, a models.dev catalog provider, or OpenAI Codex OAuth. "Is the user logged in" is now a single predicate over configured providers with a usable credential, shared by the CLI, the VS Code extension and the ACP adapter. `/feedback` opens the issue tracker. ## pythinker-code@0.8.5 ### Patch Changes - Updated dependencies [[`42da384`](42da384)]: - @pythoughts/pythinker-code-sdk@0.12.0 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: M Elkholy <melkholy@techmatrix.com>
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.
Related Issue
No issue — this is a cleanup of an internal abstraction that had grown a
provider-specific home. The problem is described below.
Problem
Model listing, capability derivation and the on-disk config shape lived in a
provider-specific module, and every other login path imported them from there:
the API-key open platforms, the OpenAI Codex OAuth flow, and the custom
api.jsonregistry all reached intomanaged-kimi-code.tsforManagedKimiCodeModelInfoandManagedKimiConfigShape. Two copies ofcapabilitiesForModelexisted side by side, byte-identical, and two variants ofthe
/modelsparser differed only in how they handled a missingcontext_length.Sitting on top of that was a managed-subscription provider that nothing else
used. Its device-code OAuth flow, token storage, usage and feedback endpoints,
agent-core OAuth service, REST routes, SDK auth facade and model provider were
all reachable only through that one provider.
What changed
The platform layer is provider-neutral.
PlatformModelInfo,PlatformConfigShape,PlatformProviderConfig,PlatformModelAliasandSupportsThinkingTypenow live inopen-platform.ts— moved rather thancopied, so the file count goes down. The duplicate
capabilitiesForModelandtoModelInfocollapse into one each; the survivor parsessupported_reasoning_efforts, so no field is lost.The managed-subscription provider is gone, and with it the device-code OAuth
stack,
managed-usage.ts,managed-feedback.ts,services/oauth/*,services/auth/managedAuth.ts, the/oauthREST routes,PythinkerAuthFacadeand
PythinkerForCodingProvider. Nothing configured any of it once modellisting stopped depending on it.
providerConfig.oauthis removed — its schema, TOML round-trip, theProviderManagerauth resolver it fed, and the mutual-exclusion rule that onlyexisted to keep it apart from static keys. Nothing writes an oauth ref now:
OpenAI Codex stores its access token as the provider
apiKeywith the refreshtoken under
source, and every other path is a plain key.Logging in is an API key, a models.dev catalog provider, or OpenAI Codex
OAuth. "Is the user logged in" is one predicate —
PythinkerHarness.isAuthenticated(), over configured providers with a usablecredential — shared by the CLI, the VS Code extension and the ACP adapter.
/feedbackopens the issue tracker.Surfaces that had options with no flow behind them are cleaned up: the
platform picker no longer offers an entry whose handler was deleted, the VS Code
welcome screen no longer renders a device-code URL panel that can never fill,
and the web client drops the device-code dialog and its
/oauthcalls —logging in there opens the provider manager, which is where API-key and catalog
providers are added.
VS Code sign-out is now non-destructive. It removes only providers a login
created (those carrying a
source) and writes once throughreplaceConfig, sohand-written
config.tomlentries survive and a failure cannot leave ahalf-signed-out config behind.
Verification
pnpm run typecheckpnpm run lintpnpm --filter ./apps/vscode run typecheckpnpm vitest run(all projects)Two regressions added, both mutation-verified (they fail when the code under
test is broken):
scope: 'oauth'refresh path, which the deletedmanaged-scope tests used to cover;
and Codex satisfies it only because it stores its access token as
apiKey.Nothing said so, so a future change to that storage would have sent every
Codex user to
AuthTokenMissingErroron their first turn.Note for reviewers
packages/migration-legacydeliberately keeps itsmanaged:kimi-codefixtures:it reads legacy configs that really did carry that provider.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.