Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions devlog/_plan/260912_devin_cli_account_login/000_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Devin CLI as an account provider

**Unit:** 260912_devin_cli_account_login
**Class:** C3 (public provider contract + a documented invariant + GUI surface)
**Goal (host):** register devin-cli as an account provider so it appears in the
dashboard accounts tab beside devin, by giving it a login entry that drives the
installed Devin CLI's own auth flow, without opencodex holding a usable Devin
bearer token.

## Why this unit exists

The dashboard's add-provider dialog has three tabs. Two of them (Free, Paid) are
rendered from the preset catalog; the Accounts tab is not. In
`gui/src/components/provider-catalog/ProviderCatalog.tsx` the preset rows are
drawn only when `tier !== "accounts"`, and the Accounts tab instead renders
`accountRows`, which is built from providers that have a login flow. The
`buckets.accounts` bucket that `bucketPresets` computes is never rendered at
all.

That is why `devin-cli` is reachable only under Free today: `authKind: "local"`
makes `isFreeProvider` true (`gui/src/provider-workspace/catalog.ts`), the same
branch that holds Ollama, vLLM and LM Studio. Reclassifying the tier alone would
remove it from Free and put it in a bucket nothing draws, so it would vanish
from the dialog entirely. The only way into the Accounts tab is to become a
provider with a login.

## The constraint this unit has to move

`src/providers/registry.ts` and `tests/providers/devin-cli-adapter.test.ts`
currently pin the opposite posture:

> The installed CLI carries its own credentials from `devin auth login`, so this
> provider takes no key and the proxy never sees a token for this provider.

That statement is about the **request path**, and it stays true: the adapter
spawns `devin acp` and the child authenticates itself. What changes is the
**dashboard path**, which gains a login entry whose job is to run the CLI's own
auth flow and read back who is signed in. The distinction the unit must keep
explicit, in code comments and in the tests, is:

- the adapter still never reads, requests, or forwards a credential at request time;
- the OAuth entry stores an identity marker, never a usable Devin bearer token.

If those two cannot both hold, the unit stops and reports rather than inventing a
token to satisfy the framework.

## Constraints

- No repository-wide local suite, typecheck, or build. Focused tests only; hosted
CI on the exact PR head is the gate. Push with `--no-verify`.
- The Devin CLI is **not installed** on the development machine and must not be
installed as part of this unit without a separate instruction. Every code path
that depends on the binary needs a documented degraded behaviour and a test
that exercises it through an injected spawn, the way
`tests/providers/devin-cli-adapter.test.ts` already drives the adapter.
- `src/lab/` must stay off the core path; nothing here touches `src/router.ts`,
`src/server/lifecycle.ts`, or `src/server/responses/core.ts`.
- Target branch is `dev`.

## Work-phase map

Dependency-ordered; each is one full PABCD cycle.

| Phase | Doc | Outcome |
|---|---|---|
| wp1 | this unit | Roadmap locked, every later phase written to diff level |
| wp2 | `010_phase1_cli_login.md` | `src/oauth/devin-cli.ts`: signed-in detection, login that drives the CLI, identity-only credential |
| wp3 | `020_phase2_reclassify.md` | Registry + OAUTH_PROVIDERS registration, invariant text, tests that pinned `local` |
| wp4 | `030_phase3_surface_and_land.md` | Accounts-tab proof against the running service, docs/locale, PR, merge |

## Open risks carried into wp2

1. **CLI absent.** Signed-in detection cannot be proven end to end on this
machine. wp2 must therefore make the binary lookup injectable and prove both
branches (found / not found) with the existing `resolveDevinCliBinary`
override seam, and wp4 must state plainly that the live signed-in path is
unproven here.
2. **No documented status subcommand.** If the CLI exposes no non-interactive way
to report the signed-in account, the login entry can only report "the CLI
reports it is signed in" without an identity. That is still enough for an
accounts row, but it changes the credential shape, so wp2 decides this against
the subagent finding recorded in `001_cli_auth_survey.md` and amends
`010_` before building.
3. **Refresh.** The OAuth framework expects a refresh path. `src/oauth/devin.ts`
throws `invalid_grant` because Cognition mints no refresh token; the CLI entry
has the same shape and should reuse that posture rather than extending an
expiry it cannot honour.

Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# 001 — What the surfaces actually require

Findings from three parallel read-only investigations (subagents Carson, Gibbs,
Rawls), recorded here so each later phase starts from evidence rather than from
the transcript.

## The Accounts tab is fed by OAUTH_PROVIDERS, not by the preset catalog

`GET /api/oauth/providers` returns `listOAuthProviders()`, which is
`Object.keys(OAUTH_PROVIDERS)` minus `chatgpt`
(`src/server/management/oauth-account-routes.ts:137-140`,
`src/oauth/index.ts:335-371`). The GUI turns that list into the Accounts rows in
`gui/src/pages/providers-page-utils.ts:8-25`. A provider does **not** need to be
in `config.json` to appear. So membership in `OAUTH_PROVIDERS` is the whole
admission rule.

## What an OAuth entry must provide

`OAuthProviderDef` (`src/oauth/index.ts:184-196`) requires `login`, `refresh`,
`providerConfig`, `defaultModel`. `providerConfig` is not hand-written: `oauthConfig(id)`
calls `deriveOAuthProviderConfig`, which finds the registry row **only when
`authKind === "oauth"`** and throws otherwise (`src/providers/derive.ts:350-353`).
That is why the registry reclassification and the OAuth registration are one
atomic change, not two independent edits.

`OAuthCredentials` requires `access: string`, `refresh: string`, `expires: number`;
`normalizeCredential` drops the whole credential if any of the three is missing or
mistyped (`src/oauth/store.ts:447-502`).

## The durable-key precedent already exists

`devin` faces the same "no refresh endpoint" problem and solves it without
inventing one: it stores the durable key as both `access` and `refresh`, sets
`expires: Number.MAX_SAFE_INTEGER`, declares `defaultRefreshPolicy: "disabled"`,
and its `refresh` throws `invalid_grant` so a forced refresh marks the account
`needsReauth` instead of pretending success (`src/oauth/devin.ts:50-72, 155-166`,
`src/oauth/index.ts:310-315`). `orcarouter-oauth` does the same. An empty
`refresh: ""` is explicitly the wrong shape — it makes `detectOAuthWarning` report
`stale_credentials` from the moment of login.

This unit reuses that shape, with one difference that has to stay visible: for
`devin` the stored string is a real API key; for `devin-cli` it is a non-secret
presence marker, because there is no token for opencodex to hold.

## The fail-closed check that makes this a migration

`src/server/auth-cors.ts:731-737` rejects a saved provider row whose
`authMode === "local"` when its registry entry is not local:

> `provider ${name} cannot use authMode "local" — its registry entry requires ${entry.authKind} auth`

`derive.ts:217-231` seeds `authMode` from `authKind`, so every config saved while
`devin-cli` was local carries `authMode: "local"`. Flipping the registry to
`oauth` without a migration turns those configs into a startup rejection. This is
the single highest-risk item in the unit and `020` owns it.

## Everything else `"local"` currently controls for this provider

From `gui/src/provider-workspace/`: `catalog.ts:137-143` treats local as
configuration-ready; `catalog.ts:170-174` puts it in the Free tier;
`auth.ts:21-22` returns `null` so no auth surface is drawn; `kind.ts:12-21`
classifies it as kind `local` for the rail filter. Under `oauth` all four change
behaviour, which is the intent — an OAuth row gets an auth surface and a login
button — but `030` has to look at the rail, not only the modal.

From `src/providers/`: `fastwire.ts:109` returns `"none"` for local, so no
Authorization header is attached. This matters: the `devin-cli` adapter never
travels the fetch path at all (`buildRequest` is a placeholder), so the header
policy is inert for it either way. `quota.ts:2899` and `key-failover.ts` skip
local rows; under `oauth` they take the OAuth branches, which is correct because
there is now an account to reason about.

## What the Devin CLI itself stores

The CLI keeps its own credential on disk as `credentials.toml`. opencodex never
reads it; the adapter only spawns `devin acp` and the child authenticates itself
(`src/adapters/devin-cli/adapter.ts:1-8`). The CLI is **not installed** on this
machine, so the exact path and any non-interactive status subcommand are
unconfirmed. `010` therefore treats both the path and the status probe as
injected dependencies with a proven not-found branch, and `030` states plainly
that the live signed-in path is unproven here.

101 changes: 101 additions & 0 deletions devlog/_plan/260912_devin_cli_account_login/002_audit_resolution.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# 002 — Audit resolution: the reclassification is the wrong mechanism

Independent adversarial audit of `000`/`010`/`020`/`030` returned **VERDICT: FAIL**
with four blockers. Three are fixable in place. The first invalidates the central
decision, and the roadmap changes rather than arguing with it.

## Blocker 1 (fatal to the original design)

