chore: merge current dev into vision-reasoning takeover branch - #1242
Closed
Wibias wants to merge 375 commits into
Closed
chore: merge current dev into vision-reasoning takeover branch#1242Wibias wants to merge 375 commits into
Wibias wants to merge 375 commits into
Conversation
The numeric requirement spec object has a union shape ({min, step} for
minContextWindow vs {min, max, step} for minQuotaHeadroom), so indexing
[NUMERIC_REQUIREMENT_SPEC[key].max] fails the GUI typecheck. Add
max: undefined to the minContextWindow entry so both members share the
same shape; behavior is unchanged (max={undefined} renders no max
attribute).
… pin The issue-quality workflow's translate job migrated from the actions/ai-inference permission (models: read) to the Copilot CLI permission (copilot-requests: write) in 3a7a72d, but the regression test still pinned models: read. The test was guarding that the job stays job-scoped (no top-level issues:write, no actions:write), so it now accepts either permission name and keeps that guard.
…rmission-test test(ci): accept the Copilot permission in the issue-quality workflow pin
Thread CodexAccountUsabilityOptions through the tier-aware selection paths (pickPriorityPreemption and the round-robin/fill-first branches of pickAlternateCodexAccount), so nativeMainSelectionOnly is honored consistently with the quota path during main-profile draining. Close and block an open Select when disabled flips: the dropdown is no longer rendered and the option buttons are disabled, so a shared controller flipping priorityUpdatingId cannot leave clickable options that silently drop the second update. Strengthen the tests: - exact-selector regression now configures a live pin alongside the priorities and asserts the pin survives, covering the stronger pin-ceiling case; - routing-level regression proves selection options reach the tier and alternate paths; - config-set pin-release test asserts each written order value; - controller-contract test guards the slice windows instead of slicing potentially-empty ranges; - toast-tone no-op test uses the real feedback selectors. Docs and copy accuracy: - switch strings state that existing account-affine threads and in-flight requests keep their captured account, and only new/unbound requests use the selected account's order tier (en/ja/ko/ru); - README qualifies lowest-usage as the quota policy and replaces unconditional thread-pinning claims; - web-dashboard adds the Pool/Direct boundary and full pin-release conditions across locales; - CLI current reports priority-aware selection; - US-English afterward and a split Russian pin-release sentence.
test: replace hardcoded old username with generic user in test fixture
Three findings from react-doctor 0.9.3: - saveProfile/removeProfile read the fetch Response body before checking response.ok, so an HTTP error payload was parsed as success. Route both through readJsonIfOk (which checks res.ok before consuming) and read the structured error body explicitly on the non-OK branch. - the candidate card used an index-derived React key. Draft candidates now carry a stable client-side key (newDraftCandidate) that is stripped by routingProfilePutBody and never reaches the server; add/update keep it. react-doctor --scope changed --base upstream/dev: No issues found. Verified: root+GUI typecheck, GUI lint, 610 GUI tests, 128 focused server tests.
feat(codex): selection order for the account pool
The destination fallback added here lets a canonical preset saved under an unknown name recover its registry-owned discovery policy by transport. The existing coverage proved only that a renamed row picks up a path and query; it never asserted the filter, and it had no negative cases at all. Adds five tests, each driven red once against a real sabotage: - recovers path, query AND filter (literal expectations, not a re-read of the same registry row \u2014 the first formulation compared the spec against itself and stayed green when the filter was replaced) - refuses a name that matches a registry entry whose transport does not - refuses OAuth destinations reached by an unknown name - refuses non-key auth modes, templated base URLs, and overridable destinations - keeps every fallback-eligible absolute discovery URL same-origin with its own base URL The last one closes a real gap: an absolute spec.url overrides the configured base, so a cross-origin one on a fallback-eligible row would send a user's key to an origin they never configured. DeepInfra is the current instance (base /v1/openai, discovery /v1/models, same origin) and the test pins that invariant. Also corrects the OAuth narrative in all five locales: it said six presets plus Copilot while the registry has eight OAuth rows, and the command list omitted ocx login command-code. Pre-existing on dev, corrected here because this change rewrites the surrounding preset counts.
CodeRabbit, Codex connector, and owner review items on #1108: - routing.createProfile key replaces the reused routing.detail on the create button; guide synced; localized in all six locales. - routingProfilePutBody now owns the serialized request contract: mode is part of the returned body (not a caller-side spread), and updates carry expectedRevision for optimistic concurrency. - requirementSelect test helper scopes to the Hard requirements fieldset so health/cost/quota keys in other fieldsets cannot false-match. - management regression test covers the mode:update success path (one save, one catalog refresh) plus a 409 stale-revision conflict. - dry-run error fallback and UnknownEvidenceMode option labels are localized. - parseProfiles validates the complete nested DTO shape so a malformed management response cannot crash the load path; GUI regression test added. - PUT update migrates config references on alias change (disabledModels, subagentModels, injectionModel, shadowCallIntercept, claudeCode) and syncs Claude agent defs, mirroring the /api/combos pattern. - delete confirmation and candidate remove aria-label are localized. react-doctor 0.9.3 --scope changed --base upstream/dev: No issues found. Verified: root+GUI typecheck, GUI lint+i18n lint, 611 GUI tests, 137 focused server tests.
…el-apis feat(providers): add SambaNova and Nebius presets
…s change CodeRabbit follow-up on cf3d4b5: migrateProfileModelReferences rewrote subagentModels and claudeCode.modelMap values but missed the sibling config.subagentModelFallback chain and modelMap keys, which are the inbound ids matched for reroute in src/claude/inbound.ts. Both now follow an alias rename, with the migration regression test extended to cover them.
…y-model-apis feat(providers): add DigitalOcean and Scaleway presets
CodeRabbit follow-up on bcec06d: migrating modelMap keys via Object.fromEntries silently drops a mapping when the map already contains the new alias as a key with a different target. Detect that collision before any mutation and reject the update with 409 alias_reference_conflict so no mapping is lost; regression test covers both keys with different targets.
fix(providers): replay DeepSeek reasoning for opencode-zen (#994)
feat(routing): add routing profiles dashboard editor
… changes
The maintainer GUI-waiver comment ("not touching gui") never re-ran the
enforce-target gate because pull_request_target types do not include issue
comments. Add an issue_comment trigger so the waiver takes effect when
posted, resolving the PR number from the issue payload and falling the
checkout back to the default branch.
Also stop running lint:gui unconditionally in the local prepush hook: it now
runs only when the push touches gui/, mirroring doctor:gui:if-changed. CI
already gated GUI lint behind the changes filter; the local hook now matches.
…tests The CJS validator test asserted the exact base-SHA checkout ref and did not cover the new issue_comment trigger. Update the ref assertion to the fallback form and add a test pinning that a maintainer GUI-waiver comment re-runs the gate via the issue_comment event.
feat(providers): add Nscale and Vultr presets
…olidate bot comments The issue_comment trigger fired for any comment on any issue from any user. Guard the enforce-target job so only maintainer (OWNER/COLLABORATOR/MEMBER) comments on actual PRs re-run the write-capable gate; a comment on a plain issue or from a contributor is skipped, with a defensive in-script re-check matching the job-level if. Also consolidate the PR gate and PR hygiene bot messages into the single opencodex-pr-gate comment. The hygiene workflow now writes its status block into the gate comment (preserving the gate section) instead of posting a second standalone message, and the gate rebuild preserves an existing hygiene block so neither workflow clobbers the other.
…te comment concurrency Two CodeRabbit findings on the consolidated gate comment: - The hygiene block delimiters matched anywhere in the comment body. A contributor-controlled changed filename could embed delimiter text mid-line and corrupt the block boundary on the next rewrite. Anchor both delimiters to complete lines via a shared regex used by the existence check and the replacement, with a regression test for embedded delimiter text. - The gate and hygiene workflows each had their own per-PR concurrency group while both read-modify-write the same consolidated comment. A concurrent gate rebuild and hygiene update could run from stale snapshots and the last write would drop the other's section. Share one per-PR concurrency group between the two workflows and pin it in the workflow tests.
…rifiable checklists Codex-bot review findings on the issue_comment trigger: - The checkout fell back to the repository default branch (main) on issue_comment, which can lag the integration branch the gate enforces. Fall back to dev (the gate's only allowed base) so comment-triggered runs evaluate with the gate's own current scripts. - issue_comment events carry no pull_request.head.sha, so eventHeadSha fell back to the live head and a completed checklist with no recorded completion head was accepted as attesting the current head. Pass an empty eventHeadSha on issue_comment so completionIsStale resets the checklist (fail closed) instead of promoting readiness from an unverifiable attestation. Regression tests cover both.
…reverting concurrent changes Adopts #1203 by @estelledc — the first three commits are theirs, cherry-picked with authorship intact. The approach was right: expose the existing `providers.<id>.contextWindow` / `modelContextWindows` contract at the management and UI layers without touching catalog derivation, which already materializes those values when upstream metadata is absent. Four corrections, all found by independent audit. **Only the selected model was saved.** The drafts map held edits for every model but the PATCH was keyed on `contextModelId`, so a value typed into model A and then abandoned by switching to B vanished — no error, no warning. The PR's own test pinned that as correct. It now sends every model the user typed into. **But "every model that differs" would have been wrong the other way.** The 10s poll can refresh a field while the modal is open; diffing drafts against live state would then call an untouched field dirty and revert someone else's change. Two conditions are required: the user touched it, AND the value differs from what the modal opened with. Both apply to the provider default too, which was previously sent unconditionally and would stamp a stale number over a concurrent update. The snapshot holds canonical numbers, not the raw text. Retyping 64000 as "64,000" is not an edit, and treating it as one would resurrect the same stale-write. When nothing survives the comparison, no PATCH is sent at all and the feedback says so rather than claiming an update. **`Number.isInteger(1e100)` is true.** Both the management validator and the form accepted it; it would persist and serialize into the catalog as an enormous number that can make Codex reject the file. Both now require a safe integer. The default is only validated when touched, so a value inherited from a hand-edited config cannot block an unrelated per-model save. **An override for a model that left live discovery was unreachable.** It sat in the drafts map, absent from the picker, impossible to inspect or clear. Tests: the exact #1073 reproduction is split in two, because a single case setting `modelContextWindows` keeps passing with the provider-wide fallback deleted. Ablations were driven red in their real defect shape rather than as artificially strong mutants — notably, comparing against live `groups` while keeping the touched guard is only visible when a field is edited, reverted, and changed server-side, which the suite now covers. Translated provider docs (ko/ja/ru/zh-cn) described both fields as caps only, which reads as the opposite of the fix for non-English users. Co-authored-by: zhouxun <zhouxun.13@bytedance.com> Closes #1073
Pinning the package NAME to our own still left the VERSION free: `@bitkyc08/opencodex@99.99.99-JaneDoe` is a valid-looking spec, and a semver prerelease identifier can encode anything. That is the same lesson the `/healthz` version taught in round 13 — I applied it there and not here. There is no trusted resolved version available at this call site, so the spec is not rendered at all. `code: ETARGET · notarget: no matching version` already tells a user their requested version does not exist, which is the diagnostic that matters. Both attack inputs are regression tests.
Adopted from PR #1171 by @byongshintv, rebuilt on the current stack. Original closed in favor of this commit. An unlimited A6API key reports zero finite credit totals. Finite-total validation then treated that as a terminal failure and returned before the key could be represented at all, so a perfectly working key looked dead in the dashboard. The unlimited branch now runs ahead of that validation and emits the generic `customWindows` row the GUI and CLI already consume, preserving expiry. It accepts `true`, `1`, and `"true"` for the upstream flag. Two known limitations, stated rather than discovered later: `creditsUsd` and its expiry are not yet surfaced by the GUI — visibility comes from `customWindows` — and neither changes existing behavior for finite keys. Confirmed to fail with the unlimited branch disabled.
… plan Audit found resolveTrustedWindowsPowerShellExe() already resolves and validates the executable through GetSystemDirectoryW. Writing a third SystemRoot/PATH lookup would reintroduce the substitution surface the plan exists to close.
…dable Builds on the contributor fix by luvs01 (#1180), which replaced the USERDOMAIN\USERNAME ACL principal with the effective token SID. Three things that fix left open: The synthetic principal POSIX CI needs lived in windows-secret-acl.ts and was chosen before the injected runner. That ordering made a lookup FAILURE unreachable outside Windows, so the two cases that defend the fail-closed boundary and the timedOutPaths isolation were guarded with `if (process.platform !== "win32") return;` and never ran on Linux or macOS. A test that silently returns on two of three CI platforms is not coverage of a security boundary. The synthetic value moves to the resolver as its own seam, runner selection becomes explicit > synthetic > default, and both guards are gone. sanitizedAclError re-attaches only allow-listed codes, and EACLIDENTITY was not among them. A required-mode harden therefore threw with the cause in the message but `error.code === undefined`, so no caller could branch on "the SID could not be resolved" versus "icacls stalled". The existing test matched the message and hid this. The absence of a name-shaped fallback is now stated as the fix rather than left as an omission. `DOMAIN\User` has a valid shape, but shape is not evidence of the token's subject, and both variables are writable by whatever launched us. runIcacls grants the principal Full Control and then removes inheritance, so a wrong principal either leaves another account holding the secret or strands the file with no usable ACE. An independent audit rejected an earlier draft of this change that restored that fallback for the optional read path. Coverage now runs the sync and async paths across required and optional on every platform, and asserts zero icacls invocations when the environment names a plausible-looking account. Ablation: reverting the runner ordering makes identityCalls 0 and the required harden succeed (2 red); dropping EACLIDENTITY from the allow-list makes both toMatchObject assertions fail (2 red). Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> Closes #1149
The audit named this the ready gate, and it was right to. Every existing test proves a piece — admission, the route allowlist, CORS, the bind scope, the injected port — and none of them prove the thing the feature exists for: that a real `codex app-server`, spawned the way a third-party host spawns it, reaches the proxy without a credential. That seam is between two processes. Not a `bun test` file. The repository does not depend on `@openai/codex`, so a test that skips when it is absent would report green on machines that never ran it. This fails loudly and its output is the evidence. The oracle is a routed model id generated at run time. Codex caches model lists and falls back to a bundled catalog when a refresh fails, so "did model/list succeed" proves nothing — a broken path looks identical to a working one. A name no bundled catalog can contain can only have arrived through our listener. Writing it surfaced two things worth recording. `model/list` reads `model_catalog_json`; it does not call the provider's `/v1/models`, so the first version watched Codex return its five bundled ids while never touching the listener — the exact false-negative shape the unique id exists to expose, pointed at the harness instead of the feature. And a hand-written catalog fixture is a liability: Codex rejects the whole file on any schema mismatch and silently falls back, so the catalog is now built with our own serializer, which also means the script exercises the bytes `ocx sync` writes. Result on Codex 0.146.0, isolated CODEX_HOME with no models_cache.json and OPENCODEX_API_AUTH_TOKEN stripped from the child environment: 9/9, ending with POST /v1/responses observed on the loopback listener. Ablation: pointing base_url at a dead port makes the last check red, so the harness is watching the hop rather than asserting its own setup. Refs #1102
react-doctor's prefer-module-scope-pure-function, and it is right: the function closes over nothing, so rebuilding it on every render is wasted work. The prepush doctor gate rejected the push over it.
…t_index tables Final-gate audit found the cross-table fallback could hand a reasoning content_part the MESSAGE canonical id when both tables held the same output_index, and an out-of-order part event passed through unrepaired. Track raw upstream id -> canonical id at item registration and rewrite part/delta events by exact raw-id match first; the index table remains only for events without a known raw id. A reused index can no longer borrow the sibling item's id, and function_call part events stay untouched.
…d drop the positional guess Final-audit round 2 reproduced two defects in the raw-id rewrite: a flat raw-id map collapsed items sharing one placeholder id into the last item's canonical id, and the index fallback could still hand a function_call part event (or an already-canonical id) the sibling message's identity on a reused index. The map is now keyed by (output_index, rawId), and events that carry an item_id are rewritten only on an exact key match — an unknown id is left alone instead of guessed by position. The index table serves only item_id-less events behind repairMissingTerminalIds, the pre-existing contract. Regression tests pin all three reproductions.
…ervision fix(lifecycle): consolidate identity-bound restart and service install cleanup
Wibias
marked this pull request as ready for review
August 7, 2026 21:10
github-actions
Bot
added
the
chore
github-actions
Bot
added
the
intake: hygiene-blocked
Contributor
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
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
13 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Temporary maintainer integration PR used to merge the current
devtip intomaintainer/vision-reasoning-sidecar, which starts from the contributor head of #1002. This preserves the original implementation while bringing it forward before maintainer fixes are applied.