docs: add openspec for SANDBOX-1806 shared types and bash session - #2
Conversation
- Add openspec config, proposal, design, tasks, and capability specs - shared-types spec: ExecRequest, ExecResponse, AssignRequest contracts - persistent-bash-session spec: delimiter protocol, concurrent pipe readers, SIGKILL timeout with 5s deadline, deferred crash recovery, eager init, no output truncation Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
WalkthroughAdds spec, proposal, task, and design documents plus repo metadata describing shared HTTP wire-format types ( ChangesSANDBOX-1806: shared types + persistent bash session
sequenceDiagram
participant Client as Client
participant Server as CLI-MCP Server
participant Agent as Sandbox Agent
participant Bash as Bash Process
Client->>Server: POST /exec (ExecRequest)
Server->>Agent: Forward ExecRequest
Agent->>Bash: Write wrapped command (START marker + command + END marker)
Bash-->>Agent: Stream stdout/stderr (delimited)
Agent->>Bash: If timeout → send SIGKILL to command pgid
Bash-->>Agent: EOF / exit marker
Agent->>Server: ExecResponse (stdout, stderr, exit_code, duration_ms)
Server->>Client: HTTP response (ExecResponse)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labelsdocumentation, proposal 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
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
`@openspec/changes/archive/2026-05-08-shared-types-and-bash-session/specs/shared-types/spec.md`:
- Line 23: Update the ExecResponse DurationMs requirement to allow zero for
sub-millisecond executions: change the phrase "a positive `DurationMs` value" to
"a non-negative `DurationMs` value (>= 0)" for the example assertion that the
ExecResponse SHALL contain `Stdout`, `Stderr`, `ExitCode`, and `DurationMs`, and
make the same wording change for the other identical occurrence in the file so
`DurationMs` can be 0 for very fast commands.
🪄 Autofix (Beta)
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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f995674e-e3f5-468a-b5c2-c79da611d8b4
📒 Files selected for processing (7)
openspec/changes/archive/2026-05-08-shared-types-and-bash-session/.openspec.yamlopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.mdopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/proposal.mdopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/specs/persistent-bash-session/spec.mdopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/specs/shared-types/spec.mdopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/tasks.mdopenspec/config.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
openspec/config.yamlopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/specs/shared-types/spec.mdopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.mdopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/proposal.mdopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/specs/persistent-bash-session/spec.mdopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/tasks.md
🪛 LanguageTool
openspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.md
[style] ~66-~66: To elevate your writing, try using a synonym here.
Context: ...ning flag) for race conditions that are hard to get right ### Decision 4: Crash rec...
(HARD_TO)
[style] ~120-~120: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...rade-offs Risk: Commands producing very large output could OOM the sandbox agent → **...
(EN_WEAK_ADJECTIVE)
Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
alexeykazakov
left a comment
There was a problem hiding this comment.
Looks good overall. I have one question/comment regarding streaming though.
| **Rationale:** | ||
| - Avoids pipe buffer deadlocks that occur with sequential reads (a command writing heavily to stderr blocks while we wait for stdout) | ||
| - Simpler than channel-based approach — just accumulate into buffers | ||
| - No streaming needed since we return the full response at the end |
There was a problem hiding this comment.
Ideally we would want to support streaming. MCP spec/sdk does support streaming. TARSy also supports streaming from MCP Servers.
It's not a deal breaker though. If it makes implementation significantly easier.
There was a problem hiding this comment.
Thanks. I investigated how both tarsy and mcp-server-devsandbox currently handle tool results — both use batch CallTool → complete CallToolResult (TARSy's "streaming" is LLM/timeline events, not MCP tool output). Neither side consumes incremental tool results today.
That said, streaming is clearly the direction for longer-running commands. I've updated Decision 2 in the design to acknowledge this and added a "Future Extension: Streaming Execution" section to the bash session spec that sketches the ExecuteStream() API shape.
The key insight: the internal pipe readers already work line-by-line, so the architecture is streaming-ready by design. Adding ExecuteStream() later is additive (~30-40 lines) and doesn't require refactoring Execute(). When TARSy/the MCP SDK adds support for partial tool results or progress notifications, we can layer streaming on top without changing the foundation.
- Amend Decision 2 in design.md to acknowledge streaming-ready internals and batch-first rationale (TARSy consumes batch today) - Add "Future Extension: Streaming Execution" section to bash session spec with sketched ExecuteStream() API and open design questions - Reference cross-system dependency (TARSy + MCP SDK partial results) for when streaming becomes actionable Co-authored-by: cursor-agent <cursor-agent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.md (1)
134-137:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve contradiction between “Open Questions: None” and streaming’s unresolved questions.
Line 136 says no open questions, but Line 50 explicitly references open design questions for
ExecuteStream(). This inconsistency can mislead planning and tracking for SANDBOX-1806 follow-ups.🤖 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 `@openspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.md` around lines 134 - 137, The doc currently contradicts itself: replace the "Open Questions — None" statement with an explicit list of unresolved items for ExecuteStream() (or mark ExecuteStream() as resolved); specifically, update the Open Questions section to list the outstanding design questions referenced earlier for ExecuteStream() (including the SANDBOX-1806 follow-ups and any decisions still pending), or remove/modify the earlier reference to ExecuteStream() so both places agree.
🤖 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.
Outside diff comments:
In `@openspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.md`:
- Around line 134-137: The doc currently contradicts itself: replace the "Open
Questions — None" statement with an explicit list of unresolved items for
ExecuteStream() (or mark ExecuteStream() as resolved); specifically, update the
Open Questions section to list the outstanding design questions referenced
earlier for ExecuteStream() (including the SANDBOX-1806 follow-ups and any
decisions still pending), or remove/modify the earlier reference to
ExecuteStream() so both places agree.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e37b947c-fd02-45fd-a6e8-d3667ef73503
📒 Files selected for processing (2)
openspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.mdopenspec/changes/archive/2026-05-08-shared-types-and-bash-session/specs/persistent-bash-session/spec.md
🚧 Files skipped from review as they are similar to previous changes (1)
- openspec/changes/archive/2026-05-08-shared-types-and-bash-session/specs/persistent-bash-session/spec.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
openspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.md
🪛 LanguageTool
openspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.md
[style] ~65-~65: To elevate your writing, try using a synonym here.
Context: ...ning flag) for race conditions that are hard to get right ### Decision 4: Crash rec...
(HARD_TO)
[style] ~119-~119: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...rade-offs Risk: Commands producing very large output could OOM the sandbox agent → **...
(EN_WEAK_ADJECTIVE)
SANDBOX-1806
Assisted By : Cursor
Summary by CodeRabbit
New Features
Documentation
Chores