Flipping `authKind` to `oauth` couples the REQUEST path to a credential that
carries no meaning. `src/router.ts:317-318` forces `authMode` from the registry
for oauth entries, and `src/server/responses/core.ts:4323` then always calls
`getValidAccessTokenSnapshot`, which throws `OAuthLoginRequiredError` when no
account set exists (`src/oauth/index.ts:576-578`) and stamps
`apiKey: resolved.accessToken` at `:4401`.

Today a configured `devin-cli` row answers with no opencodex credential at all,
because the child authenticates itself. Under the original plan every turn would
401 until someone clicked Login, and a dashboard logout would break inference
while the CLI stayed signed in. `020`'s boundary forbids touching
`responses/core.ts`, so the plan could not have special-cased its way out.

The audit also killed a claim in `000`: reclassifying does NOT make the row vanish.
`providerTier` only puts the canonical OpenAI forward provider in `accounts`
(`gui/src/provider-workspace/catalog.ts:160-181`), so an oauth preset with a
non-loopback base URL lands in **Paid**, which is rendered. The original
motivation sentence was wrong about the failure mode while being right that the
Accounts tab is unreachable from the preset catalog.

## The corrected mechanism

Accounts-tab admission is `OAUTH_PROVIDERS` membership — `listOAuthProviders()`
is `Object.keys(OAUTH_PROVIDERS)` minus `chatgpt`
(`src/oauth/index.ts:369-371`, `src/server/management/oauth-account-routes.ts:139-140`).
Nothing in that path reads `authKind`.

`authKind: "oauth"` was only needed because `oauthConfig(id)` derives
`providerConfig` through `deriveOAuthProviderConfig`, which filters on it
(`src/providers/derive.ts:350-353`). But `providerConfig` is an ordinary
`OcxProviderConfig` field — it can be built from the registry row directly.

**So: register `devin-cli` in `OAUTH_PROVIDERS` and leave `authKind: "local"`
alone.** The Accounts row appears; the request path keeps seeing a local
provider, demands no token, and behaves exactly as it does today. The
`auth-cors` migration in `020` and its whole new migration module become
unnecessary, because no persisted `authMode` ever mismatches.

This also resolves the honesty problem that made the original design
uncomfortable: opencodex no longer needs a marker to stand in for a bearer
token on the request path, because the request path never asks. The stored
credential exists only so the Accounts row has a state to show.

The residual risk moves to `isOAuthProvider("devin-cli")` becoming true, which
switches on `ocx login` (`src/oauth/login-cli.ts:86-88`), changes `ocx account`
(`src/cli/account-api.ts:83-93`), and admits the row to generic 429 failover
(`src/oauth/generic-account-failover.ts:97-98`). wp2 must prove each of those
three is either intended or inert for a stdio adapter, and `openUrl("")` in the
CLI login path must not be reached.

## Blocker 2 — preset duplication

`dashboardPreset: true` keeps the row in `deriveProviderPresets`
(`src/providers/derive.ts:365`), so it would show on a preset tab as well as
Accounts. Set `dashboardPreset: false`, matching `devin` and `cursor`, and update
the assertion at `tests/providers/devin-cli-adapter.test.ts:33` that currently
pins it true. With `authKind` staying local the preset tab would otherwise be
Free, not Paid, but the duplication is the same defect either way.

## Blocker 3 — login cannot inherit stdio

`010` said to run `devin auth login` with inherited stdio. Dashboard login is
`POST /api/oauth/login` inside the proxy, typically a launchd process with no
TTY. Use kiro's working shape instead: piped spawn with `stdin: "ignore"`
(`src/oauth/kiro.ts:151-156`), surface the CLI's own output through
`ctrl.onProgress`, and treat a login that cannot complete without a terminal as
a reported failure rather than a hang. If the CLI turns out to require a TTY, the
honest end state is an Accounts row that reports signed-in status and tells the
operator to run `devin auth login` in their own terminal — wp2 decides this
against the real binary and records which it was.

## Blocker 4 — wrong label file

Accounts rows use `oauthLabel` → `OAUTH_LABELS[id] ?? id`
(`gui/src/pages/providers-shared.ts:49-59`), not `formatProviderDisplayName`.
Without an `OAUTH_LABELS` entry the row reads `devin-cli`. `030`'s write set
moves from `gui/src/provider-icons.ts` to `gui/src/pages/providers-shared.ts`.

## Structure obligation the plan missed

`structure/AGENTS.md:49` binds changes in `src/oauth/` and `src/providers/` to
`runtime.md`, `subagents.md`, `transports/inventory.md`, and
`providers/xai-grok.md`, not only `adapters/registry.md`. wp4 checks each for a
sentence this change falsifies.

