refactor(gateway): kill dead code, dedupe loaders, split planner, structure errors#489
Merged
raahulrahl merged 4 commits intomainfrom Apr 20, 2026
Merged
refactor(gateway): kill dead code, dedupe loaders, split planner, structure errors#489raahulrahl merged 4 commits intomainfrom
raahulrahl merged 4 commits intomainfrom
Conversation
Entire src/util/ (28 files), src/effect/, src/global/, src/id/, plus several orphaned _shared/ modules (filesystem, npm, flock, etc.) were carried over from the opencode fork but never imported by anything in src/ or tests/. Also strip the matching @/* and @opencode-ai/shared/* tsconfig path aliases — only the deleted files referenced them. 55 files, 3,446 lines removed. Typecheck clean, 216/216 tests still pass.
…arkdownWithSchema Both the agent and recipe loaders followed the same pattern — splitFrontmatter → parseYaml → build candidate → safeParse → throw on error. Extract that chain into `parseMarkdownWithSchema` (plus a thin `parseFrontmatterDoc` helper) in the frontmatter util. Each loader now only owns its candidate shape. Net: fewer lines in agent/recipe, single source of truth for the error message format. Typecheck clean, 216/216 tests pass.
planner/index.ts was 653 lines mixing three responsibilities: the public PlanRequest schemas + Service/layer, the per-(peer,skill) tool factory, and a bundle of pure helpers (name normalization, verified-label logic, <remote_content> envelope, json-schema→zod). Split into: planner/index.ts 297 schemas + Service + layer planner/skill-tool.ts 216 buildSkillTool + padToolDescription planner/util.ts 184 pure helpers, re-exported from index session/prompt.ts shed its bottom 70 lines of buildSystemPrompt, mapFinishReason, evtUsage, and wrapTool into session/prompt-util.ts so the layer factory reads top-to-bottom without scrolling past helpers. Typecheck clean, 216/216 tests pass. No behavior changes — tests consuming `../planner` exports still work via re-exports in index.ts.
…w ZodError
Clients previously received the ZodError's raw JSON-array message in the
`detail` field — unreadable and forced a second parse to find the bad
field. Route 400s (invalid body) and 500s (session_failed) now run through
a shared `formatErrorDetail` that turns Zod failures into a `{path,
message}[]` under `issues` plus a human-readable `detail` summary; non-Zod
errors pass their message through unchanged.
Adds 5 unit tests for the formatter covering single-field, multi-field,
root-level, Error, and string-thrown cases.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (65)
📝 WalkthroughWalkthroughThis PR significantly simplifies the codebase by removing numerous utility modules and shared services from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
✨ 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 |
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
1 participant
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.
Summary
Four-stage cleanup of
gateway/— each stage an independent commit, each verified with typecheck + full test run.Net: ~3,260 lines removed, 0 behavior changes.
What's dead and gone (Stage A)
Verified with grep across src/ + tests/ that nothing imports these before deleting:
The `_shared/util/` now contains exactly one file: `frontmatter.ts` (the actually-used markdown parser).
DRY win (Stage B)
`agent/index.ts` and `recipe/index.ts` both ran the same pipeline: `splitFrontmatter → parseYaml → build candidate → safeParse → throw on error`. Consolidated into `parseMarkdownWithSchema({path, raw, kind, schema, build})` in frontmatter.ts. Each loader now only owns its candidate shape.
Planner split (Stage C)
`planner/index.ts` was 653 lines of schemas + service + tool factory + pure helpers. New layout:
```
planner/index.ts 297 schemas + Service + layer (public API)
planner/skill-tool.ts 216 buildSkillTool + padToolDescription
planner/util.ts 184 normalizeToolName, findDuplicateToolIds,
computeVerifiedLabel, wrapRemoteContent,
jsonSchemaToZod, etc.
```
Tests consuming `../planner` exports (collisions, verified-label, schemas) still work via re-exports in `index.ts`. No test changes required.
Also trimmed `session/prompt.ts` (462 → 391 lines) by lifting `buildSystemPrompt`, `mapFinishReason`, `evtUsage`, and `wrapTool` into `session/prompt-util.ts`.
Error shape fix (Stage D)
Before: `/plan` 400 responses returned `{ error: "invalid_request", detail: "" }` — unreadable and forced clients to re-parse.
After: ZodError → `{ error, detail: "question: must be non-empty; agents.0.endpoint: Invalid url", issues: [{path, message}, ...] }`. Non-Zod errors pass the message through unchanged. Same treatment on the `session_failed` 500 path.
Test plan
Review guide
Commits are ordered for readability — A is pure deletion (zero risk), B is a small consolidation, C is internal reorganization with re-exports preserving the public API, D adds behavior + tests. Review in order; each stands alone if you want to split the PR later.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor