SANDBOX-1812: add openspec for agent HTTP client - #15
Conversation
- Add proposal, design, spec, and tasks for AgentClient - Typed HTTP client for sandbox agent API (/exec, /assign, /health) - Structured error hierarchy: NetworkError, StatusError, DecodeError - Functional options pattern for configuration (timeout, port, HTTP client) Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used🔀 Multi-repo context codeready-toolchain/mcp-server-devsandbox, codeready-toolchain/host-operator, codeready-toolchain/toolchain-e2e, codeready-toolchain/mcp-common, codeready-toolchain/toolchain-common, codeready-toolchain/apiLinked repositories findingscodeready-toolchain/mcp-server-devsandbox [::codeready-toolchain/mcp-server-devsandbox::]
codeready-toolchain/host-operator [::codeready-toolchain/host-operator::]
codeready-toolchain/toolchain-e2e [::codeready-toolchain/toolchain-e2e::]
codeready-toolchain/mcp-common [::codeready-toolchain/mcp-common::]
codeready-toolchain/toolchain-common [::codeready-toolchain/toolchain-common::]
codeready-toolchain/api [::codeready-toolchain/api::]
WalkthroughThis PR adds an OpenSpec change set for a new typed ChangesAgent HTTP Client OpenSpec
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/2026-07-08-agent-http-client/design.md`:
- Around line 135-140: Update the Decision 7 guidance in the AgentClient design
to reflect that successful Execute response bodies should be size-bounded before
JSON decoding, not left unlimited. In the AgentClient/ExecuteResponse handling
description, add a documented cap or streamed/limited decode strategy for large
ExecResponse.Stdout and ExecResponse.Stderr payloads, and make sure the text
clearly distinguishes this from the existing truncation behavior for StatusError
and DecodeError bodies.
In `@openspec/changes/2026-07-08-agent-http-client/tasks.md`:
- Around line 20-43: The AgentClient methods still leak response bodies on
successful requests. Update Execute, Assign, and HealthCheck to explicitly close
resp.Body on 200-paths, not just on error handling, and drain if needed before
closing to preserve connection reuse. Use the existing Execute, Assign, and
HealthCheck implementations in the agent HTTP client tasks as the place to add
the success-path cleanup alongside the current status/decoding logic.
🪄 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: d0db0109-bc8f-4412-a6f9-48b3c4d51165
📒 Files selected for processing (5)
openspec/changes/2026-07-08-agent-http-client/.openspec.yamlopenspec/changes/2026-07-08-agent-http-client/design.mdopenspec/changes/2026-07-08-agent-http-client/proposal.mdopenspec/changes/2026-07-08-agent-http-client/specs/agent-client/spec.mdopenspec/changes/2026-07-08-agent-http-client/tasks.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/mcp-common(manual)codeready-toolchain/mcp-server-devsandbox(manual)codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: build-test-coverage
🧰 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/2026-07-08-agent-http-client/proposal.mdopenspec/changes/2026-07-08-agent-http-client/specs/agent-client/spec.mdopenspec/changes/2026-07-08-agent-http-client/tasks.mdopenspec/changes/2026-07-08-agent-http-client/design.md
🪛 LanguageTool
openspec/changes/2026-07-08-agent-http-client/design.md
[locale-violation] ~107-~107: The phrase ‘in future’ is British English. Did you mean: “in the future”?
Context: ...xternal-network communication is needed in future, upgrading to TLS/mTLS is a localized c...
(IN_FUTURE)
🔀 Multi-repo context codeready-toolchain/mcp-server-devsandbox, codeready-toolchain/mcp-common, codeready-toolchain/toolchain-common, codeready-toolchain/host-operator, codeready-toolchain/toolchain-e2e
Linked repositories findings
codeready-toolchain/mcp-server-devsandbox
pkg/mcpinit/init.go:213-225already exposesGET /healthand returns JSON{"status":"healthy"}or{"status":"unhealthy", ...}with a 503 on failure. This aligns with the newAgentClient.HealthCheckcontract, so the spec’s unauthenticated health check is consistent with an existing endpoint. [::codeready-toolchain/mcp-server-devsandbox::]- Same file,
pkg/mcpinit/init.go:205-225, shows the server’s HTTP mode is already behind a kube-rbac-proxy setup and the repo docs expectAuthorization: Bearer <token>on requests. That matches the new spec’s bearer-token requirement forExecute, but it also highlights thatHealthCheckmust remain unauthenticated as documented. [::codeready-toolchain/mcp-server-devsandbox::]
codeready-toolchain/mcp-common
- No matches for
ExecRequest,ExecResponse,AssignRequest,AgentClient,/exec,/assign, or/healthwere found. I didn’t observe any consumer or shared type here that would be directly affected by the new client spec. [::codeready-toolchain/mcp-common::]
codeready-toolchain/toolchain-common
- No matches for the agent client types/endpoints were found. The hits were unrelated health-check configuration in operator code, not the sandbox agent contract. [::codeready-toolchain/toolchain-common::]
codeready-toolchain/host-operator
- No matches for the agent client types/endpoints were found. The only nearby hits were unrelated
/api/v1/healthchecks and KubernetesStatusErrorusage. [::codeready-toolchain/host-operator::]
codeready-toolchain/toolchain-e2e
- No matches for the agent client types/endpoints were found. The hits were unrelated test/deploy health checks and
pods/execRBAC references. [::codeready-toolchain/toolchain-e2e::]
🔇 Additional comments (3)
openspec/changes/2026-07-08-agent-http-client/.openspec.yaml (1)
1-2: LGTM!openspec/changes/2026-07-08-agent-http-client/proposal.md (1)
1-44: LGTM!openspec/changes/2026-07-08-agent-http-client/specs/agent-client/spec.md (1)
1-168: LGTM!
- Add response body size limit for successful Execute responses in design - Document resp.Body.Close() on all paths including success in tasks Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
Assisted By: Cursor
Summary by CodeRabbit
Execute,Assign,HealthCheck).