Skip to content

feat(windows): add tray controls and restart-safety diagnostics - #306

Merged
lidge-jun merged 11 commits into
lidge-jun:devfrom
himomohi:fix/autostart-health-visibility
Jul 23, 2026
Merged

feat(windows): add tray controls and restart-safety diagnostics#306
lidge-jun merged 11 commits into
lidge-jun:devfrom
himomohi:fix/autostart-health-visibility

Conversation

@himomohi

@himomohi himomohi commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • detect restart-unsafe Codex routing and fail closed for stale, disabled, conflicting, or malformed lifecycle state
  • add a Windows-only notification-area controller with one-click Start proxy, Stop and restore native, Restart, Open dashboard, Open logs, and Exit tray actions
  • add ocx tray install|start|stop|status|uninstall and GUI controls for per-user login registration under HKCU\...\Run
  • keep tray registration across package updates while preserving the user's prior running/stopped state; uninstall removes the owned tray registration
  • expose secret-free startup/tray health through the management API and a localized Startup safety dashboard page
  • distinguish OCX-owned routing, custom local gateways, remote gateways, and CLI-only Windows shims so native GPT routing is never falsely marked protected

Windows persistence and security

  • copies the PowerShell tray controller into the ACL-hardened OPENCODEX_HOME; the mutable package source is never the persisted executable
  • uses fixed argv throughout (no Invoke-Expression, cmd /c, taskkill, or user-supplied command strings)
  • validates exact Run-value ownership and treats missing, foreign, unreadable, or wrong-typed registry values as stale
  • state files cannot select executable or deletion paths: start, stop, repair, and rollback resolve through the current trusted installation
  • uses a per-home mutex/event pair to enforce one tray instance and graceful shutdown
  • the tray is a controller only; login registration is not counted as proxy restart protection

Verification

  • bun run typecheck
  • bun run lint:gui
  • bun run build:gui
  • bun run privacy:scan
  • git diff --check
  • bun test --isolate ./tests/service.test.ts — 34 passed, 0 failed
  • bun test --isolate ./tests/autostart-health.test.ts ./tests/windows-tray.test.ts — 19 passed, 0 failed
  • bun test --isolate ./tests/cli-help.test.ts — 11 passed, 0 failed
  • npm pack --json --dry-run — includes src/tray/windows.ts, src/tray/windows-tray.ps1, management control, and the built GUI
  • real Windows lifecycle in isolated OPENCODEX_HOME/CODEX_HOME: install/start/stop/restart/uninstall, parent-process survival, PID-changing proxy restart, Run-key cleanup, live-unregistered-tray recovery, ACL inspection, and 12 concurrent starts producing one tray instance
  • real Unicode/metacharacter path (%TEMP% ! ^ ( ) & 검증) registration, launch, health, and uninstall
  • Codex in-app Browser: GUI install/show/exit/uninstall/reinstall, stale-registry fail-closed state, one-click stale-tray recovery, copy buttons, and narrow viewport
  • adversarial gates: Browser 96/100, Windows lifecycle 96/100, tray 94/100, API/security 96/100, final independent code gate 97/100; no remaining code blocker

A loaded-host aggregate run completed with 3462 passed, 3 skipped, 67 failed, and 7 errors. The failures were dominated by concurrent Windows ACL/temp-directory collisions and existing fixed-duration timeout tests; the affected focused suites passed immediately in isolation. Fresh GitHub CI is the aggregate merge gate.

Checklist

  • Scope and Windows-only behavior are documented.
  • All update lanes preserve or safely stop tray persistence.
  • Security-sensitive registry, executable-path, ACL, and rollback boundaries received adversarial review.
  • No credentials are stored in tray state, Run values, process arguments, diagnostics, or API responses.

Summary by CodeRabbit

