fix(backend): include surface flag in OpenAPI stale-contract hint#10294
fix(backend): include surface flag in OpenAPI stale-contract hint#10294kodjima33 wants to merge 1 commit into
Conversation
The stale/missing-file hint from export_openapi.py's --check told users to run `export_openapi.py --write <path>`, omitting the surface-selection flag. Surface is a separate flag (defaults to `public`), not inferred from the output path, so following the hint verbatim for a non-public surface silently exports the public surface into the wrong file (e.g. 17-path public spec overwriting the 309-path app-client spec) and --check then validates the wrong surface as up to date. check_spec now takes the surface and prefixes the correct flag (`--app-client ` / `--surface integration-public `; empty for public) in both the stale and does-not-exist hints. Regression test asserts each surface's hint carries its selector. Verified: exercised check_spec directly for all three surfaces plus the does-not-exist branch (correct flag in every hint, public unchanged); full tests/unit/test_openapi_contract.py green (15 passed) via the pinned openapi runner venv. fixes #10217
|
Closing — opened by an automated bot and left unreviewed. The bot policy has changed: it now tests and merges fixes directly (or reports them), and no longer leaves open PRs for review. If this fix is still wanted, reopen and it'll be landed properly (tested → merged). 🤖 |
|
Hey @kodjima33 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Before your next PR, please skim:
If this was declined for direction or taste, maintainers should cite an invariant ID or open a proposed one — ask if that citation is missing. Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! |
What
The
--checkfailure hint inbackend/scripts/export_openapi.pynow includes the surface-selection flag, so following it verbatim regenerates the correct surface.Why
From #10217:
check_spechad no knowledge of which surface was being checked, so itsis stale/does not existhints always emitted the bare--write <path>command, which defaults to the public surface.Change
check_spec(path, generated, surface='public')now prefixes the correct selector via a newsurface_flag()helper:--app-clientfor app-client,--surface integration-publicfor integration-public, empty for public (default invocation unchanged).main()passesargs.surfaceintocheck_spec.test_stale_hint_includes_surface_flag_for_non_public_surfacesasserts each surface's hint carries its selector and that public stays flag-free.Verified
check_speccode path for all three surfaces plus the does-not-exist branch (via the pinned openapi runner venv): every hint carries the right flag; the public hint is byte-for-byte the previous default.... run backend/scripts/export_openapi.py --app-client --write <path>... --surface integration-public --write <path>... export_openapi.py --write <path>(unchanged)tests/unit/test_openapi_contract.py— 15 passed (including the new regression test).black --line-length 120 --skip-string-normalizationclean on both files.Auto-generated from issue feedback by the mini issues-improver. Review before merge.