fix(backend): include surface flag in OpenAPI stale-contract hint (#10217)#10255
fix(backend): include surface flag in OpenAPI stale-contract hint (#10217)#10255kodjima33 wants to merge 1 commit into
Conversation
…0217) The stale/missing hints printed by export_openapi.py's contract check told users to run `export_openapi.py --write <path>`, omitting the surface flag. Surface selection is a separate flag (defaults to `public`), not inferred from the output path — so following the hint verbatim for the app-client or integration-public spec silently regenerated the public surface and overwrote the wrong contract (53k-line delta observed on the app-client spec), and `--check` then validated the wrong surface as up to date. check_spec now takes the surface and prepends the selecting flag (`--app-client ` / `--surface integration-public `; nothing for public) to both hints, matching the command that actually regenerates that surface. Failure-Class: none Verification: exercised the real check_spec/surface_flag from source over a tmp spec for all three surfaces + missing-file path — app-client emits `--app-client --write`, integration-public emits `--surface integration-public --write`, public stays `--write`. Added test_check_spec_hint_includes_surface_flag.
|
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! |
Closes #10217
Bug
backend/scripts/export_openapi.py's contract check prints a fix-it hint on a stale or missing spec:The surface (
public/app-client/integration-public) is selected by a separate flag that defaults topublicand is not inferred from the output path. So following the hint verbatim for the app-client or integration-public spec regenerates the public surface and overwrites the wrong contract — the reporter saw a 17-path public spec written over the 309-path app-client spec (~53k lines deleted) — and a subsequent--check(also missing the flag) then validated the wrong surface as "up to date".Fix
check_specnow receives the surface and prepends the selecting flag to both the stale and does-not-exist hints via a smallsurface_flag()helper:app-client→--app-client --write <path>integration-public→--surface integration-public --write <path>public→ unchanged (--write <path>)The
main()--checkcall passesargs.surfacethrough. No behavior change for the public surface; existing callers/test remain valid.Verification
Exercised the real
check_spec/surface_flagfunctions from source over a temp spec for all three surfaces plus the missing-file path — each hint now names the flag that actually regenerates that surface. Addedtest_check_spec_hint_includes_surface_flagas the regression test (extendstests/unit/test_openapi_contract.py). Backend Python env is not installable in the watchdog sandbox, so the full pytest suite runs in CI.🤖 automated by hourly watchdog; opened for review, not merged.