Fix jupyter console frame and add to frame type selector#8750
Merged
haraldschilly merged 9 commits intomasterfrom Feb 20, 2026
Merged
Fix jupyter console frame and add to frame type selector#8750haraldschilly merged 9 commits intomasterfrom
haraldschilly merged 9 commits intomasterfrom
Conversation
…selector Bug 1: The jupyter console (View > Console) opened a panel showing "jupyter console --existing PATH" but the terminal never rendered. Root cause: frame tree args stored as Immutable.js Lists failed MsgPack serialization over conat, so the backend received garbled args and could not spawn the jupyter console process. Fix: add normalizeArgs() to convert Immutable Lists to plain string arrays. Bug 2: The jupyter console was only accessible from the View menu, not the frame type selector (+ button). It was also mis-classified as "Terminal" in the title bar. Fix: introduce a dedicated "shell" frame type in the jupyter EDITOR_SPEC, override shell()/new_frame()/ set_frame_type() in JupyterEditorActions to create "shell" frames with the correct jupyter console command and args. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
haraldschilly
added
the
PR-TODO-cocalc2
Contributor
Author
|
@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: a4fe6c731e
ℹ️ 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".
Address codex review comments and discovered issues: - clear() action now handles type==="shell" (was throwing "not implemented") - Shell frames reinitialize when command/args change (kernel restart, frame type switch) via a new useEffect in TerminalFrame watching both command and stringified args - watchJupyterStore updates ALL shell frames on connection_file change, not just the most recent one - set_frame_type handles shell→terminal transition by clearing command/args - setShellFrameCommand uses close_terminal + set_frame_tree instead of set_command (which the backend doesn't implement) - Hidden frames clear stale terminal refs so visibility effect can reinit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d kernel-aware shell frames
Backend (project/conat/terminal/manager.ts):
- Store CreateTerminalOptions in a persistent map instead of a closure
variable so options survive service reuse and can be updated.
- createTerminalService now reapplies options (via createTerminal) even
when the service already exists, so reconnecting with a new
connection_file starts a fresh session.
- kill handler uses closeTerminal() to properly remove session, service,
and options entries — prevents zombie sessions that caused blank
terminals on reconnect.
Frontend (jupyter-editor/actions.ts):
- watchJupyterStore now watches both backend_state and connection_file.
connection_file alone can be stale after kernel stops.
- New hasLiveKernelConnection() gate requires project running +
backend_state==="running" + connection_file present.
- setShellFrameCommand falls back to clearShellFrameCommand when kernel
is not live, putting the frame into "no command" mode.
- get_shell_spec gates on hasLiveKernelConnection so the frame type
selector doesn't offer stale console connections.
- Removed fire-and-forget conn_write({ cmd: "kill" }) that raced with
close_terminal.
Frontend (terminal-editor/terminal.tsx):
- Show "Kernel not running" message for shell frames without a command
instead of a blank terminal or connection error.
- init_terminal early-returns when shellNeedsKernel is true.
- useEffect deps include frameType so shell↔terminal transitions trigger
reinit, and no longer early-returns when terminalRef is null so
"needs kernel" → "has command" transitions properly initialize.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a799ef2087
ℹ️ 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".
… project state - ConnectedTerminal.close() now sends api.kill() for jupyter-console terminals before disconnecting, preventing stale session resurrection when reopening the same console frame. - watchJupyterStore also watches projects store for project run-state changes, so shell frames are resynced when the project stops/starts. - Initial syncShellFrames() call on setup ensures page reload immediately reconciles shell frames with current kernel/project state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
terminal() was calling _get_most_recent_shell_id() which matches shell frames with command===undefined (kernel-not-running state). Switch to _get_most_recent_terminal_id() so only actual terminal-type frames are reused, not shell placeholders. Fixes codex review comment on PR #8750. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 283264337e
ℹ️ 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/packages/frontend/frame-editors/terminal-editor/connected-terminal.ts
Outdated
Show resolved
Hide resolved
Contributor
|
I fixed this in cocalc-ai in these two commits so no need to tag this as needed to be rebased. |
williamstein
removed
the
PR-TODO-cocalc2
…close-kill - shell() now always creates/reuses a "shell" frame type instead of falling back to a plain terminal when kernel is not running. setShellFrameCommand handles kernel state, showing "Kernel not running" placeholder instead of opening bash. - Remove api.kill() from ConnectedTerminal.close() — killing the backend session on every frame close destroys shared sessions in multi-tab and collaborative scenarios. Backend createTerminal already handles stale sessions via options comparison. - Clean up projects/jupyter store listeners on JupyterEditorActions.close() to prevent leaked references and stale syncShellFrames calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84a68c6dde
ℹ️ 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".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
Project restart required. This PR includes backend changes to
packages/project/conat/terminal/manager.ts(terminal session lifecycle). After deploying, projects must be restarted for the fix to take effect.Summary
Bug 1 — Console never renders: The jupyter console (View > Console in a notebook) opened a panel showing "jupyter console --existing PATH" but the actual terminal never appeared. Root cause: frame tree args stored as Immutable.js Lists failed MsgPack serialization over conat, so the backend received garbled args and could not spawn the process. Fixed by adding
normalizeArgs()to convert Immutable Lists to plainstring[].Bug 2 — Console not in frame type selector: The jupyter console was only accessible from the View menu, not the frame type selector ("+" button). It was also mis-classified as "Terminal" in the title bar. Fixed by introducing a dedicated
"shell"frame type in the jupyter EDITOR_SPEC, with overrides forshell(),new_frame(), andset_frame_type()inJupyterEditorActions.Bug 3 — Terminal lifecycle for shell frames (codex review follow-up):
clear()action threw "not implemented" for shell frames — fixed by handlingtype === "shell"alongside"terminal".close_terminal()+set_frame_tree(). TheTerminalFramecomponent now watchesframeType,command, andargsfor changes and reinitializes automatically.watchJupyterStorenow watches bothbackend_stateandconnection_fileand updates ALL shell frames, not just the most recent.connection_filealone is unreliable since it persists after kernel stops.command/argsso it reverts to bash.Bug 4 — Backend terminal session zombies:
TerminalManager.kill()only calledsession.close()without removing the session from its maps, leaving zombie entries that caused blank terminals on reconnect. Fixed by usingcloseTerminal()which properly removes session, service, and options entries.CreateTerminalOptionswere stored in a closure variable that was never updated after initial service creation. Moved to a persistentoptionsmap so reconnecting with a newconnection_fileactually starts a fresh session.createTerminalService()now reapplies options viacreateTerminal()even when the service already exists.Bug 5 — UX for kernel-not-running state:
hasLiveKernelConnection()gate requires project running +backend_state === "running"+connection_filepresent. Used insetShellFrameCommand()andget_shell_spec().watchJupyterStoredetects the state change and automatically connects existing shell frames.Bug 6 — Stale session on console close/reopen & project state sync:
watchJupyterStorenow also watches the projects store for project run-state changes, resyncing shell frames when the project stops/starts.syncShellFrames()call on setup ensures page reload immediately reconciles shell frames with current kernel/project state.Bug 7 — View > Console fell back to plain terminal when kernel not running:
shell()now always creates/reuses a"shell"frame type instead of falling back tosuper.shell()(plain terminal).setShellFrameCommand()handles kernel state — shows "Kernel not running" placeholder when kernel is down, auto-connects when kernel starts.Bug 8 — Leaked store listeners on editor close:
projects.on("change")andstore.on("change")listeners registered bywatchJupyterStorewere never removed whenJupyterEditorActionswas closed, causing leaked references and stalesyncShellFrames()calls. Fixed by storing a cleanup function and calling it inclose().Bug 9 —
terminal()action reused cleared shell frames:terminal()called_get_most_recent_shell_id()which matched shell frames withcommand === undefined(kernel-not-running state). Switched to_get_most_recent_terminal_id()to only match actual terminal-type frames.Test plan
🤖 Generated with Claude Code