|
| 1 | +# Tool Schema vs Runtime Schema Boundary Design Note |
| 2 | + |
| 3 | +## Status |
| 4 | +Deferred architectural cleanup. |
| 5 | + |
| 6 | +## Problem |
| 7 | +The codebase currently defines similar contract shapes in two places: |
| 8 | + |
| 9 | +- `src/tools.ts` — OpenCode-facing tool argument schemas built with `tool.schema` |
| 10 | +- `src/runtime/schema.ts` — internal runtime/domain schemas built with Zod |
| 11 | + |
| 12 | +This looks like obvious duplication, but the two layers serve different purposes and do not currently share a safe implementation surface. |
| 13 | + |
| 14 | +## Evidence in the current codebase |
| 15 | + |
| 16 | +### Tool boundary (`src/tools.ts`) |
| 17 | +The tool layer uses `const z = tool.schema` and builds raw arg shapes for OpenCode tools such as: |
| 18 | +- `FlowPlanApplyArgsShape` |
| 19 | +- `WorkerResultArgsShape` |
| 20 | +- `FlowReviewRecordFeatureArgsShape` |
| 21 | +- `FlowReviewRecordFinalArgsShape` |
| 22 | + |
| 23 | +These shapes are consumed by `tool({ args: ... })` and are explicitly tested for SDK compatibility in `tests/config.test.ts`. |
| 24 | + |
| 25 | +### Runtime boundary (`src/runtime/schema.ts`) |
| 26 | +The runtime layer uses Zod directly and adds stricter internal rules, for example: |
| 27 | +- `WorkerResultSchema` is a discriminated union with cross-field constraints |
| 28 | +- `PlanSchema` and `SessionSchema` define normalized runtime state |
| 29 | +- parsing here is the authoritative runtime/domain validation step |
| 30 | + |
| 31 | +### Tests that encode the boundary |
| 32 | +`tests/config.test.ts` already documents the intended separation: |
| 33 | +- raw tool schemas must remain SDK-compatible |
| 34 | +- raw tool schemas may accept some structurally valid payloads that runtime schemas reject |
| 35 | +- runtime schemas are stricter than tool arg schemas for cross-field rules |
| 36 | + |
| 37 | +That is a strong signal that the duplication is not accidental copy-paste alone; part of it is boundary design. |
| 38 | + |
| 39 | +## Why direct unification is risky |
| 40 | +A previous cleanup attempt confirmed the risk. |
| 41 | + |
| 42 | +### 1. Different schema engines / expectations |
| 43 | +`tool.schema` comes from `@opencode-ai/plugin`, while runtime validation uses Zod directly. |
| 44 | +Even when their APIs look similar, they are not interchangeable enough to be treated as one source of truth without adapter work. |
| 45 | + |
| 46 | +### 2. Different validation goals |
| 47 | +The tool layer wants: |
| 48 | +- SDK-compatible raw arg shapes |
| 49 | +- permissive structural validation at the integration boundary |
| 50 | + |
| 51 | +The runtime layer wants: |
| 52 | +- normalized internal domain objects |
| 53 | +- stricter semantic validation |
| 54 | +- discriminated unions and cross-field refinements |
| 55 | + |
| 56 | +### 3. Tests explicitly rely on the difference |
| 57 | +The existing tests intentionally assert that some payloads pass raw tool-schema validation but fail runtime-schema validation. A full unification would either: |
| 58 | +- weaken runtime validation, or |
| 59 | +- tighten tool-boundary validation and risk breaking the SDK contract/ergonomics |
| 60 | + |
| 61 | +## Recommended design decision |
| 62 | +### Keep the boundaries separate for now |
| 63 | +Treat the current duplication as **boundary duplication**, not just implementation duplication. |
| 64 | + |
| 65 | +Source of truth should remain split by responsibility: |
| 66 | +- `src/tools.ts` owns **integration-boundary raw shapes** |
| 67 | +- `src/runtime/schema.ts` owns **internal domain/runtime schemas** |
| 68 | + |
| 69 | +## Safe future improvement options |
| 70 | +If you want to reduce duplication later, prefer one of these conservative designs. |
| 71 | + |
| 72 | +### Option A — Shared primitive fragments only (recommended if revisited) |
| 73 | +Create a small module that exports only primitive, boundary-safe facts such as: |
| 74 | +- feature id regex/message |
| 75 | +- enum value arrays from `src/runtime/contracts.ts` |
| 76 | +- maybe field-name constants / tiny shape descriptors that are plain data only |
| 77 | + |
| 78 | +Do **not** export full reusable schema objects across the boundary. |
| 79 | + |
| 80 | +This reduces drift on low-level facts while preserving the tool/runtime separation. |
| 81 | + |
| 82 | +### Option B — Runtime schema remains authoritative, tool layer uses explicit adapters |
| 83 | +Create explicit conversion/adaptation helpers from raw tool payloads to runtime payloads. |
| 84 | + |
| 85 | +Pattern: |
| 86 | +1. tool arg schema accepts SDK-compatible input |
| 87 | +2. adapter normalizes/reshapes raw args if needed |
| 88 | +3. runtime schema parses the adapted payload |
| 89 | + |
| 90 | +This avoids pretending the two schema surfaces are the same thing. |
| 91 | + |
| 92 | +### Option C — Full unification (not recommended right now) |
| 93 | +Only consider this if: |
| 94 | +- the OpenCode tool schema surface becomes demonstrably interoperable with Zod |
| 95 | +- you are willing to redesign tests and boundary behavior together |
| 96 | +- you treat it as a feature/architecture change, not cleanup |
| 97 | + |
| 98 | +## Concrete next steps if revisited |
| 99 | +1. Extract only low-risk shared primitives: |
| 100 | + - feature id regex/message |
| 101 | + - shared enum arrays (already mostly centralized in `src/runtime/contracts.ts`) |
| 102 | +2. Add one adapter helper for the highest-drift payload first: |
| 103 | + - likely `flow_run_complete_feature` |
| 104 | +3. Preserve the existing test contract: |
| 105 | + - raw tool schema remains SDK-compatible |
| 106 | + - runtime schema remains stricter |
| 107 | +4. Stop if a change starts forcing the two boundaries to behave identically. |
| 108 | + |
| 109 | +## Recommendation |
| 110 | +Do **not** pursue full schema unification as a cleanup pass. |
| 111 | + |
| 112 | +If you want to continue in this area, treat it as a small architecture task with an explicit adapter strategy, not as simple deduplication. |
0 commit comments