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