Summary

  • New Features

    • Added Startup Safety page with restart-safety diagnostics, Windows tray indicators, and copyable recovery commands.
    • Added GET /api/startup-health for secret-free startup health reporting (also reflected in settings/status).
    • Introduced Windows tray management via ocx tray (install/start/stop/status/uninstall).
  • Documentation

    • Updated Web Dashboard guides and API documentation for Startup Safety.
  • Bug Fixes / Improvements

    • Hardened Windows tray lifecycle during updates to minimize disruption.
  • Tests

    • Expanded automated tests covering startup-health logic, CLI status output, and tray/shim/service diagnostics.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change adds startup-health classification for Codex routing, services, and shims; exposes diagnostics through APIs and CLI commands; adds a localized dashboard with risk indicators, tray controls, and recovery commands; preserves the Windows tray during updates; and updates documentation.

Changes

Startup safety diagnostics

Layer / File(s) Summary
Health model and platform probes
src/codex/*, src/service.ts, tests/autostart-health.test.ts, tests/service.test.ts, tests/codex-shim.test.ts
Routing, service, and shim state are classified into native, protected, or at-risk startup health with fail-closed diagnostics and repair recommendations.
API, cache, and CLI integration
src/server/*, src/cli/*, tests/cli-status-json.test.ts
Startup health is cached, exposed through management endpoints, included in CLI status JSON, and printed by status and doctor commands.

Windows tray lifecycle

Layer / File(s) Summary
Tray persistence and controller
src/tray/*, src/server/windows-tray-control.ts, tests/windows-tray.test.ts
Windows tray installation, registry persistence, status checks, PowerShell control, lifecycle actions, and command-safety validation are added.
Update coordination
bin/ocx.mjs, src/update/*
Self-update and GUI update flows stop the tray before package replacement, refresh installation after success, and restore the tray on failure when it was previously running.

Dashboard and documentation

Layer / File(s) Summary
Startup dashboard
gui/src/App.tsx, gui/src/pages/*, gui/src/styles.css, gui/src/i18n/*
A localized Startup page displays routing, service, shim, tray, stale-data, and recovery-command state, with navigation risk indicators and responsive styling.
Dashboard and API documentation
docs-site/src/content/docs/guides/*/web-dashboard.md, structure/05_gui-and-management-api.md
Documentation describes startup-safety diagnostics, clarified Codex autostart behavior, Windows tray scope, recovery commands, and GET /api/startup-health.

Estimated code review effort: 5 (Critical) | ~90 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Dashboard
  participant ManagementAPI
  participant StartupHealthCache
  participant CodexRouting
  participant ServiceDiagnostic
  participant CodexShim
  Dashboard->>ManagementAPI: GET /api/startup-health
  ManagementAPI->>StartupHealthCache: getCachedStartupHealth(config)
  StartupHealthCache->>CodexRouting: classify current routing
  StartupHealthCache->>ServiceDiagnostic: diagnose service state
  StartupHealthCache->>CodexShim: diagnose shim state
  CodexRouting-->>StartupHealthCache: routing status
  ServiceDiagnostic-->>StartupHealthCache: service status
  CodexShim-->>StartupHealthCache: shim status
  StartupHealthCache-->>ManagementAPI: startup health DTO
  ManagementAPI-->>Dashboard: status and recovery commands
Loading

