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.
|
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 |
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. |
This was referenced Sep 21, 2026
|
✅ Deterministic PR hygiene checks passed. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 PR description omits most changed subsystems
The summary mentions only Cursor echo handling. The diff also changes release automation, runtime ownership, updates, provider migration, and desktop UI without corresponding verification details.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🔍 Required release and cross-platform validation is undocumented
Repository rules require broader validation for release and packaging changes. The description lists only Cursor tests despite substantial Linux, macOS, Windows, workflow, and desktop changes.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🔍 Release changes require explicit security review
This PR changes publication recovery, artifact verification, updater signatures, and packaging. Repository policy requires explicit maintainer-sponsored security review before merge.
Was this helpful? React with 👍 or 👎 to provide feedback.
| endpoint: ProxyEndpoint, | ||
| anchor: PhysicalPosition<f64>, | ||
| ) -> tauri::Result<()> { | ||
| let popup = ensure(app, endpoint)?; |
There was a problem hiding this comment.
🟡 Proxy restarts can strand the tray popup on a stale endpoint
The popup’s navigation guard retains the first proxy endpoint. After a port change, it rejects the new endpoint and leaves usage stale or blank.
Learn more
The popup window is reused by ensure, but its on_navigation callback permanently captures the endpoint passed when the window was created. show detects that the current URL differs and navigates, yet that navigation is evaluated against the old endpoint and denied.
Example: The popup is created while the proxy listens on port 10100. After a restart selects port 10101, opening the tray calls show with 10101, but the existing callback still accepts only 10100, so the popup cannot load the live dashboard.
Recommended fix: Track the popup’s current endpoint and recreate the window when it changes, or make the navigation policy consult mutable endpoint state. Add a regression test covering reuse after a port change.
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: 22333a8267
ℹ️ 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
+392
to
+397
| const initialRuntimeObservation = readCurrentRuntimeTarget(); | ||
| const initialRuntimeTarget = initialRuntimeObservation.kind === "target" ? initialRuntimeObservation.target : null; | ||
| let sawRuntimePort = initialRuntimeTarget !== null; | ||
| let sawRuntimeHostname = initialRuntimeTarget?.hostname !== null && initialRuntimeTarget?.hostname !== undefined; | ||
| if (initialRuntimeTarget) { | ||
| bakePort = initialRuntimeTarget.port; |
There was a problem hiding this comment.
In the npm/pnpm update path, any structurally valid runtime-port.json now sets sawRuntimePort, even when its PID is dead. If a crashed proxy left this file behind and the configured port subsequently changed, the config fallback is skipped and the post-update service repair or direct restart uses the stale bakePort, bringing the proxy back on the wrong endpoint. Preserve the previous live-PID check (as the Bun updater still does in src/update/index.ts) or otherwise prove the record is live before treating its port as authoritative.
Useful? React with 👍 / 👎.
Comment on lines
388
to
+398
| 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.
When a startup operation outlives the deadline, this guard only publishes a failed state; it neither cancels/supersedes run nor clears Startup.running. The failure enables Retry, but begin rejects that retry at its compare_exchange, and if the original operation eventually returns it can continue emitting phases and overwrite the terminal failure. The timeout path should invalidate or abort the old generation and release the running slot before presenting a usable Retry action.
Useful? React with 👍 / 👎.
Comment on lines
+205
to
+207
| .user_agent(&window::webview_user_agent()) | ||
| .initialization_script(initialization_script()) | ||
| .on_navigation(popup_navigation_allowed(endpoint, app_handle.clone())) |
There was a problem hiding this comment.
The popup's navigation callback permanently captures the endpoint used when the window was first created. If the popup is opened during a failed startup and a retry later resolves another port, ensure reuses the existing window and show tries to navigate it to the new endpoint, but this callback rejects that new origin and leaves the popup on the old/dead runtime. Resolve the current endpoint inside the callback or recreate/reconfigure the popup when its endpoint changes.
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. |
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 two overlapping fork PRs that both rework
src/adapters/cursor/envelope-echo.ts:Tests
bun test tests/providers/cursor/cursor-envelope-echo-retry.test.ts— 47 passcursor-adapter.test.ts+cursor-tool-continuation.test.ts— 54 passUpstream status
Upstream PR creation is currently denied for this account (
CreatePullRequestpermission error, repo-specific). Will submit upstream when the restriction lifts; supersedes #347 and #380.