Conversation
chore: sync latest dev into account actions fix
Sync upstream dev through 57077ca.
chore: sync latest dev into account actions fix
chore: validate current dev with account actions fix
…he tool-catalog nudge (lidge-jun#4415) * refactor(devin): retire the ACP adapter and give the shared adapter the tool-catalog nudge Both Devin provider rows already stream Cognition's Connect-RPC api-server on the `devin` adapter. They differ only in where the credential came from: a browser sign-in through RegisterUser, or the `devin-session-token` the installed CLI already wrote to its own credentials.toml. A second adapter registered under the id `devin-cli` still spawned `devin acp` and drove the child over Agent Client Protocol on stdio. Nothing routed to it: `routedProviderConfig` pins the adapter from the registry for any registry id, so only a custom-named row such as `"devin-acp"` could select it. It is removed rather than kept, because the premise that justified it was false. The CLI's credential is the ordinary cloud token, so importing it does everything the child did without a placeholder `buildRequest`, a disabled `parseStream`, an identity-only `baseUrl` no request may connect to, and a subprocess running in the operator's own tree. `projectDevinCliAuthMode` used to warn and change nothing when a saved row still named that adapter, reasoning that routing already pinned the transport. That held only for the registry id. With the adapter gone a custom-named row has nothing pinning it and would throw `Unknown adapter: devin-cli` on every request, so the migration now rewrites every row naming the retired id whatever the row is called, and repoints a row still carrying the identity-only `cli.devin.ai` host at the api-server in the same pass. The nudge is the other half. Every non-OpenAI adapter that advertises a client tool catalog injects `buildNonOpenAIToolCatalogNudgeForTools` into its system prompt; Devin advertises a real catalog on proto field #10 and was the only one without the paragraph. It goes into `mapOcxMessagesToDevin`, which covers both provider rows at once. The wire-name callback is `tool => tool.name` rather than the default namespaced form, because `mapOcxToolsToDevin` writes the bare name — a nudge listing names the model is never offered is worse than none. The ACP wire could never have carried it: `session/prompt` takes prompt text only, with `capabilities: {}` and `mcpServers: []`. * fix(devin): share the api-server default and drop the out-of-scope GUI comment The migration wrote its own copy of the Cognition host, so a later change to the shared default would have left it writing the old address. It now imports DEVIN_DEFAULT_API_SERVER. The GUI comment rewrite went back to its dev state. It was unrelated cleanup by the PR checklist's own standard, and because the screenshot gate is path-based it was asking a comment-only diff to produce a screenshot of nothing. It can go in on its own.
…ion (lidge-jun#4411) * fix(codex): refresh the catalog when paginated history refuses injection An explicit `ocx sync` is also the refresh path for side profiles that consume the OpenCodex catalog without injection. Since paginated Codex rollouts began refusing external writes, that refusal was reaching `syncModelsToCodex` through the validate-only preflight and failing the whole sync, so the model catalog went stale on a home whose history simply requires its native writer. The preflight refusal now carries a structured `historyPreflightFailureReason` instead of only display text. When an explicit sync sees `history_paginated_requires_native_writer`, it keeps the injector's refusal intact, publishes through the existing catalog owner, and returns `catalog-only` with config and conversation files untouched. Unattended sync and every other config or integrity refusal keep their hard failure. `refreshOutcome` distinguishes a validated catalog commit from a refused refresh, so `refreshCodexModelCatalog` no longer rewrites the models cache after a refusal and `ocx sync` exits non-zero when a catalog-only refresh did not complete. * docs(devlog): record the 2.43/2.46 releases and the open closeout units Carries the planning and delivery records that were still sitting untracked in the working tree: the 2.43 and 2.46 release units, the per-work-phase execution and delivery records for the 260905 open-work closeout, the provider runtime stack unit, the 249 bulk closeout, and the beginner PDF plan. The 006 dispositions and 060 ledger updates record two maintainer decisions made during that campaign: `bun run test:changed` was removed from the local verifier set because its import-graph selection reaches most of the suite on the touched layers, and from wp4 onward the campaign accepted the final `dev` tip CI run as batch evidence instead of per-PR exact-head runs. * test(codex): cover the catalog-only path for a paginated-history refusal Three cases the change has to keep apart: an explicit sync refreshes the catalog and reports `catalog-only` when the injector refuses on `history_paginated_requires_native_writer`; a refused refresh under the same condition stays unsuccessful and writes no cache; and an unattended sync keeps the hard failure it always had. * fix(devlog): drop a maintainer email address from the wp4 reverify record `privacy:scan` flags any address outside its allowlist, and the carried closeout record quoted the `-c user.email` value used to re-author two layers. The address was never the point of the note, so it is replaced with a description of the flag pair. This is what failed `test 3/4`, `gates`, and `macos 2/2` on the previous head. * docs(structure): record the catalog-only exception and declare refreshOutcome Two residuals a delegated review found on this PR. `structure/config.md` owns the sync preflight contract and still said every deterministic refusal leaves the catalog and cache untouched, which this change makes untrue for one reason code. And `refreshOutcome` reached callers through a spread without ever being declared on `CodexSyncResult`.
Both work phases landed on dev (lidge-jun#4411 as 2d3c05f, lidge-jun#4415 as 213065e), so the unit moves to _fin with an outcome record. It records what the removal actually corrected: the ACP adapter was written on the untested assumption that OpenCodex could not hold the installed CLI's credential, then kept on a second assumption that leaving it registered was harmless because routing pinned the registry id away from it. That was true for the registry id and false for the custom-named row the documentation itself recommended, which is the row that would have broken on removal. It also corrects two beliefs this unit started with: devin-cli has not run over ACP since the account-login unit, and its token usage is reported, not missing. What is genuinely absent is account quota, which Cognition exposes only through Enterprise-scoped endpoints.
…nd cache defects (lidge-jun#4418) * docs(devlog): plan Devin hardening and the cached-token companion Locks the roadmap for the devin-cli token transition, the cloud-direct transport and usage decode, and the cached-token display companion, before any implementation lands. * docs(devlog): fold the roadmap audit findings into the Devin hardening plan An independent review of the first draft failed it on three counts. Field 7 needs its own uint64 decoder and must suppress field 28 within a message, because both arrive together and the adapter last-writes usage. The input-versus-cache mapping is derived from the frame rather than assuming Cognition is exclusive, since guessing wrong inflates input and bills cache at the uncached rate. And missing_ui_screenshot is path-based on gui/src, so the display phase carries a real screenshot instead of avoiding the word. * fix(devin-cli): harden the imported CLI session against path, read, and cache defects Five defects on the credential-import path. An empty APPDATA or XDG_DATA_HOME resolved to a cwd-relative credentials path, because `??` treats an empty string as a set value; a file planted beside the proxy would have imported as the operator's own CLI session. Every read failure collapsed into `undefined`, so a permission error on an existing file was reported as 'not signed in' and sent the operator to `devin auth login`, which does not fix it. The outcome is now missing, unreadable, incomplete or ok, each with its own message, and the parse is bounded at 64 KiB. A bare JWT in Metadata.api_key went out without the `devin-session-token$` prefix and came back as an opaque permission_denied, which reads as a revoked account rather than a malformed credential. Only a three-segment JWT is reshaped, so a Codeium UUID, an sk-ws key and a cog_ key pass through untouched. Logout cleared the shared user-JWT and catalog cache only for provider `devin`, and account deletion never cleared it, so a CLI-imported key's JWT outlived its own logout. Both ids now clear on both paths. redactSecretString recognised neither a Devin session token nor a bare JWT, and a Connect trailer can quote the request that carried one.
…un#4421) A cached request's total is mostly cache. A 58,000-token prompt that is 57,000 cache read and 1,000 fresh was printed as a bare 58,000 on every surface except the logs table row, so it read as a different, smaller request than the row directly beside it. formatTokensWithCache renders the total with its cached companion — 5.8만 c5.7만, 58K c57K — reusing the marker the logs.tokens.cacheRead label already documents as "cache read (c)", so no new i18n key is needed. A provider that reports no cache is untouched, and a turn served entirely from cache still shows the marker, since that is the row worth seeing. No backend change: /api/logs forwards the whole usage object and /api/usage already emits cache on summary, models and providers. The loss was purely client-side, in the row types and the aggregators, so the fix widens UsageModel, UsageProvider, UsageSummary30d, the CLI CostRow, and summarizeFilteredLogs to keep the fields that were already arriving. The log detail panel is deliberately left alone: it already has separate cache read and cache write cells, and stacking the companion onto its total would duplicate them.
…es (lidge-jun#4419) * fix(devin): read usage from ModelUsageStats and classify cloud failures A cached Devin turn reported a bare token total with no cached subset, so its log row looked like a smaller request than it was. The decoder was reading GetChatMessageResponse field 28. Field 28 is response_dimension_groups, the rows the IDE renders; field 7 is ModelUsageStats, the per-turn accounting. The old path worked by accident: ResponseDimension.uid is that message's field 5, which the entry walker treats as a metric id, so cache numbers appeared only when the service happened to render cache rows. Field 7 carries cache read and cache write unconditionally. Both fields arrive in the same message and the adapter keeps the last usage event, so decoding both is not enough: field 7 now suppresses field 28 within a message and is yielded last, and it needs its own uint64 decoder because the field-28 walker reads a fixed32 float out of a sub-message. Whether Cognition's input_tokens already includes cache is unsettled, and guessing inclusive is the expensive error: normalizeCostTokens only rejects read + write > input, so an inflated input passes validation and bills cached tokens at the uncached rate. The mapping is therefore derived from the frame. Both branches agree on the 58k-prompt case that prompted this. Two further classification defects. CloudChatError carried no HTTP status, so inferHttpStatusFromAdapterMessage turned an upstream 429 into a 502 and core's failover never rotated or backed off. And a cancelled turn said "Devin turn was aborted.", which isClientClosedMessage does not recognise, so a client hanging up was logged as an upstream failure; it now emits the phrase the classifier knows, with status 499. Usage frames are merged per field instead of replaced, because the counters are cumulative and a later partial frame used to zero an earlier count. * fix(devin): report field-7 usage ahead of finish and derive the merged total Review follow-ups on the usage decode. The authoritative ModelUsageStats event was yielded after the rest of the frame, so a frame that also carried finish reported usage behind the turn's end. It is now yielded first, which makes the order independent of where the service places the field. mergeDevinUsage took the max of two totals alongside the per-field maxima, which can leave totalTokens different from input + output; the cost and log paths read that total. The total is now derived from the merged counts. Regression coverage for what the change is actually for: field 7 suppressing the display rows within one frame and landing before finish, the display rows still decoding when no field 7 is present, a partial frame not zeroing an earlier count, and an HTTP status becoming a structured classification. A Connect trailer still carries no HTTP status, so a cap delivered that way keeps the older message-inference path. That is noted at the throw site as a follow-up rather than silently left open.
…idge-jun#4423) * fix(devin): map Connect trailer codes onto the status core acts on Only the HTTP status line carried a status, so a cap or an expired credential delivered as a Connect EOS trailer fell through to inferHttpStatusFromAdapterMessage and became a generic 502 — not an auth prompt, not a backoff, and nothing core's failover acts on. connectTrailerHttpStatus maps the Connect codes Cognition actually sends, and treats permission_denied carrying "your limit will reset" or "reached overall message rate limit" as the quota refusal it is rather than an authorization failure. It reads the raw trailer message, not the enriched text, so the tool-blocklist wrapper cannot trip the quota regex. An unrecognised code returns undefined and keeps the older inference path. Review follow-up in the same change: unimplemented maps to 501, and the blanket "5xx is retryable" rule was putting retryable: true on the SSE failure a client reads for a call the service will never implement. * docs(devlog): record the trailer-status mapping as closed 020 listed the Connect trailer mapping as the deferred half of the cloud-direct work. It landed, along with the 501 retryability fix the review caught, so the doc records the outcome and the two accepted residuals.
Sync latest lidge-jun/opencodex dev into PR lidge-jun#4062 feature branch.
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
2 participants
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.
Temporary same-fork validation PR for exact-head CI after syncing upstream dev. No merge intended.