Prelaunch hardening cleanup#297
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR implements prelaunch hardening across three dimensions: (1) schema reset policy replaces migration replay; (2) backend capability reporting gates simulator UI; (3) ESLint modernization with flat-config and expanded coverage. Supporting changes thread relay-client context through request dispatch and refactor tests. ChangesPrelaunch Hardening Implementation
🎯 3 (Moderate) | ⏱️ ~25 minutes Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/features/simulator/api/simulator.queries.ts (1)
13-18: ⚡ Quick winUse infinite staleness for this WebSocket-backed capability query.
This hook is consuming backend state over the WS protocol, so
staleTime: 30_000is out of policy and adds avoidable refetch churn. UseInfinityhere.As per coding guidelines: "Frontend subscribes to server state via WebSocket using `useQuerySubscription()` with `staleTime: Infinity` and `refetchOnWindowFocus: false`" and "Set `staleTime: Infinity` and `refetchOnWindowFocus: false` for WebSocket-managed server state since the WebSocket handles freshness".♻️ Proposed change
const query = useQuery({ queryKey: SIMULATOR_CAPABILITIES_QUERY_KEY, queryFn: () => simulatorService.getCapabilities(), - staleTime: 30_000, + staleTime: Infinity, refetchOnWindowFocus: false, enabled: options.enabled ?? true, });🤖 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 `@apps/web/src/features/simulator/api/simulator.queries.ts` around lines 13 - 18, The query for SIMULATOR_CAPABILITIES_QUERY_KEY (created with useQuery and calling simulatorService.getCapabilities) is WebSocket-backed and should use infinite staleness; change staleTime from 30_000 to Infinity and keep refetchOnWindowFocus: false (or switch to useQuerySubscription if codebase standard prefers that hook) while leaving enabled: options.enabled ?? true intact.
🤖 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 `@apps/backend/src/services/simulator-context.ts`:
- Around line 163-167: hasSimulatorToolchain currently checks
SIM_ENV["DEVELOPER_DIR"]/usr/bin/simctl which can miss valid setups; replace
this filesystem check in function hasSimulatorToolchain with invoking xcrun
--find simctl (e.g., via child_process.execSync or spawnSync) and return true if
the command succeeds and yields a path, otherwise return false; ensure you catch
errors from exec/spawn and treat them as non-existent, and update any imports to
include child_process if needed.
---
Nitpick comments:
In `@apps/web/src/features/simulator/api/simulator.queries.ts`:
- Around line 13-18: The query for SIMULATOR_CAPABILITIES_QUERY_KEY (created
with useQuery and calling simulatorService.getCapabilities) is WebSocket-backed
and should use infinite staleness; change staleTime from 30_000 to Infinity and
keep refetchOnWindowFocus: false (or switch to useQuerySubscription if codebase
standard prefers that hook) while leaving enabled: options.enabled ?? true
intact.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6221d55c-5411-4f7a-a454-460a05bb04fc
📒 Files selected for processing (48)
.megagoal/prelaunch-hardening/NOTES.md.megagoal/prelaunch-hardening/ROADMAP.md.megagoal/prelaunch-hardening/goals/01-prelaunch-schema-cleanup.md.megagoal/prelaunch-hardening/goals/02-static-quality-gate-coverage.md.megagoal/prelaunch-hardening/goals/03-simulator-capability-truth.md.megagoal/prelaunch-hardening/goals/04-high-risk-complexity-pass.md.megagoal/prelaunch-hardening/goals/05-final-ship-gate.mdDEVELOPMENT.mdapps/backend/cli.tsapps/backend/src/lib/database.tsapps/backend/src/services/agent/commands.tsapps/backend/src/services/query-engine.tsapps/backend/src/services/query-request-dispatcher.tsapps/backend/src/services/simulator-context.tsapps/backend/test/unit/lib/database.test.tsapps/backend/test/unit/runtime/agent-process.test.tsapps/backend/test/unit/services/agent-commands-simulator.test.tsapps/backend/test/unit/services/event-persistence-integration.test.tsapps/backend/test/unit/services/query-request-dispatcher.test.tsapps/backend/test/unit/services/simulator-capabilities.test.tsapps/backend/test/unit/shared/events.test.tsapps/backend/test/unit/shared/schema.test.tsapps/web/src/app/layouts/ContentTabBar.tsxapps/web/src/app/layouts/ContentView.tsxapps/web/src/app/layouts/MainContent.tsxapps/web/src/app/layouts/content-tabs.tsapps/web/src/features/browser/hooks/useWebview.tsapps/web/src/features/browser/ui/BrowserTab.tsxapps/web/src/features/browser/ui/FocusModeOverlay.tsxapps/web/src/features/browser/ui/InspectPromptOverlay.tsxapps/web/src/features/browser/webview-manager.tsapps/web/src/features/settings/ui/sections/ExperimentalSection.tsxapps/web/src/features/simulator/api/simulator.queries.tsapps/web/src/features/simulator/api/simulator.service.tsapps/web/src/features/simulator/index.tsapps/web/src/platform/capabilities.tsdocs/codebase-audit-2026-05-18.mdeslint.config.mjspackage.jsonshared/events.tsshared/schema.tsshared/types/index.tsshared/types/simulator.tstest/unit/session/calculateTurnStats.test.tstest/unit/session/subagentMessageList.test.tstest/unit/shared/chatInsertStore.test.tstest/unit/workspace/content-tabs.test.tstest/unit/workspace/workspaceStore.test.ts
💤 Files with no reviewable changes (7)
- apps/web/src/features/browser/ui/BrowserTab.tsx
- apps/web/src/features/browser/ui/FocusModeOverlay.tsx
- apps/web/src/features/browser/ui/InspectPromptOverlay.tsx
- test/unit/shared/chatInsertStore.test.ts
- apps/web/src/features/browser/webview-manager.ts
- apps/web/src/platform/capabilities.ts
- apps/web/src/features/browser/hooks/useWebview.ts
![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: b20b7e4fcd
ℹ️ 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".
|
|
||
| export const PRELAUNCH_REQUIRED_COLUMNS = { | ||
| workspaces: ["status"], | ||
| sessions: ["agent_harness", "error_category"], |
There was a problem hiding this comment.
For local databases that already have sessions.agent_harness but still contain the old value codex, this column-only prelaunch gate passes even though the removed migration used to rewrite those rows to codex-sdk. Existing sessions with messages then hit the harness-lock path in sendMessage and are rejected as Cannot switch agent from codex to codex-sdk, instead of getting the intended reset hint or working after migration. Please either validate/fail on stale agent_harness = 'codex' rows here or keep the data rewrite before accepting the DB.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7751b36. Backend startup now fails fast with the pre-launch reset hint when a current-shape local DB still contains stale sessions.agent_harness = codex rows, so those sessions cannot later trip the harness-lock path.
| function hasSimulatorToolchain(): boolean { | ||
| const developerDir = SIM_ENV["DEVELOPER_DIR"]; | ||
| if (!developerDir) return false; | ||
| return existsSync(join(developerDir, "usr/bin", "simctl")); |
There was a problem hiding this comment.
On a macOS backend with Xcode installed but without the simbridge helper built or packaged, this reports simulator streaming as available even though startStream immediately fails with simbridge binary not found. Since the frontend now uses simulatorCapabilities to enable the setting/tab, these installs get an apparently supported simulator UI that cannot start a stream. Please include the same findSimBridgePath() check in the capability result, or return an unavailable reason when the stream helper is missing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7751b36. Simulator capabilities now require findSimBridgePath() to resolve before reporting availability, and the capability tests cover the missing-simbridge case.
Greenlight — Round 1Fixed all 3 actionable AI review comments in
Verification before push:
|
1 participant
Summary
Verification
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores