|
| 1 | +# OpenAPI Translation Cleanup Plan |
| 2 | + |
| 3 | +## Goal |
| 4 | + |
| 5 | +Trim `packages/opencode/src/server/routes/instance/httpapi/public.ts` until OpenAPI generation is mostly a direct projection of the `HttpApi` route declarations, without breaking the generated SDK surface. |
| 6 | + |
| 7 | +The main failure mode to eliminate is spec-only behavior: anything that appears in `/doc` or the SDK but is not accepted by runtime `HttpApi` validation. |
| 8 | + |
| 9 | +## Current Culprit |
| 10 | + |
| 11 | +`public.ts` exports `PublicApi` with a large `OpenApi.annotations({ transform })` hook. That hook rewrites the generated spec for legacy SDK compatibility. |
| 12 | + |
| 13 | +The highest-risk rewrite is `InstanceQueryParameters`, which injected `directory` and `workspace` into every instance route in OpenAPI even when the runtime query schema did not accept them. This caused the SDK and `/doc` to advertise calls that could fail with `400` at runtime. |
| 14 | + |
| 15 | +## Non-Negotiables |
| 16 | + |
| 17 | +- Do not break the generated JavaScript SDK without an explicit versioned migration plan. |
| 18 | +- Runtime route schemas are the source of truth for accepted params, payloads, and responses. |
| 19 | +- `/doc`, generated SDK types, and runtime validation must agree for every endpoint. |
| 20 | +- Prefer endpoint or schema annotations over post-generation spec surgery. |
| 21 | +- Remove one category of rewrite at a time, with focused compatibility checks. |
| 22 | + |
| 23 | +## PR Checklist |
| 24 | + |
| 25 | +Status legend: `[x]` done locally, `[~]` in progress locally, `[ ]` not started. |
| 26 | + |
| 27 | +Current combined PR scope: |
| 28 | + |
| 29 | +- `[x]` PR 1 drift tests: added OpenAPI/runtime query assertions and a negative fixture in `test/server/httpapi-query-schema-drift.test.ts`. |
| 30 | +- `[x]` PR 2 injection removal: removed broad `directory` / `workspace` post-generation injection from `public.ts` and replaced it with explicit runtime query schemas on affected routes. |
| 31 | +- `[ ]` PR 3+ cleanup: leave query override, path pattern, error shape, auth, and component-shape rewrites for later PRs. |
| 32 | + |
| 33 | +### PR 1: Add OpenAPI/Runtime Query Drift Tests |
| 34 | + |
| 35 | +- `[x]` Add or extend `packages/opencode/test/server/httpapi-query-schema-drift.test.ts`. |
| 36 | +- `[x]` Import `OpenApi.fromApi` and `PublicApi`. |
| 37 | +- `[x]` Generate the public spec in-process with `OpenApi.fromApi(PublicApi)`. |
| 38 | +- `[x]` Add a route inventory for the existing runtime reproducers: `session`, `file`, `experimental`, and `instance` routes. |
| 39 | +- `[x]` For each inventory entry, assert every OpenAPI query parameter is declared by the runtime query schema. |
| 40 | +- `[x]` Add a negative regression fixture that fails on spec-only `directory` / `workspace` params. |
| 41 | +- `[x]` Keep this part test-only. |
| 42 | + |
| 43 | +Verification: |
| 44 | + |
| 45 | +- `[x]` `bun test --timeout 5000 test/server/httpapi-query-schema-drift.test.ts` from `packages/opencode`. |
| 46 | +- `[x]` `bun typecheck` from `packages/opencode`. |
| 47 | + |
| 48 | +### PR 2: Delete Spec-Only Workspace Query Injection |
| 49 | + |
| 50 | +- `[x]` Edit `packages/opencode/src/server/routes/instance/httpapi/public.ts`. |
| 51 | +- `[x]` Delete `InstanceQueryParameters`. |
| 52 | +- `[x]` Delete the `isInstanceRoute` constant. |
| 53 | +- `[x]` Delete the branch that prepends `directory` and `workspace` to every instance operation. |
| 54 | +- `[x]` Keep `normalizeParameter(param, route)` for parameters that are actually produced by `HttpApi`. |
| 55 | +- `[x]` Add `WorkspaceRoutingQuery` / `WorkspaceRoutingQueryFields` to runtime query schemas for affected routes. |
| 56 | +- `[x]` Regenerate SDK and inspect diff. Result: no `directory` / `workspace` request-param removals; generated SDK diff is declaration ordering only. |
| 57 | + |
| 58 | +Notes: |
| 59 | + |
| 60 | +- Added `WorkspaceRoutingQuery` in `middleware/workspace-routing.ts` as the canonical runtime schema for middleware-consumed query params. |
| 61 | +- Replaced v2 union-query schemas with plain struct query schemas so `OpenApi.fromApi` emits their query params directly. This intentionally exposes the beta `/api/session` pagination/filter params in the SDK; cursor mutual-exclusion rules now live in the handlers, while `directory` / `workspace` remain allowed with cursors for routing. |
| 62 | + |
| 63 | +Expected code shape: |
| 64 | + |
| 65 | +```ts |
| 66 | +for (const param of operation.parameters ?? []) normalizeParameter(param, `${method.toUpperCase()} ${path}`) |
| 67 | +``` |
| 68 | + |
| 69 | +Verification: |
| 70 | + |
| 71 | +- `[x]` `bun test --timeout 5000 test/server/httpapi-query-schema-drift.test.ts` from `packages/opencode`. |
| 72 | +- `[x]` `bun dev generate > /tmp/opencode-openapi.json` from `packages/opencode`. |
| 73 | +- `[x]` `./packages/sdk/js/script/build.ts` from repo root. |
| 74 | +- `[x]` Inspect SDK diff for removed `directory` / `workspace` params. Result: none after explicit runtime schemas; v2 list/message now also expose their existing beta pagination/filter query params in the SDK. |
| 75 | +- `[x]` `bun typecheck` from `packages/opencode`. |
| 76 | + |
| 77 | +### PR 3: Replace Broad Query Type Override Sets With Route-Level Helpers |
| 78 | + |
| 79 | +- Edit `packages/opencode/src/server/routes/instance/httpapi/public.ts`. |
| 80 | +- Remove broad name-based assumptions from `QueryNumberParameters` and `QueryBooleanParameters` one field at a time. |
| 81 | +- Add shared query schema helpers near route group code if needed, for example in `groups/metadata.ts` or a new `groups/query.ts`. |
| 82 | +- Prefer route declarations like `Schema.NumberFromString.check(...)` and boolean string decoders like the existing `QueryBoolean` in `groups/session.ts`. |
| 83 | +- Keep only route-specific `QueryParameterSchemas` entries when SDK compatibility requires a public encoded type that Effect OpenAPI cannot emit yet. |
| 84 | + |
| 85 | +Concrete first targets: |
| 86 | + |
| 87 | +- Replace `roots` / `archived` reliance on `QueryBooleanParameters` with explicit route schema helpers. |
| 88 | +- Replace `start` / `cursor` / `limit` reliance on `QueryNumberParameters` with explicit route schema constraints where missing. |
| 89 | +- Keep `GET /find/file limit`, `GET /session/{sessionID}/diff messageID`, and `GET /session/{sessionID}/message limit` overrides until their route schemas generate identical SDK types directly. |
| 90 | + |
| 91 | +Verification: |
| 92 | + |
| 93 | +- Focused HTTP tests for changed query fields. |
| 94 | +- `bun dev generate > /tmp/opencode-openapi.json` from `packages/opencode`. |
| 95 | +- `./packages/sdk/js/script/build.ts` from repo root. |
| 96 | +- Inspect generated SDK request param types before deleting each override. |
| 97 | +- `bun typecheck` from `packages/opencode`. |
| 98 | + |
| 99 | +### PR 4: Move Path Parameter Patterns Into ID Schemas |
| 100 | + |
| 101 | +- Audit `PathParameterSchemas` and `pathParameterSchema()` in `public.ts`. |
| 102 | +- Check source schemas in files like `packages/opencode/src/session/schema.ts`, `packages/opencode/src/permission/schema.ts`, and pty schema definitions. |
| 103 | +- Add or fix `ZodOverride` / OpenAPI-compatible annotations on branded ID schemas so generated path params include the same patterns without `public.ts` overrides. |
| 104 | +- Delete one path override only after generated OpenAPI is unchanged for that param. |
| 105 | + |
| 106 | +Concrete first targets: |
| 107 | + |
| 108 | +- `sessionID` |
| 109 | +- `messageID` |
| 110 | +- `partID` |
| 111 | +- `permissionID` |
| 112 | +- `ptyID` |
| 113 | + |
| 114 | +Leave ambiguous route-local `id` overrides for workspace routes until they are renamed or explicitly typed in endpoint params. |
| 115 | + |
| 116 | +Verification: |
| 117 | + |
| 118 | +- `bun dev generate > /tmp/opencode-openapi.json` from `packages/opencode`. |
| 119 | +- `./packages/sdk/js/script/build.ts` from repo root. |
| 120 | +- Inspect generated path param types and patterns. |
| 121 | +- `bun typecheck` from `packages/opencode`. |
| 122 | + |
| 123 | +### PR 5: Replace Built-In Error Rewrites With Declared API Errors |
| 124 | + |
| 125 | +- Edit route group files under `packages/opencode/src/server/routes/instance/httpapi/groups/`. |
| 126 | +- Replace SDK-visible `HttpApiError.BadRequest` / `HttpApiError.NotFound` with explicit error schemas from `packages/opencode/src/server/routes/instance/httpapi/errors.ts` or add new ones there. |
| 127 | +- Update handlers to fail with the declared API errors at the boundary. |
| 128 | +- Remove matching cases from `normalizeLegacyErrorResponses()` only after generated OpenAPI remains SDK-compatible. |
| 129 | +- Do this group by group, starting with one small route group. |
| 130 | + |
| 131 | +Concrete first targets: |
| 132 | + |
| 133 | +- `groups/config.ts` `PATCH /config` bad request. |
| 134 | +- `groups/session.ts` endpoints that already translate domain not-found errors. |
| 135 | +- `groups/file.ts` if any handler currently relies on built-in error shape. |
| 136 | + |
| 137 | +Verification: |
| 138 | + |
| 139 | +- Focused HTTP tests asserting response body shape for changed error paths. |
| 140 | +- `bun dev generate > /tmp/opencode-openapi.json` from `packages/opencode`. |
| 141 | +- `./packages/sdk/js/script/build.ts` from repo root. |
| 142 | +- Inspect SDK error union diff. |
| 143 | +- `bun typecheck` from `packages/opencode`. |
| 144 | + |
| 145 | +### PR 6: Remove Auth/Security Spec Rewrites If SDK Can Tolerate It |
| 146 | + |
| 147 | +- Audit `delete operation.security`, `delete operation.responses?.["401"]`, and `delete spec.components?.securitySchemes` in `public.ts`. |
| 148 | +- Decide whether SDK should expose auth in generated operation metadata. |
| 149 | +- If preserving no-auth SDK surface is required, leave this rewrite and document it as intentional compatibility code. |
| 150 | +- If removing it, update SDK generation expectations and docs in the same PR. |
| 151 | + |
| 152 | +Verification: |
| 153 | + |
| 154 | +- `./packages/sdk/js/script/build.ts` from repo root. |
| 155 | +- Inspect generated client call signatures and error unions. |
| 156 | +- Do not merge if auth churn changes normal SDK call ergonomics unintentionally. |
| 157 | + |
| 158 | +### PR 7: Tackle Component Shape Rewrites One At A Time |
| 159 | + |
| 160 | +- Audit these in `public.ts`: `normalizeComponentNames`, `collapseDuplicateComponents`, `applyLegacySchemaOverrides`, `normalizeComponentDescriptions`, `stripOptionalNull`, `fixSelfReferencingComponents`. |
| 161 | +- For each rewrite, make a tiny PR that removes or narrows only that rewrite. |
| 162 | +- If generated SDK type names churn broadly, stop and either keep the rewrite or fix `effect-smol` generation first. |
| 163 | + |
| 164 | +Concrete first targets: |
| 165 | + |
| 166 | +- Delete cosmetic `normalizeComponentDescriptions` if SDK output does not change materially. |
| 167 | +- Narrow `applyLegacySchemaOverrides` entries that correspond to schemas already fixed at the source. |
| 168 | +- Keep `stripOptionalNull` until there is an explicit SDK migration plan, because it likely affects many optional fields. |
| 169 | + |
| 170 | +Verification: |
| 171 | + |
| 172 | +- `bun dev generate > /tmp/opencode-openapi.json` from `packages/opencode`. |
| 173 | +- `./packages/sdk/js/script/build.ts` from repo root. |
| 174 | +- Inspect generated SDK type-name and optionality diffs. |
| 175 | + |
| 176 | +## Upstream Middleware Query Support |
| 177 | + |
| 178 | +Long-term, `WorkspaceRoutingMiddleware` should declare the query fields it reads once, and `HttpApi` should use that declaration for both runtime validation and OpenAPI generation. |
| 179 | + |
| 180 | +Target in `effect-smol`: |
| 181 | + |
| 182 | +- Extend `HttpApiMiddleware.Service` config with optional query schema support, or add a dedicated middleware query annotation. |
| 183 | +- Make runtime request decoding include middleware query schemas. |
| 184 | +- Make `OpenApi.fromApi` emit middleware query params for endpoints using that middleware. |
| 185 | + |
| 186 | +Once available, remove `WorkspaceRoutingQueryFields` spreads from route groups and declare `directory` / `workspace` only on `WorkspaceRoutingMiddleware`. |
| 187 | + |
| 188 | +## Suggested PR Order |
| 189 | + |
| 190 | +1. Add drift detection tests only. |
| 191 | +2. Remove `InstanceQueryParameters` spec injection; rely on `WorkspaceRoutingQueryFields` already present in runtime schemas. |
| 192 | +3. Convert query type overrides into route/schema-level helpers where possible. |
| 193 | +4. Convert path parameter overrides into schema annotations or upstream fixes. |
| 194 | +5. Replace built-in error response rewrites with explicit declared API errors by route group. |
| 195 | +6. Tackle component naming/nullability rewrites only after SDK compatibility snapshots are stable. |
| 196 | + |
| 197 | +## Verification Checklist Per PR |
| 198 | + |
| 199 | +- Focused HTTP tests for changed routes. |
| 200 | +- OpenAPI drift tests. |
| 201 | +- `bun dev generate > /tmp/opencode-openapi.json` from `packages/opencode`. |
| 202 | +- `./packages/sdk/js/script/build.ts` from repo root. |
| 203 | +- Inspect generated SDK diff for public API churn. |
| 204 | +- `bun typecheck` from `packages/opencode`. |
0 commit comments