Repository navigation
Add credential-free Sprite base builder - #9618
lawrencecchen wants to merge 4 commits into
Conversation
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
📝 WalkthroughWalkthroughAdds a strict Bash script to create credential-free Fly Sprite cmux checkpoints. Adds documentation for Sprite setup, authentication, client enrollment, revocation, and public connections. Updates exited hosted-surface geometry handling and adds Unix regression coverage. ChangesFly Sprites support
Exited hosted-surface sizing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant build-sprite-base.sh
participant Sprite CLI
participant Fly Sprite
participant cmux-tui service
Operator->>build-sprite-base.sh: provide options and SPRITE_TOKEN
build-sprite-base.sh->>Sprite CLI: create Sprite and enable public URL
Sprite CLI->>Fly Sprite: provision Sprite
build-sprite-base.sh->>Fly Sprite: install pinned cmux
build-sprite-base.sh->>cmux-tui service: register and stop service
build-sprite-base.sh->>Fly Sprite: remove runtime state and create checkpoint
build-sprite-base.sh->>cmux-tui service: optionally start service
build-sprite-base.sh-->>Operator: print checkpoint metadata
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors)
✅ Passed checks (23 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
🤖 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 `@cmux-tui/docs/sprites.md`:
- Around line 57-63: Update the invitation-creation instructions around cmux
enroll create so the command executes inside the Sprite through authenticated
sprite exec, using the specified organization and Sprite, before returning the
invitation; preserve the existing state directory and enrollment arguments.
In `@cmux-tui/scripts/build-sprite-base.sh`:
- Around line 143-148: Update the build metadata output in the script’s
reporting block to emit the required tagged localhost report link at
http://127.0.0.1:17320/<tag>. Derive the URL from the existing build tag value,
and preserve the current metadata lines without substituting a file:// URL or
local artifact path.
- Around line 69-71: Update the missing-credential error in
build-sprite-base.sh’s SPRITE_TOKEN validation to use product-level wording
without exposing the SPRITE_TOKEN environment-variable name. Keep the stderr
output and exit status unchanged, and instruct users to source the authorized
credential file before retrying.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d3d133f0-c649-4f2e-98ab-3e1b4f1173e3
📒 Files selected for processing (4)
cmux-tui/README.mdcmux-tui/docs/README.mdcmux-tui/docs/sprites.mdcmux-tui/scripts/build-sprite-base.sh
| cmux enroll create \ | ||
| --session sprite \ | ||
| --state-dir /home/sprite/.local/share/cmux-sprite/remote \ | ||
| --advertise wss://<sprite-host>/v1/link \ | ||
| --ttl 300 \ | ||
| --json | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run invitation creation inside the Sprite.
This block runs cmux enroll create on the reader host, but --state-dir /home/sprite/... identifies daemon state inside the Sprite. The command will target a local admin socket or fail. Wrap this command in authenticated sprite exec -o <org> -s <sprite> -- ..., or state that the backend performs that exact operation before it returns the invitation.
The later approval step already specifies authenticated Sprite exec. The creation step needs the same execution boundary.
🤖 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 `@cmux-tui/docs/sprites.md` around lines 57 - 63, Update the
invitation-creation instructions around cmux enroll create so the command
executes inside the Sprite through authenticated sprite exec, using the
specified organization and Sprite, before returning the invitation; preserve the
existing state directory and enrollment arguments.
| if [[ -z "${SPRITE_TOKEN:-}" ]]; then | ||
| echo "SPRITE_TOKEN is required" >&2 | ||
| exit 2 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced script around the reported validation and the surrounding
# output behavior without executing repository code.
if [ -f cmux-tui/scripts/build-sprite-base.sh ]; then
nl -ba cmux-tui/scripts/build-sprite-base.sh | sed -n '1,140p'
printf '\n--- occurrences of SPRITE_TOKEN ---\n'
rg -n 'SPRITE_TOKEN|sprite|echo ".*error|>&2' cmux-tui/scripts/build-sprite-base.sh || true
else
printf 'Referenced file not found\n'
fi
printf '\n--- shell syntax safety probe ---\n'
sh -n cmux-tui/scripts/build-sprite-base.sh && echo "syntax ok" || echo "syntax error"Repository: manaflow-ai/cmux
Length of output: 194
Information Disclosure (CWE-209): Generation of Error Message Containing Sensitive Information
Reachability: Internal
Do not expose the credential environment-variable name in the error.
The script writes SPRITE_TOKEN is required to stderr, which exposes a credential configuration name to callers who run the script without credentials. Use product-level text instead, such as “A Sprite management credential is required. Source the authorized credential file and retry.”
🤖 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 `@cmux-tui/scripts/build-sprite-base.sh` around lines 69 - 71, Update the
missing-credential error in build-sprite-base.sh’s SPRITE_TOKEN validation to
use product-level wording without exposing the SPRITE_TOKEN environment-variable
name. Keep the stderr output and exit status unchanged, and instruct users to
source the authorized credential file before retrying.
Source: Coding guidelines
| printf '\nSPRITE_BASE_ORG=%s\n' "$org" | ||
| printf 'SPRITE_BASE_NAME=%s\n' "$name" | ||
| printf 'SPRITE_BASE_CHECKPOINT=%s\n' "$checkpoint" | ||
| printf 'SPRITE_BASE_CMUX_VERSION=%s\n' "$cmux_version" | ||
| printf 'SPRITE_BASE_DAEMON_STATE=absent\n' | ||
| printf 'SPRITE_BASE_SERVICE=%s\n' "$([[ "$keep_running" -eq 1 ]] && echo running || echo stopped)" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Report the checkpoint build with the required tagged link.
Lines 143-148 report build metadata but do not emit http://127.0.0.1:17320/<tag>. Add the required tagged build-report link. Do not replace it with a file:// URL or a local artifact path.
As per coding guidelines, shell builds must use the tagged localhost report link.
🤖 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 `@cmux-tui/scripts/build-sprite-base.sh` around lines 143 - 148, Update the
build metadata output in the script’s reporting block to emit the required
tagged localhost report link at http://127.0.0.1:17320/<tag>. Derive the URL
from the existing build tag value, and preserve the current metadata lines
without substituting a file:// URL or local artifact path.
Source: Coding guidelines
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@cmux-tui/crates/cmux-tui-core/src/mux.rs`:
- Around line 16220-16225: The test setup around
insert_terminal_identity_surface must create the “exited-cell-pixels” workspace
in the empty test_mux state before projecting the terminal. Capture the created
workspace and pass its workspace.key instead of the literal workspace
identifier, preserving the existing projection and set_cell_pixel_size flow.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 86798f76-e09d-47bd-8094-9be2b591483a
📒 Files selected for processing (1)
cmux-tui/crates/cmux-tui-core/src/mux.rs
| let surface = insert_terminal_identity_surface( | ||
| &mux, | ||
| "00112233445566778899aabbccddeeff", | ||
| "11111111111111111111111111111111", | ||
| "exited-cell-pixels", | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="cmux-tui/crates/cmux-tui-core/src/mux.rs"
rg -n -C 8 \
'fn test_mux|insert_terminal_identity_surface|project_terminal_to_workspace_in_state|exited_host_cell_pixels_converge_without_a_live_host' \
"$file"Repository: manaflow-ai/cmux
Length of output: 11417
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="cmux-tui/crates/cmux-tui-core/src/mux.rs"
echo "project_terminal_to_workspace_in_state implementation:"
sed -n '12192,12310p' "$file" | cat -n
echo
echo "create_empty_workspace implementation:"
rg -n -C 12 'fn create_empty_workspace|pub fn create_empty_workspace|create_empty_workspace<' "$file"
echo
echo "workspace_registry field and init:"
rg -n -C 6 'workspace_registry:|workspace_registry:' "$file" | head -n 220Repository: manaflow-ai/cmux
Length of output: 19180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("cmux-tui/crates/cmux-tui-core/src/mux.rs")
text = p.read_text()
checks = [
("unknown workspace key error in projection", 'fn project_terminal_to_workspace_in_state'),
("workspace lookup before allocation in projection", 'state.workspaces[destination]').count('state.workspaces[destination]'),
("project_terminal_to_workspace_in_state returns an error when workspace key is missing", 'state.workspaces[destination].ok_or_else(|| anyhow::anyhow!("unknown workspace key {workspace_key}"))?', 'state.workspaces[destination].ok_or_else(|| anyhow::anyhow!("unknown workspace key {workspace_key}"))?'),
]
for name, needle in [("unknown workspace key error in projection", '!state.workspaces.iter().all(|workspace| workspace.key != workspace_key)'), ("missing workspace exact error", 'state.workspaces[destination].ok_or_else(|| anyhow::anyhow!("unknown workspace key {workspace_key}"))?'), ("projection allocation before project_resource", 'project_resource')]
:
# This is just a quick scan helper
pass
PYRepository: manaflow-ai/cmux
Length of output: 929
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
text = Path("cmux-tui/crates/cmux-tui-core/src/mux.rs").read_text()
# Locate the exact projection body text.
start = text.index("fn project_terminal_to_workspace_in_state(")
end = text.index("/// Move an existing tab", start)
proj = text[start:end]
checks = {
"projection exists": True,
"early no-existing-terminal unknown workspace check": '!state.workspaces.iter().all(|workspace| workspace.key != workspace_key)' in proj,
"main workspace exists check before allocation": 'state.workspaces[destination].ok_or_else(|| anyhow::anyhow!("unknown workspace key {workspace_key}"))?' in proj,
"uses existing workspace's active screen/pane when available": "state.workspaces[destination].active_screen_ref().map(|screen| screen.active_pane)" in proj,
"allocates a pane and screen only after selected existing workspace": "state.workspaces[destination].screens.push(Screen" in proj,
"allocated destination matches projected workspace": "state.workspaces[destination].screens.push(Screen" in proj and proj.index("state.workspaces[destination].screens.push(Screen") > proj.index("return Ok((Some(current), false));"),
}
for key, result in checks.items():
print(f"{key}: {result}")
if not all(checks.values()):
raise SystemExit(1)
for line_no, line in enumerate(proj.splitlines(), 1):
if "ok_or_else" in line and "unknown workspace key" in line:
print("workspace_error_line:", line_no, line.strip())
PYRepository: manaflow-ai/cmux
Length of output: 484
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="cmux-tui/crates/cmux-tui-core/src/mux.rs"
echo "Relevant error checks:"
rg -n -C 3 'unknown workspace|all\(workspace|all\(|Position' "$file"
echo
echo "Projection source:"
sed -n '12192,12245p' "$file" | cat -n
echo
echo "Project helper source around workspace selection:"
sed -n '8410,8445p' "$file" | cat -nRepository: manaflow-ai/cmux
Length of output: 24776
Create the passed workspace before projecting into it.
project_terminal_to_workspace_in_state requires the workspace key to exist at cmux-tui/crates/cmux-tui-core/src/mux.rs:12208 and cmux-tui/crates/cmux-tui-core/src/mux.rs:12217; passing "exited-cell-pixels" into the empty test_mux() state makes projection fail before set_cell_pixel_size runs. Create the workspace and pass the returned workspace.key.
🤖 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 `@cmux-tui/crates/cmux-tui-core/src/mux.rs` around lines 16220 - 16225, The
test setup around insert_terminal_identity_surface must create the
“exited-cell-pixels” workspace in the empty test_mux state before projecting the
terminal. Capture the created workspace and pass its workspace.key instead of
the literal workspace identifier, preserving the existing projection and
set_cell_pixel_size flow.
|
Closing this obsolete Sprite branch. Head |
|
Closing as obsolete. The Sprite checkpoint-builder path is no longer the accepted cloud direction. Its exited-terminal cell-metric behavior is already present via merged #9387; no unique implementation remains here. |
2 participants
Summary
Testing
bash -n cmux-tui/scripts/build-sprite-base.shshellcheck cmux-tui/scripts/build-sprite-base.shv1with daemon state absentCMUX_SPRITE_OKfrom its terminalcmux@0.9.11TUI against the enrolled Spritedc935e5476, created clean checkpointv3, and attached the actual TUI without the convergence errorNotes