Make remote workspaces interactive and stable - #42
Conversation
|
@codex review |
![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: b2443a21aa
ℹ️ 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".
| if (started) { | ||
| setTranscript(started.transcript); | ||
| setDrafting(false); | ||
| props.onSelectSession(started.session.sessionId); |
There was a problem hiding this comment.
When a user starts a first prompt and then navigates elsewhere through the still-active sidebar before it settles, this async handler survives the workspace change or component unmount and unconditionally invokes the captured onSelectSession. Completion therefore forces the user back to the newly created remote session, potentially long after they left; invalidate the request on scope change/unmount and gate this navigation on the original workspace still being active.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a59393d41
ℹ️ 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".
| useEffect(() => { | ||
| promptRequestRef.current += 1; | ||
| setDrafting(false); | ||
| setDraft(""); | ||
| setOptimisticPrompt(null); |
There was a problem hiding this comment.
When a prompt is running and the user navigates directly to another remote workspace or profile, this effect invalidates the old completion but clears only the draft-related state. starting, sendingSessionId, and stopping remain set, and the stale-request guard prevents the old completion from clearing them, so the new workspace's New session/composer controls remain disabled indefinitely and Stop may target the old session under the new profile. Reset these operation UI flags on scope changes and cover same-component workspace navigation as another variant of the reported lifecycle bug.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 299711c90a
ℹ️ 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".
| } finally { | ||
| settled?.cancel(); | ||
| await settled?.promise.catch(() => {}); | ||
| await port?.close({ abort: false }).catch(() => {}); |
There was a problem hiding this comment.
When a remote turn runs longer than 30 minutes, the waiter rejects, but this finally still calls port.close({ abort: false }). That is not a detach operation: PiRpcSessionPort.close invokes rpc.stop, and RemoteHostDaemon.stopRpc(false) ends the Pi process's stdin and sends SIGTERM after two seconds. Thus a long-running remote task is terminated even though the UI presents Stop as the explicit cancellation path; the timeout path should disconnect without stopping the remote RPC process.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3114b6871c
ℹ️ 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".
| await this.refreshSessions(profile.id); | ||
| return await this.openSession(profile.id, sessionId, true); | ||
| } finally { | ||
| this.releaseOperation(profile.id, operation); |
There was a problem hiding this comment.
When a prompt exceeds 30 minutes or its transport disconnects, the new detach path intentionally leaves the remote RPC process running, but this finally immediately removes its ActiveRemoteOperation. After the IPC request rejects, the renderer removes the running/Stop UI, and abortSession can no longer find the operation, so reconnecting or returning to the workspace cannot cancel work that is still consuming remote resources; a subsequent prompt may instead attach to the still-busy RPC process. Keep detached operations discoverable or reattachable until the daemon reports settlement, and cover the timeout/disconnect-to-Stop variants. This is fresh evidence beyond the earlier timeout finding because the new detach() fix preserves the process while making it untracked.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f619c9ca8
ℹ️ 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".
| private readonly store: ProfileStore; | ||
| private readonly runtime: ManagedRemoteRuntime; | ||
| private readonly statuses = new Map<string, RemoteProfileStatus>(); | ||
| private readonly activeOperations = new Map<string, ActiveRemoteOperation>(); |
There was a problem hiding this comment.
When Jasmine is exited and relaunched while a prompt is running, the SSH client disconnects but the remote daemon deliberately leaves its RPC process alive; this in-memory map is then recreated empty, so the relaunched renderer offers Send with no Stop control and reserveOperation accepts another turn for the same profile. Fresh evidence beyond the prior detach finding is that the final fix tracks reconnects only within this service instance and provides no startup discovery or persistence. Rehydrate the daemon's active session state on startup and cover the app-restart lifecycle variant.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| if (active?.port) await active.port.close({ abort: true }).catch(() => {}); | ||
| if (active) this.releaseOperation(profile.id, active); |
There was a problem hiding this comment.
When Stop remote runtime is invoked from settings during a prompt, this releases the operation before the original startSession/promptSession request has reached its own cleanup. The running UI disappears and a new prompt can reserve the profile while the old handler may still proceed into refreshSessions/openSession, racing runtime.stop and potentially overwriting the final disconnected status with ready or failed. Mark the operation as stopping and wait for its request to settle before releasing it.
Useful? React with 👍 / 👎.
| const replacement = await options.runtime.openSession(options.profile, { | ||
| sessionId: options.sessionId, | ||
| afterSeq: cutoff |
There was a problem hiding this comment.
When the SSH interruption includes a daemon restart or host reboot, cutoff belongs to the old daemon while the replacement daemon's sequence starts again at zero. Passing the old value as afterSeq and retaining it in waitForRemotePromptSettled causes every new agent_settled or exit event to be ignored as seq <= cutoff, leaving the operation in repeated 30-minute reconnect cycles even though the original process is already gone. Detect a daemon epoch/sequence reset and fail or reset the waiter, and add this reconnect variant to the regression coverage.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aaae4270db
ℹ️ 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".
| await this.runtime.stop(profile); | ||
| this.releaseOperation(profile.id, operation); |
There was a problem hiding this comment.
When Stop is clicked while startSession is still opening its SSH/RPC port, operation.port is null, so this branch stops the current daemon and immediately resolves active.done by releasing the reservation even though the original handler can continue into ensureDaemon/rpc.start. Another workspace can then reserve the profile and collide with that handler, and stopProfile's new await active.done does not actually wait for cleanup. Fresh evidence beyond the earlier profile-stop finding is that the newly added completion promise is resolved here before the originating request settles; let that request's finally own the release and cover this opening-port variant.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| if (operation.port) { | ||
| await operation.port.abort(); |
There was a problem hiding this comment.
If the SSH transport fails while the user clicks Stop on an active port, port.abort() rejects here after abortRequested was set. The prompt handler classifies the same disconnect as detached and starts its monitor, but that monitor immediately skips its loop and declines to release because abortRequested is true; this branch also exits by throwing, leaving the profile permanently reserved in a reconnecting/stopping state until Jasmine restarts. Ensure this failure path reaches one owner that releases the operation, and add the disconnect-during-Stop regression variant.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91616fa967
ℹ️ 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".
| child.kill(); | ||
| await egress?.close(); |
There was a problem hiding this comment.
When a client-proxy prompt times out or its control SSH connection drops, the new detach path reaches this finally and closes the EgressBroker, which terminates both the reverse tunnel and local gateway. The detached RPC process therefore loses its only provider egress, while monitorDetachedOperation merely inspects the daemon and never recreates the broker, so the work advertised as continuing remotely can fail or remain stuck. Preserve or re-establish the egress lease while detached and cover the client-proxy timeout/disconnect variants.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| setDrafting(false); | ||
| props.onSelectSession(started.session.sessionId); | ||
| } | ||
| if (!result) setDraft(text); |
There was a problem hiding this comment.
When the remote turn settles successfully but the subsequent refreshSessions or openSession reconciliation fails, both hook methods return null, so this branch restores the text as though it was never sent. Retrying then submits the same turn again—or creates a second session for a failed first-session reconciliation—even though the original work is already durable remotely. Distinguish pre-send failure from post-settlement sync failure and cover both start and existing-session variants.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 336b665791
ℹ️ 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".
| checkedAt: new Date().toISOString(), | ||
| busy: false | ||
| }); | ||
| this.recoverActiveOperation(profile, runtimeInfo); |
There was a problem hiding this comment.
When Jasmine restarts during a remote prompt, navigation initializes to newChat, so the route effect does not refresh any remote profile; this recovery hook runs only after refreshSessions, which requires the user to open or expand that profile. Until then the operation is unreserved, and a client-proxy turn has no replacement egress tunnel after the old process exits, so it can fail or collide with another submission. Fresh evidence beyond the earlier restart finding is that the new recovery implementation is still reachable only from this interactive refresh path rather than service initialization.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| await port.prompt(text); | ||
| await callbacks.onPromptAccepted?.(); |
There was a problem hiding this comment.
If the transport drops after rpc.send has written the prompt into the remote process but before Pi's command response reaches this await, port.prompt() rejects and onPromptAccepted never runs even though the daemon may report the RPC as busy and continue executing it. RemoteProfileService consequently rethrows the detached failure, the renderer restores the draft, and retrying after the monitor releases the operation can submit the same work twice. Fresh evidence beyond the post-settlement synchronization finding is the unhandled gap between the daemon's delivery acknowledgement and Pi's later RPC response; mark the submission accepted at the former boundary.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| info = await this.runtime.inspectRuntime(profile, { install: false }); | ||
| } catch (error) { | ||
| if (!isRetryableError(error)) throw error; |
There was a problem hiding this comment.
When a second SSH interruption occurs during detached polling after the runtime probe succeeds, inspectRuntime can wrap the failed daemon proxy connection as daemon-proxy-failed, whose retryable flag defaults to false. This branch therefore throws instead of polling again, and the monitor's finally releases the reservation and retained egress resources while the remote RPC can still be busy. Fresh evidence beyond the earlier detached-registration finding is that the new monitor classifies its own transient connection failure as terminal; normalize daemon connection failures as retryable or keep the operation reserved until daemon state is known.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc88a7f69b
ℹ️ 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".
| * says so, and this is what makes that promise true. | ||
| */ | ||
| async removeProfile(profileId: string): Promise<void> { | ||
| if (this.activeOperations.has(profileId)) { |
There was a problem hiding this comment.
When Jasmine starts while a remote prompt is active and the SSH recovery probe is still pending, the user can remove the profile before this map is populated. Recovery can then reserve and monitor the now-deleted profile after its local workspaces and transcripts have been erased, while the running remote operation loses its only usable profile entry. Await startupRecovery before this check, as the start, prompt, and stop paths already do.
Useful? React with 👍 / 👎.
| if (!isRetryableError(error)) throw error; | ||
| await new Promise((resolve) => setTimeout(resolve, DETACHED_OPERATION_POLL_MS)); | ||
| continue; |
There was a problem hiding this comment.
When Stop is requested for a detached operation during a transient SSH outage, runtime.stop() reports remote-stop-failed without setting retryable, so this branch throws immediately and the outer finally releases the reservation and retained client-proxy egress even though the remote task may still be running. Fresh evidence beyond the earlier attached-port abort comment is that this failure occurs in the detached monitor's daemon-stop command; classify transport stop failures as retryable or otherwise retain the operation, and cover this detached Stop variant.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| if ("pending" in result) { | ||
| onToastRef.current("Remote prompt accepted; waiting to synchronize the session"); | ||
| return result; |
There was a problem hiding this comment.
When a first prompt returns pending because it detached or because post-settlement synchronization failed, this path only shows a toast and returns; the status listener updates profile status but never reloads sessions or workspaces. Consequently, even after the main-process monitor synchronizes the durable session, the new session remains absent from the current workspace until the user manually refreshes, despite the toast saying Jasmine is waiting to synchronize it. Arrange a completion-driven refresh and cover both new-session and existing-session pending variants.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99af83a21a
ℹ️ 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".
| const stopped = await props.onAbortSession(starting ? undefined : sendingSessionId ?? sessionOperation?.sessionId ?? undefined); | ||
| if (!stopped) setStopping(false); |
There was a problem hiding this comment.
When a detached or startup-recovered operation is stopped successfully, this leaves stopping set to true; clearing sessionOperation only hides the button and does not reset the component state. If the user then starts another prompt in the same workspace, the newly displayed Stop button inherits the stale loading/disabled state and cannot cancel that prompt. Reset this flag when the tracked operation disappears or before beginning the next submission.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
| if (error instanceof PiRemoteError && error.code === "runtime-not-installed") return; | ||
| if (!isRetryableError(error) || attempt === STARTUP_RECOVERY_ATTEMPTS - 1) return; | ||
| await new Promise((resolve) => setTimeout(resolve, 250 * 2 ** attempt)); |
There was a problem hiding this comment.
If Jasmine starts while the remote host is briefly unavailable, three retryable probe failures permanently end startup recovery; there is no later background retry unless the user happens to refresh that profile. For an already-running client-proxy prompt, the previous app process has closed its egress tunnel, so giving up here can leave the remote turn without provider access while the UI also has no tracked operation. Fresh evidence beyond the earlier restart finding is that recovery now runs at startup but stops permanently after this bounded loop; retain a background retry or otherwise keep the operation discoverable until the host answers.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 957c04038b
ℹ️ 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".
| // Only client-proxy work depends on this app staying online after | ||
| // the bounded startup gate. Direct profiles remain daemon-owned and | ||
| // are rediscovered by their next explicit session refresh. | ||
| if (profile.network.mode === "client-proxy") void this.retryStartupRecovery(profile); |
There was a problem hiding this comment.
When the initial probes for a client-proxy profile all fail, this fire-and-forget retry is no longer tracked by startupRecoveryByProfile, so startSession and promptSession stop waiting for recovery while it is still unresolved. If the host returns and the user sends first, the new request reserves the empty local slot and rpc.start can reclaim the matching daemon RPC even while it is busy, allowing another prompt to be submitted before the old one is recovered. Fresh evidence beyond the earlier bounded-retry report is that the replacement retry now runs outside the gate; keep it associated with the profile until it either reserves the recovered operation or proves none exists, and cover the reconnect-versus-Send race.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46d647eb5e
ℹ️ 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".
| await delayUnref(delayMs); | ||
| if (this.cancelledStartupRecovery.has(profile.id)) return; | ||
| try { | ||
| const info = await this.runtime.inspectRuntime(profile, { install: false }); |
There was a problem hiding this comment.
When an offline client-proxy profile is edited while persistent startup recovery is running, this loop continues probing the captured pre-edit RemoteProfile. Because startSession and promptSession await that same recovery promise, correcting the SSH host, port, or network mode has no effect and the composer can remain in Starting indefinitely against the old configuration; reload the profile during retries or cancel and restart its recovery when the profile is updated, with a regression for the edit-during-recovery path.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| { phase: "session", remediation: "Open the remote session history to verify what was saved before retrying." } | ||
| ); | ||
| } | ||
| if (!active?.busy) { |
There was a problem hiding this comment.
When the Pi RPC child crashes or is killed while a prompt is detached but the daemon itself remains alive, info.daemonId still matches while activeRpc becomes null; this branch therefore refreshes and clears the operation as though an idle RPC had confirmed successful settlement. Unlike the earlier daemon-restart case, the daemon epoch has not changed, so the new check does not catch this failure and the UI can report synchronization for incomplete work; treat a missing same-daemon RPC as an unconfirmed failure and cover that process-exit variant.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 686a69d817
ℹ️ 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".
| * connection, which leaves remote work running. | ||
| */ | ||
| async stopProfile(profileId: string): Promise<RemoteProfileStatus> { | ||
| await this.awaitProfileStartupRecovery(profileId); |
There was a problem hiding this comment.
When Jasmine starts with an offline client-proxy profile, recoverProfileOnStartup enters the unbounded retryStartupRecovery loop, so this await never completes and Stop remote runtime never reaches runtime.stop() or returns a failure. The fresh evidence beyond the earlier bounded/global recovery reports is that persistent recovery is now intentionally infinite but this same-profile Stop path has no cancellation mechanism, leaving the visible control loading until the host reconnects; explicit Stop should cancel or bypass recovery before attempting its own bounded stop.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbe5b8aa3f
ℹ️ 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".
| // Explicit Stop supersedes startup recovery. Do not wait for an unbounded | ||
| // offline client-proxy gate; any in-flight probe checks this cancellation | ||
| // before it can reserve the recovered operation. | ||
| this.cancelledStartupRecovery.add(profileId); |
There was a problem hiding this comment.
When Stop remote runtime is attempted during a transient SSH outage, this permanently cancels the profile's startup-recovery loop before runtime.stop() reports failure. For a startup-recovered client-proxy prompt, the remote process may still be running but no background probe will reconnect its required egress when the host returns, leaving the work untracked until an incidental manual session refresh; clear the cancellation or restart recovery when the stop fails, and cover this failed-Stop variant.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f98998d880
ℹ️ 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".
|
|
||
| /** Creates the session and runs its first prompt on one RPC port, then publishes the durable result. */ | ||
| async startSession(profileId: string, cwd: string, text: string): Promise<RemoteSessionStartResult | RemoteSessionSubmissionPending> { | ||
| await this.awaitProfileStartupRecovery(profileId); |
There was a problem hiding this comment.
When Send is issued while startup recovery is pending—especially for an offline client-proxy profile whose retry loop is unbounded—this await occurs before reserveOperation. RemoteSessionPage immediately renders Stop from its local starting/sending state, but abortSession finds no entry in activeOperations and returns false, leaving the IPC request and composer stuck until SSH recovers. Reserve a cancellable operation before waiting, or let the composer abort cancel the per-profile recovery.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5ed336657
ℹ️ 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".
| // Only client-proxy work depends on this app staying online after | ||
| // the bounded startup gate. Direct profiles remain daemon-owned and | ||
| // are rediscovered by their next explicit session refresh. | ||
| if (currentProfile.network.mode === "client-proxy") await this.retryStartupRecovery(profile.id); |
There was a problem hiding this comment.
For a remote-direct profile that still has a busy prompt when Jasmine restarts offline, this branch abandons recovery after three probes and removes the Send gate. If the host later returns and the user prompts the same session before an explicit refresh, daemon.ts lines 224–226 reclaims the matching RPC before checking busy, and sendRpc writes the second prompt into the already-running process. Continue discovery for direct profiles as well, or reject turn-starting commands when a reclaimed RPC is busy, and cover this egress-mode variant.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b3b5105d3
ℹ️ 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".
| await callbacks.onPromptDispatched?.(); | ||
| await port.prompt(text, [], callbacks.onPromptAccepted); |
There was a problem hiding this comment.
When the control connection disconnects after openSession but before port.prompt() invokes DaemonClient.request, that request rejects before calling writable.write, yet this callback has already marked the prompt as dispatched. RemoteProfileService consequently returns pending, causing the renderer to discard the draft, while detached monitoring sees the idle RPC as settled even though the prompt was never sent. Move the dispatch boundary into the transport write path and cover this pre-write disconnect for both new and existing sessions.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| } finally { | ||
| if (!removed) this.cancelledStartupRecovery.delete(profileId); |
There was a problem hiding this comment.
When removal cancels persistent startup recovery for an offline profile but ProfileStore.remove() then fails because of a write or lock error, the profile remains configured while its recovery promise has already exited. Clearing only the cancellation flag here does not schedule another probe, so an undiscovered remote operation—especially a client-proxy turn needing its egress restored—remains untracked and a later submission can race it. Restart recovery on every non-removed exit and cover the failed-removal variant.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Summary
agent_settled, reconciles the listing, and opens the returned transcriptsessions.readand daemon status (daemonId/activeRpc) protocol required by remote transcripts and restart recoveryRegression coverage
Validation
npm.cmd run buildnpm.cmd run test:unit: 23/23 suitesnpm.cmd run test:renderer: 22 files / 176 testsnpm.cmd run harness:checknpm.cmd run harness:inspectnpm.cmd run harness:visualplus an off-screen real-profile screenshot review of the remote draft pageControl+Shift+Space; the 3 startup-timing cases passed separately. CI must confirm the complete clean-session matrix.root@172.16.115.11:4560main-service run with an isolated temporary profile/cwd and local mock provider: durable session, user turn, assistant turn,sessions.read, and status patches[ready]all passed; remote profile/session/mock/cwd cleanup verified419eec35e672d098056279c4e4157cbaa10042b4f73ced8c571b04c8431e0835(the 0.1.1 build and dry-run below were repeated for 0.1.2)@jasmine-ai/pi-remote@0.1.2dry-run package: 42 files, runtime verify passedPost-review fix
tests/unit/remote-sessions.mjs)