[WRONG BRANCH] release: promote verified 2.53.0-preview.20260913 product tree to preview - #4504
Merged
Merged
Conversation
Carry #4319 with native raw-ladder distinction and regression coverage. Co-authored-by: Keito Itagaki <171206780+ke-1t@users.noreply.github.com>
Document reasoningEffortMode in all seven translated guides. Keep the original Chat-wire regression; Responses summary is not a Chat request field.
…he tool-catalog nudge (#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 (#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 (#4411 as 2d3c05f, #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.
…ility (#4410) Meta Muse rejects function names over 64 characters. On api.meta.ai only, rewrite long or charset-unsafe tool identities to collision-safe wire names and restore the originals inbound before namespace restore and the undeclared-tool guard.
…nd cache defects (#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.
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.
Match kiro-style sanitize fallbacks, rebuild the shared 55-char prefix pair, and drive handleResponses restore through a key-auth fixture pointed at api.meta.ai so registry oauth no longer 401s the inbound cases.
…es (#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.
…ents (#4410) The undeclared-tool guard reads name straight off response.function_call_arguments.done, outside any function_call item, so a hashed Meta Muse alias reached the client and could fail the turn as an undeclared tool. Restore now matches that event and its delta alongside the item shapes.
…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.
…n test (#4410) Meta Muse aliases names with unsupported characters as well as overlength names. Record additional_tools among rewrite locations, poll the passthrough continuation cache instead of a fixed sleep, and assert replayed history on the second outbound body.
…undary (#4410) Upstream sees every aliased declaration even when tool_choice narrows what it may call, so a wire name in that catalog is not evidence that restoring it into an executable client name is permitted. Narrow the restorable map the same way authorizedAliases already does for the namespace layer.
Reopens the device-flow phase closed as NOOP in 260903_muse_spark_plan_oauth/020. That close listed exactly what a reopen would need (endpoints, client id, payload shapes, identity semantics, error taxonomy, file and registry names, ToS seam); a working second-party implementation now supplies all of it, and the HIGH_RISK GUI entry already exists. The vendor restriction has not changed, so the unit ships under the same explicit-owner-decision posture as anthropic and google-antigravity. Seven documents: 000 plan and work-phase map, 001 measured endpoint and payload reference with our current gaps at path:line, 002 design decisions (namespaced credential field instead of an overloaded bearer, staticHeaders instead of a transport hook, capability-gated quota probe, why PKCE does not apply), and diff-level phase docs 010-040 for the device core, login integration, version header plus on-demand quota, and the test matrix and gate. No source changes in this commit.
Six findings, none rebutted. Four came from reading the pinned test contracts and two from independent grok-4.6 reviewers; the plan as first written would not have compiled and would have broken four existing tests. - The muse credential field moves from wp3 to wp2: the module that returns it cannot compile without it (TS2353 excess-property check). - The device call now forwards fetchImpl/sleep/now, so no test reaching that path can call auth.meta.com for real. - A Keychain read that times out stays a throw. Falling through would have broken meta-muse-oauth.test.ts:159-166 and, worse, started a browser grant to solve a permissions dialog. - A host with no paste surface now gets the device reason composed WITH the existing dev.meta.ai and META_MODEL_API_KEY guidance rather than instead of it. - The credential field's outbound protection is corrected: hand-built allowlists, not a kiro-specific redactor, so the prohibition is written into the docstring. - The quota probe gains a success TTL enforced even against a forced refresh, because ?refresh=1 and the reset poller both bypass the quota cache.
The wp2 cell of the work-phase map now names src/oauth/types.ts alongside the device module, matching where fold 4 put the credential field. Test row 41d now asserts sleep and now are forwarded, not just fetchImpl, and row 55c drops a forceRefresh framing that does not apply: fetchMuseKeyQuotaSnapshot takes no such parameter, so the forced-refresh assertion belongs at the dispatcher instead.
The 260903 close quoted here names tests/oauth-tos-warning.test.ts, but the warning test shipped at tests/gui/oauth-tos-warning.test.ts. The quote stays verbatim and a note names the real path, so a reader following the citation does not hit a dead end. Found by the citation checker written for this cycle's Check phase.
…d row
Security review of the overlay-tolerant seed comparison found one gap the
allowPrivateNetwork deny does not cover. The canonical OpenAI seed defines only
adapter, authMode, baseUrl and codexAccountMode, so matchesCanonicalProviderSeed
ignores every other key on the merge-based write paths. headers is one of them,
and it is not inert: canonical OpenAI has no registry staticHeaders, the PATCH
field mask writes headers with a shallow merge, and the forward adapter applies
provider.headers to the upstream request before the incoming forward headers, so
a persisted value wins whenever the caller omits that header.
Before this commit a dashboard-session PATCH such as
{"headers":{"chatgpt-account-id":"..."}} would persist on the ChatGPT forward row
and ride every subsequent request that did not carry the header itself. POST
still refused it through the strict comparison; PATCH, the provider editor and
reload did not. Denying headers on name === "openai" restores the pre-overlay
behavior on those paths with a clearer message than a seed mismatch.
The regression test was driven red before it was accepted: removing the guard
fails exactly the new case and nothing else in the file.
dev landed #4425 on the old src/service.ts after this stack branched. Carry the LogonTrigger UserId and the matching source-contract tests into src/service/windows-taskxml.ts so a non-elevated install still registers without asking for UAC.
Keep the service facade. The Windows logon-trigger fix from #4425 is already in src/service/windows-taskxml.ts.
Merge the god-file round 1 stack: storage/cleanup, service, and kiro facades.
…erge refactor: land god-file round 1 (cleanup, service, kiro)
…i-overlays Lane S of the contributor carry train, released from its security-review hold. Carries #4447 by Veritas-7: merge-based provider writes (PATCH, the provider editor, reload) no longer fail the canonical-seed comparison because a persisted operator overlay such as selectedModels rides along in the merged candidate. Seed keys stay byte-pinned and POST keeps the strict exact-key comparison. Maintainer security review is recorded in the pull request thread and changed the outcome. The review found that the canonical OpenAI seed defines only four keys, so overlay tolerance reaches nearly every config key, and that headers was a live gap: the PATCH field mask writes it, and the forward adapter applies provider.headers to the upstream ChatGPT request before the incoming forward headers, so a persisted value wins whenever the caller omits that header. c39098b denies headers on canonical openai the way allowPrivateNetwork is denied, with a regression test that was driven red before it was accepted. Cross-platform CI run 34748483096 concluded success on c39098ba3d98d1f2fa4c6b1c4c3f9c0e1e2f0a4b, the exact head merged here. Its first attempt failed in the select-windows-runner job with no failing step, which is a runner-allocation flake rather than a code failure; re-running the failed jobs on the same commit turned the run green, so the evidence remains exact-head. Recorded follow-up, not blocking: the overlay tolerance is a denylist. A future provider field classified editor that touches a trust boundary would become silently reachable on the canonical row, and codexToolMode is the current example. The durable fix is an explicit overlay allowlist plus a guard test.
Since 321b9b1 the upgrade handler pre-opens the sideband upstream and hands it to attachLiveSidebandUpstream already OPEN. attach arms the 10s liveConnectTimer whenever liveMaxSessionMs is set, but the only non-teardown clear site is the upstream "open" listener, which can never fire for a socket that opened before the client existed. Every dictation and live-call session was therefore force-closed with "audio connection timed out" exactly ten seconds after attach. Disarm the watchdog on the successful takeover path, mirroring the open listener, and cover it with a regression test. Found by the 2.53.0 release regression audit (parallel commit audit). Local suite NOT RUN per maintainer rule; hosted exact-head CI is the gate.
Aside (Pi agent core, openai-completions) returns read_file screenshots as
tool-result parts of shape {type:"image", data, mimeType}. The chat
inbound translator only recognized image_url parts and silently dropped
these, so vision models answered that the image data never arrived
(live-reproduced against the running proxy: claude-opus-5 with a Pi-style
tool image reports the image missing, while the same request in image_url
form is described correctly). Claude-shaped clients sending Anthropic
source-object image parts over the same endpoint were dropped the same way.
Normalize both shapes to the data-URI form the Responses pipeline already
handles.
Local suite NOT RUN per maintainer rule; hosted exact-head CI is the gate.
…nnect-timer fix(live): disarm sideband connect watchdog on the pre-opened path
…parts fix(chat): accept Pi and Anthropic-shaped image parts on the chat wire
…view Product tree is dev at eb81eaa. Only package.json differs, carrying the preview channel version 2.53.0-preview.20260913. This promotion follows a 364-commit regression audit (origin/main..981b53e) by 15 parallel subagent lanes plus two audit-spawned fixes reviewed and merged to dev: #4496 (live sideband connect watchdog) and #4497 (chat image part shapes). Zero unresolved P0/P1 at promotion time. Exact-head hosted CI green on eb81eaa (run 34750934849). Local suite NOT RUN per task rule; hosted exact-head CI is the gate.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
|
Important Review skippedToo many files! This PR contains 877 files, which is 577 over the limit of 300. To get a review, reduce the PR to 300 files or fewer by splitting it into smaller PRs or changing its base branch. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (26)
📒 Files selected for processing (877)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
changed the title
Contributor
⏳ DRAFT
What to do
Its title has been prefixed with |
lidge-jun
changed the title
lidge-jun
marked this pull request as ready for review
September 13, 2026 10:30
github-actions
Bot
changed the title
Owner
Author
|
Maintainer integration record: merging this preview promotion as repository owner. Exact-head verification: all required checks green on head b06e54c (ci aggregator, gates, test 1-4, macos 1-2, npm-global, keyring, docker smoke, hygiene, api usage; windows/macos-control skipped by changes-gate design). enforce-target fails as "wrong base (preview); missing UI screenshot" — a path-based false positive on the whole-product-tree promotion, identical to the failure on the 2.52.0 promotions #4406/#4407, which were merged the same way; title and body contain no UI keyword requiring a screenshot. Local suite NOT RUN per task rule; hosted exact-head CI is the gate. Tree invariant verified: the promoted tree differs from audited dev eb81eaa only in the package.json version line (independent reviewer Wegener, near-pass). |
Owner
Author
리뷰 · 우선순위 65 / 80이 PR은 새 기능을 넣는 작업이 아닙니다. 이미 감사 이야기도 본문과 맞습니다. 이 댓글을 쓰는 지금 이 PR은 이미 머지·태그·npm 발행까지 끝난 상태입니다. 머지 커밋은 제목의 라인 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
This was referenced Sep 13, 2026
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
6 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.
Summary
Verification
Checklist