fix(config): combine user-edit preservation across auto-refresh, desktop apply, and policy probes - #612
Closed
luvs01 wants to merge 17 commits into
Closed
fix(config): combine user-edit preservation across auto-refresh, desktop apply, and policy probes#612luvs01 wants to merge 17 commits into
luvs01 wants to merge 17 commits into
Conversation
…imi-for-coding default (lidge-jun#5403) * release: v2.33.0-preview.20260825 * release: v2.34.0-preview.20260827 * release: v2.36.0-preview.20260829 * fix(release): pass the bump job's permissions through the reusable-workflow call (lidge-jun#3262) Both v2.40.0 release dispatches (33615174183 preview, 33615177849 main) died at startup_failure: a workflow_call cannot grant its callee more than the calling job holds, and dev-version-bump.yml's job declares contents+pull- requests write. lidge-jun#3129 wired the call but never dispatched a release, so this is its first live run. The caller job now declares exactly the callee's two permissions; no other job in release.yml gains anything. Co-authored-by: jun <jun@lidge.dev> (cherry picked from commit 7ce0ba5) * release: set preview channel version 2.48.0-preview.20260908 * release: set main channel version 2.48.0 * chore(release): promote 2.55.0-preview.20260914 to preview Promotes the dev product snapshot 62f0222 to the preview train. The 2.55.0 line carries the lidge-jun#4546 cost-guard work: one send budget per logical request with a shared final-recovery reserve, zero-is-zero refusals with a typed error rather than a synthetic 502, compact and the Kiro inner retries admitted against that budget, a finite send ceiling per root workflow with an interactive reserve a fan-out cannot take, and a healthy detour promoted on transient-hold expiry instead of released cold. The previous preview tip 2.54.0-preview.20260914 is already tagged and published and is outranked by v2.54.0, so it could not be re-released; this is a new candidate rather than a re-cut. * chore(release): promote the verified 2.55.0 product tree to main Same product tree as preview 7bdd1b2 / 2.55.0-preview.20260914, which published successfully with its registry smoke green. Only package.json version differs. * fix(kimi): update Kimi coding registry for K2.8 (adjustable thinking, 1M context, current alias default) - kimi-for-coding is the stable subscription alias Moonshot re-points at each coding release; it now routes to K2.8 Preview. Live GET /coding/v1/models lists only kimi-for-coding[-highspeed], k3, k3-256k; the k2.x ids are retired from the subscription endpoint. - K2.8 accepts the same adjustable low/high/max thinking ladder as k3 (verified live: 350K-token request accepted at max effort; upstream rejects beyond 1,048,576 with 'model token limit: 1048576'). - Bump kimi-for-coding context window to the verified 1M ceiling and advertise text+image input. - Default kimi / kimi-code presets to kimi-for-coding instead of the retired kimi-k2.7-code. - Update provider-registry parity test to match the new verified shape. * fix(kimi): retire k2.x ids from the coding picker and migrate saved configs to kimi-for-coding Address review on lidge-jun#5403: - MODEL_RENAMES gains kimi/kimi-code entries mapping the retired default kimi-k2.7-code to the live kimi-for-coding alias, so saved configs keep working after Moonshot removed the k2.x ids from the subscription endpoint. - The kimi/kimi-code presets seed only ids the endpoint still serves (live /coding/v1/models: kimi-for-coding, k3). Every preset metadata list is live-id only: seeding a retired id there re-armed the rename migration on every boot (lidge-jun#5066 shape), because the residue guard cannot skip a list that holds the retired id without the live alias. - KIMI_CODING_MODELS is replaced by KIMI_CODING_LIVE_MODELS built from KIMI_CODING_K3_MODELS + KIMI_CODING_K28_MODELS, so a future alias added to the K28 constant flows into the picker and every parallel record. - Parity tests now assert defaultModel is kimi-for-coding for both presets (a registry rollback to the retired default would otherwise pass silently). Verified: bun test on model-rename-migration, provider-registry-parity and codex-catalog (432 pass), full tests/providers sweep (only pre-existing proxy-environment timeouts fail, identical on the clean base), tsc clean. * docs(kimi): document the K2.8 coding refresh in the providers guide Address the CodeRabbit finding on lidge-jun#5403: the kimi row in the canonical English providers guide (and the zh-cn translation) now documents the kimi-for-coding default, the 1M context window, the adjustable low/high/max ladder (default max), image input, and the automatic kimi-k2.7-code migration on upgrade. Verified with the required validation: cd docs-site && bun install --frozen-lockfile && bun run build (497 pages, exit 0). * fix(kimi): repair saved K2 coding metadata * fix(kimi): drop the stale no-reasoning classification even when only the replacement id is present Address the CodeRabbit finding on the maintainer's 608d7a2: a saved row can carry kimi-for-coding in noReasoningModels while every retired id is already gone from the row (the pre-K2.8 registry seeded the alias there). The early return in dropRenamedIdsFromList required the retired id, so the stale classification survived and kept the reasoning picker disabled for the live alias. Proceed when the list contains either id and filter both. Verified: model-rename-migration + provider-registry-parity 101 pass, tsc clean; new regression test covers the replacement-id-only row. * fix(kimi): preserve explicit reasoning overrides --------- Co-authored-by: JUN <bitkyc08@gmail.com> Co-authored-by: jun <jun@junui-MacBookPro.local> Co-authored-by: jun <jun@lidge.dev> Co-authored-by: lidge-jun <243035832+lidge-jun@users.noreply.github.com> Co-authored-by: t <a@b.com> Co-authored-by: JUN <jun@lidgeai.com> Co-authored-by: panyuanyuan <panyuanyuan@hetao101.com>
…ations (lidge-jun#5417) * docs: fold the desktop app into a beta section and record install locations * docs: resync translated READMEs for desktop beta
…le publish path (lidge-jun#5405) * feat(release): add the pre-publication asset verifier The release pipeline verified checksums and generated the updater manifest inside attach-release, a job that runs after publication and is skipped on dry-run. The guarantee that gives is "packaging finished before publish"; the guarantee a release needs is "everything about to be published was verified valid before publish". desktop/scripts/verify-release-assets.ts is the verification authority. It derives the expected platform file set from the release workflow's own packaging matrices and the producer tables (build-standalone targets, collect-release-assets bundle names), verifies every recorded checksum against the bytes on disk with the bare-name rule the flat verification directory requires, verifies every updater signature cryptographically against the minisign public key pinned in tauri.conf.json (pure Ed25519 "Ed" mode, the form the Tauri bundler emits; the prehashed "ED" mode fails loudly rather than mis-verifying), generates the updater manifest and parses it back against the files it names, and writes a machine-readable receipt that a later stage can require. bundlesByTarget and platformFiles are exported from their owning scripts, and the standalone target set, archive naming, and executable naming move into scripts/standalone-targets.ts, which the builder and the verifier share — a target added to one side without the other fails verification, not the release. Unit tests in release-desktop-scripts.test.ts cover the derivation against the real workflow, checksum acceptance and the three refusal modes, signature verification with real Ed25519 fixtures (tampered payload, foreign key id, unsupported algorithm), and the full flow including the receipt. They were reviewed statically and are first executed by hosted CI. * feat(release): verify everything before publication and add a resumable publish path The pipeline now has a verify-release job between packaging and publication. It downloads the packaged artifacts, runs the verifier over them — expected platform set, checksums, updater signatures, manifest generation and parse-back — and publishes the verified bundle plus the verification receipt. publish waits for verify-release instead of verifying nothing, and attach-release downloads the verified bundle and refuses to upload unless the receipt names this run's version and commit. Checksum verification and latest.json generation moved out of attach-release into verify-release, so they now run on dry-run too: a dry run proves the same chain a real release relies on. npm and GitHub are not published atomically, so a run that acknowledged npm publication and failed afterwards needs a path that completes the GitHub side without republishing. The new resume-after-npm-publish dispatch input is that path: the preflight requires the version to already exist on npm and refuses to combine with dry-run, the publish step skips npm publish while still emitting the publication receipt the downstream steps gate on, and release creation is idempotent so a release left behind by the failed run is reused for attachment. A successful publish records these recovery instructions in the job summary at the moment they matter. The workflow-contract tests assert the new ordering graph, the absence of verification steps in attach-release, the receipt gate's ordering before the upload, and the recovery branches; the publish-needs assertion in ci-workflows.test.ts follows the new graph. Release automation changed, so this carries the explicit security review the repository requires: no permissions blocks change, no secrets are added or re-scoped, and verification (commit 1) is reviewable separately from publication ordering and the recovery input (this commit).
…lacement (lidge-jun#5406) * fix(service): make ownership state crash-safe and consent-bound Use one default-home authority with an active-home compatibility mirror, token/PID/process-instance locks, fsynced atomic replacement, mirror-first deletion, and authoritative recovery after partial commits. Bind ownership grants to the exact approved owner/install/generation/revision and to re-observed managing-CLI compatibility. OpenCodex 2.60.x, unknown managers, and registrations without protocol 1 remain guests. Local tests, typecheck, builds, installs, and runtime probes were NOT RUN by instruction; the included regressions are for hosted CI. * fix(update): fence replacement and restart with ownership leases Split package replacement, runtime stop, and service restoration authority. Unknown and desktop ownership now block package replacement; Node and Bun read the same authoritative state observations. Hold a shared mutation lease from final subject and liveness validation through replacement, and through dashboard restart. Direct bind takes the same lease, while repair children join by an exact live token. Local tests, typecheck, builds, installs, and runtime probes were NOT RUN by instruction; hosted CI is the verifier. * docs(structure): define authoritative ownership and takeover compatibility Record the authority/mirror commit protocol, consent subject precondition, managing-CLI compatibility floor, independent update authorities, and shared replacement/start lease. Local structure checks were NOT RUN by instruction; hosted CI is the verifier. * fix(service): recover incomplete locks without poisoning delegates Reclaim empty or partial state locks only after the stale grace and dead-PID proof. Canonical delegated mutation tokens are consumed from child environments, cached only while the exact parent lease remains live, and discarded before fresh acquisition. Local tests, typecheck, builds, installs, and runtime probes were NOT RUN by instruction; hosted CI is the verifier. * fix(service): make lease cleanup intent explicit Keep token-specific stale recovery as the owner of uncertain descriptor, owner-file, directory, and release cleanup paths so deterministic hygiene accepts the deliberate best-effort boundaries. Local checks were NOT RUN by instruction. * fix(service): align typed evidence with shared record selector Cast the service-owned evidence union at the shared plain-ESM selector boundary; both carry the same validated record shape, while TypeScript correctly rejects the missing index signature without the explicit boundary cast. Local checks were NOT RUN by instruction; this fixes the exact hosted typecheck diagnostic. * test: isolate corrupt authority and follow shared state paths Reset the corrupt-authority fixture before exercising valid-authority mirror recovery, and point the updater source oracle at the shared active/default path resolver instead of an inlined filename literal. Local tests were NOT RUN by instruction; this fixes the exact hosted shard failures. * test(update): follow the reconciled install-state facade Point the Node launcher, Bun state reader, and source oracle at the install-state-contract surface landed on dev, while keeping one state-record authority implementation underneath it. Local checks were NOT RUN by instruction; this fixes the exact hosted shard diagnostic.
…idge-jun#5410) On Linux, bun run build:local asked for appimage,deb in ONE tauri invocation. When AppImage bundling failed (linuxdeploy missing a host dependency), the invocation died and the deb was never attempted — a contributor following the README got zero artifacts and an error that named a tool they never invoked (observed on a real GNOME desktop, devlog plan 260921 / 120_install_verification.md). Each format now builds in its own invocation, every format is attempted, and the summary reports each outcome beside the artifacts that did build; the exit code is non-zero when any requested format failed. A failing format is retried once with --verbose: at the bundler's default log level the error is a bare "failed to run linuxdeploy" with the tool's stderr discarded, and the verbose pass is the branch where those diagnostics reach the terminal. The release workflow builds its artifacts on its own runner image and is untouched.
Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
…ater table (lidge-jun#5425) dev went red at the union of lidge-jun#5405 and lidge-jun#5391: lane F made the deb a second Linux updater target, so the verifier's derived expected set gained OpenCodex-<version>-linux-amd64.deb.sig, while the test's hand-written oracle still described the earlier world where only the AppImage was signed. Each branch was green alone; the merge was not. The fix is derivation, not list-keeping. The signed set and the manifest platform list in the fixture now come straight from platformFiles — the table that decides which bundles carry the updater key — and the produced payload list comes from the shared standalone target module and the bundle table. A future updater target changes both sides of the assertion by itself. The derivation test keeps its concrete payload anchors (a renamed or dropped bundle should still fail for a human to review) and asserts the rule instead of the roster: a bundle's signature is expected exactly when the updater table names it. Only the two test oracles changed; the verification ordering (checksums, signatures and the manifest all precede publication) is untouched.
* test(service): assert ownership parser behavior * fix(service): fence runtime start and stop ownership * fix(update): hold runtime authority through replacement * docs(runtime): record ownership mutation boundaries * test(cli): follow transactional start prewarm * test(cli): anchor fenced start refusal * test(update): assert lock boundary behavior
…e does not fork execPath (lidge-jun#5418) * fix(cli): probe endpoint liveness in-process so the standalone resolve does not fork execPath Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(cli): prove absence on every loopback host and see refusals inside AggregateError Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * ci: retrigger cross-platform run (macos 1/2 shard hit the 20-minute runner timeout) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(cli): keep a mixed aggregate out of the absence proof --------- Co-authored-by: jun <bitkyc08@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
lidge-jun#5445) * fix(desktop): name the bootstrap script in the policy the webview is actually served * fix(desktop): ship the startup surface as one page the policy can name
* feat(desktop): restore detailed usage in a compact tray popup * fix(desktop): point the popup at the endpoint module this tree has * fix(desktop): let the popup navigation gate read its own constants * docs(devlog): record the tray usage popup as built and run on macOS * fix(gui): clear the React Doctor findings in the tray popup page The pull_request check runs react-doctor at blocking: warning with no comment, review comment, or commit status, so nine findings failed the PR while being readable only inside the run's job summary. Reproduced with the repository's own pinned scan and fixed at the root rather than suppressed: - import useI18n from i18n/shared instead of through the barrel - hoist the hidden-provider and configured-model lookups into Sets in both the chart filter and filterUsage, keeping models === null as "all models" - move the pure incomplete() helper to module scope - key quota rows by an identity quotaWindows() now assigns That identity also closes a bug the index key was hiding: a provider may report two custom windows under one label, and the row key has to stay unique anyway or React reconciles two different windows onto the same row. The effect's post-await writes keep the single active() guard, which checks both disposal and the AbortSignal. The rule cannot see the property through a helper, so the finding carries a scoped suppression that states the evidence. * feat(desktop): give the tray popup the native translucent surface The popup sat next to the WidgetKit widget as a flat #202022 rectangle. macOS now gets the active HUD window material at a 12-point radius and Windows gets Acrylic, both through Tauri's own effects builder, so no new dependency is involved. Linux stays opaque: blur there belongs to the compositor and the effects path does not support it. That asymmetry is exactly where a transparent stylesheet would paint a hole instead of a panel, so one cfg constant, VIBRANT_SURFACE, decides both the native builder and the data-tray-vibrancy hook the page reads. The two cannot disagree because neither restates the other. Transparent windows on macOS require the macos-private-api feature and app.macOSPrivateApi; that forecloses Mac App Store submission, which this Developer ID DMG channel does not use. Also restores rustfmt formatting in popup.rs. The desktop shell job failed on the format step, which gates clippy and the Rust tests, so neither had run since the popup landed on the branch. * feat(gui): restyle the tray popup to the widget's vocabulary The popup and the menu bar widget showed the same numbers in two different visual languages. The page now uses the material behind a thin scrim, rounded tabular numerals with the leading figure carrying each section, secondary-tone labels, and separators only where the subject changes. The scrim is what keeps contrast stable across wallpapers; without it a light desktop washes the labels out. Every rule keeps an opaque default and layers the glass on top, so the stylesheet is correct on a platform that never sets the hook. The appended duplicate rules that had accumulated at the end of the file are folded into the blocks they were overriding. gui-tray-vibrancy-surface.test.ts reads both popup.rs and tray.css and fails if the dataset flag and the attribute selector drift apart. Nothing in either toolchain connects them, and a silent rename would either lose the glass or punch a hole. * fix(gui): keep the quota row identity out of the hardcoded-string gate The GUI lint rule reads a template literal's leading text as UI copy, so both "custom:" and the key-shaped "quota.custom." failed the gates job. A custom window is identified by its own label now, with no literal at all; the de-duplication pass is what keeps that unique, including against the three fixed keys. --------- Co-authored-by: Jyun1998 <yjunwoo14@gmail.com> Co-authored-by: JayYun98 <JayYun98@users.noreply.github.com>
…jun#5462) * fix(desktop): let the tray's left click reach the usage popup Attaching a menu to a tray icon makes the left click open that menu, and the builder never said otherwise. So on macOS and Windows the click never reached on_tray_icon_event in any visible way: the icon showed the menu, and the menu item that opens the popup is Linux-only. The popup lidge-jun#5452 added had no way to open at all on the two platforms where the left click is the whole interaction. Found by clicking it. The change reads correctly either way, which is why static review kept missing it — the handler is there, the event fires, and the wrong surface appears on top. Linux keeps the default. Its StatusNotifier hosts deliver no usable click event, so the menu is the entire interaction there and releasing it would remove the only way in. The pairing is now a test. Nothing in the type system connects .menu() to show_menu_on_left_click(), and the failure is quiet, so tray.rs reads its own production half at compile time and fails if a menu is attached without releasing the click. It slices the source at the test attribute because the assertions quote the call names they look for, and scanning the whole file would find the test's own literals and keep passing after the real calls were gone. * fix(desktop): give every platform a menu path to the usage popup Releasing the left click is not enough on macOS, and the reason is upstream: tray-icon assigns the menu to the NSStatusItem itself, so AppKit pops that menu on mouse-down before the crate's own click handler runs. show_menu_on_left_click(false) sets an ivar that handler reads, and the handler never gets the chance. Verified by reading tray-icon 0.24.2's macOS implementation after a locally built bundle kept showing the menu with the flag set. So the icon click cannot be the only way in. The Show Usage item was Linux-only because Linux hosts differ in whether a click reaches the application at all; that same reasoning applies to macOS for a different cause, and it leaves Windows as the only platform the icon alone would have served. The item is unconditional now. The click release stays: it is correct on Windows, where it does what it says. The guard covers the item too. Platform-gating it once already left two platforms with no way to the popup, so the test now fails if any line that mentions the item sits under a cfg attribute. * fix(desktop): anchor the menu-opened popup on the tray icon The menu handler passed a zero anchor, which popup::geometry clamps into the top-left corner of the work area. That was tolerable while the item was a Linux fallback; now that the menu is the ordinary way in on macOS, a window in the far corner reads as misplaced rather than as a menu. It reads the tray icon's rect and anchors on its centre. A host that cannot report a rect still gets the clamped corner, which is the best answer available there. * docs(devlog): record the glass popup opening from the tray menu Captured from a locally built bundle on macOS, against a runtime started with an isolated configuration home. The dashboard behind the panel shows through the material at the left edge, and the window is anchored under the icon rather than clamped into a corner. * fix(desktop): satisfy clippy and stop the guard matching its own prose Two things the format step had been hiding. It gates clippy and the Rust tests, so neither had run on the popup since it landed. clippy rejects the nested `if` inside the `Focused(false)` arm; it is a match guard now, with the same behaviour. The left-click guard matched the bare call name, and the comments above the menu explain why that flag is inert on macOS — so the assertion found its own prose earlier in the file than the builder and concluded the order was wrong. It matches the call site now. The ordering comparison is gone: it asserted nothing the presence of the call site does not already say. cargo test --lib tray:: and popup:: pass locally.
…jun#5451) * fix(desktop): make the startup surface unable to wait forever On Linux the bootstrap window drew its six phase rows and then never changed again: the headline stayed on the markup's default and every row stayed pending, pixel-identical at 26s and 46s. The page's own handshake deadline is 5s and it never fired, so all three invokes answered — `apply` had been handed a falsy progress and returned at its first line. Three things made that reachable. `startup_snapshot` could answer `None`. The page returns early on a falsy progress, so the one case it cannot render, a shell with no startup state, arrived as silence instead of as a diagnostic. It now always answers with a state, and a missing startup state is reported as a failure the user can read and copy. `run` looked up `AppState` before publishing anything, so the sequence could return having said nothing at all. The first report now happens before any lookup that can fail. Nothing enforced the 30-second ceiling. Several returns inside `run` report no terminal state, and a step that outlives the deadline reports none either, so the surface kept whatever it was last told for as long as the process lived. A guard bound to the run now reports a terminal state when the run does not. It is idempotent and generation-scoped, so it cannot overwrite a result the run reported and one left over from an earlier run cannot fail the retry that replaced it, and it waits a short grace past the ceiling so the run's own failure — which names the endpoint, the home and how the child ended — is the diagnostic on screen rather than the guard's thinner one. `Phase::NotStarted` is new and deliberately absent from the checklist the page draws. The state used to be seeded with `registering`, so a shell that never began rendered exactly like one that had just begun, on the surface whose whole job is to tell those apart. * test(clients): anchor the run oracles on the name, not the signature `desktop-cli-contracts` sliced the sequence at the literal `async fn run(app: &AppHandle)`. Giving `run` its start instant changed that string, `indexOf` answered -1, and `slice(-1)` returns the last character rather than failing — so every index the case computes became -1 and the four ordering assertions compared -1 against -1. It did not go quietly only because the first one is `toBeGreaterThan(-1)`; the rest would have passed on an empty string. Both run oracles now anchor on the function name, which is what they are about, and assert the anchor was found before slicing. A parameter added to the sequence is not a change to the order these cases pin.
lidge-jun#5467) * docs(devlog): land the app runtime ownership record and close the unit The planning record for this batch lived only on an unmerged branch while every lane it describes was already on dev. The decisions, the contradiction rounds, the lane split and the re-audit are the reasons the code looks the way it does, so they belong in the tree beside it. They carry no host detail; 040 says so explicitly, and a sweep for addresses, mesh names, accounts and absolute user paths finds nothing beyond loopback in technical prose. 150 records the outcome and the two things worth carrying forward: the single constant that binds the native translucent surface to the page's CSS hook, and the defect that no amount of reading would have found — tray-icon assigns the menu to the NSStatusItem, so AppKit pops it before the crate's click handler runs, and the popup had no way to open on macOS or Windows. * docs(devlog): record that the privacy scan skips devlog-only commits The scan lives in the gates job, which is gated on the ci paths filter, and that allowlist has no devlog entry. So the one change class where reading devlog matters is the one where the scan never runs, and the aggregate check goes green anyway. This pull request is an instance: sixteen devlog files, proven by a hand sweep rather than by the gate.
…top apply, and policy probes Combines three fork PRs touching live-reconcile/config edit preservation: preserve config edits during catalog auto-refresh (#578), rebase the hand-edit guard after desktop apply (#217), and make claude desktop policy probes async+cached (#381), rebased onto current dev. bun test: config-user-edits + catalog-auto-refresh-scheduler + claude-desktop-first-party + native-claude-desktop-toggle + claude-desktop-policy: 91 pass; 4 claude-desktop-first-party timeouts reproduce identically on clean dev baseline (icacls stall)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
This was referenced Sep 21, 2026
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: luvs01/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 The PR summary covers only a small part of the branch
The 144-file diff also changes ownership, updates, releases, desktop UI, providers, and Responses behavior. Update the review scope and verification evidence before merging.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🔍 Release changes need explicit security review
This workflow adds asset verification and resumable publication. Repository policy requires explicit maintainer security review for release automation; the supplied context does not record it.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
388
to
+399
| tauri::async_runtime::spawn(async move { | ||
| run(&app).await; | ||
| sleep_until(started + DEADLINE + SETTLE_GRACE).await; | ||
| settle( | ||
| &guard, | ||
| started, | ||
| generation, | ||
| format!( | ||
| "the startup sequence did not finish within {} seconds", | ||
| DEADLINE.as_secs() | ||
| ), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🟡 Timed-out desktop startup cannot actually be retried
After the guard publishes Failed, the original run stays marked running until its hung future returns. Retry then exits immediately, leaving users stuck until they restart the app.
Learn more
The deadline guard publishes a retryable Failed state, but it does not stop or supersede the task running run. That task alone clears Startup.running, while begin rejects every retry when this flag remains true. If an awaited operation never returns, the UI offers a Retry button that can never start another attempt. The old run can also publish later phases after the timeout because publication is not generation-gated.
Example: A startup operation remains pending past 32 seconds. The guard displays “Failed” with Retry enabled. Clicking Retry invokes begin, but running is still true, so no new run starts and the screen remains failed.
Recommended fix: Make timeout settlement supersede the active generation and permit a new run. Cancel the old task where possible, clear its running ownership atomically, and reject every later publication from the superseded generation.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+145
to
+147
| /** Non-blocking variant for the long-lived server request path. */ | ||
| export async function probeClaudeDesktopPolicyAsync( | ||
| options: Omit<ClaudeDesktopPolicyProbeOptions, "run"> & { readonly run?: ClaudeDesktopPolicyAsyncProbeRunner } = {}, |
There was a problem hiding this comment.
🟡 Claude Desktop apply still blocks the server event loop
The Windows gateway apply path still invokes the synchronous policy probe. Two registry timeouts can block all proxy requests for four seconds.
Learn more
The new asynchronous probe is used only by the status endpoint. The gateway apply path still calls probeClaudeDesktopPolicy, which uses spawnSync twice when the policy key is absent. Each query has a two-second timeout, and Bun cannot serve unrelated requests while either synchronous child is pending.
Example: On Windows, reg.exe hangs for both the policy and parent-key queries. Applying the gateway profile blocks streaming and management requests for about four seconds, although the new probe was intended to make policy checks non-blocking.
Recommended fix: Await getCachedClaudeDesktopPolicy or probeClaudeDesktopPolicyAsync in the apply path too. Update the injected probe seam to support the asynchronous contract without weakening existing route tests.
Was this helpful? React with 👍 or 👎 to provide feedback.
![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: 7b85f6bf4c
ℹ️ 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".
Comment on lines
388
to
+390
| tauri::async_runtime::spawn(async move { | ||
| run(&app).await; | ||
| sleep_until(started + DEADLINE + SETTLE_GRACE).await; | ||
| settle( |
There was a problem hiding this comment.
When a startup operation actually hangs beyond DEADLINE + SETTLE_GRACE, this guard publishes a Failed state with canRetry: true but leaves Startup.running set. That flag is cleared only after run returns, so for the permanent hang this guard is intended to handle, pressing Retry calls begin, loses the compare-exchange, and does nothing forever. Cancel or invalidate the old generation and release the running flag before enabling retry, while preventing the abandoned run from publishing later.
Useful? React with 👍 / 👎.
Comment on lines
+61
to
+63
| fn mark_shown() { | ||
| let elapsed = process_start().elapsed().as_millis() as u64; | ||
| SHOWN_AT_MS.store(elapsed, Ordering::Release); |
There was a problem hiding this comment.
On the first call to mark_shown, process_start() initializes its Instant at that exact point, so the millisecond-truncated elapsed value is normally 0. Because within_focus_grace also treats 0 as “never shown,” the first focus-loss event is not protected by the new 400 ms grace period and the popup can immediately flash closed on the platforms this guard is meant to handle. Store a nonzero encoded timestamp or track the unset state separately.
Useful? React with 👍 / 👎.
| | `xai` | `openai-chat` | `https://cli-chat-proxy.grok.com/v1` | OAuth uses the separate Grok CLI subscription gateway. The API-key override uses `https://api.x.ai/v1` and may inject Priority Processing. Live-first Grok catalog; `grok-4.5` is the fallback default. | | ||
| | `anthropic` | `anthropic` | `https://api.anthropic.com` | Claude models; live model list fetched from `/v1/models`. | | ||
| | `kimi` | `openai-chat` | `https://api.kimi.com/coding/v1` | Kimi K2.7/K2.6/K2.5 coding models. | | ||
| | `kimi` | `openai-chat` | `https://api.kimi.com/coding/v1` | Kimi Code Plan coding models. Defaults to the stable `kimi-for-coding` alias (currently K2.8 Preview): 1M-token context window, adjustable `low`/`high`/`max` thinking (default `max`), text + image input. Retired `kimi-k2.x` selections are migrated to the alias on upgrade. | |
There was a problem hiding this comment.
This canonical row now says the K2.x IDs are retired and migrated to kimi-for-coding, but the unchanged French, Traditional Chinese, Korean, Turkish, Japanese, and Russian guides/providers.md pages still describe K2.7/K2.6/K2.5 as the available coding models. Users selecting those locales therefore receive instructions that contradict both this page and the registry change; update those six translated rows alongside the English and Simplified Chinese versions.
AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
Owner
Author
|
Closing: this change has been submitted to the upstream repository as a separate pull request. The fork copy is closed to keep the active evaluation list focused on work not yet upstreamed. |
Owner
Author
|
Closing: this change has been submitted to the upstream repository as a separate pull request. The fork copy is closed to keep the active evaluation list focused on work not yet upstreamed. |
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
Combines three overlapping fork PRs around config user-edit preservation and the Claude desktop apply path:
armDetachedConfigBaseline+ reconcile changes.adoptPersistedClaudeCode.Tests
config-user-edits+catalog-auto-refresh-scheduler+claude-desktop-first-party+native-claude-desktop-toggle+claude-desktop-policy— 91 passclaude-desktop-first-partytimeouts reproduce identically on the clean dev baseline (transient icacls stall on this host), not introduced by this changeUpstream status
Upstream PR creation is currently denied for this account (
CreatePullRequestpermission error, repo-specific). Will submit upstream when the restriction lifts; supersedes #217, #381, #578.