Suggested reviewers: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.66% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main Windows tray controls and restart-safety diagnostics added in this PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gui/src/App.tsx`:
- Around line 141-155: Update the startup health effect containing
readStartupHealth to poll /api/startup-health at the same 30-second interval
used by fetchRuntimeVersion, while preserving cancellation cleanup and the
existing last-known state behavior on fetch failures. Ensure the interval is
cleared on unmount so startupAtRisk refreshes throughout the SPA session.

In `@gui/src/i18n/de.ts`:
- Line 4: Update the nav.startup translation to describe the startup-safety
diagnostic page rather than autostart: in gui/src/i18n/de.ts at lines 4-4, use
“Startsicherheit” or an equivalent German label; in gui/src/i18n/ru.ts at lines
6-6, use “Безопасность запуска” or an equivalent Russian label.

In `@gui/src/i18n/ja.ts`:
- Line 66: Update the startup.stale translation to convey “outdated” or
“requires updating,” not corruption, in gui/src/i18n/ja.ts lines 66-66,
gui/src/i18n/ko.ts lines 61-61, and gui/src/i18n/ru.ts lines 66-66; use natural
language for each locale, with the Russian wording equivalent to “Устарел.”

In `@src/cli/doctor.ts`:
- Around line 412-414: Update the at-risk recovery guidance in
startupHealthSummary() and the direct doctor hint to render
StartupHealth.recommendedCommand instead of startup.commands.installService,
preserving the classifier-selected remediation such as codex-shim install. In
src/cli/doctor.ts lines 335-338 and 412-414, apply this replacement; in
src/cli/index.ts line 452, retain the existing display with no direct change
because it is corrected through the shared summary.

In `@src/cli/status.ts`:
- Line 173: Update startupHealthSummary() to display health.recommendedCommand
instead of hard-coding “ocx service install” for at-risk states, preserving the
classifier-selected recovery command across supported platforms.
- Around line 7-9: Replace the duplicated startup signal collection in the CLI
status JSON flow (around lines 123-133) with the shared
collectStartupHealth(config) collector. Remove the now-unused
deriveStartupHealth, StartupHealth, isCodexRoutingInjected, and
diagnoseCodexShim imports, while preserving the existing CLI output format and
using the configured shared routing path.

In `@src/codex/autostart-health.ts`:
- Around line 68-74: Update the recommendedCommand logic associated with
startupHealthSummary so an at-risk state with serviceSupported false and an
already-healthy effective shim does not select COMMANDS.installShim; preserve
installService for supported services and restoreNative for the remaining
at-risk cases. Add a focused regression test in the existing autostart-health
test suite covering serviceSupported false with an already-healthy cli-only shim
and asserting the recommended command is restoreNative.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 44e8f1ae-e246-4a9c-898c-65ae401712bc

📥 Commits

Reviewing files that changed from the base of the PR and between 54e0bbf and dbd9fe0.

📒 Files selected for processing (26)
  • docs-site/src/content/docs/guides/web-dashboard.md
  • docs-site/src/content/docs/ja/guides/web-dashboard.md
  • docs-site/src/content/docs/ko/guides/web-dashboard.md
  • docs-site/src/content/docs/ru/guides/web-dashboard.md
  • docs-site/src/content/docs/zh-cn/guides/web-dashboard.md
  • gui/src/App.tsx
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/src/pages/Dashboard.tsx
  • gui/src/pages/Startup.tsx
  • gui/src/styles.css
  • src/cli/doctor.ts
  • src/cli/index.ts
  • src/cli/status.ts
  • src/codex/autostart-health.ts
  • src/codex/inject.ts
  • src/codex/shim.ts
  • src/server/management-api.ts
  • structure/05_gui-and-management-api.md
  • tests/autostart-health.test.ts
  • tests/cli-status-json.test.ts
  • tests/codex-shim.test.ts

Comment thread gui/src/App.tsx
Comment thread gui/src/i18n/de.ts Outdated
Comment thread gui/src/i18n/ja.ts Outdated
Comment thread src/cli/doctor.ts
Comment thread src/cli/status.ts Outdated
Comment thread src/cli/status.ts
Comment thread src/codex/autostart-health.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dbd9fe0a1f

ℹ️ 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 thread src/codex/autostart-health.ts Outdated
Comment thread src/codex/inject.ts Outdated
@himomohi himomohi changed the title fix: surface restart-unsafe Codex routing feat(windows): add tray controls and restart-safety diagnostics Jul 23, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

♻️ Duplicate comments (1)
src/cli/status.ts (1)

6-8: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Still duplicating collectStartupHealth() instead of calling it.

This re-implements the exact field-gathering that collectStartupHealth(config) (src/codex/autostart-health.ts:106-123) already does — diagnoseService(), diagnoseCodexShim(), getCodexRoutingKind(), codexAutoStartEnabled(config), then a hand-built deriveStartupHealth({...}) call with an identical field list. doctor.ts already calls the shared collector correctly; keeping status.ts on a separate hand-rolled path risks the two surfaces silently diverging the next time someone adds/renames a field in StartupHealthInputs.

You still need service/codexShim locally for the service.summary/codexShim.summary JSON fields, so keep those two diagnostic calls, but delegate the startup derivation itself.

♻️ Proposed refactor
-import { diagnoseService } from "../service";
-import { deriveStartupHealth, type StartupHealth } from "../codex/autostart-health";
-import { getCodexRoutingKind } from "../codex/inject";
+import { diagnoseService } from "../service";
+import { collectStartupHealth, type StartupHealth } from "../codex/autostart-health";
   const service = diagnoseService();
   const serviceSummary = service.summary;
   const codexShim = diagnoseCodexShim();
   const codexShimSummary = codexShim.summary;
-  const startup = deriveStartupHealth({
-    routingKind: getCodexRoutingKind(),
-    autostartEnabled: codexAutoStartEnabled(config),
-    serviceInstalled: service.installed,
-    serviceViable: service.viable,
-    serviceEnabled: service.enabled,
-    serviceRunning: service.running,
-    serviceStale: service.stale,
-    serviceConflict: service.conflict,
-    serviceSupported: service.supported,
-    shimInstalled: codexShim.installed,
-    shimHealthy: codexShim.healthy,
-    platform: process.platform,
-  });
+  const startup = collectStartupHealth(config);
As per path instructions, `src/**` changes must not bypass shared routing/config layers.

Also applies to: 122-139

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/status.ts` around lines 6 - 8, Update the status command’s startup
health flow to use the shared collectStartupHealth(config) collector instead of
manually gathering inputs and calling deriveStartupHealth. Retain the local
diagnoseService() and diagnoseCodexShim() calls for the service.summary and
codexShim.summary JSON fields, while sourcing startup from the shared collector
so routing and configuration remain centralized.

Source: Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gui/src/pages/Startup.tsx`:
- Around line 217-222: Extract the duplicated risk-detail i18n selection into a
shared resolveRiskDetailKey helper accepting routingKind and shimCoverage, and
use it in gui/src/pages/Startup.tsx lines 217-222. Replace the inline ternary
chain in gui/src/pages/Dashboard.tsx lines 629-640 with the same helper,
preserving the custom-local, cli-only, and default riskDetail selection order.
- Around line 271-298: Allow the uninstall action in the startup tray controls
to render when tray.stale is true, while preserving the existing loading, error,
installed, and tray-presence checks. Update the uninstall button condition
around runTrayAction("uninstall") without changing the install, start, or stop
gating.
- Around line 254-262: Update the shim StateBadge in the startup detail row so
its ok prop reflects both shim health and autostart status, making “Installed
but disabled” use the non-healthy styling. Keep the existing yes and no text
selection unchanged, and preserve healthy styling only when the shim is healthy
and autostart is enabled.

In `@src/cli/help.ts`:
- Around line 52-59: Update the tray help entry’s usage string to include the
supported --no-start option alongside --json, matching the usage documented by
windowsTrayCommand while preserving the existing command syntax and help
details.

In `@src/cli/index.ts`:
- Around line 430-436: Within the Windows tray removal step, update the status
check to call getWindowsTrayStatus() once, store its result in a local variable,
and use that cached status for both installed and stale checks before invoking
uninstallWindowsTray().
- Around line 313-315: Remove the redundant dynamic import inside the service
diagnosis flow and add `serviceStartableFromTray` to the existing static import
from "../service". Update the `serviceStartableFromTray(service)` call to use
that statically imported symbol while preserving the surrounding
`diagnoseService` logic.
- Around line 306-349: The CLI dispatcher lacks focused tests for the tray proxy
command flows. Add tests covering __tray-start, __tray-restart, and
__startup-health, mocking findLiveProxy, diagnoseService, serviceCommand, spawn,
waitForProxy, and handleStop; verify installed-but-not-viable handling,
direct-proxy fallback, and that restart stops without starting when handleStop
fails.

In `@src/codex/inject.ts`:
- Around line 193-200: Update isLoopbackUrl to recognize IPv6 loopback hostnames
returned as "[::1]" by URL.hostname, while preserving existing localhost and
IPv4 handling. Add a regression test covering an IPv6 openai_base_url with a
port and verify it is classified as loopback rather than custom-remote.

In `@src/tray/windows-tray.ps1`:
- Around line 171-218: Update the menu Opening handler to render the cached tray
state maintained by Update-TrayState instead of invoking synchronous network
probes when the context menu opens. Preserve the existing 3-second timer refresh
and ensure menu labels, enabled states, and icons use the latest cached
$script:* values; confirm this freshness contract without adding a second probe
path.

In `@src/update/index.ts`:
- Around line 173-195: Add regression coverage for the Windows tray handoff
across src/update/index.ts lines 173-195, src/update/job.ts lines 522-570, and
bin/ocx.mjs lines 185-193. In tests targeting the job flow, stub
getWindowsTrayStatus, stopWindowsTray, startWindowsTray, and runLoggedCommand to
verify clean stop and refresh, restoration plus job failure on stop/proxy-stop
failure, restoration and consistent state on refresh failure, and idle-tray
preservation via --no-start. Add launcher-level coverage for the legacy npm
self-update path to verify the equivalent stop/start handoff.

---

Duplicate comments:
In `@src/cli/status.ts`:
- Around line 6-8: Update the status command’s startup health flow to use the
shared collectStartupHealth(config) collector instead of manually gathering
inputs and calling deriveStartupHealth. Retain the local diagnoseService() and
diagnoseCodexShim() calls for the service.summary and codexShim.summary JSON
fields, while sourcing startup from the shared collector so routing and
configuration remain centralized.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 78eb6ce0-de39-424d-8d03-2e7299fb9df0

📥 Commits

Reviewing files that changed from the base of the PR and between dbd9fe0 and 3c8e5c0.

📒 Files selected for processing (35)
  • bin/ocx.mjs
  • docs-site/src/content/docs/guides/web-dashboard.md
  • docs-site/src/content/docs/ja/guides/web-dashboard.md
  • docs-site/src/content/docs/ko/guides/web-dashboard.md
  • docs-site/src/content/docs/ru/guides/web-dashboard.md
  • docs-site/src/content/docs/zh-cn/guides/web-dashboard.md
  • gui/src/App.tsx
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/src/pages/Dashboard.tsx
  • gui/src/pages/Startup.tsx
  • gui/src/styles.css
  • src/cli/doctor.ts
  • src/cli/help.ts
  • src/cli/index.ts
  • src/cli/status.ts
  • src/codex/autostart-health.ts
  • src/codex/inject.ts
  • src/codex/shim.ts
  • src/server/management-api.ts
  • src/server/startup-health-cache.ts
  • src/server/windows-tray-control.ts
  • src/service.ts
  • src/tray/windows-tray.ps1
  • src/tray/windows.ts
  • src/update/index.ts
  • src/update/job.ts
  • structure/05_gui-and-management-api.md
  • tests/autostart-health.test.ts
  • tests/service.test.ts
  • tests/windows-tray.test.ts

Comment thread gui/src/pages/Startup.tsx
Comment thread gui/src/pages/Startup.tsx
Comment thread gui/src/pages/Startup.tsx
Comment thread src/cli/help.ts
Comment thread src/cli/index.ts
Comment thread src/cli/index.ts Outdated
Comment thread src/cli/index.ts
Comment thread src/codex/inject.ts Outdated
Comment thread src/tray/windows-tray.ps1
Comment thread src/update/index.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/tray/windows.ts (1)

255-275: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Async fallback duplicates the sync syncRegistryAbsenceIsProven probe instead of sharing it.

readOwnedRunValueAsync's catch block (lines 261-271) re-implements the same "query bare RUN_KEY, then fall back to checking RUN_PARENT_KEY via windowsRegistryParentShowsRunKey" sequence that the sync path already factors into syncRegistryAbsenceIsProven (lines 211-224). Keeping two copies of this fail-closed decision tree risks them drifting apart on a future edit (e.g., someone tightens the sync version's status check but forgets the async one).

Consider extracting an asyncRegistryAbsenceIsProven() helper mirroring the sync one, so both entry points share a single source of truth for this security-relevant logic.

♻️ Suggested refactor
+async function asyncRegistryAbsenceIsProven(): Promise<boolean> {
+  try {
+    await runRegistryAsync(["query", RUN_KEY, "/reg:64"]);
+    return true;
+  } catch (runError) {
+    if (Number((runError as { code?: unknown }).code) !== 1) return false;
+    try {
+      const parent = await runRegistryAsync(["query", RUN_PARENT_KEY, "/reg:64"]);
+      return !windowsRegistryParentShowsRunKey(parent);
+    } catch {
+      return false;
+    }
+  }
+}
+
 async function readOwnedRunValueAsync(runValue = windowsTrayRunValue(getConfigDir())): Promise<string | null> {
   try {
     const output = await runRegistryAsync(["query", RUN_KEY, "/v", runValue, "/reg:64"]);
     return parseWindowsTrayRunValue(output, runValue);
   } catch (error) {
     if (Number((error as { code?: unknown }).code) === 1) {
-      try {
-        await runRegistryAsync(["query", RUN_KEY, "/reg:64"]);
-        return null;
-      } catch (runError) {
-        if (Number((runError as { code?: unknown }).code) === 1) {
-          try {
-            const parent = await runRegistryAsync(["query", RUN_PARENT_KEY, "/reg:64"]);
-            if (!windowsRegistryParentShowsRunKey(parent)) return null;
-          } catch { /* fall through to the fail-closed error */ }
-        }
-      }
+      if (await asyncRegistryAbsenceIsProven()) return null;
     }
     throw new Error("Unable to verify Windows tray registry status.");
   }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tray/windows.ts` around lines 255 - 275, Extract the duplicated
registry-absence decision tree from readOwnedRunValueAsync into an
asyncRegistryAbsenceIsProven helper that mirrors syncRegistryAbsenceIsProven,
including the RUN_KEY query and RUN_PARENT_KEY fallback with
windowsRegistryParentShowsRunKey. Update the async catch path to call this
helper and preserve the existing fail-closed error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/windows-tray.test.ts`:
- Around line 97-106: Add focused regression coverage for the fail-closed
unreadable-registry behavior through readOwnedRunValue/readOwnedRunValueAsync,
rather than only testing windowsRegistryParentShowsRunKey. Expose these
functions for testing or inject a mock registry runner, then verify bare Run
queries with non-1 failures and CurrentVersion parent-query failures throw,
while preserving the existing missing-key behavior.

