Repository navigation
fix(api): unbreak master — rate-limit headers, import validate, agent reject - #875
Merged
Merged
Conversation
… reject Four integration regressions from the #861/#862/#863 merge train left master red. Fixes: - Rate-limit headers (#801): #863 rewrote the limiter without re-emitting X-RateLimit-* headers. headers_enabled on the SlowAPI Limiter can't be used (the decorator then tries to inject into dict-returning endpoints and raises), and SlowAPIMiddleware exempts decorated routes. Add a pure-ASGI RateLimitHeadersMiddleware that injects the headers from scope-state view_rate_limit onto every HTTP response (incl. 401 login) and leaves WebSocket scopes untouched. - Import validate (#862): ImportRow carried a hard field_validator on minion_id, so the dry-run validate endpoint 500'd on intentionally-bad rows instead of reporting them. Move format checking into node_import.validate_row (soft, status="invalid") and re-check defensively on the commit path so a bad minion_id can still never be persisted. - Agent reject (#795): the endpoint correctly commits status="rejected" in its own session; the test's module-scoped db_session (expire_on_commit=False) was asserting against a stale identity-mapped instance. Refresh before asserting. Verified against a Postgres-backed fleet_test: the 4 previously-red tests plus their full files (32) and the related rate-limit/import unit tests (45) pass.
github-actions
Bot
added
breaking-change
The behavioural rate-limit tests failed in the full suite (429 too early / "state shared across IP keys") while passing in isolation. Root cause: four integration app fixtures (conftest.app_with_test_db and the local fixtures in test_auth_endpoints, test_check_minion_id, test_node_import_api) did `limiter_module.limiter = Limiter(...)` before importing the routes. The @limiter.limit decorators bind to whichever limiter object exists at first route import and can't be rebound, so the throwaway test_limiter from the first such module became the decorators' limiter for the whole session — while the behavioural tests reset the *production* limiter and saw stale 429s. Fix: don't swap. Share the real production limiter and just zero its in-memory storage on setup/teardown so counts don't bleed across modules. This keeps app.state.limiter and the decorator limiter as the same object everywhere. Verified: full integration suite (excl. the Redis-only vnc/webssh ws tests that can't run in the sandbox) is 326 passed / 1 xfailed, including all of test_rate_limit_behavioral_801 in full-suite order.
Co-authored-by: Cursor <[email protected]>
…ion-red Co-authored-by: Cursor <[email protected]> # Conflicts: # VERSION # frontend/package.json
…transport Master went red after #877 (local-timezone date formatting) and the ImportRow soft-validation change merged together: - test_ist_timestamps / test_processes_tab_psutil_613 asserted hardcoded "Asia/Kolkata" / " IST" strings; now assert the dynamic getTimezone() + timeZoneName/formatIST contract introduced by #877 (#796). - test_validators_752 TestImportRow expected ValidationError on bad ids, but ImportRow is the validate dry-run transport and must carry invalid rows; assert it accepts the string while MINION_ID_RE rejects it.
Coverage Report✅ 85.9% on Gate passed |
added 3 commits
June 24, 2026 22:40
mutmut 3.6.0 renamed the [tool.mutmut] config keys and shipped a config-loader regression that crashes `mutmut results` with "can only concatenate list (not 'str') to list". Since the mutation.yml "Print mutation results" step has no continue-on-error, this failed the check on every PR touching validator/redaction tests (e.g. this branch's test_validators_752.py edit). Pin <3.6.0 so the existing string-valued config keeps working.
…ion-red # Conflicts: # VERSION # frontend/package.json
#878 extracted Escape-to-close + focus trapping into the shared useFocusTrap hook, so ConfirmDialog.tsx no longer contains the literal "Escape". Assert it delegates to useFocusTrap and that the hook handles Escape, instead of grepping the component for the moved string.
Coverage Report✅ 85.9% on Gate passed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Master went red after the #861/#862/#863 merge train. This restores it by fixing the 4 failing integration tests — fixing production behaviour where there was a real bug, and test wiring where the production code was already correct.
X-RateLimit-*headers.headers_enabled=Trueon the SlowAPILimiteris unusable (its decorator then tries to inject headers into our dict-returning endpoints and raisesparameter response must be an instance of Response), andSlowAPIMiddlewareexempts decorated routes. Added a pure-ASGIRateLimitHeadersMiddlewarethat injects the headers fromscope.state.view_rate_limitonto every HTTP response (including the 401 login) and leaves WebSocket scopes untouched.ImportRowhad a hardfield_validatoronminion_id, so the dry-run validate endpoint 500'd on intentionally-bad rows instead of categorising them. Format checking moved intonode_import.validate_row(soft →status="invalid"), with a defensive re-check on the commit path so a badminion_idcan never be persisted.status="rejected"correctly in its own session; the test's module-scopeddb_session(expire_on_commit=False) asserted against a stale identity-mapped instance. Now refreshes before asserting (no weakening).Test plan
fleet_testruffclean, app buildsMade with Cursor