Skip to content

[AEP][medium] AEP-20260924-BE002: cap limits and fail closed on bad history - #330

Merged
mattmre merged 3 commits into
mainfrom
aep/medium/AEP-20260924-BE002/api-limits
Sep 24, 2026
Merged

mattmre merged 3 commits into
mainfrom
aep/medium/AEP-20260924-BE002/api-limits

Conversation

@mattmre

@mattmre mattmre commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Description

Caller-controlled dashboard limits are capped at 5000. A non-integer /api/events limit is HTTP 400. A non-integer model-scope limit keeps 20 and returns JSON. A non-integer cleanup limit keeps 25. Zero and negative limits return no rows. OPTIONS is 204 without a bearer. Corrupt history files raise instead of becoming an empty success.

This score is for de1fcab, rebased onto main after #329. The earlier SHA 6dae0f2 returned HTTP 500 for limit=abc on the model-scope and cleanup routes.

Type of Change

  • Bug fix

Testing

Parent re-ran test_dashboard_api_limits.py and test_do_get_routes_disk_llm_estimate: 9 passed. Ruff passed. Review grok-tierb-330b-20260924 repeated that run and probed the live handlers. No browser.

Brutal Honesty

  • Blank ?limit= is parsed as an omitted limit, not as 400.
  • Model-scope imports torch inside the handler. This environment's probe stubbed that import so the limit path could run. The limit parse itself was not stubbed.
  • The options test restores DASHBOARD_TOKEN and DASHBOARD_CORS_ORIGIN.

EVIDENCE: 9 dashboard limit tests passed on de1fcab; review grok-tierb-330b-20260924
SMOKE: targeted unittest only; bash scripts/smoke.sh was not run on this branch
BHS_SELF_DRAFT: 100
BHS_SELF_DRAFT_AGENT: grok-aep-fix-20260924
BHS_TIER_B: 100
BHS_TIER_B_AGENT: grok-tierb-330b-20260924
BHS_TIER_B_SEVERITY: none
BHS_OFFICIAL: 100
CARRY_FORWARD: none
DEFERRED_SCOPE: history-route 400 stays on PR 336; inline timestamp stays on PR 337
LOOP_ITERATIONS: 2
OPERATOR_OVERRIDE: none

@mattmre

mattmre commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

Review of 6dae0f2, stacked on the dashboard UI branch. Events limit=abc is 400, do_OPTIONS is 204 without a bearer, and zero or negative model-scope limits become an empty list. One acceptance criterion from AEP-20260924-BE002 is still open.

handle_api_model_scope_events, handle_api_model_scope_features, and handle_api_model_scope_interventions call _nonnegative_limit inside try (dashboard_server.py:1979, :2002, :2025). _nonnegative_limit("abc") raises ValueError. The except Exception then sends HTTP 500 (:1995-1996 and the matching lines on the other two handlers). BE002 step 1 says a non-integer limit keeps the handler default (20) and must not 500. AC3 says ?limit=abc is not HTTP 500 and the body does not contain the exception text.

handle_api_evidence_cleanup_plan (:1967-1968) does int(query_params["limit"][0]) inside the same kind of try. A non-integer becomes HTTP 500. BE002 step 1 says that limit keeps the cleanup default of 25.

test_dashboard_api_limits.py checks _nonnegative_limit on 0, -3, and 9000. It does not call the three model-scope handlers with abc, so this 500 stays green.

Required fix: on ValueError from those four parses, use the handler default (20 for model-scope, 25 for cleanup) and return the normal JSON. Do not send 500. Add a handler test that fails if abc calls send_error_response(500, ...).

/api/events?limit=abc staying 400 is the separate events contract. Leave that.

@mattmre

mattmre commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

Second review note on 6dae0f2, in addition to the model-scope HTTP 500.

test_dashboard_api_limits.py test_options_preflight_does_not_require_a_bearer sets dashboard_server.DASHBOARD_TOKEN = "secret" and does not restore the previous value. _explicit_open_mode only toggles DASHBOARD_ALLOW_UNAUTHENTICATED. After that test, test_dashboard_server.py test_do_get_routes_disk_llm_estimate (test_dashboard_server.py:1215) gets 401 because the token is still set. A combined run of the dashboard modules was 93 tests, 1 failed, 2 skipped. Restore the previous token (and DASHBOARD_CORS_ORIGIN, which that test also sets) in a finally.

Base automatically changed from aep/high/AEP-20260924-FE001/dashboard-ui to main September 24, 2026 14:42
A non-integer events limit is HTTP 400 instead of the whole log. Model-scope
limits no longer treat zero as the full list. CORS preflight can return 204
without a bearer. A campaign, validation, preflight, or evidence-index file
that is not a JSON object reaches the handler 500 instead of an empty success.
@mattmre
mattmre force-pushed the aep/medium/AEP-20260924-BE002/api-limits branch from 0b93b62 to de1fcab Compare September 24, 2026 14:43
@mattmre

mattmre commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

Fix pushed and rebased onto current main. Tip is de1fcab.

  • A non-integer model-scope limit keeps 20 and returns JSON. A non-integer cleanup limit keeps 25. A non-integer keep_latest keeps 1. /api/events?limit=abc is still HTTP 400.
  • The options preflight test restores DASHBOARD_TOKEN and DASHBOARD_CORS_ORIGIN.
  • Parent re-ran test_dashboard_api_limits.py and test_do_get_routes_disk_llm_estimate together: 9 passed.

Not merged. The body still scores the previous SHA. A fresh Tier B review of de1fcab is required before that score can move.

@mattmre
mattmre merged commit e664b2c into main Sep 24, 2026
12 checks passed
@mattmre
mattmre deleted the aep/medium/AEP-20260924-BE002/api-limits branch September 24, 2026 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant