Merge remote-tracking branch 'upstream/dev' into review/pr-744-antigravity

Author: Wibias

.github/workflows/ci.yml

@@ -40,30 +40,125 @@ on:
 permissions:
   contents: read
 
+# Retrigger CI after dir-fsync / oauth deadline follow-ups (tip 34a1ac46).
 concurrency:
   group: cross-platform-ci-${{ github.ref }}
   cancel-in-progress: true
 
 jobs:
+  # Which Windows runner this run is allowed to use.
+  #
+  # READ THIS BEFORE TREATING IT AS A SECURITY BOUNDARY: it is not one.
+  #
+  # On `pull_request` this workflow is loaded from the PR head, so the `case`
+  # below is owned by the proposed patch exactly like an `if:` guard would be.
+  # A hostile PR can delete the branch and hardcode the self-hosted labels into
+  # `$GITHUB_OUTPUT`, and `runs-on` will honour it. That this job runs on
+  # `ubuntu-latest` changes nothing — the untrusted part is its OUTPUT, not its
+  # host. `.github/workflows/ci.yml` is in this workflow's `pull_request.paths`,
+  # so such an edit triggers its own run.
+  #
+  # What actually keeps untrusted code off a self-hosted runner lives OUTSIDE
+  # this file, where a PR cannot reach it: the fork-PR approval policy
+  # (`all_external_contributors`) and the judgement of whoever clicks approve.
+  # Runner groups would be the other lever, but they are an organisation
+  # feature and this repository is user-owned, so the approval policy is the
+  # only one available here. GitHub's own guidance is to avoid self-hosted
+  # runners on public repositories for this reason.
+  #
+  # So read the routing below as a COST control that keeps honest pull requests
+  # on GitHub-hosted runners, not as a guarantee about hostile ones.
+  #
+  # `push` on dev/main/preview requires the push permission, and
+  # `workflow_dispatch` requires write access, so both carry a trusted author.
+  # A trusted author is not audited code: merging a contributor PR into `dev`
+  # fires `push`, and its dependencies and postinstall hooks then run here.
+  select-windows-runner:
+    name: select windows runner
+    runs-on: ubuntu-latest
+    timeout-minutes: 2
+    outputs:
+      runner: ${{ steps.pick.outputs.runner }}
+      label: ${{ steps.pick.outputs.label }}
+    steps:
+      - name: Pick runner
+        id: pick
+        env:
+          # Read through env rather than interpolating directly into the script:
+          # `github.event_name` is a fixed vocabulary, but keeping the habit means
+          # no future edit here can grow a script-injection sink.
+          EVENT_NAME: ${{ github.event_name }}
+          USE_SELF_HOSTED: ${{ vars.OCX_SELF_HOSTED_WINDOWS }}
+        shell: bash
+        run: |
+          set -euo pipefail
+          trusted=no
+          case "$EVENT_NAME" in
+            push|workflow_dispatch) trusted=yes ;;
+          esac
+
+          # Repository variable OCX_SELF_HOSTED_WINDOWS is an OPERATIONAL switch,
+          # not a security control: a PR that rewrites this script ignores it for
+          # the same reason it ignores the event check above. Its job is to keep CI
+          # working when the box is off or busy. Anything other than `1` —
+          # including unset, the state before a runner exists — falls back to
+          # windows-latest.
+          if [ "$trusted" = "yes" ] && [ "${USE_SELF_HOSTED:-}" = "1" ]; then
+            echo 'runner=["self-hosted","Windows","X64","ocx-home"]' >> "$GITHUB_OUTPUT"
+            echo 'label=self-hosted (ocx-home)' >> "$GITHUB_OUTPUT"
+          else
+            echo 'runner="windows-latest"' >> "$GITHUB_OUTPUT"
+            echo 'label=windows-latest' >> "$GITHUB_OUTPUT"
+          fi
+
   test:
-    name: ${{ matrix.os }}
-    runs-on: ${{ matrix.os }}
+    name: ${{ matrix.name }}
+    needs: select-windows-runner
+    runs-on: ${{ matrix.runner }}
     # Windows dominates this matrix: on run 30459554635 the same suite took
     # ubuntu 4.6min / macos 5.6min / windows 11.8min. Against the previous
     # 12-minute ceiling that left ~12s of headroom, so runner variance decided
     # the result rather than the code under review — #711's rerun finished at
     # 11.8min and passed while #653's was killed at 12.0min (issue #717).
     # A cancelled job renders as `fail` in `gh pr checks`, so that flakiness
-    # reads as a broken PR. 20 minutes keeps a green Windows run green with
-    # real margin; it is not a licence for the suite to grow into it. If
-    # Windows approaches this ceiling too, fix the 2.5x platform gap instead
-    # of raising the number again.
-    timeout-minutes: 20
+    # reads as a broken PR. After the 2026-08-01 state-store admission merge,
+    # Windows under `bun test --isolate` on #827 hit the 20-minute kill while
+    # still green mid-suite (~19m of tests). 30 minutes is the margin for that
+    # tip — not a licence to absorb hung tests (see oauth mutation waitMs
+    # unref fix). Shrink the suite / close the platform gap rather than raising
+    # this again for ordinary variance.
+    timeout-minutes: 30
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, windows-latest, macos-latest]
+        # `name` is spelled out rather than derived from `runner`: a runner given
+        # as a label array renders as "self-hosted Windows X64 ocx-home", so the
+        # check name would change with the routing and break any branch
+        # protection rule that names it. Keeping `windows` fixed means the
+        # required check stays the same whichever machine served it.
+        include:
+          - name: ubuntu
+            runner: ubuntu-latest
+          - name: windows
+            runner: ${{ fromJSON(needs.select-windows-runner.outputs.runner) }}
+          - name: macos
+            runner: macos-latest
     steps:
+      - name: Show selected runner
+        if: matrix.name == 'windows'
+        shell: bash
+        run: echo "windows leg on ${{ needs.select-windows-runner.outputs.label }}"
+
+      # A self-hosted runner keeps its working directory between jobs. Without an
+      # explicit wipe, a file deleted in the commit under test survives on disk
+      # and the suite passes against a tree that no longer exists in git.
+      # `--ephemeral` registration de-registers the runner after each job but does
+      # not clean the workspace, so this step is what makes the checkout honest.
+      - name: Clean workspace (self-hosted only)
+        if: runner.environment == 'self-hosted'
+        shell: bash
+        run: git clean -xffd . || true
+
       - name: Checkout
         uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
 
@@ -113,6 +208,10 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
+        # Deliberately NOT routed to the self-hosted box. This job runs
+        # `npm install -g`, which writes into the machine's global prefix and
+        # would leave an `ocx` on a maintainer's personal PATH. It is an
+        # 8-minute job, so there is nothing to win by moving it.
         os: [ubuntu-latest, windows-latest, macos-latest]
     steps:
       - name: Checkout

bin/ocx.mjs

