From 434311ccc33f9504a6783a8e11bf3a2957ee4ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D7=A0=CF=85=CE=B1=CE=B7=20=D7=A0=CF=85=CE=B1=CE=B7=D1=95?= =?UTF-8?q?=CF=83=CE=B7?= Date: Sat, 15 Aug 2026 23:08:56 -0700 Subject: [PATCH] feat: record the SDK's contract with its consumers osapi publishes a Go SDK that osapi-orchestrator is built on, and the rules governing it live in two documents addressed to people working inside osapi. The consumer cannot see the contract it depends on. Each rule was checked against the code before being written, so the capability records what is true rather than what was intended. Co-Authored-By: Claude Opus 5 (1M context) --- .../specify-sdk-standards/.openspec.yaml | 2 + .../changes/specify-sdk-standards/design.md | 99 +++++++++++++++++++ .../changes/specify-sdk-standards/proposal.md | 43 ++++++++ .../specs/sdk-standards/spec.md | 93 +++++++++++++++++ .../changes/specify-sdk-standards/tasks.md | 20 ++++ 5 files changed, 257 insertions(+) create mode 100644 openspec/changes/specify-sdk-standards/.openspec.yaml create mode 100644 openspec/changes/specify-sdk-standards/design.md create mode 100644 openspec/changes/specify-sdk-standards/proposal.md create mode 100644 openspec/changes/specify-sdk-standards/specs/sdk-standards/spec.md create mode 100644 openspec/changes/specify-sdk-standards/tasks.md diff --git a/openspec/changes/specify-sdk-standards/.openspec.yaml b/openspec/changes/specify-sdk-standards/.openspec.yaml new file mode 100644 index 0000000..0c73c8f --- /dev/null +++ b/openspec/changes/specify-sdk-standards/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-15 diff --git a/openspec/changes/specify-sdk-standards/design.md b/openspec/changes/specify-sdk-standards/design.md new file mode 100644 index 0000000..67ead7a --- /dev/null +++ b/openspec/changes/specify-sdk-standards/design.md @@ -0,0 +1,99 @@ +## Context + +`osapi` publishes a Go SDK at `pkg/sdk/client`. `osapi-orchestrator` consumes it +by pinned version — every operation it exposes is an SDK call, and its +`internal/engine` and `pkg/orchestrator` packages import it directly. + +The rules governing that SDK live in `osapi`: + +| Rule | Stated in | +| ---------------------------------------- | ------------------------------------- | +| Method names are clean verbs | `CLAUDE.md` | +| No generated type in a public signature | `docs/docs/sidebar/sdk/guidelines.md` | +| JSON tags on every exported result field | `docs/docs/sidebar/sdk/guidelines.md` | +| Errors wrapped, nil bodies guarded | `docs/docs/sidebar/sdk/guidelines.md` | + +Both documents are addressed to people working inside `osapi`. Neither is +visible to the repository that depends on them. + +## Goals / Non-Goals + +**Goals.** Record what the SDK guarantees, so a consumer can read the contract +rather than infer it. + +**Non-Goals.** Changing the SDK. The rules already hold — this records them. + +## Decisions + +### Recording, not correcting + +Every rule was checked against the code before being written down: + +- No service method repeats its service name. +- No public method signature contains a `gen` type. Generated types appear only + inside method bodies, where they are constructed from SDK types. +- No exported field on a result type lacks a `json` tag. +- `osapi-orchestrator` never imports the generated package. + +A requirement written from documentation alone records what someone intended. A +requirement checked against the code records what is true. + +*Alternative considered:* write the requirements from the two documents and +verify afterwards. Rejected — the verification is what distinguishes a rule that +holds from one that was aspirational, and doing it first meant the requirement +could be phrased around what the code actually does. + +### The tags are load-bearing + +`JSON tags required` reads like style until you find what depends on it: results +are converted to generic maps by marshalling to JSON and unmarshalling into a +map. An untagged field arrives under its Go name — `Hostname` rather than +`hostname` — which does not match the key the API returned, so the lookup misses +and the value is silently absent. + +The requirement states that dependency, because a rule whose reason is invisible +is one a future contributor will relax. + +### `omitempty` is a semantic choice + +The distinction the requirement draws is between a field whose absence carries +meaning and one whose value must always be readable. `Changed` is the example +that matters: omitted when false, a consumer cannot distinguish "this mutation +changed nothing" from "this SDK version does not report changes". + +*Alternative considered:* require `omitempty` on all optional fields and leave +it there. Rejected — that is a rule about pointers and slices, and it misses the +case the SDK actually gets wrong. + +### Scope stops at the SDK boundary + +This capability covers what `pkg/sdk` guarantees its consumers. It does not +cover how the API those methods call is designed, how a provider behaves when +one runs, or how Go is written across the organization. Those are separate +capabilities with separate readers. + +*Alternative considered:* one capability covering the SDK and the API it wraps. +Rejected — the SDK's audience is a consuming repository, and the API's audience +is someone adding an endpoint. A capability serving both would be read by +neither. + +## Risks / Trade-offs + +- **A recorded rule is harder to change than an undocumented one.** That is the + intent: the SDK has a consumer, and a contract that can be changed without + noticing is what breaks it. +- **The five resource verbs may not fit a future operation.** The requirement + allows an action verb where none of the five describes the operation, rather + than forcing a bad fit. + +## Migration Plan + +None. The rules hold; this records them. The two source documents stay in place +and point at the capability. + +## Open Questions + +- Should `osapi-orchestrator` state that it consumes this contract? It would + make the dependency visible from the consumer's side, but every repository + naming the capabilities it depends on is a larger convention than this change + should introduce. diff --git a/openspec/changes/specify-sdk-standards/proposal.md b/openspec/changes/specify-sdk-standards/proposal.md new file mode 100644 index 0000000..9e0c487 --- /dev/null +++ b/openspec/changes/specify-sdk-standards/proposal.md @@ -0,0 +1,43 @@ +## Why + +`osapi` publishes a Go SDK at `pkg/sdk/client`. `osapi-orchestrator` is built on +it: every operation it exposes is an SDK call, and it pins the module by +version. The rules that make the SDK usable by a second repository are written +down in two places inside `osapi`, and both are documentation rather than +requirements: + +- `CLAUDE.md` states that method names MUST be clean verbs and never repeat the + service name. +- `docs/docs/sidebar/sdk/guidelines.md` states that no generated type may appear + in a public signature, that every exported result field needs a JSON tag, and + how errors are wrapped. + +Both bind a consumer that cannot see them. `osapi-orchestrator` has no way to +discover the contract it depends on except by reading another repository's +contributor documentation, and nothing detects a change to that contract until +the consumer breaks. + +The rules hold today. Verified against the code: no service method stutters, no +public signature exposes a `gen` type, no exported result field lacks a JSON +tag, and `osapi-orchestrator` never imports `gen`. That is what makes this a +recording rather than a correction. + +## What Changes + +- Add an `sdk-standards` capability recording what the SDK guarantees its + consumers. +- Leave both source documents in place, pointing at the capability. + +## Capabilities + +### Added Capabilities + +- `sdk-standards`: the contract between `pkg/sdk` and the repositories built on + it — method naming, type exposure, result shape, and error handling. + +## Impact + +- `osapi`: `CLAUDE.md` and `sdk/guidelines.md` state the rules once and point at + the capability for the rest. +- `osapi-orchestrator`: can read the contract it depends on without reading + another repository's contributor documentation. diff --git a/openspec/changes/specify-sdk-standards/specs/sdk-standards/spec.md b/openspec/changes/specify-sdk-standards/specs/sdk-standards/spec.md new file mode 100644 index 0000000..e3e0bbc --- /dev/null +++ b/openspec/changes/specify-sdk-standards/specs/sdk-standards/spec.md @@ -0,0 +1,93 @@ +## Purpose + +Records what the SDK at `pkg/sdk` guarantees the repositories built on it, so a +consumer can read the contract it depends on rather than infer it from another +repository's contributor documentation. + +## ADDED Requirements + +### Requirement: A method name does not repeat its service + +A service method SHALL be named for the action alone. The service already +supplies the namespace, so repeating it in the method reads twice at every call +site. + +Methods SHALL use `List`, `Get`, `Create`, `Update`, and `Delete` for operations +on a resource. An operation with no persistent resource — a one-shot action or a +command execution — MAY use a verb naming what it does. + +#### Scenario: A service gains a read method + +- **WHEN** a service is added for a domain +- **THEN** its read method is `Get`, not the domain name followed by `Get` + +#### Scenario: An operation has no resource + +- **WHEN** an operation performs an action rather than acting on a stored + resource +- **THEN** it is named for the action, because none of the five resource verbs + describes it + +### Requirement: Generated types stay inside the SDK + +A public method signature SHALL NOT contain a type from the generated OpenAPI +package. The SDK exists to hide that package; a signature naming one requires +every consumer to import it, and re-exports each regeneration as a breaking +change. + +A consumer needing to import the generated package indicates the SDK is missing +a wrapper, rather than indicating the consumer should import it. + +#### Scenario: A request needs a generated body type + +- **WHEN** a method sends a request whose body is a generated type +- **THEN** the method accepts an SDK-defined type and builds the generated one + internally + +#### Scenario: A consumer reaches for the generated package + +- **WHEN** a consumer cannot express a call without importing the generated + package +- **THEN** the SDK adds the missing wrapper, rather than the consumer adding the + import + +### Requirement: Every exported result field carries a JSON tag + +Every exported field on a result type SHALL carry a `json` tag naming the key in +`snake_case`. + +The tags are load-bearing rather than decorative: results are converted to +generic maps by round-tripping through JSON, and an untagged field arrives under +its Go name, which does not match the key the API returned. + +A field whose absence is meaningful SHALL use `omitempty`. A field a caller must +always be able to read SHALL NOT, so that a false or empty value is +distinguishable from a field that was never set. + +#### Scenario: A result is converted to a map + +- **WHEN** a result is converted to a generic map +- **THEN** its keys match the API's, because each field names its key + +#### Scenario: A mutation reports whether it changed anything + +- **WHEN** a mutation result reports that nothing changed +- **THEN** the field is present and false, rather than omitted + +### Requirement: Errors reaching a consumer carry context + +An error returned from an SDK method SHALL name the operation that produced it. + +A response body SHALL be checked for nil after its status is checked, because a +status alone does not establish that a body was returned. + +#### Scenario: A call fails inside a wrapped client + +- **WHEN** an underlying call fails +- **THEN** the returned error names the SDK operation, so a consumer's log + identifies the call without a stack trace + +#### Scenario: A success status arrives with no body + +- **WHEN** a response carries a success status and no body +- **THEN** the SDK returns an error rather than dereferencing it diff --git a/openspec/changes/specify-sdk-standards/tasks.md b/openspec/changes/specify-sdk-standards/tasks.md new file mode 100644 index 0000000..e18fbf7 --- /dev/null +++ b/openspec/changes/specify-sdk-standards/tasks.md @@ -0,0 +1,20 @@ +## 1. Record the capability + +- [x] 1.1 Verify each rule against the code rather than the documentation + stating it +- [x] 1.2 Write the `sdk-standards` capability +- [x] 1.3 Record the decisions and their rejected alternatives in design.md + +## 2. Point the sources at the capability + +- [ ] 2.1 `osapi` — `docs/docs/sidebar/sdk/guidelines.md` keeps its worked + examples and points at the capability for the rules +- [ ] 2.2 `osapi` — `CLAUDE.md` drops the SDK naming block, which the capability + now states + +## 3. Verification + +- [ ] 3.1 Confirm no service method repeats its service name +- [ ] 3.2 Confirm no public signature contains a generated type +- [ ] 3.3 Confirm every exported result field carries a JSON tag +- [ ] 3.4 Confirm no consumer imports the generated package