Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openspec/changes/specify-sdk-standards/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-15
99 changes: 99 additions & 0 deletions openspec/changes/specify-sdk-standards/design.md
Original file line number Diff line number Diff line change
@@ -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.
43 changes: 43 additions & 0 deletions openspec/changes/specify-sdk-standards/proposal.md
Original file line number Diff line number Diff line change
@@ -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.
93 changes: 93 additions & 0 deletions openspec/changes/specify-sdk-standards/specs/sdk-standards/spec.md
Original file line number Diff line number Diff line change
@@ -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
20 changes: 20 additions & 0 deletions openspec/changes/specify-sdk-standards/tasks.md
Original file line number Diff line number Diff line change
@@ -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