[WRONG BRANCH] promote dev onto preview for v2.35.0-preview - #2825
Merged
Conversation
…blic API
`normalizeXaiResponsesWebSearch` was gated on `isXaiPublicApi`, so it ran for
api.x.ai and skipped cli-chat-proxy.grok.com — the OAuth lane. That gate
contradicted the evidence already in this file: the 2026-08-22 probe recorded in
`normalizeToolGroup` and in `isXaiResponsesDestination` had found the two hosts
to be one dialect. Re-probed 2026-08-27 against the CLI proxy, one field per
request: `web_search_preview` -> 422 `unknown variant`, `external_web_access` ->
400 on every value including `true`, `search_context_size` -> 400, while
`user_location` and `search_content_types` -> 200. Identical to the public API.
Two consequences, both reproduced at the adapter before the fix.
A cached/index-only declaration became a LIVE web search. This normalizer omits
the whole tool when `external_web_access` is not `true`, precisely so dropping
the flag cannot silently widen network access. On the CLI proxy the normalizer
never ran, and the downstream capability strip only DELETES that flag and keeps
the tool — so a caller asking for no network reached xAI with an ordinary
`{type:"web_search"}`. That is the widening this file exists to refuse, on the
lane OAuth users actually take.
A legacy `web_search_preview` reached the proxy verbatim and 422'd the entire
turn, because the type conversion also lives behind the same gate.
Both are fixed by scoping the gate to `isXaiResponsesDestination`, which covers
both hosts, and `isXaiPublicApi` is removed with its last caller.
`responses-routed-web-search-fields` had a case asserting that an unclassified
row leaks the fatal fields at the CLI adapter; the host-scoped normalizer now
strips them regardless of classification, which was already true for api.x.ai,
so that expectation is corrected rather than adjusted. Reverting the gate turns
the new coverage red.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fills in the outcome sections for the four release phases: the preview prerelease, the main promotion, the stable publish, and the deploy proof. Two plan pages had to change on contact rather than merely be annotated. 030 planned to keep main's stale 2.33.0 through the promotion conflict, following #2553 and #2507. CI failed that correctly: tests/release-version-line.test.ts arrived in this very delta and asserts the in-tree version is never behind the highest release tag, and once v2.34.0-preview.20260827 existed, 2.33.0 ordered behind it. The promotion was rebuilt to carry dev's 2.34.0, which makes the promotion merge itself the release commit. 040 therefore lost its only hand step. It is now a pure workflow dispatch against the promotion merge, since release.ts would skip the bump anyway. 050 carries the full record: every sha, every run id, the docs deploy and live-site proof, the unpacked-tarball and installed-binary proof, and the two disclosures - #2745 still unmerged pending security review, and the +11 open CodeQL alerts this train carries onto latest.
docs(devlog): record the v2.34.0 release train outcome
…scan-2762 fix(ci): restore post-release dev gates
fix(cli): neutralize usage report terminal controls
fix(xai): normalize web search on the Grok CLI proxy, not just the public API
fix(integrations): ignore JSON object key order in ownership
…ma-bounds-2763 fix(moonshot): intersect nested schema bounds
…he-options-2765 fix(openai): strip unsupported forward cache options
Phase-0 docs-only cycle for the CLI-operability unit: research the real management API surface (183 reachable routes, 108 mutating), the CLI surface (52 runners, 20 disconnected help sources), the GUI capability map (13 GUI-only classes), and per-issue root causes for #2696-#2705. The finding that shapes the roadmap: nothing binds the CLI surface to the API surface. Help lives in a hand-written banner plus 20 module USAGE constants with zero consumers outside their own files, so it is free to drift, and no test fails when a route lands with no verb. Records diff-level docs for wp2-wp9 and the decisions deliberately deferred.
The A-gate reviewer verified ~40 file:line anchors and found the diagnosis sound but three proposed patches unbuildable and one guard vacuous. Each finding re-verified against source before amending: - wp6 patched a function that does not exist; the real gate is accountLabelForAttribution (summary.ts:687), and widening the Codex label predicate would have made the edit a no-op. - wp6's stamp site sits inside isGenericFailoverProvider, which needs oauth authMode, and the rotation paths need >=2 accounts. A single-account xai user would never have been attributed while every test passed. Stamp moved outside the gate with a named activation scenario. - The recurrence-guard regex matched handleLogin and neither target, so it would have greened over the regression it guards. Now red-first. - Command counts were 49/43/52; they are 58/52/57. - The new-verb sketch had four wrong helper signatures. - accounts[] is blanked under any filter (summary.ts:943), so the natural agent query returned an empty table; it now says why. - auto-switch and reset-credits already have verbs. Also splits wp3's overloaded contract work into wp3b and adds the third parity check without which an under-declared route registry was undetectable.
The audit split wp3's contract work into its own phase but left the stack diagram and phase table describing the old eight-branch chain. Records why wp3b is a dependency-ordered successor rather than an effort bucket, and why wp6 deliberately follows wp4.
The update-recovery case starts a real detached proxy, and its cleanup ran the reap only when `node ocx.mjs stop` exited non-zero. That exit code is a claim, not proof: `stop` also reports success when it finds no live runtime to stop, which is indistinguishable here from one it failed to stop. On that path nothing killed the proxy, and the block went straight on to `rmSync` the fixture tree. The survivor was reparented to init with its package tree, `src` symlink, and config deleted underneath it — unable to serve, unable to exit. One escapee sat at 99% of a core for three and a half hours, 202 minutes of CPU time, listening on nothing. It also held the test machine lock, which is how it surfaced. Cleanup now resolves the pid before anything destroys the record of it, runs `stop` for its graceful path, then verifies liveness and reaps regardless of what `stop` claimed. `rmSync` moves inside a nested finally so it still always runs, but strictly after the reap: deleting the tree out from under a live detached process is what turned a missed kill into a permanently spinning orphan. Reaping the recorded pid is enough because bin/ocx.mjs mirrors its Bun child's exit, so the node launcher follows it down. Reaping alone would still fail silently, because the case passed the whole time it was leaking. An afterAll now audits the pid, so a future regression is red instead of invisible. Verified by A/B: with `stop` stubbed to exit 0 without killing, the old cleanup reports 15 pass / 0 fail and leaves the orphan, the new one reports 15 pass / 0 fail and leaves nothing, and the old cleanup under the new guard turns red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nnot match prose
The PR hygiene gate failed with `empty_catch` on a documentation-only PR. The
scanner is right about what it saw and wrong about what it means: it reads added
lines with a text regex, and two devlog lines quote the very construct they are
arguing should be removed. A prose citation of a defect is indistinguishable from
the defect when the check is textual.
Rewording the citations is the honest fix here. Loosening the regex to ignore
markdown would weaken a real guard for every future PR, and a `suppression-approved`
label would record an exception that does not exist -- there is no empty catch in
this diff to approve.
Verified against the gate's own exported helper rather than by eye: hasEmptyCatch
now returns false for every file in this unit, while still returning true for
`try{}catch{}` and false for a handled catch, so the detector is intact.
WP9MSG && git log --oneline -1
Three defects made unattended agent operation impossible. The provider and models dispatch runners awaited their handler and returned a literal 0, so index.ts called process.exit(0) and erased the exit code the handler had set. 'ocx provider quota' against a stopped proxy printed an error and exited 0 (#2697). responseMessage read only error/message/detail, so the reason and hint the server sets on a refusal never reached the operator: a fenced management plane was indistinguishable from a generic failure (#2698). The account client was worse, collapsing every transport error into a bare sentinel that discarded the cause, and exiting 1 for everything including 404 and 409. The service exports its token file as OPENCODEX_API_AUTH_TOKEN, and nothing refused a management token there. When the two match the server fences the whole management plane closed at boot, so every /api/* returns 503 even on a loopback install that never needed a data-plane secret (#2696). The write path now refuses it, and doctor names an install already in that state. The recurrence guard for the exit-code defect corrected its own plan: the allowlist is debug and login, verified because both exit(1) from inside the handler, not the three commands the plan guessed. login reporting success for a failed OAuth flow is left to wp3b's exit-code contract. Closes #2696 Closes #2697 Closes #2698
Reading process.exitCode only reports the current command's outcome if nothing earlier in the process set it. Four handlers in index.ts write it (handleEnsure, handleTrayProxyStart, handleProxyRestart, handleStop), and while only one command runs per process today, the service runner already resets first for exactly this reason. Mirror it rather than depending on that invariant holding.
Two independent reviewers found the same defect: the fix was half-inert. apiError gained a status parameter and apiJson gained transportError, and no production caller passed or read either, so the 404 to 4 and 409 to 5 mapping never ran and the retained transport cause was never printed. The behavior existed only in this phase's own tests while the commit message claimed it shipped. Status is now threaded through all 19 apiError call sites and the cause through the 14 status === 0 guards; two new tests assert the call sites rather than the helpers, so it cannot go inert again. tray had the identical #2697 defect: windowsTrayCommand reports failure through process.exitCode and returns void, so ocx tray install printed an error and exited 0. The recurrence guard could not see it because the pattern was anchored on 'await handle\w+(', scoping it to a naming convention rather than to the defect class. Broadened to 'await [\w.]+(', which surfaced four more candidates; each was verified in its handler before being allowlisted with its own reason. Also corrects the login allowlist reason (it throws rather than exits 1), passes the env seam through to configuredAdminToken so doctor's check no longer reads real machine state for half its comparison, and extends assertNotAdminToken with the equality arm that catches an operator-set admin token without the ocx_admin_ prefix. The #180 matrix test asserting a 404 exits 1 now expects 4. That issue is a feature request, not an exit-code contract; the test described then-current behavior, and the change is the one 010.3 specified and the PR disclosed.
A review of the #2696 work noted that doctor announced a total management outage at the same level as a directory-permission note. OAuthDoctorCheck gains a FAIL level for a condition that makes the surface unusable rather than degraded, and the credential-collision check uses it. Doctor's exit code remains wp3b's scope.
Four parallel read-only reviews of 020 against the current tree all returned FAIL, on one shared root cause: the doc counts grep line hits and calls them routes. Nine amendments in 021. The load-bearing one is check 3. 020 calls it "the unit's central claim -- if this gate can pass vacuously, nothing else in the unit holds", and specifies it as registryRoutes == literalCount + allowlistCount. That identity cannot balance against the real source: one literal line can register two routes (PUT || PATCH), 19 literal lines decide their method in an enclosing block, and two live routes use a negated guard the prescribed === scan never sees. One module -- routing-analytics-routes.ts -- has one route and zero literals, so a literal-keyed test omits it entirely. For GET /api/storage the scan finds only the dead shadowed copy and never the live one; the gate would have mistaken the corpse for the patient. It now reconciles on distinct (method, path) pairs and fails loudly rather than guessing a method it cannot resolve. The 188-literal figure was measured over a wider file set than the "40+ invisible" claim it justifies: 159 in management scope, and the 29-line difference is exactly the codex-auth and native-profile files counted as invisible. Real non-literal count is 18, now enumerated with mechanism. The lab exemption claimed 20 reads. The family is 21 routes, 7 of them mutating, and a local SQLite read cannot start an automation run -- so accept criterion 2 was violated on day one by routes that already exist. Those seven get a bounded deferred-verb exemption naming wp7 as owner, and the parity test asserts the owner phase still exists, so an exemption cannot outlive its debt. 020.4 aimed the version field at isOpencodexHealthz, which returns boolean and cannot carry a value; the body it validates is discarded one frame up in proxyIdentityAt. It also omitted tests/proxy-liveness.test.ts, where 12 exhaustive toEqual assertions break, and asked for a push into a warnings array that does not exist -- the only candidate funnels into codexRuntime.warning, which would file a stale-PATH warning under the Codex runtime heading. 020.2's premise was that 20 USAGE constants are dead. 20 is the file count; 37 constants exist, 12 are exported, and the 12 exports are one-line aliases whose targets carry 243 live references threaded through rejectArgs into CliUsageError. Deleting them is a contract change to how the CLI reports argument errors, not a deletion sweep. Also: 020 never mentioned the Lab boundary, whose protected set includes src/server/management-api.ts -- the file that would import the new route registry. 021 gives the data-only mechanism and the guard command that proves it. Accept criterion 5 is new: the reconciliation gate must be driven red before it counts as evidence. 020's original gate would have failed on correct code while believing itself rigorous, which is what a gate nobody has attacked looks like.
The A-gate reviewer for 021 died twice on provider rate limits, so the audit was done directly. That substitution is recorded in A10 rather than glossed: an unaudited amendment correcting an unaudited plan is the same failure one level up. Four defects, two blocking. A1 told the scanner to resolve a route's method from the same line, the next two lines, or the enclosing block. All three miss how lab-routes.ts actually works: a preceding sibling guard at :352 (`if (req.method !== "GET") return null`) narrows the eight path literals that follow it. storage-log-guard-routes.ts inverts the nesting again, with the path outer at :112 and the method as an inner early return at :113. A forward-and-outward scanner resolves neither, so the fix for 020's arithmetic was itself unimplementable. Method resolution is now a small ordered walk over guard statements that maintains a narrowing context -- which means admitting the fail-loud requirement cannot be satisfied by a regex. A3 asked the parity test to check that every deferred-verb owner still exists in the goalplan. The goalplan is gitignored and untracked, so that test passes here and cannot find the file in CI -- a vacuous gate introduced by the fix rather than by the original. The exemption now names a tracked devlog doc instead, so the assertion means the same thing in both places. A5 had each module's const USAGE become a table lookup. Those constants are top-level, and ESM answers a top-level cycle with undefined rather than an error, so a future back-import would silently empty the usage text in every rejectArgs call -- a quiet regression in the exact error-reporting surface these issues are about. capabilities.ts is now a leaf data module with a guard test and a non-empty assertion. A6 proposed capability entries for help and --version. Neither reaches the dispatch table: both exit in the CLI head at root.ts:28, so --version has no runner key to parity-check against. And help's absence is not a gap -- tests/cli-registry.test.ts:14-21 excludes it in a comment and a headHandled set as a deliberate decision. A6 read that decision as an oversight. Head-handled surfaces now live in a separate list that feeds the banner and the JSON output without touching runner-key parity. Also verified rather than assumed: agent-settings-routes.ts and oauth-account-routes.ts are correctly absent from A2's non-literal table. Their startsWith/slice hits manipulate payloads, not paths.
…ofile test CI failed in `tests/cli-native-profile.test.ts`: a 409 from `stage/finish` returned 5 where the test expected 1. Both cannot be right, so this is a contract question rather than a broken assertion, and it gets answered rather than silenced. 5 is correct. `apiError` maps 404 to 4 and 409 to 5 so a script can tell a missing account from a concurrent mutation, which is the whole point of #2698; the plan for this unit declares that vocabulary explicitly. The test asserted 1 only because every account-family failure used to exit 1 regardless of status -- it was pinning the defect, not the intent. What the test actually covers, the idempotent cancel fallback and the absence of a spurious cleanup warning, is unchanged and still asserted. The expectation is updated with that reasoning recorded inline, so the next reader sees why 5 is not a regression. The comment citing the old bare-catch defect is reworded for the same reason as the devlog lines: the hygiene scanner matches the construct in prose. WP9MSG && git log --oneline -1
… source Nothing in this repository related the CLI surface to the management API. This adds the declared inventory that later phases register against, plus the gate that keeps it honest. The registry is declared, not harvested, because 18 routes are unreachable by any text scan of their own file: four regexes over model paths, three over lab ids, an endsWith, a pathname.slice, a prefix decode, three path constants, and two negated pathname !== guards. The negated pair matters most. GET /api/routing-analytics has no equality literal anywhere, so a scan built on === never sees it. And for GET /api/storage a === scan finds only the dead shadowed copy in logs-usage-routes.ts, never the live guard in storage-log-guard-routes.ts -- it would have reconciled against the corpse and called the surface covered. The reconciliation counts (method, path) pairs, never grep line hits. Line counting cannot balance against this source: /api/codex-auth/pool-strategy registers PUT and PATCH from one guard spanning four lines, and nineteen path guards decide their method somewhere other than their own line. Both nesting orders occur and neither is inferable from the other -- lab-routes.ts:352 narrows to GET before eight path literals, while storage-log-guard-routes.ts:112 opens with the path and puts the method guard inside at :113. So the scanner walks statements in order maintaining a method narrowing context, and when it cannot resolve a method it returns null and the test fails. It never assumes GET; a scanner that guesses produces a number nobody can trust, which is how the earlier route figures became unreproducible. Resolved surface: 192 pairs from source plus 18 declared non-literal, 108 of them mutating. The gate was driven red before being trusted. Injecting POST /api/combos/red-first-probe into a real handler failed two checks independently and named the route in both. Two further failures found during construction were also real: the generator emitted the negated-guard routes twice once the scanner learned to resolve them, and the inertness assertion flagged this file's own header, which explains why it must not import Lab. That inertness is load-bearing. route-registry.ts is imported by src/server/management-api.ts, which tests/core-lab-boundary.test.ts protects: a user with one provider and no Lab must execute no Lab code. Route paths are strings, so declaring /api/lab/status creates no module edge. The module imports nothing at all, and a test asserts it. Exemptions carry a mandatory reason. The seven mutating /api/lab routes are the interesting case: they were previously going to be waved through as "local-transport" alongside the reads, but ocx lab reads local SQLite and cannot start an automation run or import a community bundle. They now carry deferred-verb with an owner phase and a tracked devlog doc the test verifies exists -- deliberately a repository file and not the goalplan, which is gitignored, so the assertion means the same thing in CI as it does locally.
An agent driving ocx had no way to discover what it could do except by parsing help text, and help text was twenty per-module USAGE constants plus a banner a test explicitly licensed to drift from the command registry. This adds the table that relates each CLI capability to the management routes it drives, and the verb that emits it. ocx capabilities human tree ocx capabilities --json full table with routes and flags ocx capabilities --json --mutating-only ocx capabilities --route /api/usage which commands drive this route capabilities.ts is a leaf module and a test enforces it. Every command module declares its usage text as a top-level const USAGE evaluated at import time, so a cycle back into this table would resolve to undefined under ESM rather than throwing -- silently emptying the usage string rejectArgs hands to CliUsageError, degrading the very error reporting these issues are about. The test asserts no relative import and no dynamic import, plus that every rendered invocation and summary is non-empty, so a cycle fails loudly. Head-handled surfaces stay out of CLI_COMMANDS. --version, -v, and version exit in the CLI head at root.ts before dispatch, so none is a runner key; help is excluded there deliberately, documented in cli-registry.test.ts as a head-handled pseudo-case. Listing either as an ordinary command entry would break the runner-key parity assertion, so they live in HEAD_CAPABILITIES and contribute to the banner and JSON output without touching parity. --route on a route no verb drives exits 4 rather than printing an empty list and returning 0. Reporting success for a request that accomplished nothing is the same dishonesty the transport phase just removed; it does not get reintroduced at the discovery layer. The registry cross-check earned its place immediately: the first version of this table declared ocx status as driving GET /api/status, and the test proved that route does not exist. collectStatus identity-probes /healthz and reads local config, so the capability now correctly declares no route and says so. Flag parsing is order-independent by construction, scanning argv for the flag rather than reading a fixed position -- positional reading is why ocx restore back --json ignores its flag today.
docs(skill): add the repo-owned ocx operating skill
Co-authored-by: Cursor <cursoragent@cursor.com>
…sktop `--json=true` and Unicode dashes still ran doctor and printed prose as success. Desktop status/show threw a generic Error so usage exited 1. Co-authored-by: Cursor <cursoragent@cursor.com>
fix(cli): close the gap-audit findings where the CLI misreported its own outcome
…Kiro again A turn whose replayed history already ENDS with a delivered final answer had nothing to ask upstream, but the adapter asked anyway. `buildKiroPayload` appends a trailing user turn because the protocol requires one, and cf1a572 made that turn a neutral acknowledgement rather than a resume instruction. What it could not remove is the inference itself: a neutral prompt is still a prompt, so the model answered the closed task again and a finished turn kept behaving like a still-open goal. 60537f0 additionally stopped advertising the completion tool for that shape, which narrowed the loop without ending it. The boundary is now that no request happens at all. `ProviderAdapter` gains an optional `localTerminal` hook, consulted in `handleResponsesInner` after adapter resolution and BEFORE the request is built: no build (so no token estimate), no send (so `sendCount` stays 0), and the turn completes from a locally constructed terminal. Placement is load-bearing in three ways, each of which ruled out a simpler spot: `buildRequest` cannot express it — the contract returns an `AdapterRequest`, and events only exist once a `Response` reaches `parseStream`. Faking a `Response` inside `fetchResponse` is worse: it records a physical send. An outputless `done` from the ordinary event path cannot express it either. `guardEmptyCompletionEventStream` treats a terminal with no content event as a failed turn, suppresses it, and re-invokes the identical request; a second empty terminal becomes `empty_completion_retry_failed`. Routing the fix through that path would have re-sent the very request it exists to prevent — which is why the regression runs with `emptyCompletionRetry` both enabled and disabled rather than only in the default configuration. `runTurn` was considered and rejected: it routes every Kiro request into the custom transport branch, which waits on provider pacing and increments `sendCount` before any local decision, still meets the empty-completion guard afterwards, and would force the adapter to re-own transport, retries, failover, cancellation, and accounting it already gets from the standard path. The predicate is the existing `hasTrailingDeliveredFinalAnswer`, so there is one notion of "delivered": it keys off role and `phase`, which is why a user message merely QUOTING the acknowledgement is unaffected, and it returns false at the first non-assistant message, so a genuine follow-up still reaches Kiro. The adapter-owned bounded retry is untouched because it builds with a forced `text_fallback` mode and never passes through this hook. `localTerminalReason` is recorded on the request log so a zero-send request is explainable rather than looking like a lost one. It is a fixed identifier, never conversation-derived. Scope: this fixes the repeated inference only. The user-visible duplicate answer — ordinary text released as `commentary` and the completion answer as `final_answer` in the SAME inference, split into two messages by the bridge — is a separate mechanism and is planned as wp2 in devlog/_plan/260828_kiro_turn_termination/020. Verification: 214 tests pass across kiro-adapter, kiro-stream, server-kiro-completion-e2e, core-lab-boundary and repo-hygiene; 113 more across the empty-completion and request-log suites; `bun x tsc --noEmit` clean; `privacy:scan` green. All four new short-circuit assertions were driven red by reverting the hook call, so none is vacuous. Full-suite validation was skipped at the user's explicit instruction.
…r SSE return The local-terminal short-circuit returned its SSE body raw. Every other streaming return in handleResponsesInner wraps the bridge output in trackStreamLifetime, which is what releases the turn admission lease when the body finishes or the client disconnects. A turn that already holds all of its output would otherwise keep a lease alive until something else reclaimed it — the one resource the no-inference path had no excuse to hold. Found while checking the short-circuit against the neighbouring return sites rather than by a failing test, which is why no assertion changes here: the existing local-terminal regressions still pass, and a lease leak is not observable through them.
The merged stack left GET /api/providers off the undeclared-route ratchet, apiError sites coalescing status to 1, a dispatch source scan that still expected console on sync-cache, and a stale generated skill surface. Co-authored-by: Cursor <cursoragent@cursor.com>
fix(cli): unblock dest CI after the agentic-control stack
…timate
The C-phase verifier failed wp1 criterion 3 on evidence, and it was right. The
local terminal constructed explicit zero usage, but `usageForFinalLog` marks all
Kiro and Cursor usage estimated provider-wide, so the row came out as
`{inputTokens: 0, outputTokens: 0, estimated: true}` with
`usageStatus: "estimated"`. Zero counts from a turn that made no inference are
exact. A row claiming otherwise is indistinguishable from a real turn whose usage
frame never arrived — which is the wrong signal on the one path whose whole point
is that nothing was sent.
The provider-wide marking exists because those adapters can only guess a real
inference's usage. There is nothing to guess when there was no inference, so
`usageForFinalLog` takes an explicit `locallyAnswered` argument rather than
inferring it from the numbers: a genuine turn that happened to consume zero
tokens is still an estimate, and a locally answered turn whose adapter DID
estimate something still says so. It defaults false, so no existing caller
changes behaviour.
The second finding is fixed too: `localTerminalReason` lived only on the internal
context and never reached `RequestLogEntry`, so the field could not actually make
a zero-send row explainable as its own comment claimed. It is now copied into the
entry, sanitized like the other metadata strings.
Both gaps existed because the committed regressions asserted behaviour — nothing
was sent — and never read the resulting log. `tests/server-kiro-completion-e2e.test.ts`
now inspects the request-log row for both response modes: the reason field,
`usageStatus: "reported"`, absent `estimated`, zero counts, `sendCount === 0`,
and no `inputTokenEstimate`. Reverting the `locallyAnswered` argument turns
exactly those two red.
Verification: 363 tests pass across the kiro, usage-log, request-log,
empty-completion, core-lab-boundary and repo-hygiene suites;
`bun x tsc --noEmit` clean; `privacy:scan` green. Full-suite validation remains
skipped per the user's instruction.
Re-verification failed criterion 3 again, one level down. The parent request-log row read exact, but `addFinalRequestLog` finalizes its active attempt first through `finishRequestAttempt`, which ran `finalizedUsage` without the new `locallyAnswered` argument. So the row said `usageStatus: "reported"` while its own embedded attempt still said `estimated: true` — and the attempt is the detailed accounting a maintainer actually reads for a zero-send turn. The flag travels ON the attempt rather than as a `finishRequestAttempt` parameter. That function is called from six places (core, collaboration, compact, encrypted-payload, policy-fallback, and the request-log module itself), and a new positional argument would silently default to the wrong answer at any call site that was missed. Setting it once where the local terminal is decided cannot be forgotten by a later caller. The gap survived the first fix for the same reason it existed: the accounting tests asserted the row and the attempt's `sendCount`/`inputTokenEstimate`, but not the attempt's usage provenance. They now assert `usageStatus` and absent `estimated` on every embedded attempt. Verification: 205 tests pass across server-kiro-completion-e2e, usage-log, request-log, kiro-adapter, tool-catalog-nudge and cursor-exec-empty-result; `bun x tsc --noEmit` clean. Full-suite validation skipped per instruction.
…after a wasted one `EMPTY_EXEC_OUTPUT_MESSAGE` is a repair: it fires only once a model has already spent a call and read a blank result. That recovers the turn but cannot recover the round trip, and it leaves the model guessing whether its command failed or its output was merely dropped. Live 2026-08-28, a routed Kiro session did exactly that — reported a blank exec result as possible context loss, and learned the echo rule only from the repair text. The rule now also appears in the tool-catalog nudge, so a code-mode turn is told up front that nothing in the isolate is echoed automatically and that a bare trailing `await tools.<name>(...)` is discarded. The wording is one exported constant shared by both surfaces and by the Cursor guidance note: two surfaces describing the same isolate must not drift, because that is how a model gets told two different things about whether its output survived. Scope is gated on an actual code-mode exec being advertised. A flat shell bridge echoes stdout on its own, so the same sentence there would be false — asserted directly rather than left to the reader. Verification: 205 tests pass across server-kiro-completion-e2e, usage-log, request-log, kiro-adapter, tool-catalog-nudge and cursor-exec-empty-result; `bun x tsc --noEmit` clean. The kiro-adapter assertion checks the ACTUAL wire prompt rather than the builder alone, since the session that misread a blank result was a routed Kiro turn. Full-suite validation skipped per instruction.
…client to echo phase Suppressing a finished turn keyed off the inbound `phase: "final_answer"` alone, which is a field the proxy emits but no client is obliged to send back. Measured against the running proxy: the same history WITH phase sent nothing upstream, and WITHOUT it spent 7686 input tokens redoing the closed task. `localTerminalReason` appeared 0 times across 1534 logged Kiro turns even though the suppression code was deployed and loaded, so the feature had never once fired in production. The proxy now records the final answer it actually emitted, keyed by the normalized conversation digest it already computes for logging, and consults that record when the inbound phase is missing. An explicit phase still wins; the record is an additional source. Scope is the load-bearing constraint. Only the 32-hex normalized digest may key the map, so no raw header or account identifier can be retained here, and an unresolvable scope means "no record" rather than a global fallback. Matching requires the remembered answer to still be the trailing content-bearing message: every legitimate next turn contains a new user message somewhere, so "a later user message exists" would suppress nothing, while keying off the trailing turn distinguishes a re-sent closed history from a genuine follow-up. A local terminal emits no output and therefore writes no record, which is what keeps suppression alive across the repeated identical histories the live loop actually produced.
…and-turn-termination fix(kiro): explain empty exec output and stop reopening a delivered final answer
Contributor
|
✅ Deterministic PR hygiene checks passed. |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (219)
📝 WalkthroughWalkthroughThe version is bumped to 2.35.0. The changeset adds a generated CLI capability registry, honest exit codes, new ChangesOCX CLI Agentic Control Platform
Estimated code review effort: 4 (Complex) | ~75 minutes Cursor Adapter Umbrella Catalog and Diagnostics
Estimated code review effort: 4 (Complex) | ~60 minutes Kiro Local-Terminal Turn Suppression
Estimated code review effort: 4 (Complex) | ~40 minutes Storage Cleanup Policy Metadata Persistence
Estimated code review effort: 3 (Moderate) | ~25 minutes Ownership Semantic Fingerprinting
Estimated code review effort: 3 (Moderate) | ~20 minutes OAuth Account Attribution and Provider Compatibility Fixes
Estimated code review effort: 4 (Complex) | ~50 minutes Historical Merge and Release Process Records
Estimated code review effort: 1 (Trivial) | ~10 minutes ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
github-actions
Bot
changed the title
Contributor
⏳ DRAFT
What to do
UI screenshot waived by the |
# Conflicts: # package.json
lidge-jun
marked this pull request as ready for review
August 28, 2026 14:22
lidge-jun
added
suppression-approved
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6667e34517
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * static `maxModeVerified` gate so ultra generalizes automatically as evidence | ||
| * arrives — never from window size (devlog 260828 blocker-4 fold). | ||
| */ | ||
| let liveCursorMaxModeBases: ReadonlySet<string> = new Set(); |
There was a problem hiding this comment.
When multiple Cursor providers or failover accounts expose different maxModeModels, this process-global set makes request behavior depend on whichever discovery completed last: provider-fetch.ts records one authenticated provider's roster here, while every later resolveCursorSelection reads the same value. An Ultra request can therefore enable unsupported Max Mode for another account, or lose valid evidence after another provider reports none. Store this evidence per provider/account and resolve requests against the selected credential's discovery result.
Useful? React with 👍 / 👎.
Comment on lines
+49
to
+50
| const base = baseProviderLabel(providerName); | ||
| if (base === "openai" || base === "anthropic") return; |
There was a problem hiding this comment.
For a custom OAuth provider whose valid name ends in -main or -p<hex6>, baseProviderLabel strips that suffix before hashing; two distinct configured providers using the same upstream account ID then receive the same label and are merged by usage/summary.ts, which keys account rows only by label. Names such as anthropic-main are worse because the normalized value triggers the attribution skip entirely. Hash the actual configured provider identity, applying only explicit built-in aliases rather than legacy persisted-log normalization.
Useful? React with 👍 / 👎.
lidge-jun
marked this pull request as ready for review
August 28, 2026 14:35
This was referenced Aug 28, 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
4 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
Promote the
devintegration line ontopreviewfor the v2.35.0-preview release train. This is a maintainer promotion, not a feature PR: the branch isorigin/devatd7a82a8fcverbatim, sogit diff origin/dev HEADis empty.devhad accumulated 111 commits ahead of both release branches while npmlateststill served 2.34.0 andpreviewserved 2.34.0-preview.20260827, so the in-tree 2.35.0 line had never been published on either channel.The head carries the Kiro turn-termination work merged in #2819. Its operationally significant fix: turn suppression had been keyed off a Responses
phasefield that clients are not obliged to echo back, solocalTerminalReasonfired 0 times across 1,534 logged Kiro turns despite being deployed. The proxy now remembers the final answer it emitted, scoped to the normalized conversation digest, so suppression no longer depends on client cooperation.enforce-targetwill failwrong_base—ALLOWED_BASESis["dev"], so every promotion to a release branch trips it by construction. Precedent: #2757, #2760, #2553, #2507.Verification
devCross-platform CI atd7a82a8fc— completed: success on the exact commit this branch points at (ci, gates, hygiene, all 4 test shards, macos, windows, npm-global on all three OSes, keyring on all three OSes, linux-systemd, storage policy, api usage, react-doctor all green).origin/devwith no additional commits;git diff origin/dev codex/promote-preview-2350is empty.main(oldest 2026-08-13, newest 2026-08-27) and are not introduced by this delta.Checklist
origin/devverbatimSummary by CodeRabbit
New Features
ocx capabilitiescommand for machine-readable command and route discovery.Bug Fixes
Documentation
ocxagent skill documentation and CLI references.