feat(opencode): add upstream HttpApi backend route coverage - #1416
Conversation
|
Warning Review limit reached
More reviews will be available in 34 minutes and 4 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThree new GET endpoints are added to the HTTP API: ChangesNew HTTP API Endpoints: capabilities, directories, shells
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/opencode/src/server/routes/instance/httpapi/groups/project.ts (1)
40-43: ⚡ Quick winUse
Schema.ClassforProjectDirectory.
ProjectDirectoryis a multi-field schema and should follow the repo convention withSchema.Classinstead ofSchema.Struct.Suggested refactor
-const ProjectDirectory = Schema.Struct({ - directory: Schema.String, - strategy: Schema.optionalKey(Schema.String), -}) +class ProjectDirectory extends Schema.Class<ProjectDirectory>("ProjectDirectory")({ + directory: Schema.String, + strategy: Schema.optionalKey(Schema.String), +}) {}As per coding guidelines, "Use
Schema.Classfor multi-field data in Effect schemas".🤖 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 `@packages/opencode/src/server/routes/instance/httpapi/groups/project.ts` around lines 40 - 43, The ProjectDirectory schema definition uses Schema.Struct for a multi-field schema, but repository conventions require using Schema.Class instead. Replace Schema.Struct with Schema.Class in the ProjectDirectory definition that contains the directory and strategy fields to align with the Effect schema coding guidelines for multi-field data structures.Source: Coding guidelines
packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts (1)
45-49: ⚡ Quick winPrefer
Schema.ClassforShellItem.
ShellItemis multi-field and should useSchema.Classper the Effect schema conventions in this repo.Suggested refactor
-const ShellItem = Schema.Struct({ - path: Schema.String, - name: Schema.String, - acceptable: Schema.Boolean, -}) +class ShellItem extends Schema.Class<ShellItem>("ShellItem")({ + path: Schema.String, + name: Schema.String, + acceptable: Schema.Boolean, +}) {}As per coding guidelines, "Use
Schema.Classfor multi-field data in Effect schemas".🤖 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 `@packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts` around lines 45 - 49, The ShellItem definition currently uses Schema.Struct but should be refactored to use Schema.Class since it contains multiple fields. Change the ShellItem definition from using Schema.Struct to Schema.Class while keeping all three field definitions (path as Schema.String, name as Schema.String, and acceptable as Schema.Boolean) exactly as they are structured currently.Source: Coding guidelines
packages/opencode/src/server/routes/instance/httpapi/handlers/project.ts (1)
70-83: ⚡ Quick winPrefer a named
Effect.fnfor the newdirectorieshandler flow.This new route path is currently anonymous (
Effect.gen(...)inline). UsingEffect.fn("ProjectHttpApi.directories")here keeps traces and debugging labels consistent with the repo’s Effect conventions.Suggested refactor
- .handleRaw("directories", (ctx) => - Effect.gen(function* () { + const directories = Effect.fn("ProjectHttpApi.directories")(function* (ctx: { params: { projectID: string } }) { const projectInfo = yield* Project.Service.use((svc) => svc.get(ProjectID.make(ctx.params.projectID))) if (!projectInfo) return yield* projectFailure(new NotFoundError({ message: `Project not found: ${ctx.params.projectID}` })) const directories = yield* Project.Service.use((svc) => svc.sandboxes(projectInfo.id)) const result = [projectInfo.worktree, ...directories] .filter((directory) => directory !== "/") .map((directory) => ({ directory })) return HttpServerResponse.jsonUnsafe(result) - }).pipe( + }) + + return handlers + .handleRaw("directories", (ctx) => + directories(ctx).pipe( Effect.catch(projectFailure), Effect.catchDefect(projectFailure), - ), - ), + ), + )As per coding guidelines,
packages/opencode/**/*.ts: “UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers...”.🤖 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 `@packages/opencode/src/server/routes/instance/httpapi/handlers/project.ts` around lines 70 - 83, The handleRaw "directories" handler currently uses an anonymous Effect.gen for its flow, which doesn't provide proper tracing labels. Refactor this by extracting the Effect.gen logic into a named Effect.fn call with the identifier "ProjectHttpApi.directories" to align with the repository's Effect conventions for tracing and debugging. Replace the inline Effect.gen(function* () { ... }) with Effect.fn("ProjectHttpApi.directories", ...) that generates the same effect chain, ensuring the error handling with projectFailure is preserved in the pipe.Source: Coding guidelines
🤖 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 `@packages/opencode/src/shell/shell.ts`:
- Around line 95-99: The unix() function reads /etc/shells but doesn't properly
normalize lines before filtering. Currently it checks if line.trim() is truthy
in the filter condition but uses the original untrimmed line value, which
preserves whitespace padding and allows indented comments (lines with leading
whitespace before #) to be treated as valid shell candidates. Fix this by
trimming each line first, then checking if the trimmed line is truthy and
doesn't start with "#", so you filter on the actual trimmed value rather than
checking trimmed truthiness while keeping the untrimmed value. Additionally, add
a fallback return statement after the return statement with Array.from in case
the final parsed array is empty.
In `@packages/sdk/openapi.json`:
- Line 2751: The import statement in the code sample on line 2751 (and also on
lines 2801 and 3788) is missing the closing quote around the module name
"`@opencode-ai/sdk`". The import statement currently reads "import {
createOpencodeClient } from \"`@opencode-ai/sdk`\n" but should be "import {
createOpencodeClient } from \"`@opencode-ai/sdk`\"\n". Add the missing closing
double quote after the package name `@opencode-ai/sdk` in all three locations to
make the JavaScript import valid.
---
Nitpick comments:
In `@packages/opencode/src/server/routes/instance/httpapi/groups/project.ts`:
- Around line 40-43: The ProjectDirectory schema definition uses Schema.Struct
for a multi-field schema, but repository conventions require using Schema.Class
instead. Replace Schema.Struct with Schema.Class in the ProjectDirectory
definition that contains the directory and strategy fields to align with the
Effect schema coding guidelines for multi-field data structures.
In `@packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts`:
- Around line 45-49: The ShellItem definition currently uses Schema.Struct but
should be refactored to use Schema.Class since it contains multiple fields.
Change the ShellItem definition from using Schema.Struct to Schema.Class while
keeping all three field definitions (path as Schema.String, name as
Schema.String, and acceptable as Schema.Boolean) exactly as they are structured
currently.
In `@packages/opencode/src/server/routes/instance/httpapi/handlers/project.ts`:
- Around line 70-83: The handleRaw "directories" handler currently uses an
anonymous Effect.gen for its flow, which doesn't provide proper tracing labels.
Refactor this by extracting the Effect.gen logic into a named Effect.fn call
with the identifier "ProjectHttpApi.directories" to align with the repository's
Effect conventions for tracing and debugging. Replace the inline
Effect.gen(function* () { ... }) with Effect.fn("ProjectHttpApi.directories",
...) that generates the same effect chain, ensuring the error handling with
projectFailure is preserved in the pipe.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 832fc393-3bd5-495e-b631-14d1e7f0bd46
⛔ Files ignored due to path filters (2)
packages/sdk/js/src/v2/gen/sdk.gen.tsis excluded by!**/gen/**packages/sdk/js/src/v2/gen/types.gen.tsis excluded by!**/gen/**
📒 Files selected for processing (13)
packages/opencode/script/route-inventory.tspackages/opencode/src/server/routes/instance/httpapi/groups/experimental.tspackages/opencode/src/server/routes/instance/httpapi/groups/project.tspackages/opencode/src/server/routes/instance/httpapi/groups/pty.tspackages/opencode/src/server/routes/instance/httpapi/handlers/experimental.tspackages/opencode/src/server/routes/instance/httpapi/handlers/project.tspackages/opencode/src/server/routes/instance/httpapi/handlers/pty.tspackages/opencode/src/shell/shell.tspackages/opencode/test/server/experimental-routes.test.tspackages/opencode/test/server/project-routes.test.tspackages/opencode/test/server/pty-routes.test.tspackages/opencode/test/server/route-inventory-harness.test.tspackages/sdk/openapi.json
Summary
Adds local ProductionApi coverage for three upstream HttpApi backend JSON routes that have clear PawWork semantics:
GET /experimental/capabilitiesGET /project/:projectID/directoriesGET /pty/shellsAlso classifies the remaining upstream-only candidates without local product semantics as explicitly deferred in the route inventory harness, while keeping retired
/questionroutes out of local OpenAPI coverage.Why
Issue #936 is migrating backend routes to the Effect HttpApi surface. The latest upstream inventory still had a small set of upstream HttpApi routes that were neither local ProductionApi routes nor documented local exceptions. This PR closes the safe backend parity gap and leaves code-backed inventory classifications for the routes that should not be locally implemented yet.
Related Issue
Related to #936.
Human Review Status
Pending
Review Focus
Please focus on whether the three newly exposed HttpApi routes map to the lowest correct local layer, and whether the deferred inventory classifications are narrow enough to avoid hiding real migration work.
Risk Notes
The
/pty/shellsroute touches host shell discovery on macOS, Linux, and Windows by reusing the local shell helper; it avoids spawning shells and only checks candidate paths. The PR also updates generated OpenAPI and v2 SDK output to matchProductionApi. The visible UI check is skipped because no UI or copy changed.How To Verify
Screenshots or Recordings
Not applicable: no visible UI changes.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.