## Effect on the work-phase map

wp2 and wp3 swap emphasis: wp2 still builds `src/oauth/devin-cli.ts` (now with a
piped spawn and no marker-as-bearer concern), wp3 becomes registration plus the
`dashboardPreset` flip and the three `isOAuthProvider` consequences, with the
`authKind` flip and its migration DELETED. wp4 is unchanged apart from the label
file and the structure docs.

Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# 003 — Blocker 1 resolved: import-first login, oauth classification kept

`002` proposed dodging the request-path coupling by leaving `authKind: "local"`
and registering in `OAUTH_PROVIDERS` anyway. The operator rejected the premise:
devin-cli is not a local runtime. It is a CLI that requires a vendor account —
demonstrated by installing it and signing in, after which
`devin auth status` reports `Logged in (via Devin)` with its credential at
`~/.local/share/devin/credentials.toml`. Ollama, vLLM and LM Studio have no
account at all; grouping devin-cli with them was a taxonomy error.

So the classification is `oauth`, and blocker 1 has to be solved rather than
avoided.

## The resolution

Blocker 1 said: with `authKind: "oauth"`, `src/router.ts:317-318` forces
`authMode`, `src/server/responses/core.ts:4323` calls
`getValidAccessTokenSnapshot`, and that throws `OAuthLoginRequiredError` when no
account set exists.

That is only a defect while no credential is stored. Once the login entry has
run, a credential exists, the snapshot resolves, `apiKey` is stamped onto a
provider config the adapter never reads, and the turn proceeds exactly as it does
today. Requiring one sign-in before an account provider answers is not a
regression — it is what an account provider means, and it is what the operator
asked for.

**No change to `core.ts` or `router.ts` is needed.** The boundary in `000` holds.

> **Superseded in part by `020`.** Audit round 2 disproved the startup-import half
> of this section: `projectStartupConfigRepairs` is a synchronous projector
> persisted through `mutatePersistedConfig`, which writes `config.json` only,
> while `getValidAccessTokenSnapshot` reads the auth store. A boot pass there
> cannot mint a credential, so existing installs DO need one sign-in after
> upgrade. `020` carries the corrected, honest version. The login-time
> import-first design below stands unchanged; only the boot-import claim is dead.

## What does have to be built: import-first, so nobody is broken mid-flight

A user who has `devin-cli` configured today and is signed into the CLI must not
wake up to 401s. Kiro already solves this shape (`src/oauth/kiro.ts:335-429`):
login imports an existing CLI session rather than starting a browser flow.

wp2 therefore builds `loginDevinCli` import-first:

1. `devin auth status` — confirmed present and non-interactive on 3000.10.21,
printing `Logged in (via Devin).` plus the credential path. This is the probe;
the subcommand is no longer a guess (`001` recorded it as unconfirmed).
2. already signed in -> return the marker credential immediately, no browser.
3. signed out -> run `devin auth login` with kiro's piped-spawn shape
(blocker 3), surfacing the CLI's own `Visit <url> ... paste the code` prompt
through `ctrl.onAuth`/`onManualCodeInput` — that flow is confirmed: the CLI
prints a PKCE URL and accepts a pasted one-time code, which is exactly the
shape `onManualCodeInput` exists for.

And wp3 adds a startup import for existing installs: when `devin-cli` is
configured, has no stored credential, and `devin auth status` says signed in,
store the marker so the first turn after upgrade succeeds without a click. Same
repair pass as the other two migrations
(`src/providers/model-rename-startup.ts`).

## Blocker 2, 3, 4 — unchanged from `002`

`dashboardPreset: false`; piped spawn; `OAUTH_LABELS` in
`gui/src/pages/providers-shared.ts` is the Accounts label, not
`provider-icons.ts`. The `로컬` badge disappears on its own once `auth` stops
being `"local"` (`ProviderCatalog.tsx` badge ladder), which is the mark the
operator asked to have removed.

## Corrections to earlier docs from the live install

- credential path is `$XDG_DATA_HOME/devin/credentials.toml`
(`~/.local/share/devin/...`), not `~/.config`. `010`'s path resolver changes.
- `devin auth status` exists and is non-interactive. `010`'s "unconfirmed
subcommand" hedge is replaced by a real probe, and its test case 4 becomes a
regression guard rather than a guess.
- A separate live defect was found and already landed on `dev` outside this unit:
the adapter passed `DEVIN_PERMISSION_MODE=ask`, which the CLI rejects with exit
2, so every default-configuration turn failed (PR #4332, `e7f7487b3d`). This
unit assumes that fix is present.

Loading
Loading