Skip to content

Commit 4fed364

Browse files
authored
chore(api): update API skills and enforce doc formatting (#4330)
1 parent f3ae09c commit 4fed364

65 files changed

Lines changed: 3055 additions & 2218 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/skills/api/SKILL.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ You are helping the user add or modify API endpoints in OpenMeter.
2626
api/spec/packages/aip/src/
2727
├── main.tsp # Top-level imports
2828
├── openmeter.tsp # Service definition, routes, and interface wiring
29-
├── konnect.tsp # Konnect-specific service definition
29+
├── konnect.tsp # Konnect-specific service definition (must mirror openmeter.tsp's tags + route interfaces)
3030
├── common/ # Shared types: errors, pagination, parameters
3131
├── shared/ # Shared resources: ULID, request/response wrappers, tags
3232
├── meters/ # Domain: models + operations
@@ -47,6 +47,8 @@ Each domain typically has:
4747

4848
Routes are wired in `api/spec/packages/aip/src/openmeter.tsp` via interface declarations with `@route` and `@tag` decorators.
4949

50+
`api/spec/packages/aip/src/konnect.tsp` is the parallel Konnect-flavoured service definition. The two files are **not** identical — Konnect has its own service metadata, namespace name, `@useAuth` configuration, security scheme models, and intentionally exposes a narrower subset of the OpenMeter surface. But for any domain that *is* exposed in both, every new domain `import`, `@tagMetadata(...)` entry, and `@route` / `@tag` interface must be added to both files in the same edit. Diff the two files before generating to spot accidental drift; existing differences are expected, but a tag/route you just added showing up in only one file is a bug.
51+
5052
## Workflow
5153

5254
Follow these steps in order:
@@ -57,8 +59,8 @@ For a new domain/resource:
5759

5860
1. Create a new directory under `api/spec/packages/aip/src/<domain>/`
5961
2. Add `index.tsp`, model file(s), and `operations.tsp`
60-
3. Import the domain in `api/spec/packages/aip/src/openmeter.tsp`
61-
4. Wire up the route interface in `openmeter.tsp`
62+
3. Import the domain in `api/spec/packages/aip/src/openmeter.tsp` **and** `api/spec/packages/aip/src/konnect.tsp` (unless the domain is intentionally OpenMeter-only — confirm with the user before excluding it from Konnect)
63+
4. Wire up the route interface (and any new `@tagMetadata`) in **both** `openmeter.tsp` and `konnect.tsp`. After editing, run `diff openmeter.tsp konnect.tsp` and check that your new imports / tags / interfaces appear on both sides — pre-existing differences (service metadata, namespace name, `@useAuth`, security scheme models) are intentional and unrelated to your change.
6264

6365
For modifying an existing endpoint:
6466

@@ -342,7 +344,9 @@ OpenMeter v3 APIs follow [Kong's AIP](https://kong-aip.netlify.app/list/) conven
342344
| `rules/aip-158-pagination.md` | Page-based and cursor-based pagination |
343345
| `rules/aip-160-filtering.md` | Filter query syntax, `Common.*FieldFilter` types, label dot-notation |
344346
| `rules/aip-129-labels.md` | Label key constraints, PATCH-with-null semantics |
345-
| `rules/aip-193-errors.md` | RFC-7807 error responses, `Common.ErrorResponses`, 403-before-404 rule |
347+
| `rules/aip-193-errors.md` | AIP-193 RFC-7807 error responses, `invalid_parameters`, 403-before-404 rule |
348+
| `rules/openmeter-error-types.md` | OpenMeter `Common.*` error types wiring AIP-193 onto operations |
349+
| `rules/inline-errors.md` | Inline (partial / non-fatal) errors via `Shared.BaseError<T>` for 2xx responses |
346350
| `rules/aip-composition.md` | Composition-over-inheritance (spread, `model is`, `@discriminator`) |
347351
| `rules/aip-docs.md` | `@doc`/`/** */` requirements, `@operationId`, `@summary` |
348352
| `rules/aip-181-stability.md` | `x-private` / `x-unstable` / `x-internal` stability markers |

.agents/skills/api/rules/aip-193-errors.md

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,18 +34,7 @@ When a caller lacks access to a resource:
3434
- Return `403 Forbidden` if the resource is owned by the caller's organization (they exist but can't touch it)
3535
- Return `404 Not Found` otherwise — this prevents data-existence leakage across tenants
3636

37-
## OpenMeter Common types
37+
## See also
3838

39-
Use `Common.ErrorResponses` (= `BadRequest | Unauthorized | Forbidden`) on every operation, then add specific types explicitly. Types with ★ are OpenMeter extensions beyond what AIP-193 documents by name.
40-
41-
| Type | Status | Source | When to add explicitly |
42-
| ----------------------------- | ------ | ----------- | --------------------------------------------------- |
43-
| `Common.NotFound` | 404 | AIP-193 | GET, PATCH, PUT, DELETE by ID |
44-
| `Common.Conflict` | 409 | AIP-193 | Create operations that may conflict |
45-
| `Common.Gone` | 410 | ★ OpenMeter | PUT/PATCH when the resource was soft-deleted |
46-
| `Common.PayloadTooLarge` | 413 | ★ OpenMeter | Endpoints accepting large bodies or bulk operations |
47-
| `Common.UnprocessableContent` | 422 | ★ OpenMeter | Semantically invalid requests |
48-
49-
```tsp
50-
get(@path meterId: Shared.ULID): Shared.GetResponse<Meter> | Common.NotFound | Common.ErrorResponses;
51-
```
39+
- `rules/openmeter-error-types.md` — OpenMeter `Common.*` types that wire AIP-193 onto operations, including OpenMeter-only status codes (410, 413, 422).
40+
- `rules/inline-errors.md` — inline errors returned **inside** a 2xx response body (partial successes, pre-flight validation on draft resources). AIP-193 itself only covers transport-level error responses.
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# Inline (partial / non-fatal) errors
2+
3+
Some endpoints return errors **inside** a 2xx response body. These are **not** RFC-7807 problem details — RFC-7807 (see `aip-193-errors.md`) covers errors that fail the whole request. Inline errors live as a field on the response or resource model and describe per-item or pre-flight failures that did not stop the response from being produced.
4+
5+
## When to use
6+
7+
- **Partially successful responses** — the operation processed some items and reports per-item failures alongside the successful results (typically a `data[]` plus `errors[]` pair on the response model).
8+
- **Not-yet-finalized resources** — the resource itself is in a non-final state (e.g. a draft) and exposes pre-flight findings that block promotion to the final state. The GET itself succeeds; the findings are part of the resource shape.
9+
10+
If the whole request fails, return an RFC-7807 response instead — see `aip-193-errors.md`.
11+
12+
## Required shape: `Shared.BaseError<T>`
13+
14+
All inline errors compose `Shared.BaseError<T>` from `api/spec/packages/aip/src/shared/errors.tsp`. It defines the three uniform fields every inline error must carry, so SDKs and UIs can render them generically:
15+
16+
| Field | Type | Required | Meaning |
17+
| ------------ | ----------------- | -------- | ------------------------------------------------------------------ |
18+
| `code` | `T` | yes | Machine-readable error code |
19+
| `message` | `string` | yes | Human-readable description |
20+
| `attributes` | `Record<unknown>` | optional | Additional structured context (field path, offending value, IDs) |
21+
22+
`T` is the type of `code`. Pick one:
23+
24+
- **Domain-scoped enum (preferred)** — codes are discoverable and stable, and clients can switch on them. Include an `Unknown: "unknown"` zero member for forward compatibility.
25+
- **`string`** — only when the set of codes is genuinely open-ended (e.g. validation rules that grow over time without API churn).
26+
27+
## Composition pattern
28+
29+
Use `model … is Shared.BaseError<T>` and add domain-specific identifying fields as needed (e.g. the input that produced the error, a JSON path, a resource id). Do **not** redeclare `code`, `message`, or `attributes`.
30+
31+
```tsp
32+
import "../shared/index.tsp";
33+
34+
namespace MyDomain;
35+
36+
/** Machine-readable code for a {operation} error. */
37+
@friendlyName("MyOperationErrorCode")
38+
enum MyOperationErrorCode {
39+
Unknown: "unknown",
40+
// domain-specific codes...
41+
}
42+
43+
/** Inline error returned by {operation}. */
44+
@friendlyName("MyOperationError")
45+
model MyOperationError is Shared.BaseError<MyOperationErrorCode> {
46+
/** Identifier from the request that produced this error. */
47+
@visibility(Lifecycle.Read)
48+
@summary("Subject")
49+
subject?: string;
50+
}
51+
```
52+
53+
For an open-ended code set, pass `string`:
54+
55+
```tsp
56+
@friendlyName("MyValidationError")
57+
model MyValidationError is Shared.BaseError<string> {
58+
/** JSON path to the offending field. */
59+
@visibility(Lifecycle.Read)
60+
@summary("Field")
61+
field: string;
62+
}
63+
```
64+
65+
## Wiring into the response
66+
67+
Add the inline errors as a field on the response or resource model — typically named `errors` (partial responses) or a domain-specific name like `validation_errors` (pre-flight findings on a draft resource).
68+
69+
```tsp
70+
@friendlyName("MyOperationResponse")
71+
model MyOperationResponse {
72+
@visibility(Lifecycle.Read)
73+
data: MyOperationResult[];
74+
75+
/** Per-item failures encountered while processing the request. */
76+
@visibility(Lifecycle.Read)
77+
errors: MyOperationError[];
78+
}
79+
```
80+
81+
## Naming
82+
83+
- Model: `<Domain><Operation>Error` or `<Domain>ValidationError`.
84+
- Code enum (when used): `<ModelName>Code` (drop the trailing `Error`, append `Code`).
85+
- All fields use `@visibility(Lifecycle.Read)` — inline errors are always server-produced.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# OpenMeter error response types
2+
3+
OpenMeter wires AIP-193 (`rules/aip-193-errors.md`) onto operations through the `Common.*` types in `api/spec/packages/aip/src/common/`. Use `Common.ErrorResponses` (= `BadRequest | Unauthorized | Forbidden`) on every operation, then add specific types explicitly. Types with ★ are OpenMeter extensions beyond what AIP-193 documents by name.
4+
5+
| Type | Status | Source | When to add explicitly |
6+
| ----------------------------- | ------ | ----------- | --------------------------------------------------- |
7+
| `Common.NotFound` | 404 | AIP-193 | GET, PATCH, PUT, DELETE by ID |
8+
| `Common.Conflict` | 409 | AIP-193 | Create operations that may conflict |
9+
| `Common.Gone` | 410 | ★ OpenMeter | PUT/PATCH when the resource was soft-deleted |
10+
| `Common.PayloadTooLarge` | 413 | ★ OpenMeter | Endpoints accepting large bodies or bulk operations |
11+
| `Common.UnprocessableContent` | 422 | ★ OpenMeter | Semantically invalid requests |
12+
13+
```tsp
14+
get(@path meterId: Shared.ULID): Shared.GetResponse<Meter> | Common.NotFound | Common.ErrorResponses;
15+
```
16+
17+
For inline errors returned **inside** a 2xx response body (partial successes, pre-flight validation on draft resources), see `rules/inline-errors.md`.

api/spec/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"type": "module",
55
"scripts": {
66
"generate": "pnpm --filter @openmeter/api-spec-legacy run generate && pnpm --filter @openmeter/api-spec-aip run generate",
7-
"format": "prettier --list-different --find-config-path --write .",
7+
"format": "prettier --list-different --find-config-path --write . && pnpm --filter @openmeter/api-spec-aip run format",
88
"lint": "prettier --check . && pnpm --filter @openmeter/api-spec-legacy run lint && pnpm --filter @openmeter/api-spec-aip run lint",
99
"lint:fix": "prettier --write ."
1010
},

api/spec/packages/aip/lib/index.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { defineLinter } from '@typespec/compiler'
22
import { casingErrorsRule, casingRule } from './rules/casing.js'
3-
import { docDecoratorRule } from './rules/docs.js'
3+
import { docDecoratorRule, docFormatRule } from './rules/docs.js'
44
import { friendlyNameRule } from './rules/friendly-name.js'
55
import { operationSummaryRule } from './rules/operation-summary.js'
66
import { operationIdKebabCaseRule } from './rules/operation-id.js'
@@ -13,6 +13,7 @@ const rules = [
1313
casingRule,
1414
casingErrorsRule,
1515
docDecoratorRule,
16+
docFormatRule,
1617
friendlyNameRule,
1718
noNullableRule,
1819
operationSummaryRule,

api/spec/packages/aip/lib/rules/docs.js

Lines changed: 144 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,17 @@
1-
import { createRule, paramMessage, getDoc } from '@typespec/compiler'
1+
import {
2+
createRule,
3+
defineCodeFix,
4+
getDoc,
5+
getSourceLocation,
6+
paramMessage,
7+
} from '@typespec/compiler'
8+
import * as prettier from 'prettier'
9+
import {
10+
detectNewline,
11+
extractMarkdownFromDocComment,
12+
getIndentBefore,
13+
wrapMarkdownAsDocComment,
14+
} from './utils.js'
215

316
export const docDecoratorRule = createRule({
417
name: 'doc-decorator',
@@ -60,3 +73,133 @@ export const docDecoratorRule = createRule({
6073
},
6174
}),
6275
})
76+
77+
/**
78+
* Format a doc-comment Markdown body through Prettier.
79+
* Returns the formatted Markdown body (no `/** *\/` framing).
80+
* @param {string} markdown
81+
* @param {{ printWidth?: number, proseWrap?: 'always' | 'never' | 'preserve' }} [options]
82+
*/
83+
async function formatDocMarkdown(markdown, options = {}) {
84+
if (markdown.trim() === '') return ''
85+
return await prettier.format(markdown, {
86+
parser: 'markdown',
87+
printWidth: options.printWidth ?? 80,
88+
proseWrap: options.proseWrap ?? 'always',
89+
})
90+
}
91+
92+
/**
93+
* Build a code fix that replaces a doc comment with a precomputed string.
94+
* The Prettier work happens before this is constructed; the fix callback is
95+
* sync and just emits the replacement.
96+
*
97+
* @param {import('@typespec/compiler').SourceLocation} location
98+
* The full `/** ... *\/` source range.
99+
* @param {string} newText The replacement text, including `/**` and `*\/`.
100+
*/
101+
function createFormatDocCommentCodeFix(location, newText) {
102+
return defineCodeFix({
103+
id: 'format-doc-comment',
104+
label: 'Format doc comment',
105+
fix(context) {
106+
return context.replaceText(location, newText)
107+
},
108+
})
109+
}
110+
111+
/**
112+
* Collect every `DocNode` reachable from the program by walking semantic
113+
* targets that can carry doc comments. We use the existing semantic listener
114+
* surface (model/property/enum/etc.) rather than a private AST walker.
115+
*/
116+
function collectDocNodes(target, sink) {
117+
const node = target.node
118+
if (!node || !node.docs || node.docs.length === 0) return
119+
for (const doc of node.docs) sink.push(doc)
120+
}
121+
122+
export const docFormatRule = createRule({
123+
name: 'doc-format',
124+
severity: 'warning',
125+
description:
126+
'Format doc comment bodies as Markdown using Prettier (proseWrap=always).',
127+
messages: {
128+
default:
129+
'Doc comment is not formatted. Apply the suggested fix to reformat as Markdown.',
130+
},
131+
// Async because Prettier 3.x's `format` is async.
132+
async: true,
133+
create: (context) => {
134+
/** @type {import('@typespec/compiler').DocNode[]} */
135+
const docNodes = []
136+
137+
const collect = (target) => collectDocNodes(target, docNodes)
138+
139+
return {
140+
model: collect,
141+
modelProperty: collect,
142+
enum: collect,
143+
enumMember: collect,
144+
union: collect,
145+
unionVariant: collect,
146+
operation: collect,
147+
interface: collect,
148+
scalar: collect,
149+
namespace: collect,
150+
151+
async exit() {
152+
// Deduplicate: a doc may be visited via multiple semantic kinds.
153+
const seen = new Set()
154+
const work = []
155+
for (const doc of docNodes) {
156+
if (seen.has(doc)) continue
157+
seen.add(doc)
158+
work.push(processDoc(doc, context))
159+
}
160+
await Promise.all(work)
161+
},
162+
}
163+
},
164+
})
165+
166+
/**
167+
* Compute a formatted replacement for a single DocNode and, if it differs
168+
* from the source, report a diagnostic with an attached code fix.
169+
* @param {import('@typespec/compiler').DocNode} doc
170+
* @param {import('@typespec/compiler').LinterRuleContext<any>} context
171+
*/
172+
async function processDoc(doc, context) {
173+
const location = getSourceLocation(doc)
174+
const source = location.file.text
175+
const raw = source.slice(location.pos, location.end)
176+
177+
// Defensive: only format actual `/** ... */` blocks.
178+
if (!raw.startsWith('/**') || !raw.endsWith('*/')) return
179+
180+
const indent = getIndentBefore(source, location.pos)
181+
const newline = detectNewline(source)
182+
183+
let markdown
184+
try {
185+
markdown = extractMarkdownFromDocComment(raw)
186+
} catch {
187+
return
188+
}
189+
190+
let formatted
191+
try {
192+
formatted = await formatDocMarkdown(markdown)
193+
} catch {
194+
// If Prettier can't parse the body, leave it alone.
195+
return
196+
}
197+
198+
const replacement = wrapMarkdownAsDocComment(formatted, indent, newline)
199+
if (replacement === raw) return
200+
201+
context.reportDiagnostic({
202+
target: doc,
203+
codefixes: [createFormatDocCommentCodeFix(location, replacement)],
204+
})
205+
}

0 commit comments

Comments
 (0)