Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
manzt
added this pull request to stack #10823
September 13, 2026 22:10
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
manzt
force-pushed
the
manzt/sandbox-ui
branch
from
September 13, 2026 22:12
7054ac6 to
79d2941
Compare
manzt
marked this pull request as ready for review
September 13, 2026 22:13
Contributor
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
11 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/core/network/connection-notice.ts">
<violation number="1" location="frontend/src/core/network/connection-notice.ts:15">
P2: When a sandbox sync fails while the connection is `CLOSED`, this `OPEN` guard skips `sync.error` and replaces the diagnostic with the generic `connection.reason`. Let `sync.error` take precedence regardless of connection state.</violation>
</file>
<file name="frontend/src/components/editor/chrome/panels/packages-panel.tsx">
<violation number="1" location="frontend/src/components/editor/chrome/panels/packages-panel.tsx:170">
P2: While a sandbox sync is pending, this fieldset disables only package installation; `UpgradeButton` and `RemoveButton` remain clickable outside it. Disable those mutation controls during sync to prevent package changes racing with manifest application.</violation>
</file>
<file name="frontend/src/components/editor/chrome/panels/sandbox-panel.tsx">
<violation number="1" location="frontend/src/components/editor/chrome/panels/sandbox-panel.tsx:80">
P1: When sandbox environment preparation succeeds but the kernel then fails to start, this `Retry sync` action only reapplies the manifest and leaves the notebook disconnected because `syncSandbox` does not request a reconnect for an existing sandbox. Route this recovery action through the kernel reconnect path, or only show it for actual sandbox-sync failures.</violation>
</file>
<file name="marimo/_server/api/endpoints/packages.py">
<violation number="1" location="marimo/_server/api/endpoints/packages.py:280">
P1: When the server is running in single-process sandbox mode, `manager.sandbox` is false even though `GLOBAL_SETTINGS.SANDBOX_MODE` is `"single"`. This branch makes all pre-start sandbox responses empty and prevents the UI from reading or editing the manifest before kernel startup; gate on the configured sandbox mode instead.</violation>
</file>
<file name="marimo/_utils/inline_script_metadata.py">
<violation number="1" location="marimo/_utils/inline_script_metadata.py:411">
P3: This change fixes a real bug: a `pyproject` frontmatter value beginning with a `#` comment used to be left unwrapped, so `script_metadata.loads` returned None and the block was silently ignored. No regression test covers the new behavior. Add a test that a comment-leading raw TOML `pyproject` is wrapped and parsed (e.g. `get_headers_from_markdown`/`_get_pyproject_from_filename` on a `.md` with `pyproject: |\n # comment\n dependencies = ["numpy"]`), and that a value already starting with `# /// script` is not double-wrapped.</violation>
</file>
<file name="tests/_environments/test_async_preparation.py">
<violation number="1" location="tests/_environments/test_async_preparation.py:142">
P2: `source.write_text(...)` runs inside the async test and is a blocking pathlib I/O call, which ruff's ASYNC240 flags. Every sibling pathlib call in this file (`markdown.read_text()`, `carrier.exists()`, `Path(owner.path).exists()`) carries `# noqa: ASYNC240`, so this new call without the comment will fail the `make py-check` lint step. Append `# noqa: ASYNC240` to the write.</violation>
</file>
<file name="frontend/src/components/editor/chrome/panels/sandbox-controller.tsx">
<violation number="1" location="frontend/src/components/editor/chrome/panels/sandbox-controller.tsx:85">
P3: `sync()` returns `false` without setting any error when the connection is `WebSocketState.CONNECTING` or when the websocket ends `CLOSED` after reconnect. The visible consequence: in `SandboxStartupPanel` the "Retry sync" button is not disabled during startup (`disabled={!actions}` only), so clicking it during environment preparation silently does nothing, and in `saveAndSync` a reconnect that ends CLOSED silently aborts with the dialog left open and no diagnostic in the dialog itself (only the sidebar notice). Set an error (or disable the button while the connection is CONNECTING) on these false paths so the UI explains why nothing happened.</violation>
<violation number="2" location="frontend/src/components/editor/chrome/panels/sandbox-controller.tsx:100">
P2: `waitFor(connectionAtom, ...)` after `onReconnect()` has no timeout. If reconnection stalls in `WebSocketState.CONNECTING` (network partition, server backoff), the promise never settles: `sync()` stays in the `finally`-guarded region, so `inFlight.current` stays `true`, `operation.pending` stays `true`, and the dialog/footer display an endless "Syncing…" with all sync buttons permanently disabled until reload. Wrap the wait in a timeout and treat a timeout as a sync failure (clear the in-flight flag and set `operation.error`).</violation>
</file>
<file name="frontend/src/core/packages/useInstallPackage.ts">
<violation number="1" location="frontend/src/core/packages/useInstallPackage.ts:40">
P3: When a package install succeeds but the backend responds with restartRequired (success=false, restart_required=true, e.g. sandbox sync), onSuccess is no longer invoked, while it was before this change. Since restartRequired still means the packages were saved successfully, callers like packages-panel.tsx that clear the input on success will now leave the stale package text in the field after a "Changes saved — restart required" install. If that is unintended, invoke onSuccess?.() in the restartRequired branch as well; if it is deliberate, the asymmetry between the two success-like paths is worth a comment.</violation>
</file>
<file name="marimo/_environments/script_metadata.py">
<violation number="1" location="marimo/_environments/script_metadata.py:117">
P2: `write_manifest` does a read-modify-write of the whole notebook file with no lock against the running kernel, so a concurrent notebook autosave can be clobbered. The `_stable_carrier_lock` serializes manifest writers among themselves, but the kernel's save path does not take it; if the kernel writes the file between `source.read()` and `destination.write()`, the freshly read `script` (with cells from before the autosave) is written back and the autosaved cell edits are lost. Replace the in-place truncating write with an atomic write (temp file + `os.replace`) to at least avoid torn/interleaved writes.</violation>
</file>
<file name="frontend/src/core/websocket/useMarimoKernelConnection.tsx">
<violation number="1" location="frontend/src/core/websocket/useMarimoKernelConnection.tsx:597">
P3: A transient close during the initial startup CONNECTING phase drops the phase. When previous.state is CONNECTING with phase 'preparing-environment'/'starting-kernel' (not 'reconnecting'), the CONNECTING branch falls through to plain `{ state: CONNECTING }`, so connection-notice.ts shows the generic 'Connecting…' title and a later startup failure loses the phase needed to label it 'Sandbox setup failed'. Treat any CONNECTING→CONNECTING retry close as reconnecting, or preserve the existing phase.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| variant="text" | ||
| size="xs" | ||
| disabled={!actions} | ||
| onClick={() => actions?.sync()} |
Contributor
There was a problem hiding this comment.
P1: When sandbox environment preparation succeeds but the kernel then fails to start, this Retry sync action only reapplies the manifest and leaves the notebook disconnected because syncSandbox does not request a reconnect for an existing sandbox. Route this recovery action through the kernel reconnect path, or only show it for actual sandbox-sync failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/chrome/panels/sandbox-panel.tsx, line 80:
<comment>When sandbox environment preparation succeeds but the kernel then fails to start, this `Retry sync` action only reapplies the manifest and leaves the notebook disconnected because `syncSandbox` does not request a reconnect for an existing sandbox. Route this recovery action through the kernel reconnect path, or only show it for actual sandbox-sync failures.</comment>
<file context>
@@ -0,0 +1,173 @@
+ variant="text"
+ size="xs"
+ disabled={!actions}
+ onClick={() => actions?.sync()}
+ >
+ Retry sync
</file context>
| if isinstance(sandbox, NotebookSandbox): | ||
| return sandbox, sandbox.source, sandbox.backend | ||
| return None, None, None | ||
| if not manager.sandbox: |
Contributor
There was a problem hiding this comment.
P1: When the server is running in single-process sandbox mode, manager.sandbox is false even though GLOBAL_SETTINGS.SANDBOX_MODE is "single". This branch makes all pre-start sandbox responses empty and prevents the UI from reading or editing the manifest before kernel startup; gate on the configured sandbox mode instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_server/api/endpoints/packages.py, line 280:
<comment>When the server is running in single-process sandbox mode, `manager.sandbox` is false even though `GLOBAL_SETTINGS.SANDBOX_MODE` is `"single"`. This branch makes all pre-start sandbox responses empty and prevents the UI from reading or editing the manifest before kernel startup; gate on the configured sandbox mode instead.</comment>
<file context>
@@ -249,3 +260,142 @@ def _get_filename(request: Request) -> str | None:
+ if isinstance(sandbox, NotebookSandbox):
+ return sandbox, sandbox.source, sandbox.backend
+ return None, None, None
+ if not manager.sandbox:
+ return None, None, None
+ key = file_key or manager.workspace.get_unique_file_key()
</file context>
Suggested change
| if not manager.sandbox: | |
| if GLOBAL_SETTINGS.SANDBOX_MODE is None: |
Comment on lines
+15
to
+16
| (sync.pending || sync.error) && | ||
| connection.state === WebSocketState.OPEN |
Contributor
There was a problem hiding this comment.
P2: When a sandbox sync fails while the connection is CLOSED, this OPEN guard skips sync.error and replaces the diagnostic with the generic connection.reason. Let sync.error take precedence regardless of connection state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/core/network/connection-notice.ts, line 15:
<comment>When a sandbox sync fails while the connection is `CLOSED`, this `OPEN` guard skips `sync.error` and replaces the diagnostic with the generic `connection.reason`. Let `sync.error` take precedence regardless of connection state.</comment>
<file context>
@@ -0,0 +1,84 @@
+ const sync = get(sandboxSyncAtom);
+ if (
+ sandbox &&
+ (sync.pending || sync.error) &&
+ connection.state === WebSocketState.OPEN
+ ) {
</file context>
Suggested change
| (sync.pending || sync.error) && | |
| connection.state === WebSocketState.OPEN | |
| (sync.error || | |
| (sync.pending && connection.state === WebSocketState.OPEN)) |
| <div className="flex-1 flex flex-col overflow-hidden"> | ||
| <InstallPackageForm context={dependencies.context} onSuccess={refetch} /> | ||
| {(isTreeSupported || isSandbox) && ( | ||
| <fieldset disabled={syncing} className="contents"> |
Contributor
There was a problem hiding this comment.
P2: While a sandbox sync is pending, this fieldset disables only package installation; UpgradeButton and RemoveButton remain clickable outside it. Disable those mutation controls during sync to prevent package changes racing with manifest application.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/chrome/panels/packages-panel.tsx, line 170:
<comment>While a sandbox sync is pending, this fieldset disables only package installation; `UpgradeButton` and `RemoveButton` remain clickable outside it. Disable those mutation controls during sync to prevent package changes racing with manifest application.</comment>
<file context>
@@ -122,58 +163,49 @@ const PackagesPanel: React.FC = () => {
<div className="flex-1 flex flex-col overflow-hidden">
- <InstallPackageForm context={dependencies.context} onSuccess={refetch} />
- {(isTreeSupported || isSandbox) && (
+ <fieldset disabled={syncing} className="contents">
+ <InstallPackageForm context={dependencies.context} />
+ </fieldset>
</file context>
| }, [requests, filename, connection.state, setSandbox]); | ||
|
|
||
| const sync = useEvent(async () => { | ||
| if (inFlight.current || connection.state === WebSocketState.CONNECTING) { |
Contributor
There was a problem hiding this comment.
P3: sync() returns false without setting any error when the connection is WebSocketState.CONNECTING or when the websocket ends CLOSED after reconnect. The visible consequence: in SandboxStartupPanel the "Retry sync" button is not disabled during startup (disabled={!actions} only), so clicking it during environment preparation silently does nothing, and in saveAndSync a reconnect that ends CLOSED silently aborts with the dialog left open and no diagnostic in the dialog itself (only the sidebar notice). Set an error (or disable the button while the connection is CONNECTING) on these false paths so the UI explains why nothing happened.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/chrome/panels/sandbox-controller.tsx, line 85:
<comment>`sync()` returns `false` without setting any error when the connection is `WebSocketState.CONNECTING` or when the websocket ends `CLOSED` after reconnect. The visible consequence: in `SandboxStartupPanel` the "Retry sync" button is not disabled during startup (`disabled={!actions}` only), so clicking it during environment preparation silently does nothing, and in `saveAndSync` a reconnect that ends CLOSED silently aborts with the dialog left open and no diagnostic in the dialog itself (only the sidebar notice). Set an error (or disable the button while the connection is CONNECTING) on these false paths so the UI explains why nothing happened.</comment>
<file context>
@@ -0,0 +1,257 @@
+ }, [requests, filename, connection.state, setSandbox]);
+
+ const sync = useEvent(async () => {
+ if (inFlight.current || connection.state === WebSocketState.CONNECTING) {
+ return false;
+ }
</file context>
| showPackageRestartToast(); | ||
| } else if (response.success) { | ||
| showAddPackageToast(packages); | ||
| onSuccess?.(); |
Contributor
There was a problem hiding this comment.
P3: When a package install succeeds but the backend responds with restartRequired (success=false, restart_required=true, e.g. sandbox sync), onSuccess is no longer invoked, while it was before this change. Since restartRequired still means the packages were saved successfully, callers like packages-panel.tsx that clear the input on success will now leave the stale package text in the field after a "Changes saved — restart required" install. If that is unintended, invoke onSuccess?.() in the restartRequired branch as well; if it is deliberate, the asymmetry between the two success-like paths is worth a comment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/core/packages/useInstallPackage.ts, line 40:
<comment>When a package install succeeds but the backend responds with restartRequired (success=false, restart_required=true, e.g. sandbox sync), onSuccess is no longer invoked, while it was before this change. Since restartRequired still means the packages were saved successfully, callers like packages-panel.tsx that clear the input on success will now leave the stale package text in the field after a "Changes saved — restart required" install. If that is unintended, invoke onSuccess?.() in the restartRequired branch as well; if it is deliberate, the asymmetry between the two success-like paths is worth a comment.</comment>
<file context>
@@ -37,10 +37,10 @@ export function useInstallPackages(): {
showPackageRestartToast();
} else if (response.success) {
showAddPackageToast(packages);
+ onSuccess?.();
} else {
showAddPackageToast(packages, response.error);
</file context>
Comment on lines
+597
to
+602
| (previous.state === WebSocketState.CONNECTING && | ||
| previous.phase === "reconnecting")) | ||
| ) { | ||
| return { ...status, phase: "reconnecting" }; | ||
| } | ||
| return status; |
Contributor
There was a problem hiding this comment.
P3: A transient close during the initial startup CONNECTING phase drops the phase. When previous.state is CONNECTING with phase 'preparing-environment'/'starting-kernel' (not 'reconnecting'), the CONNECTING branch falls through to plain { state: CONNECTING }, so connection-notice.ts shows the generic 'Connecting…' title and a later startup failure loses the phase needed to label it 'Sandbox setup failed'. Treat any CONNECTING→CONNECTING retry close as reconnecting, or preserve the existing phase.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/core/websocket/useMarimoKernelConnection.tsx, line 597:
<comment>A transient close during the initial startup CONNECTING phase drops the phase. When previous.state is CONNECTING with phase 'preparing-environment'/'starting-kernel' (not 'reconnecting'), the CONNECTING branch falls through to plain `{ state: CONNECTING }`, so connection-notice.ts shows the generic 'Connecting…' title and a later startup failure loses the phase needed to label it 'Sandbox setup failed'. Treat any CONNECTING→CONNECTING retry close as reconnecting, or preserve the existing phase.</comment>
<file context>
@@ -564,7 +581,26 @@ export function useMarimoKernelConnection(opts: {
+ if (
+ status.state === WebSocketState.CONNECTING &&
+ (previous.state === WebSocketState.OPEN ||
+ (previous.state === WebSocketState.CONNECTING &&
+ previous.phase === "reconnecting"))
+ ) {
</file context>
Suggested change
| (previous.state === WebSocketState.CONNECTING && | |
| previous.phase === "reconnecting")) | |
| ) { | |
| return { ...status, phase: "reconnecting" }; | |
| } | |
| return status; | |
| if ( | |
| status.state === WebSocketState.CONNECTING && | |
| (previous.state === WebSocketState.OPEN || | |
| previous.state === WebSocketState.CONNECTING) | |
| ) { | |
| return { ...status, phase: "reconnecting" }; | |
| } | |
| return status; |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings affect sandbox synchronization, notebook isolation, startup guards, and manifest handling.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds asynchronous sandbox startup and recovery, pre-kernel manifest APIs, conflict-safe editing, and package-panel state handling.
Changes:
- Added sandbox manifest and synchronization APIs.
- Added conflict-safe manifest editing and recovery workflows.
- Added frontend startup, package, connection, and testing updates.
File summaries
| File | Review summary |
|---|---|
tests/_server/api/endpoints/test_packages.py |
No final comment. |
tests/_environments/test_script_metadata.py |
No final comment. |
tests/_environments/test_sandbox_interface.py |
No final comment. |
tests/_environments/test_async_preparation.py |
No final comment. |
packages/openapi/src/api.ts |
No final comment. |
packages/openapi/api.yaml |
No final comment. |
marimo/_utils/inline_script_metadata.py |
Moderate (1 vote): Check for the exact # /// script marker rather than a prefix to avoid misclassifying raw TOML comments. |
marimo/_server/session_manager.py |
No final comment. |
marimo/_server/models/packages.py |
No final comment. |
marimo/_server/api/endpoints/packages.py |
Critical (1 vote): Guard pending mutations by notebook file rather than session ID. Moderate (1 vote): Mirror or exclude explicitly configured virtual environments in the pre-session branch. Moderate (1 vote): Map OSError and PermissionError from manifest writes to an operation error. |
marimo/_environments/script_metadata.py |
Critical (2 votes): Reject already-commented script markers before wrapping to prevent nested blocks and manifest corruption. |
marimo/_environments/sandbox.py |
Moderate (1 vote): Keep runtime-managed marimo display-only and suppress its upgrade action. |
marimo/_environments/backends.py |
No final comment. |
marimo/_cli/development/commands.py |
No final comment. |
frontend/src/core/websocket/useMarimoKernelConnection.tsx |
No final comment. |
frontend/src/core/websocket/types.ts |
No final comment. |
frontend/src/core/websocket/__tests__/useMarimoKernelConnection.hook.test.tsx |
No final comment. |
frontend/src/core/wasm/bridge.ts |
No final comment. |
frontend/src/core/run-app.tsx |
No final comment. |
frontend/src/core/packages/useInstallPackage.ts |
No final comment. |
frontend/src/core/packages/toast-components.tsx |
No final comment. |
frontend/src/core/packages/sandbox-state.ts |
No final comment. |
frontend/src/core/packages/package-data.ts |
No final comment. |
frontend/src/core/network/types.ts |
No final comment. |
frontend/src/core/network/resolve.ts |
No final comment. |
frontend/src/core/network/requests-toasting.tsx |
No final comment. |
frontend/src/core/network/requests-static.ts |
No final comment. |
frontend/src/core/network/requests-network.ts |
No final comment. |
frontend/src/core/network/requests-lazy.ts |
No final comment. |
frontend/src/core/network/connection-notice.ts |
No final comment. |
frontend/src/core/MarimoApp.tsx |
No final comment. |
frontend/src/core/islands/bridge.ts |
No final comment. |
frontend/src/core/errors/state.ts |
No final comment. |
frontend/src/core/edit-app.tsx |
No final comment. |
frontend/src/components/editor/renderers/cell-array.tsx |
No final comment. |
frontend/src/components/editor/header/status.tsx |
No final comment. |
frontend/src/components/editor/header/__tests__/status.test.tsx |
No final comment. |
frontend/src/components/editor/chrome/wrapper/footer-items/backend-status.tsx |
No final comment. |
frontend/src/components/editor/chrome/panels/sandbox-panel.tsx |
Moderate (1 vote): Disable manifest editing while startup is pending. |
frontend/src/components/editor/chrome/panels/sandbox-controller.tsx |
Critical (1 vote): Associate draft state with the current filename to prevent cross-notebook edits. Moderate (1 vote): Clear or key global sandbox state by filename to prevent stale metadata. |
frontend/src/components/editor/chrome/panels/packages-panel.tsx |
Critical (1 vote): Disable dependency-tree Upgrade and Remove actions during synchronization or serialize sandbox mutations. |
frontend/src/components/editor/chrome/panels/__tests__/sandbox-recovery.test.tsx |
No final comment. |
frontend/src/components/editor/chrome/panels/__tests__/packages-panel.test.tsx |
No final comment. |
frontend/src/components/editor/app-container.tsx |
No final comment. |
frontend/src/components/editor/alerts/connection-notice.tsx |
No final comment. |
frontend/src/components/editor/alerts/connecting-alert.tsx |
No final comment. |
frontend/src/__mocks__/requests.ts |
No final comment. |
Review details
Suppressed comments (6)
frontend/src/components/editor/chrome/panels/sandbox-controller.tsx:75
sandboxAtomis global and is only updated after this request resolves, so it retains the previous notebook's manifest/backend whilefilenamehas already changed. The Packages panel and connection notice can therefore show stale sandbox metadata (and expose edit/sync actions for it); if this request fails, the stale value can remain indefinitely. Clear or key the atom by filename and only render metadata matching the current notebook.
let cancelled = false;
requests
.getSandbox({ fileKey: filename })
.then((value) => {
if (!cancelled) {
setSandbox(value);
}
})
frontend/src/components/editor/chrome/panels/sandbox-panel.tsx:161
- While startup is pending, this item remains enabled even though both manifest mutation endpoints reject
_sandbox_source(..., mutation=True)with 409. Clicking Edit manifest from the footer therefore opens an editor that cannot save and is reported as a stale-manifest conflict; disable it duringpending, as the Sync item already does.
disabled={!actions || sandbox.manifest === null}
marimo/_environments/sandbox.py:489
- Returning the runtime
marimopackage here makes it a top-level actionable node inpackages-panel.tsx: the tree rendersUpgradeButtonfor every top-level package, while onlyRemoveButtonsuppressesmarimo. Clicking Upgrade therefore callssandbox.add("marimo", upgrade=true), which can addmarimoto the notebook manifest even though the sandbox treats it as runtime-managed and explicitly forbids removing it. Keep runtime-only packages display-only or suppress the upgrade action formarimo.
marimo/_server/api/endpoints/packages.py:284 - This pre-session branch treats every
manager.sandboxsession as an ephemeral script sandbox, butIPCKernelManagerImpl.start_kernelgives an explicitly configured[tool.marimo.venv]precedence and strips the sandbox metadata routing. Before the kernel starts, this endpoint therefore advertises a manifest/editor for a runtime that will not use it, then changes tobackend: nullonce the session exists. Exclude configured-venv sessions here or mirror the kernel's sandbox selection.
marimo/_server/api/endpoints/packages.py:357 write_manifestperforms direct file writes for Python notebooks and can raiseOSError/PermissionError(frontmatter commits can also do so), but this endpoint does not catch it. A read-only or otherwise unwritable notebook therefore escapes as a 500 instead of returning the manifest-edit error that the recovery UI can display; the sync handler below already treatsOSErroras an operation failure. IncludeOSErrorin this exception mapping.
marimo/_utils/inline_script_metadata.py:411- This prefix check also treats valid raw TOML beginning with a comment such as
# /// script-not-a-blockas an already wrapped PEP 723 block. The frontmatter is then returned withoutwrap_block, so the metadata reader will not recognize its dependencies; check the first line for the exact# /// scriptmarker instead of using a prefix.
- Files reviewed: 47/47 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+170
to
+172
| <fieldset disabled={syncing} className="contents"> | ||
| <InstallPackageForm context={dependencies.context} /> | ||
| </fieldset> |
Comment on lines
+52
to
+55
| const [document, setDocument] = useState<{ | ||
| original: string; | ||
| draft: string; | ||
| } | null>(null); |
| def write_manifest(path: str, contents: str, *, previous: str) -> str: | ||
| """Replace only metadata, rejecting stale edits and invalid TOML.""" | ||
| toml_reader.reads(contents) | ||
| if any(line.startswith("///") for line in contents.splitlines()): |
Comment on lines
+270
to
+272
| if mutation and manager.is_session_starting( | ||
| state.require_current_session_id() | ||
| ): |
manzt
force-pushed
the
manzt/sandbox-ui
branch
from
September 13, 2026 22:37
50cb5c8 to
300484a
Compare
manzt
force-pushed
the
manzt/sandbox-ui
branch
from
September 13, 2026 22:54
1972543 to
fe5ca84
Compare
manzt
force-pushed
the
manzt/sandbox-ui
branch
from
September 14, 2026 15:37
fe5ca84 to
c9625b1
Compare
Bundle ReportChanges will increase total bundle size by 15.12kB (0.06%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: marimo-esmAssets Changed:
Files in
Files in
Files in
Files in
Files in
Files in
Files in
Files in
Files in
Files in
|
mscolnick
previously approved these changes
Sep 15, 2026
manzt
force-pushed
the
manzt/sandbox-ui
branch
from
September 15, 2026 15:04
c9625b1 to
240375b
Compare
Show startup progress after 500 ms, with layouts for empty notebooks and notebooks with existing cells. Present failures immediately and link to diagnostics and recovery actions in Packages. Let users edit the full manifest and sync without leaving the notebook. Keep drafts through failures and stale saves; close the editor only after successful sync. Refresh package data after mutations, include marimo in the package view, and show sandbox state beside its actions. Keep healthy footers quiet and retain the connecting indicator. Cover startup timing, manifest recovery, stale drafts, and package refreshes.
for more information, see https://pre-commit.ci
manzt
force-pushed
the
manzt/sandbox-ui
branch
from
September 15, 2026 15:05
240375b to
f7ce118
Compare
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
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.
Async startup lets the browser open before the kernel is ready. This PR adds pending states for empty notebooks and notebooks with existing cells.
Packages gains sandbox status, sync, and a full TOML manifest editor. Recovery works before a kernel exists, while failed syncs keep the editor open with the user's changes, and stale saves cannot overwrite external edits. Package changes refresh the panel automatically.