Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b6e6e191a
ℹ️ 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".
src/daemon/handlers/session.ts
Outdated
| runtime: setSessionRuntimeHintsForOpen(sessionStore, sessionName, explicitRuntime), | ||
| previousRuntime, | ||
| replacedStoredRuntime: true, |
There was a problem hiding this comment.
Defer runtime replacement until open succeeds
resolveOpenRuntimeHints writes the new runtime into sessionStore before any launch work runs, so a later failure in applyRuntimeHints, relaunch close, or dispatch(..., 'open', ...) leaves the session mutated even though open returned an error. This breaks the “atomic replace-and-open” behavior and can make subsequent retries/replays run with an unintended runtime payload from a failed attempt.
Useful? React with 👍 / 👎.
|
Summary
Add typed
runtimepayload support toopenso callers can replace session runtime and relaunch atomically.Make runtime actions replayable, validate malformed typed runtime payloads, normalize typed-runtime validation into structured daemon error responses, and forward typed runtime through batch steps.
Touched files: 13. Scope expanded beyond the initial session handler into replay/session-store/batch/docs to keep recording, replay, and typed transport consistent.
Note:
runtime setnow shares the same metro port validation path as typedopen.runtime, so out-of-range ports fail fast instead of being accepted implicitly.Validation
pnpm typecheckpnpm exec node --test src/daemon/__tests__/session-store.test.ts src/daemon/handlers/__tests__/session.test.tspnpm test:smokepnpm test:unit(still fails in pre-existingsrc/daemon/__tests__/http-server.test.tswith "Promise resolution is still pending but the event loop has already resolved")