---

Outside diff comments:
In `@src/tray/windows.ts`:
- Around line 255-275: Extract the duplicated registry-absence decision tree
from readOwnedRunValueAsync into an asyncRegistryAbsenceIsProven helper that
mirrors syncRegistryAbsenceIsProven, including the RUN_KEY query and
RUN_PARENT_KEY fallback with windowsRegistryParentShowsRunKey. Update the async
catch path to call this helper and preserve the existing fail-closed error
behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 43f804f4-bac4-4688-bd20-2e4f2a9a2b7e

📥 Commits

Reviewing files that changed from the base of the PR and between 091d44a and e5228c7.

📒 Files selected for processing (6)
  • src/codex/inject.ts
  • src/service.ts
  • src/tray/windows.ts
  • tests/autostart-health.test.ts
  • tests/service.test.ts
  • tests/windows-tray.test.ts

Comment thread tests/windows-tray.test.ts
@himomohi
himomohi force-pushed the fix/autostart-health-visibility branch from 5f6d95e to 9fcba57 Compare July 23, 2026 06:23

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
gui/src/pages/Dashboard.tsx (1)

459-460: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Prevent stale polling results from overwriting the PUT response.

In gui/src/pages/Dashboard.tsx, the five-second /api/settings poll can have a GET already in flight when this PUT succeeds. If that GET returns afterward, fetchData() overwrites the authoritative codexAutoStart and startupHealth values with stale data, causing the startup-safety warning to flicker or display the wrong state.

Track settings request/mutation epochs and ignore GET results started before a successful write, or re-fetch /api/settings after the PUT and apply only the newest response.

Proposed direction
+const settingsMutationEpoch = useRef(0);

 const toggleCodexAutoStart = async () => {
   if (!settings || settingsSaving) return;
+  const mutationEpoch = ++settingsMutationEpoch.current;
   ...
-  setSettings(prev => prev ? { ...prev, codexAutoStart: data.codexAutoStart, startupHealth: data.startupHealth ?? prev.startupHealth } : prev);
+  if (mutationEpoch === settingsMutationEpoch.current) {
+    setSettings(prev => prev ? {
+      ...prev,
+      codexAutoStart: data.codexAutoStart,
+      startupHealth: data.startupHealth ?? prev.startupHealth,
+    } : prev);
+  }
 };

The polling path must also compare its request epoch before committing fetched settings.

As per path instructions, GUI state changes must stay consistent with management API responses.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gui/src/pages/Dashboard.tsx` around lines 459 - 460, Update the settings
polling and PUT flow around fetchData and the settings mutation so stale GET
responses cannot overwrite authoritative mutation results. Track
request/mutation epochs, capture the epoch when each GET starts, and check it
before applying codexAutoStart or startupHealth; increment the mutation epoch
only after a successful PUT so any earlier in-flight poll is ignored while newer
responses remain applicable.

Source: Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@gui/src/pages/Dashboard.tsx`:
- Around line 459-460: Update the settings polling and PUT flow around fetchData
and the settings mutation so stale GET responses cannot overwrite authoritative
mutation results. Track request/mutation epochs, capture the epoch when each GET
starts, and check it before applying codexAutoStart or startupHealth; increment
the mutation epoch only after a successful PUT so any earlier in-flight poll is
ignored while newer responses remain applicable.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2db3924c-18e9-4040-bcc3-470e256ff825

📥 Commits

Reviewing files that changed from the base of the PR and between e5228c7 and 5f6d95e.

📒 Files selected for processing (27)
  • bin/ocx.mjs
  • gui/src/i18n/de.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/pages/Dashboard.tsx
  • gui/src/pages/Startup.tsx
  • gui/src/startup-health-ui.ts
  • src/cli/doctor.ts
  • src/cli/help.ts
  • src/cli/index.ts
  • src/cli/internal-dispatch.ts
  • src/cli/status.ts
  • src/cli/tray-proxy.ts
  • src/codex/autostart-health.ts
  • src/tray/windows-tray.ps1
  • src/tray/windows.ts
  • src/update/index.ts
  • src/update/job.ts
  • src/update/tray-update-plan.d.mts
  • src/update/tray-update-plan.mjs
  • tests/autostart-health.test.ts
  • tests/cli-help.test.ts
  • tests/internal-cli-dispatch.test.ts
  • tests/tray-proxy.test.ts
  • tests/update-tray-handoff.test.ts
  • tests/windows-tray.test.ts
💤 Files with no reviewable changes (1)
  • src/tray/windows-tray.ps1

@himomohi

Copy link
Copy Markdown
Contributor Author

Addressed the remaining CodeRabbit outside-diff Dashboard race in d6f5342. Settings polls now use request and successful-mutation epochs, ignore responses while a PUT is in flight, and cannot overwrite the authoritative PUT response. Added focused pure regression coverage; typecheck, GUI lint/build, privacy scan, and focused tests pass.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature has value, but the current 47-file / 3,666-line PR is not a mergeable unit. The latest head has no GitHub check runs at all, and the PR body reports an aggregate run with 67 failures and 7 errors. Focused isolated passes and self-assigned “94–97/100” review scores cannot replace a clean current-dev matrix for code that changes service lifecycle, Codex shim routing, update handoff, registry persistence, PowerShell tray execution, management API, and GUI state together.

Please split this into independently reviewable PRs (startup-health classifier/API; Windows tray lifecycle/update handoff; GUI/localization), rebase each onto current dev, and require a clean full suite plus Windows lifecycle CI. The current PR remains open for revision, but should not be merged in this form.

@lidge-jun
lidge-jun force-pushed the fix/autostart-health-visibility branch from c8fa696 to 1f6a6cd Compare July 23, 2026 12:18
@github-actions github-actions Bot added the enhancement New feature or request label Jul 23, 2026

@lidge-jun lidge-jun left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved: startup-health/tray feature set. Rebase onto split management-api completed, typecheck verified.

@lidge-jun
lidge-jun merged commit dc52f1f into lidge-jun:dev Jul 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants