Repository navigation
feat: follow openapi.yaml, and sync it daily by pull request - #51
Conversation
|
Nice direction overall: one source of truth for paths, parameters and allowed values, plus a sync that turns spec drift into a reviewable PR. I checked that the backend runs the same request check on every v1 endpoint (multipart skipped) and that 1. An installed CLI now refuses what the server already accepts (main concern)
The daily sync keeps this repo close to the server. It does nothing for a CLI that is already installed: an npm install, or a version pinned somewhere downstream, stays on its bundled spec until someone upgrades. That lag is often days.
Suggestion: add an escape hatch (an env var such as 2. Internal details in a public repo
3. Smaller points
|
|
I agree with the direction. Calling by operationId, failing the generator on an unknown keyword, the CI drift check, holding every request in the suite to the spec, and reading flag choices from the spec are all good. They also fix real bugs: One part needs to change before merge: the local JSON body check ( Problem: when the API accepts more, installed CLIs start to failEvery released CLI version freezes the spec it was built with. The spec has 159 Reproduced with a build of this PR (dummy token, no network request): So "never stricter than the server" is true only while both sides have the same spec. Over time, an old client is always stricter than the API. The daily sync shortens how far the repo lags behind the API. It does not shorten how long users take to upgrade. A new capability works only after the whole chain is done: Our own clients can upgrade quickly. External users cannot follow our schedule. They need a CLI that keeps working, not one that starts to fail the day the API ships a change. The error also says "openapi.yaml does not take this", which misleads both people and models. Why the local body check does not pay for itselfThe API already validates every request against the same openapi.yaml, and its 400 has the same shape as the one here ( Suggestions
Smaller items:
|
ac03c79 to
e7330eb
Compare
|
Thanks — all taken. Pushed as a new commit on top, with the two earlier commits reworded (force-pushed; the code in them is unchanged). 1. An installed CLI refusing what the server accepts. Went with your second option, so no escape hatch is needed: only what no version of the API would take is refused locally — a wrong type, a missing required key. A value outside an enum, a key Flag 2. Internal details. The doc comment, ARCHITECTURE.md, the PR description and both earlier commit bodies now describe it as "the way API v1 validates requests against the same openapi.yaml". Footers are 3. Smaller points.
|
spec/openapi.yaml is a verbatim copy of the published API v1 spec, and `npm run generate` turns it into src/generated/operations.ts: every operation's method, path, parameters and request-side schemas, prose stripped. Commands name an operation instead of a path, through call(), which checks path and query against the operation before sending (an undeclared or missing parameter, a value not of its type) and spells a list the way the spec says. The values a flag offers are read from the spec too; a list the spec stops giving leaves the flag open instead of crashing every command, and a test fails on it. A body goes as given: the API validates it against the openapi.yaml it runs, and a local copy would make every installed CLI refuse what the API learns after it ships. The tests hold every request the suite sends, and every example in the help, to the spec instead, body included, with a check that is not shipped. A daily workflow fetches the spec, regenerates, runs check and test, and opens a pull request listing the operations that moved and which the CLI calls; it is a feat naming those, or a chore when none is. CI fails if src/generated drifts from the spec. Which commands exist is still the CLI's decision. Following the spec changed what some commands accept: - view create --type takes form; form create --scene takes form and quickpay; table create lists RelationField - transaction list/aggregate send paid_status comma-joined - refused before sending: entry list --view with --limit, entry count of several containers with --keyword, a serial number that is not one - the form edit example used exam_setting.total_score, which the API does not take; it now uses limited_time Refs: #T17509 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
e7330eb to
8b40b9f
Compare
|
Agreed on all of it — done, and squashed into one commit (force-pushed).
Smaller items:
@kba977 this supersedes item 1 of my previous reply: rather than refusing only certain errors, the body is not checked locally at all. The sync PR title, the non-fatal |
Why
The CLI wrote down every path, parameter and allowed value by hand, a second copy of what openapi.yaml already says. The copy had fallen behind:
view create --type formandform create --scene form|quickpaywere refused before reaching the API, and theform edithelp example sent anexam_settingkey the API rejects. Nothing told anyone when the spec moved.This adds an Action that notices openapi.yaml changes and brings them to the CLI by pull request. Which commands exist stays the CLI's decision; what a request may carry — paths, parameters and their types, the values a flag offers — follows the spec.
What changed
spec/openapi.yaml: the published spec, verbatim.npm run spec:updatefetches it;npm run generatewritessrc/generated/operations.ts(every operation's method, path, parameters and request-side schemas, prose stripped). A schema keyword the generator does not know stops it rather than being dropped.src/spec.ts:call('listEntries', { path, query, body })is how every command makes a request now. It checks path and query against the operation — an undeclared or missing parameter, a value not of its type — and spells a list the way the spec says (explode: false→ comma-joined).onContainerpicks the form or table operation by--form/--table;choices/bodyChoicesfeed the flags' allowed values from the spec. A list the spec stops giving leaves its flag open rather than crashing every command at load; a test fails on it instead.--help, to the spec — body included — withsrc/testing/conformance.ts, which is not shipped. That is how theform editexample was found..github/workflows/spec-sync.yml: daily at 09:17 Asia/Shanghai. Fetches, regenerates, runscheckandtest, and opens a PR onchore/sync-openapilisting the operations and schemas that moved and which the CLI calls (scripts/spec-diff.mjs; an operation counts as changed when any schema it refers to changed). The title comes from the same comparison:feat: follow openapi.yaml changes to <operations>when an operation the CLI calls changed, achoreotherwise. A PR is opened even when the CLI no longer passes, and the run fails so it is noticed.src/generateddiffers from whatnpm run generateproduces.Behaviour changes
view create --typetakesform;form create --scenetakesformandquickpay;table create --helplistsRelationField.transaction list|aggregate --status a --status bsendspaid_status=a,b(spec:explode: false) instead ofpaid_status[]=.entry list --viewwith--limit(a view is read 50 a page),entry countof several containers with--keyword, a non-numeric entry serial.form editexample now usesexam_setting.limited_time;total_scoreis not inExamSettingInput.Setup
The sync PR is opened with
RELEASE_PLEASE_TOKENso CI runs on it. Without that secret it falls back toGITHUB_TOKEN, which needs "Allow GitHub Actions to create and approve pull requests" enabled and shows no checks on the PR.workflow_dispatchonly works from the default branch, so the workflow can first be run by hand right after merge.Testing
npm run checkandnpm test(277 tests) pass.tscfail at the call sites.spec-diff.mjsand the PR-description step were run locally against an older copy of the spec; the workflow itself has not run on GitHub yet.Refs: #T17509
🤖 Generated with Claude Code