@@ -250,12 +250,40 @@ function runNpmSelfUpdate() {
       try {
         const svcArgs = serviceReinstallArgs();
         const svc = spawnSync(process.execPath, svcArgs, { stdio: "inherit", windowsHide: true });
-        if (svc.status !== 0) {
+        let needDirectStart = svc.status !== 0;
+        if (!needDirectStart) {
+          // Exit 0 can still leave stale/missing assets that never bring the proxy
+          // back — match the GUI/CLI fallthrough so /healthz is not left dead.
+          try {
+            const st = spawnSync(process.execPath, [launcher, "status", "--json"], {
+              encoding: "utf8",
+              timeout: 20_000,
+              windowsHide: true,
+            });
+            if (st.status === 0 && typeof st.stdout === "string" && st.stdout.trim()) {
+              const parsed = JSON.parse(st.stdout);
+              const proxyUp = parsed?.proxy?.running === true || parsed?.proxy?.health?.ok === true;
+              const viable = parsed?.startup?.serviceViable === true;
+              if (!proxyUp && !viable) needDirectStart = true;
+            } else {
+              // status failed or empty — fail closed to direct start (match CLI).
+              needDirectStart = true;
+            }
+          } catch {
+            needDirectStart = true;
+          }
+        }
+        if (needDirectStart) {
           // On Windows, schtasks /create requires elevation. The launcher inherits the
           // user's (non-admin) token, so the service reinstall can fail with access
-          // denied. Fall back to a direct detached proxy start so the update never
-          // leaves the user without a running proxy.
-          console.warn("opencodex: service refresh failed — starting the proxy directly instead.");
+          // denied — or exit 0 while leaving a non-viable manager. Fall back to a
+          // direct detached proxy start so the update never leaves the user without
+          // a running proxy.
+          console.warn(
+            svc.status === 0
+              ? "opencodex: service refresh left a non-viable manager — starting the proxy directly instead."
+              : "opencodex: service refresh failed — starting the proxy directly instead.",
+          );
           console.warn("  Run 'ocx service install' as administrator to refresh the background service.");
           const env = { ...process.env };
           delete env.OCX_SERVICE;

gui/src/client-resource.ts

@@ -40,6 +40,12 @@ type Store<T> = {
   /** Store-level visibilitychange handler, installed only while this store polls. */
   visibilityListener: (() => void) | null;
   generation: number;
+  /**
+   * Set when `setClientResourceData` publishes while nobody is subscribed (session-cache
+   * seed). The first subscribe must quiet-revalidate; otherwise mount never fetches and
+   * seeded rows stay indefinitely stale with `lastAttemptOk: true`.
+   */
+  seedNeedsRevalidate: boolean;
 };
 
 /**
@@ -80,6 +86,7 @@ function getStore<T>(key: string): Store<T> {
       inflightOwner: null,
       visibilityListener: null,
       generation: 0,
+      seedNeedsRevalidate: false,
     };
     stores.set(key, store);
   }
@@ -223,6 +230,8 @@ async function runFetch<T>(
   try {
     const data = await fetcher(controller.signal);
     if (gen !== store.generation || controller.signal.aborted) return;
+    // Cleared on settle (not at subscribe) so StrictMode's aborted first mount still revalidates.
+    store.seedNeedsRevalidate = false;
     store.snapshot = {
       data,
       error: undefined,
@@ -233,6 +242,7 @@ async function runFetch<T>(
     };
   } catch (error) {
     if (gen !== store.generation || controller.signal.aborted) return;
+    store.seedNeedsRevalidate = false;
     store.snapshot = {
       ...store.snapshot,
       // Normalize so a loader that rejects with `undefined` still reads as a failure.
@@ -298,9 +308,12 @@ function subscribeResource<T>(
   store.fetcherByListener.set(onStoreChange, fetcher);
   store.subscriberCount++;
 
-  // Cold start only — keep cached data across transient 0→1 resubscribe gaps.
-  if (store.subscriberCount === 1 && store.snapshot.data === undefined) {
-    void runFetch(store, fetcher, { replaceInflight: true, owner: onStoreChange });
+  // Cold start, or a pre-subscribe seed that still needs a network check. Keep
+  // cached data across transient 0→1 resubscribe gaps when neither applies.
+  if (store.subscriberCount === 1) {
+    if (store.snapshot.data === undefined || store.seedNeedsRevalidate) {
+      void runFetch(store, fetcher, { replaceInflight: true, owner: onStoreChange });
+    }
   }
   recomputePoll(store);
 
@@ -328,7 +341,7 @@ function subscribeResource<T>(
 }
 
 /** Module-level fetch cache with useSyncExternalStore subscriptions (no fetch in useEffect). */
-export interface ClientResourceOptions {
+export interface ClientResourceOptions<T = unknown> {
   pollMs?: number;
   enabled?: boolean;
   /**
@@ -337,18 +350,34 @@ export interface ClientResourceOptions {
    * happening off-screen — a restarting server, for instance.
    */
   pauseWhenHidden?: boolean;
+  /**
+   * Optional seed applied once before the first subscribe (session-cache revisit).
+   * Lives in the store — not a render-time ref — so React Compiler / react-hooks/refs
+   * stay quiet while the mount fetch still quiet-revalidates via `seedNeedsRevalidate`.
+   */
+  initialData?: T;
+}
+
+/** Seed an empty, unsubscribed store. No-ops when data already exists or someone is listening. */
+function seedClientResourceIfEmpty<T>(key: string, data: T): void {
+  const store = getStore<T>(key);
+  if (store.subscriberCount !== 0 || store.snapshot.data !== undefined) return;
+  setClientResourceData(key, data);
 }
 
 export function useClientResource<T>(
   key: string,
   fetcher: (signal: AbortSignal) => Promise<T>,
-  options?: ClientResourceOptions,
+  options?: ClientResourceOptions<T>,
 ): ResourceSnapshot<T> & { refresh: (opts?: { forceLoading?: boolean }) => void } {
   const enabled = options?.enabled !== false;
   const pollMs = options?.pollMs;
   // Default true: a background tab has nobody reading the paint. Opt out for polls that
   // must keep running while hidden, such as waiting for a restarted server to answer.
   const pauseWhenHidden = options?.pauseWhenHidden !== false;
+  if (enabled && options?.initialData !== undefined) {
+    seedClientResourceIfEmpty(key, options.initialData);
+  }
   const fetcherRef = useRef(fetcher);
   // Sync latest fetcher every commit. No dep array on purpose: inline fetchers are
   // reallocated every render; listing them would re-subscribe forever.
@@ -411,7 +440,7 @@ export function useKeyedClientResource<T>(
   key: string,
   deps: readonly unknown[],
   load: (signal: AbortSignal) => Promise<T>,
-  options?: ClientResourceOptions,
+  options?: ClientResourceOptions<T>,
 ): ResourceSnapshot<T> & { refresh: (opts?: { forceLoading?: boolean }) => void } {
   const resource = useClientResource(key, load, options);
   const prevDepsRef = useRef<readonly unknown[] | null>(null);
@@ -447,6 +476,9 @@ export function setClientResourceData<T>(key: string, data: T) {
     hasSucceeded: true,
     lastAttemptOk: true,
   };
+  // Only pre-subscribe seeds need a follow-up fetch. Live publishers (mutation
+  // results) already hold the fresh value and must not schedule a redundant GET.
+  store.seedNeedsRevalidate = store.subscriberCount === 0;
   emit(store);
 }
 

gui/src/components/CodexAccountPool.tsx

@@ -11,7 +11,6 @@ import CodexPoolStrategySetting from "./CodexPoolStrategySetting";
 import { useCodexAutoSwitch } from "../hooks/useCodexAutoSwitch";
 import { readJsonIfOk } from "../fetch-json";
 import { CodexAccountPoolCards, CodexAccountPoolReauthBanner } from "./codex-account-pool-cards";
-import { DataSurfaceStatus } from "./data-surface";
 import { CodexAccountSwitchModal } from "./codex-account-switch-modal";
 import { CodexAccountResetModal } from "./codex-account-reset-modal";
 import { CodexAccountPoolLoadStates, CodexAccountPoolMainCard, CodexAccountPoolPageHead } from "./codex-account-pool-main-card";
@@ -57,10 +56,6 @@ export default function CodexAccountPool({ apiBase, accountModeState = null, ban
   const ownController = useCodexAccountPool(apiBase, !injectedController);
   const controller = injectedController ?? ownController;
   const { accounts, activeId, loadState, switchingId, pauseUpdatingId, pausingExhausted, load } = controller;
-  // The controller owns the visible progress signal. A forced quota refresh keeps rows on screen
-  // and can take ~1s (longer when the server has to refill per-account quota), so without this the
-  // wait is invisible; `refreshingQuota` below stays local because it only guards its own button.
-  const { refreshing } = controller;
   const [confirm, setConfirm] = useState<CodexAccountEntry | null>(null);
   const [showAdd, setShowAdd] = useState(false);
   const [reauthId, setReauthId] = useState<string | null>(null);
@@ -277,12 +272,6 @@ export default function CodexAccountPool({ apiBase, accountModeState = null, ban
         onRetry={() => { void load(); }}
       />
 
-      {/* Revalidation over existing rows. The cold branch above already owns a live region, so
-          this only renders once rows are on screen, keeping one announcement per transition. */}
-      {refreshing && accounts.length > 0 && (
-        <DataSurfaceStatus live={loadState !== "error"}>{t("common.loading")}</DataSurfaceStatus>
-      )}
-
       {!(loadState === "loading" && accounts.length === 0) && (
         <>
           <CodexAccountPoolMainCard

gui/src/components/apikeys-workspace/ApiKeysWorkspace.tsx

@@ -39,6 +39,8 @@ export interface ApiKeysWorkspaceProps {
   copied: boolean;
   filteredModels: ExternalModelRow[];
   modelsLoading: boolean;
+  /** Quiet revalidation / retry over rows already on screen — not a skeleton. */
+  modelsRefreshing?: boolean;
   modelsLoadFailed: boolean;
   modelCount: number;
   hasModelData: boolean;
@@ -76,6 +78,7 @@ export default function ApiKeysWorkspace({
   copied,
   filteredModels,
   modelsLoading,
+  modelsRefreshing = false,
   modelsLoadFailed,
   modelCount,
   hasModelData,
@@ -420,6 +423,7 @@ export default function ApiKeysWorkspace({
                 <ApiKeysModelsPanel
                   filteredModels={filteredModels}
                   modelsLoading={modelsLoading}
+                  modelsRefreshing={modelsRefreshing}
                   modelsLoadFailed={modelsLoadFailed}
                   modelCount={modelCount}
                   hasModelData={hasModelData}