Skip to content

Add test suite, CI, and a batch of correctness fixes - #6

Merged
agb1986 merged 1 commit into
mainfrom
add-tests-ci-correctness
Aug 8, 2026
Merged

agb1986 merged 1 commit into
mainfrom
add-tests-ci-correctness

Conversation

@agb1986

@agb1986 agb1986 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • Test suite (50 tests) covering the MCP tools, the REST ingest endpoint, both auth middlewares, the web dashboard (including HTML escaping), and the finance hook's parsing/description logic — backed by an in-memory MongoDB (mongomock-motor), so tests need no containers and run in under a second
  • CI: GitHub Actions workflow running ruff check + pytest on every push to main and every PR
  • Ruff configured in pyproject.toml (with import sorting and modern-syntax rules); existing code cleaned up to pass
  • Correctness fixes found during the repo review:
    • Constant-time API key comparison in the MCP/REST bearer auth (app/server.py), matching what the dashboard's basic auth already did with secrets.compare_digest
    • REST POST /entries now validates field types via a Pydantic model — previously data/keywords of any type were inserted into Mongo as-is
    • list_entries clamps limit (1–100) and skip (≥0); negative values previously raised from Mongo and limit was unbounded
    • Indexes on created_at, source, and keywords created at server startup, so list/dashboard queries stop collection-scanning as the hook-fed store grows
    • Mutable default arguments (keywords: list[str] = []) replaced with None defaults
    • except (InvalidId, Exception) in parse_id narrowed to except (InvalidId, TypeError) so unexpected errors surface

Test plan

  • ruff check . — clean
  • pytest — 50 passed locally against mongomock
  • Smoke-tested ensure_indexes + SSE app assembly (mcp.http_app + REST route + middleware) imports and builds
  • CI green on this PR (first run of the new workflow)
  • After merge: docker compose up -d --build on the CasaOS box and confirm the hook still ingests entries

🤖 Generated with Claude Code

The hook and REST endpoint have each needed a bug-fix PR already, with
no safety net to catch regressions. This adds one and fixes the small
correctness issues found during a repo review:

- 50 pytest tests covering the MCP tools, REST endpoint, both auth
  middlewares, the dashboard, and the finance hook's parsing logic,
  backed by an in-memory MongoDB (mongomock-motor) so no containers
  are needed
- GitHub Actions workflow running ruff + pytest on every push and PR
- Constant-time API key comparison in the MCP/REST bearer auth,
  matching what the dashboard's basic auth already did
- Validate REST body field types with a Pydantic model instead of
  inserting whatever arrives into Mongo
- Clamp list_entries limit/skip (negative values raised from Mongo,
  and limit was unbounded)
- Create indexes on created_at/source/keywords at startup, so list
  and dashboard queries stop collection-scanning as the store grows
- Replace mutable default arguments and a dead `except (InvalidId,
  Exception)` clause

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@agb1986
agb1986 merged commit 01f5aa5 into main Aug 8, 2026
1 check passed
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