diff --git a/.github/pr-assets/5428-reauth-unknown-flow-get.jpg b/.github/pr-assets/5428-reauth-unknown-flow-get.jpg new file mode 100644 index 00000000000..c625ed73a21 Binary files /dev/null and b/.github/pr-assets/5428-reauth-unknown-flow-get.jpg differ diff --git a/devlog/_fin/260904_repo_hygiene_campaign/000_plan.md b/devlog/_fin/260904_repo_hygiene_campaign/000_plan.md index f7f030084a9..a27c412b9ef 100644 --- a/devlog/_fin/260904_repo_hygiene_campaign/000_plan.md +++ b/devlog/_fin/260904_repo_hygiene_campaign/000_plan.md @@ -17,15 +17,17 @@ every contributor whose work is carried. ## Classification of local branches -Every branch was scored on four independent axes rather than by name: +Every branch was scored on four independent axes rather than by name. Axis 3 is +shown in its corrected form; the campaign itself ran it without `--no-renames` +(see the 2026-09-21 correction in 010_method.md): 1. `git merge-base --is-ancestor
origin/dev` — plain ancestry. 2. `git cherry origin/dev
` — patch-equivalence, which catches rebases. 3. Content landing — the files the branch touches - (`git diff --name-only origin/dev...
`) are compared two-dot against - `origin/dev` restricted to exactly those paths. Zero remaining difference - means the branch's content is already on `dev` even though a squash merge - destroyed its commit identity. + (`git diff --no-renames --name-only origin/dev...
`) are compared two-dot + against `origin/dev` restricted to exactly those paths. Zero remaining + difference means the branch's content is already on `dev` even though a + squash merge destroyed its commit identity. 4. Exact reference matching against live GitHub state: open-PR head refs, worktree-backing refs, and the PR number a scratch branch was cut for. diff --git a/devlog/_fin/260904_repo_hygiene_campaign/010_method.md b/devlog/_fin/260904_repo_hygiene_campaign/010_method.md index 773b6db6494..c8e98858bc5 100644 --- a/devlog/_fin/260904_repo_hygiene_campaign/010_method.md +++ b/devlog/_fin/260904_repo_hygiene_campaign/010_method.md @@ -7,8 +7,8 @@ A local branch is deletable when at least one holds, and no guard fires. ``` T1 ancestry git merge-base --is-ancestor
origin/dev T2 patch-equiv git cherry origin/dev
-> no '+' lines -T3 content paths = git diff --name-only origin/dev...
- git diff --name-only origin/dev
-- -> empty +T3 content paths = git diff --no-renames --name-only origin/dev...
+ git diff --no-renames --name-only origin/dev
-- -> empty T4 scratch branch name encodes a PR number whose state is MERGED or CLOSED AND the name matches the scratch prefix set AND the number is a WHOLE numeric token of the branch name @@ -22,6 +22,17 @@ report "unmerged" for work that is fully shipped. T3 asks the only question that is actually load-bearing — is there any difference left in the files this branch claims to change. +Correction, 2026-09-21: the 71 deletions recorded below ran the listing command +without `--no-renames`. Rename detection must be disabled while collecting that +path set, and any rerun after this date should use the form shown above. +Otherwise a rename contributes only its destination: if `dev` independently +contains the same destination but retains the source, the restricted second +diff is empty even though the complete tip trees differ. `--no-renames` emits +both the deleted source and added destination, so the source-side difference +prevents a false LANDED verdict. No wrongly-LANDED branch has been identified +from the earlier run; this is a preventive correction for the next sweep, not a +measured incident. + T4 is deliberately narrow. It fires only for throwaway prefixes (`pr*`, `rb-`, `jrb-`, `mtp/`, `big-`, `cf-`, `ocx-`, `wip/`, `backup/`, `candidate`, `cursor-`, `midstream`) created by earlier review and rebase runs, diff --git a/devlog/_fin/260904_repo_hygiene_campaign/100_pr_verdicts.md b/devlog/_fin/260904_repo_hygiene_campaign/100_pr_verdicts.md index bf8e5d701c0..3ff033a1c98 100644 --- a/devlog/_fin/260904_repo_hygiene_campaign/100_pr_verdicts.md +++ b/devlog/_fin/260904_repo_hygiene_campaign/100_pr_verdicts.md @@ -1,10 +1,12 @@ # 100 — Per-PR verdicts Full classification of the 53 pull requests open when the campaign started. -Method: fetch each PR head, take the files it touches -(`git diff --name-only origin/dev...`), then compare those exact paths -two-dot against `origin/dev`. Remaining differences mean the work has not -landed. +Method: fetch each PR head, take the files it touches, then compare those exact +paths two-dot against `origin/dev`. Remaining differences mean the work has not +landed. The verdicts below were produced with +`git diff --name-only origin/dev...`; any rerun must use +`git diff --no-renames --name-only origin/dev...` so a rename cannot hide +the deleted source side from the path set (2026-09-21; see 010_method.md). ## Closed diff --git a/gui/src/components/use-main-device-reauth.ts b/gui/src/components/use-main-device-reauth.ts index 626e004a5c1..44d18b9645f 100644 --- a/gui/src/components/use-main-device-reauth.ts +++ b/gui/src/components/use-main-device-reauth.ts @@ -181,6 +181,12 @@ export function useMainDeviceReauth(apiBase: string, onCompleted: () => void) { const dto = await res.json().catch(() => ({})) as FlowDto; if (!isCurrent() || flowRef.current !== flowId) return; if (!res.ok) { + if (res.status === 404 && dto.code === "unknown_flow") { + stopPolling(); + flowRef.current = null; + setState({ phase: "failed", code: "request_failed" }); + return; + } // Ownership continues from the Cancel click, including while DELETE // is unresolved. Never offer a replacement POST during that window, // and keep the existing poll cadence so a later terminal status remains observable. diff --git a/gui/tests/main-device-reauth-ownership.test.tsx b/gui/tests/main-device-reauth-ownership.test.tsx index 917cb5f80d1..e90315560b3 100644 --- a/gui/tests/main-device-reauth-ownership.test.tsx +++ b/gui/tests/main-device-reauth-ownership.test.tsx @@ -315,6 +315,17 @@ for (const failure of ["network", "http", "nonterminal"] as const) { }); } +test("unknown flow status stops polling after a failed cancellation", async () => { + await mount(); + await beginFlow("A"); + await invoke(() => hook.cancel()); + await reply(take("DELETE", "A"), { code: "unavailable" }, 503); + await act(async () => { for (const wake of sleepers.splice(0)) wake(); }); + await reply(take("GET", "A"), { code: "unknown_flow" }, 404); + expect(hook.state).toEqual({ phase: "failed", code: "request_failed" }); + expect(sleepers).toHaveLength(0); +}); + test("two successful cancellation replies complete the same flow only once", async () => { await mount(); await beginFlow("A"); diff --git a/src/adapters/qoder/scaffold-guard.ts b/src/adapters/qoder/scaffold-guard.ts index 8a1b6df6206..529a9c0c882 100644 --- a/src/adapters/qoder/scaffold-guard.ts +++ b/src/adapters/qoder/scaffold-guard.ts @@ -54,11 +54,44 @@ const MAX_MARKER_LENGTH = Math.max(...ALL_MARKERS.map(marker => marker.length)); * refuse the turn. A stem running to the end of the buffer still counts: more text may be * arriving, and reading it as prose is the one reading that could release the block body. */ -function reminderOpensHere(lowered: string, at: number): boolean { - const after = lowered[at + REMINDER_OPEN.length]; +function reminderOpensHere(text: string, at: number): boolean { + const after = text[at + REMINDER_OPEN.length]; return after === undefined || /[\s/>]/.test(after); } +/** + * Fold one UTF-16 code unit the way `toLowerCase()` does, when that yields one code unit. + * + * This keeps every match the lowercased scan used to make. U+212A KELVIN SIGN lowercases to an + * ASCII `k`, so `` was treated as tool markup; an ASCII-only fold would release it. + * A character whose lowercase form is longer (such as U+0130) is left as-is. + */ +function foldCodeUnit(code: number): number { + if (code >= 65 && code <= 90) return code + 32; + if (code < 128) return code; + const lowered = String.fromCharCode(code).toLowerCase(); + return lowered.length === 1 ? lowered.charCodeAt(0) : code; +} + +/** + * Find a lowercase ASCII marker without transforming `text`. + * + * Marker offsets must remain offsets into the original string. Unicode lowercasing can expand + * one code unit into several (for example, `İ` becomes `i` plus a combining dot), so an index + * obtained from `text.toLowerCase()` is unsafe to reuse with `text.slice()`. Folding one code + * unit at a time keeps the offsets and the matches. + */ +function indexOfMarker(text: string, marker: string, from = 0): number { + const last = text.length - marker.length; + outer: for (let at = Math.max(0, from); at <= last; at++) { + for (let offset = 0; offset < marker.length; offset++) { + if (foldCodeUnit(text.charCodeAt(at + offset)) !== marker.charCodeAt(offset)) continue outer; + } + return at; + } + return -1; +} + /** * Ceiling on a suppressed block before it is treated as unterminated. * @@ -80,9 +113,10 @@ export interface ScaffoldFilterResult { function heldSuffixLength(text: string): number { const limit = Math.min(MAX_MARKER_LENGTH - 1, text.length); for (let length = limit; length > 0; length--) { - const suffix = text.slice(text.length - length).toLowerCase(); for (const marker of ALL_MARKERS) { - if (marker.length > length && marker.startsWith(suffix)) return length; + if (marker.length > length && indexOfMarker(text, marker.slice(0, length), text.length - length) >= 0) { + return length; + } } } return 0; @@ -114,7 +148,6 @@ export class QoderScaffoldFilter { for (;;) { if (this.mode === "suppress") { const scan = this.suppressedTail + buffer; - const scanned = scan.toLowerCase(); // Unwind nesting rather than ending at the first closer. A reminder containing another // reminder would otherwise hand the outer block's remaining body — the MCP server list // in the reported leak — to the client as the model's answer, with a successful @@ -122,11 +155,11 @@ export class QoderScaffoldFilter { let cursor = 0; let close = -1; for (;;) { - const nextClose = scanned.indexOf(REMINDER_CLOSE, cursor); + const nextClose = indexOfMarker(scan, REMINDER_CLOSE, cursor); if (nextClose < 0) break; - let nextOpen = scanned.indexOf(REMINDER_OPEN, cursor); - while (nextOpen >= 0 && !reminderOpensHere(scanned, nextOpen)) { - nextOpen = scanned.indexOf(REMINDER_OPEN, nextOpen + 1); + let nextOpen = indexOfMarker(scan, REMINDER_OPEN, cursor); + while (nextOpen >= 0 && !reminderOpensHere(scan, nextOpen)) { + nextOpen = indexOfMarker(scan, REMINDER_OPEN, nextOpen + 1); } if (nextOpen >= 0 && nextOpen < nextClose) { this.suppressDepth += 1; @@ -159,11 +192,10 @@ export class QoderScaffoldFilter { let earliest = -1; let found = ""; - const lowered = buffer.toLowerCase(); for (const marker of ALL_MARKERS) { - let at = lowered.indexOf(marker); - while (at >= 0 && marker === REMINDER_OPEN && !reminderOpensHere(lowered, at)) { - at = lowered.indexOf(marker, at + 1); + let at = indexOfMarker(buffer, marker); + while (at >= 0 && marker === REMINDER_OPEN && !reminderOpensHere(buffer, at)) { + at = indexOfMarker(buffer, marker, at + 1); } if (at < 0) continue; // A closer sitting exactly where an opener starts cannot happen, so ties are impossible. diff --git a/src/integrations/raycast-detect.ts b/src/integrations/raycast-detect.ts index 7ae70edf461..e0ef3191f6d 100644 --- a/src/integrations/raycast-detect.ts +++ b/src/integrations/raycast-detect.ts @@ -44,10 +44,19 @@ export interface RaycastDetectDeps { */ const RAYCAST_DEFAULTS_DOMAIN = "com.raycast.macos.v1"; const RAYCAST_SUBSCRIPTION_KEY = "subscriptions_active"; +const DEFAULTS_PATH = "/usr/bin/defaults"; +const DEFAULTS_TIMEOUT_MS = 2_000; -export function realRaycastDetectDeps(): RaycastDetectDeps { +interface RealRaycastDetectRuntime { + platform?: string; + spawnSync?: typeof Bun.spawnSync; +} + +export function realRaycastDetectDeps(runtime: RealRaycastDetectRuntime = {}): RaycastDetectDeps { + const platform = runtime.platform ?? process.platform; + const spawnSync = runtime.spawnSync ?? Bun.spawnSync; return { - platform: process.platform, + platform, homedir: homedir(), env: process.env, exists: path => { @@ -59,9 +68,15 @@ export function realRaycastDetectDeps(): RaycastDetectDeps { }, readDefault: (domain, key) => { // `defaults` is macOS-only; elsewhere the plan is simply unknown. - if (process.platform !== "darwin") return null; + if (platform !== "darwin") return null; try { - const result = Bun.spawnSync(["defaults", "read", domain, key], { stdout: "pipe", stderr: "pipe" }); + const result = spawnSync([DEFAULTS_PATH, "read", domain, key], { + stdout: "pipe", + stderr: "pipe", + timeout: DEFAULTS_TIMEOUT_MS, + }); + // A timed-out or signal-killed probe reports exitCode === null; that + // and any non-zero exit mean the preference was not read. if (result.exitCode !== 0) return null; return result.stdout.toString().trim(); } catch { diff --git a/src/lib/bounded-body.ts b/src/lib/bounded-body.ts index 0b3769eaacf..effb247fb71 100644 --- a/src/lib/bounded-body.ts +++ b/src/lib/bounded-body.ts @@ -15,6 +15,8 @@ export interface BoundedBodyOptions { * Reader cancellation and lock release still run. Defaults to false. */ fatalUtf8?: boolean; + /** Report UTF-8 validity without rejecting malformed bodies. */ + reportUtf8Validity?: boolean; /** * Byte ceiling for retained body data. Defaults to BOUNDED_BODY_MAX_BYTES (64 KiB), * which suits error bodies; callers materializing whole success payloads (e.g. a @@ -44,6 +46,8 @@ export interface BoundedBodyResult { oversized: boolean; /** False means callers should use a status-only fallback, not `text`. */ displaySafe: boolean; + /** Present when reportUtf8Validity was requested and the retained body reached EOF. */ + utf8Valid?: boolean; } export interface BoundedBytesOptions { @@ -238,6 +242,14 @@ function decodeUtf8(chunks: readonly Uint8Array[], fatal: boolean, timedOut = fa } } +function decodeUtf8WithValidity(bytes: Uint8Array): { text: string; utf8Valid: boolean } { + try { + return { text: decodeUtf8([bytes], true), utf8Valid: true }; + } catch { + return { text: decodeUtf8([bytes], false), utf8Valid: false }; + } +} + /** * Consume the original response body under strict memory and time bounds. * @@ -326,6 +338,24 @@ export async function readBoundedResponseBody( const { value, done } = outcome as ReadableStreamReadResult; if (done) { + if (options.reportUtf8Validity) { + const bytes = retained.subarray(0, retainedBytes); + // A fatal decode that returned already proved the bytes valid; still + // honour the reporting contract instead of dropping utf8Valid. + const decoded = options.fatalUtf8 === true + ? { text: decodeUtf8([bytes], true), utf8Valid: true } + : decodeUtf8WithValidity(bytes); + return { + text: decoded.text, + truncated: false, + timedOut: false, + totalTimedOut: false, + inactivityTimedOut: false, + oversized: false, + displaySafe: true, + utf8Valid: decoded.utf8Valid, + }; + } return { text: decodeUtf8([retained.subarray(0, retainedBytes)], options.fatalUtf8 === true), truncated: false, diff --git a/src/server/responses/core-combo-failure.ts b/src/server/responses/core-combo-failure.ts index c14c3bb680b..e222c2ccaea 100644 --- a/src/server/responses/core-combo-failure.ts +++ b/src/server/responses/core-combo-failure.ts @@ -45,27 +45,28 @@ export async function consumeComboFailure( // a second body read, so this mirrors its normalization: raw 402/429, or a 5xx whose intact, // display-safe body carries a recognized quota message. let quotaConfirmedByBody = false; + const serverError = response.status >= 500 && response.status < 600; try { - const body = await readBoundedResponseBody(response, { - signal, - // Match shouldRetryCodexPoolAccountQuota before treating a 5xx body as quota evidence. - fatalUtf8: response.status >= 500 && response.status < 600, - }); - usage = usageFromComboFailureText(body.text); - if ( - response.status >= 500 && response.status < 600 - && body.displaySafe && !body.truncated - ) { + const body = await readBoundedResponseBody(response, { signal, reportUtf8Validity: serverError }); + // A 5xx body counts as quota or classification evidence only when it decoded as valid + // UTF-8, matching shouldRetryCodexPoolAccountQuota. A malformed byte keeps the status-only + // fallback, with one exception: a cyber-policy refusal must still stop the combo, so the + // replacement-decoded text may carry that verdict and nothing else. + const utf8Trusted = !serverError || body.utf8Valid === true; + if (utf8Trusted) usage = usageFromComboFailureText(body.text); + if (serverError && utf8Trusted && body.displaySafe && !body.truncated) { const quotaMessage = codexQuotaFailureMessage(body.text); quotaConfirmedByBody = quotaMessage !== undefined && isRateLimitOrQuotaFailureMessage(quotaMessage); } - if (body.displaySafe) { + if (body.displaySafe && !body.truncated) { const normalized = normalizeUpstreamErrorText(body.text, fallback); - classificationText = normalized.safeText; - upstreamCode = normalized.code; - upstreamMessage = normalized.message; - upstreamType = normalized.type; + if (utf8Trusted || isCyberPolicyCode(normalized.code) || isCyberPolicyMessage(normalized.safeText)) { + classificationText = normalized.safeText; + upstreamCode = normalized.code; + upstreamMessage = normalized.message; + upstreamType = normalized.type; + } } } catch (error) { if (signal?.aborted) throw error; diff --git a/structure/gui-and-management-api.md b/structure/gui-and-management-api.md index 5be71c0bfbc..0b95f0146af 100644 --- a/structure/gui-and-management-api.md +++ b/structure/gui-and-management-api.md @@ -420,7 +420,7 @@ single forms, and the shell pattern is the part worth keeping stable: | Subagents | Featured-roster selection workspace (`gui/src/components/subagents-workspace/`). | | Combos | Rail, detail panel, and an add flow (`gui/src/components/ComboWorkspace.tsx`). | | Add provider | Catalog browser plus form and OAuth panes (`gui/src/components/provider-catalog/`, `gui/src/components/AddProviderModal.tsx`). The catalog browses four tabs — Accounts, Free, Local, Paid — where Local is a catalog-only bucket peeled out of `bucketPresets` after `presetTier` has classified; the workspace `providerTier` stays three-way, so the rail, the free-paid sort and the Free count still treat a local runtime as free. Search sits above the tabs and reaches every tab at once: while a query is live the list renders all four groups with headings and the strip becomes jump chips with counts rather than a tablist, because moving the selected tab would change the row kind under the user (a preset-select button becomes a login row). ArrowDown from the search input focuses the first enabled result action; if none is available, focus stays in the input. The tab strip wraps within narrow modals. Every nonempty note has a full-text button so narrow rows never hide content permanently; the native note dialog closes during teardown and restores focus to its trigger. Provider notes clamp to two lines and open in full in a stacked native `` owned by `AddProviderModal`, which also owns the search text so its `window` Escape handler can unwind popup, then query, then dialog. | -| Codex accounts | Account pool cards, add-account flow, switch and reset modals (`gui/src/components/CodexAccountPool.tsx`, `gui/src/components/AddCodexAccountModal.tsx`), plus the generic account-targeting picker opt-in on `gui/src/pages/codex-set-multiauth.tsx`. Add/delete/login completion is projected to one boolean before presentation; pending catalog work is a warning, not a failed account mutation. The main card's native-main device reauth (#3898) is owned by `gui/src/components/use-main-device-reauth.ts`: the dedicated `/api/codex-auth/main/reauth-device` namespace only — never the pool login route — with flowId-owned polling, an allowlisted verification URL, and no token fields accepted from payloads. The main-device reauth hook retains flow ownership from the Cancel click, while DELETE is unresolved and after retryable failure; polling normally continues. A concurrent GET HTTP error cannot expose a replacement login POST before DELETE settles. If a retryable DELETE failure races with a non-2xx GET while the flow is pending or committing, either response order preserves same-flow Cancel retry, restores the last server-provided device code, verification URL, and phase when needed, and keeps the existing poll cadence so a later terminal result remains observable. Outside same-flow cancellation ownership, a GET HTTP failure still stops polling without starting a second login POST. The cancellation-failure indication survives pending status updates until a trusted terminal result releases ownership. A successful DELETE with a terminal `failed` DTO releases it and uses the same closed failure-code mapping as polling; only `succeeded` notifies login completion. Unrecognized or nonterminal DTO status values remain retryable. A DELETE response with HTTP 404 and code `unknown_flow` releases the expired flow and shows the existing generic failure state so device re-login is available again; it claims neither login success nor confirmed cancellation. Confirmed cancellation also makes device re-login available. Start, polling and cancellation completions verify their controller or flow ownership after asynchronous response reads; replaced flows and unmounted hooks cannot update a newer flow or notify completion. Effect setup restores mounted state after the StrictMode development cleanup cycle. | +| Codex accounts | Account pool cards, add-account flow, switch and reset modals (`gui/src/components/CodexAccountPool.tsx`, `gui/src/components/AddCodexAccountModal.tsx`), plus the generic account-targeting picker opt-in on `gui/src/pages/codex-set-multiauth.tsx`. Add/delete/login completion is projected to one boolean before presentation; pending catalog work is a warning, not a failed account mutation. The main card's native-main device reauth (#3898) is owned by `gui/src/components/use-main-device-reauth.ts`: the dedicated `/api/codex-auth/main/reauth-device` namespace only — never the pool login route — with flowId-owned polling, an allowlisted verification URL, and no token fields accepted from payloads. The main-device reauth hook retains flow ownership from the Cancel click, while DELETE is unresolved and after retryable failure; polling normally continues. A concurrent retryable GET HTTP error (any non-2xx other than 404 `unknown_flow`) cannot expose a replacement login POST before DELETE settles. If a retryable DELETE failure races with such a retryable non-2xx GET while the flow is pending or committing, either response order preserves same-flow Cancel retry, restores the last server-provided device code, verification URL, and phase when needed, and keeps the existing poll cadence so a later terminal result remains observable. A GET with HTTP 404 and code `unknown_flow` is terminal even during cancellation ownership: the flow no longer exists, so polling stops, the flow is released, and the existing generic failure state appears, as on the DELETE path. Outside same-flow cancellation ownership, a GET HTTP failure still stops polling without starting a second login POST. The cancellation-failure indication survives pending status updates until a trusted terminal result releases ownership. A successful DELETE with a terminal `failed` DTO releases it and uses the same closed failure-code mapping as polling; only `succeeded` notifies login completion. Unrecognized or nonterminal DTO status values remain retryable. A DELETE response with HTTP 404 and code `unknown_flow` releases the expired flow and shows the existing generic failure state so device re-login is available again; it claims neither login success nor confirmed cancellation. Confirmed cancellation also makes device re-login available. Start, polling and cancellation completions verify their controller or flow ownership after asynchronous response reads; replaced flows and unmounted hooks cannot update a newer flow or notify completion. Effect setup restores mounted state after the StrictMode development cleanup cycle. | | Dashboard overview | Overview, Providers, and Models tabs at the page level (`gui/src/pages/Dashboard.tsx`), the 30-day token and coverage stats in the overview head (`gui/src/pages/dashboard-overview-head.tsx`), and the effort-cap, injection, maintenance, sidecar, and memory panels below it (`gui/src/pages/dashboard-overview-panels.tsx`). | The native-main reauth poller captures an immutable accepted flow id for queued callbacks. diff --git a/structure/overview.md b/structure/overview.md index e75cf6430ee..d5cd3746564 100644 --- a/structure/overview.md +++ b/structure/overview.md @@ -240,7 +240,7 @@ Raw reasoning content and provider-authored summaries remain distinct on the Res Connected-browser pairing and dashboard failure meanings follow the [management UI contract](gui-and-management-api.md#dashboard-surfaces); machine enrollment alone does not authenticate a browser. -Native-main reauthentication keeps its existing polling cadence when a non-2xx status races with retryable cancellation for the same owned flow; the [dashboard flow-ownership contract](gui-and-management-api.md#dashboard-surfaces) defines terminal release and completion notification. +Native-main reauthentication keeps its existing polling cadence when a non-2xx status races with retryable cancellation for the same owned flow; the [dashboard flow-ownership contract](gui-and-management-api.md#dashboard-surfaces) defines terminal release and completion notification. A GET answered with 404 `unknown_flow` is the exception: the flow no longer exists, so polling stops and the generic failure state appears. Cline CLI is a managed file integration: its provider settings and catalog share one recoverable journal operation. The [paired-file contract](clients/integrations.md#cline-paired-files) defines its stop/restart requirement. Pool quota producers and account commands follow the [bounded raw-observation contract](providers/openai-tiers.md#bounded-pool-quota-observations), separate from the latest display snapshot and capacity estimates. diff --git a/structure/transports/inventory.md b/structure/transports/inventory.md index bfd34825f7d..3cb8053a5ff 100644 --- a/structure/transports/inventory.md +++ b/structure/transports/inventory.md @@ -98,6 +98,14 @@ promises are observed, and a cancellation that never settles cannot extend the r After an attached read, cleanup removes the abort listener, cancels any inactivity timer, and attempts to release the reader lock. `tests/server/bounded-body.test.ts` covers these paths. +`readBoundedResponseBody` accepts `reportUtf8Validity`: the body decodes with replacement +characters instead of rejecting, and a result that reached EOF carries `utf8Valid`. Combined with +`fatalUtf8`, a returned body is valid by construction and reports `true`. Timeout and oversized +results omit the field. `consumeComboFailure` uses it for 5xx bodies: only a valid body supplies +quota evidence, usage, or classification, and a malformed body keeps the status-only fallback +unless its lenient decode identifies a cyber-policy refusal, which must still stop the combo +(`tests/providers/cyber-policy-error-fidelity.test.ts`). + `src/oauth/orcarouter.ts` applies this reader to a successful `POST /api/v1/auth/keys` response with a 65,536-byte (64 KiB) ceiling. One 30-second signal, combined with caller cancellation, covers both fetching the response headers and consuming the body; no separate body or inactivity diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 023bddbe114..ab82bee7a43 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -11,6 +11,12 @@ Plaintext collaboration restoration treats a null namespace as absent, rejects n ## Responses HTTP/SSE +`src/server/responses/core-combo-failure.ts` keeps a cyber-policy stop from bounded +replacement-decoded error text when a 5xx body has malformed UTF-8. Every other use of a +malformed 5xx body (usage, quota and reset evidence, ordinary classification) keeps the +status-only fallback. Rebuilt failures retain the non-replayable marker; cyber-policy failures +carry neither Retry-After nor quota-reset metadata. + `/v1/responses` is the main Codex-facing endpoint. The server parses Responses input, routes to a provider, lets the selected adapter speak the upstream protocol, then bridges adapter events back to Responses-compatible streaming output. For an opted-in key-auth provider, a hosted-search continuation stays bound to the API-key selection that served the first leg; the contract is the [hosted-search continuation binding](../providers-and-adapters.md#hosted-search-continuation-binding). diff --git a/tests/clients/raycast-detect.test.ts b/tests/clients/raycast-detect.test.ts index 4e4268b29b3..3d5fc49e6a2 100644 --- a/tests/clients/raycast-detect.test.ts +++ b/tests/clients/raycast-detect.test.ts @@ -1,5 +1,9 @@ import { describe, expect, test } from "bun:test"; -import { detectRaycast, type RaycastDetectDeps } from "../../src/integrations/raycast-detect"; +import { + detectRaycast, + realRaycastDetectDeps, + type RaycastDetectDeps, +} from "../../src/integrations/raycast-detect"; /** * Stubbed deps only. The real detector spawns `defaults` and reads the @@ -29,6 +33,21 @@ function fakeDeps( } describe("detectRaycast", () => { + test("the macOS preference probe uses the system binary with a bounded runtime", () => { + let invocation: { command: string[]; timeout?: number } | undefined; + const spawnSync = ((command: string[], options: { timeout?: number }) => { + invocation = { command, timeout: options.timeout }; + return { exitCode: 0, stdout: Buffer.from("1") }; + }) as unknown as typeof Bun.spawnSync; + + const deps = realRaycastDetectDeps({ platform: "darwin", spawnSync }); + expect(deps.readDefault("com.raycast.macos.v1", "subscriptions_active")).toBe("1"); + expect(invocation).toEqual({ + command: ["/usr/bin/defaults", "read", "com.raycast.macos.v1", "subscriptions_active"], + timeout: 2_000, + }); + }); + test("darwin: a Pro subscription, the app bundle and the revealed ai folder", () => { const deps = fakeDeps("darwin", ["/Applications/Raycast.app", "/home/u/.config/raycast/ai"], { defaultValue: "1" }); expect(detectRaycast(deps)).toEqual({ @@ -55,6 +74,17 @@ describe("detectRaycast", () => { expect(detectRaycast(fakeDeps("darwin", [], { defaultValue: "" })).plan).toBe("unknown"); }); + test("darwin: a timed-out or killed defaults probe is unknown, not a false positive", () => { + // A probe the 2s timeout kills reports a null or non-zero exit code; neither reads the + // preference. stdout carries "1" so a probe that ignored the exit code would report Pro. + for (const exitCode of [null, 143]) { + const spawnSync = (() => ({ exitCode, exitedDueToTimeout: true, stdout: Buffer.from("1") })) as unknown as typeof Bun.spawnSync; + const deps = realRaycastDetectDeps({ platform: "darwin", spawnSync }); + expect(deps.readDefault("com.raycast.macos.v1", "subscriptions_active")).toBeNull(); + expect(detectRaycast(deps).plan).toBe("unknown"); + } + }); + test("win32: LOCALAPPDATA\\Programs\\Raycast is the install path and the plan is unknown", () => { const local = "C:\\Users\\u\\AppData\\Local"; const deps = fakeDeps("win32", [`${local}\\Programs\\Raycast`, "C:\\Users\\u\\.config\\raycast\\ai"], { diff --git a/tests/providers/cyber-policy-error-fidelity.test.ts b/tests/providers/cyber-policy-error-fidelity.test.ts index 7adc39f3f22..e1b45aa1833 100644 --- a/tests/providers/cyber-policy-error-fidelity.test.ts +++ b/tests/providers/cyber-policy-error-fidelity.test.ts @@ -18,6 +18,7 @@ import { consumeComboFailure } from "../../src/server/responses/core"; import { handleResponses } from "../../src/server/responses"; import type { AdapterEvent, OcxConfig } from "../../src/types"; import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; +import { isNonReplayableResponse, markResponseNonReplayable } from "../../src/lib/upstream-retry"; import { createTestTranslatorBudget, withTestTranslatorBudget } from "../helpers/translator-budget"; const createOpenAIChatAdapter = (...args: Parameters) => @@ -180,6 +181,55 @@ describe("cyber_policy error fidelity", () => { }); }); + test("malformed UTF-8 does not erase a cyber-policy stop", async () => { + const bytes = new Uint8Array([ + ...new TextEncoder().encode(JSON.stringify(CYBER_ERROR_BODY)), + 0xff, + ]); + const failure = await consumeComboFailure(new Response(bytes, { status: 502 })); + expect(failure.response.status).toBe(400); + expect(failure.upstreamCode).toBe(CYBER_POLICY_ERROR_CODE); + expect(comboFailureDecision(502, failure.classificationText, { + code: failure.upstreamCode, + })).toBe("stop"); + }); + + test("bounded recovery preserves a non-replayable malformed cyber stop", async () => { + const bytes = new Uint8Array([...new TextEncoder().encode(JSON.stringify(CYBER_ERROR_BODY)), 0xff]); + const upstream = new Response(bytes, { + status: 502, + headers: { "retry-after": "120", "x-codex-primary-reset-at": "2000000000" }, + }); + markResponseNonReplayable(upstream); + const failure = await consumeComboFailure(upstream); + expect(failure.response.status).toBe(400); + expect(failure.upstreamCode).toBe(CYBER_POLICY_ERROR_CODE); + expect(failure.nonReplayable).toBe(true); + expect(isNonReplayableResponse(failure.response)).toBe(true); + expect(failure.retryAfter).toBeUndefined(); + expect(failure.resetAt).toBeUndefined(); + expect(failure.response.headers.get("retry-after")).toBeNull(); + expect(comboFailureDecision(502, failure.classificationText, { code: failure.upstreamCode })).toBe("stop"); + }); + + test("malformed UTF-8 keeps the status-only fallback for a non-cyber 5xx body", async () => { + // Only a cyber-policy verdict may be read from replacement-decoded text. Any other code + // or prose from a malformed body (here a quota code that would widen the cooldown scope) + // must not reach classification, exactly as before the body was decoded leniently. + const bytes = new Uint8Array([ + ...new TextEncoder().encode(JSON.stringify({ + error: { message: "You exceeded your current quota", type: "insufficient_quota", code: "insufficient_quota" }, + })), + 0xff, + ]); + const failure = await consumeComboFailure(new Response(bytes, { status: 502 })); + expect(failure.response.status).toBe(502); + expect(failure.classificationText).toBe("Provider error 502"); + expect(failure.upstreamCode).toBeUndefined(); + expect(failure.resetAt).toBeUndefined(); + expect(failure.usage).toBeUndefined(); + }); + test("drops Codex reset headers as well as Retry-After for a cyber-policy failure", async () => { const upstream = new Response(JSON.stringify(CYBER_ERROR_BODY), { status: 429, diff --git a/tests/providers/qoder-scaffold-guard.test.ts b/tests/providers/qoder-scaffold-guard.test.ts index 65f06d7bc15..4c7a26d5143 100644 --- a/tests/providers/qoder-scaffold-guard.test.ts +++ b/tests/providers/qoder-scaffold-guard.test.ts @@ -35,6 +35,22 @@ describe("QoderScaffoldFilter", () => { expect(first.text + filter.flush().text).toBe("Before.After."); }); + test("uses original-string offsets when Unicode lowercasing would expand", () => { + const expandingPrefix = "İ".repeat(64); + const filter = new QoderScaffoldFilter(); + const result = filter.push(`${expandingPrefix}${REMINDER}After.`); + expect(result.fail).toBeNull(); + expect(result.text + filter.flush().text).toBe(`${expandingPrefix}After.`); + expect(result.text).not.toContain("internal-notes"); + }); + + test("uses original-string offsets to find a closer after expanding Unicode", () => { + const filter = new QoderScaffoldFilter(); + const result = filter.push(`${"İ".repeat(64)}After.`); + expect(result.fail).toBeNull(); + expect(result.text + filter.flush().text).toBe("After."); + }); + test("catches a marker split across deltas", () => { const filter = new QoderScaffoldFilter(); // The opening tag arrives in three pieces; a per-delta scan would miss it entirely. @@ -136,6 +152,25 @@ describe("QoderScaffoldFilter", () => { expect(result.fail).toContain(""); }); + test("folds a Kelvin-sign spelling of invoke the way lowercasing did", () => { + // U+212A lowercases to an ASCII k in one code unit, so the old lowercased scan caught it. + const kelvin = "\u212A"; + const filter = new QoderScaffoldFilter(); + const result = filter.push(`Checking.\n\ncd /srv/private && git status\n`); + expect(result.text).toBe("Checking.\n"); + expect(result.text).not.toContain("git status"); + expect(result.fail).not.toBeNull(); + }); + + test("holds a Kelvin-sign invoke prefix split across deltas", () => { + const kelvin = "\u212A"; + const filter = new QoderScaffoldFilter(); + const first = filter.push("Checking.\n\ncd /srv/private && git status`); + expect(first.text + second.text).toBe("Checking.\n"); + expect(second.fail).not.toBeNull(); + }); + test("does not open a block on a word that merely starts with the tag name", () => { // The opener is matched without its ">", so it needs a token boundary of its own. const filter = new QoderScaffoldFilter(); @@ -159,6 +194,17 @@ describe("QoderScaffoldFilter", () => { }); describe("guardQoderScaffolding", () => { + test("never emits a reminder after a Unicode case-folding expansion", () => { + const { events, emit } = collect(); + const guarded = guardQoderScaffolding(emit); + const prefix = "İ".repeat(64); + guarded({ type: "text_delta", text: `${prefix}${REMINDER}` }); + guarded({ type: "done", stopReason: "stop" }); + expect(textOf(events)).toBe(prefix); + expect(textOf(events)).not.toContain("internal-notes"); + expect(events[events.length - 1]!.type).toBe("done"); + }); + test("strips the reminder and still completes the turn", () => { const { events, emit } = collect(); const guarded = guardQoderScaffolding(emit); diff --git a/tests/server/account-pool-management-api.test.ts b/tests/server/account-pool-management-api.test.ts index 1c66163c87c..30d32dcb5a7 100644 --- a/tests/server/account-pool-management-api.test.ts +++ b/tests/server/account-pool-management-api.test.ts @@ -603,11 +603,15 @@ describe("legacy pool contract goldens (#wp5)", () => { } }); - test("a bad strategy and a bad stickyLimit are rejected identically on every kind", async () => { + test("a bad strategy and a bad stickyLimit are rejected identically on every kind, with one null exception", async () => { // One validator, three adapters. The kinds keep their own request and response shapes -- // that is what the goldens above pin -- but the VALUE rules are now a single implementation, // so "quota, round-robin, fill-first" and the 1..100 sticky bound cannot drift apart per // kind. Before this, the generic kind carried a private copy of both. + // ONE deliberate exception: `strategy: null` is not a bad value on the legacy generic + // endpoint -- it clears the saved strategy and answers 200. Codex, Anthropic, and the + // unified /api/pool/settings route all reject the same null with 400. A future reader + // who sees the 200 must not "fix" it back without deciding that contract first. const codex = async (payload: Record) => { const req = new Request("http://localhost/api/codex-auth/pool-strategy", { method: "PUT", headers: { "Content-Type": "application/json" }, body: JSON.stringify(payload), @@ -615,19 +619,45 @@ describe("legacy pool contract goldens (#wp5)", () => { const resp = await handleCodexAuthAPI(req, new URL(req.url), makeCodexConfig()); return resp!.status; }; - const server = startServer(0); + const previousHome = process.env.OPENCODEX_HOME; + const testDir = mkdtempSync(join(tmpdir(), "ocx-pool-validator-")); + let server: ReturnType | undefined; try { + process.env.OPENCODEX_HOME = testDir; + saveConfig({ + port: 0, + hostname: "127.0.0.1", + defaultProvider: "google-antigravity", + providers: { + "google-antigravity": { adapter: "google", baseUrl: "https://daily-cloudcode-pa.googleapis.com", authMode: "oauth" }, + }, + } as OcxConfig); + server = startServer(0); const oauth = async (payload: Record) => { const res = await fetch(new URL("/api/oauth/accounts/pool", server.url), { method: "PUT", headers: { "content-type": "application/json" }, body: JSON.stringify(payload), }); return res.status; }; - for (const strategy of ["weighted", "", 3, null]) { + const oauthJson = async (payload: Record) => { + const res = await fetch(new URL("/api/oauth/accounts/pool", server.url), { + method: "PUT", headers: { "content-type": "application/json" }, body: JSON.stringify(payload), + }); + return { status: res.status, body: await res.json() as { strategy?: unknown } }; + }; + for (const strategy of ["weighted", "", 3]) { expect(await codex({ strategy })).toBe(400); expect(await oauth({ provider: "anthropic", strategy })).toBe(400); expect(await oauth({ provider: "google-antigravity", strategy })).toBe(400); } + expect(await codex({ strategy: null })).toBe(400); + expect(await oauth({ provider: "anthropic", strategy: null })).toBe(400); + // The generic legacy contract: null clears the saved strategy. Prove the clear actually + // happened -- a 200 that left the old strategy in place would be a silent no-op. + expect(await oauth({ provider: "google-antigravity", strategy: "round-robin" })).toBe(200); + const cleared = await oauthJson({ provider: "google-antigravity", strategy: null }); + expect(cleared.status).toBe(200); + expect(cleared.body).toHaveProperty("strategy", null); // 0 and 101 sit just outside the shared bound; 1 and 100 are the edges that must pass. for (const stickyLimit of [0, 101, 1.5]) { expect(await codex({ stickyLimit })).toBe(400); @@ -638,7 +668,13 @@ describe("legacy pool contract goldens (#wp5)", () => { expect(await codex({ stickyLimit })).toBe(200); } } finally { - await server.stop(true); + try { + await server?.stop(true); + } finally { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(testDir); + } } }); diff --git a/tests/server/bounded-body.test.ts b/tests/server/bounded-body.test.ts index 2a142719ae6..80d9cf0b76e 100644 --- a/tests/server/bounded-body.test.ts +++ b/tests/server/bounded-body.test.ts @@ -22,6 +22,35 @@ function responseFromChunks(...chunks: Uint8Array[]): Response { } describe("readBoundedResponseBody", () => { + test("reportUtf8Validity is honoured on the fatal decode path at EOF", async () => { + const valid = await readBoundedResponseBody(responseFromChunks(encoder.encode('{"ok":true}')), { + fatalUtf8: true, + reportUtf8Validity: true, + }); + expect(valid.utf8Valid).toBe(true); + let caught: unknown; + try { + await readBoundedResponseBody(responseFromChunks(new Uint8Array([0xff])), { + fatalUtf8: true, + reportUtf8Validity: true, + }); + } catch (error) { caught = error; } + expect(boundedBodyDecodeFailure(caught)).toBe("invalid_utf8"); + }); + + test("reportUtf8Validity reports a malformed body at EOF without rejecting it", async () => { + const valid = await readBoundedResponseBody(responseFromChunks(encoder.encode("ok")), { + reportUtf8Validity: true, + }); + expect(valid).toMatchObject({ text: "ok", utf8Valid: true, displaySafe: true, truncated: false }); + const malformed = await readBoundedResponseBody(responseFromChunks(new Uint8Array([0x6f, 0xff])), { + reportUtf8Validity: true, + }); + expect(malformed).toMatchObject({ text: "o\uFFFD", utf8Valid: false, displaySafe: true, truncated: false }); + const unrequested = await readBoundedResponseBody(responseFromChunks(encoder.encode("ok"))); + expect(unrequested.utf8Valid).toBeUndefined(); + }); + test("only actual decoder exceptions carry the decode discriminator", async () => { for (const bytes of [new Uint8Array([0xff]), new Uint8Array([0xe2, 0x82])]) { let caught: unknown;