feat: FastAPI scaffold + GroupMe webhook handler #10

Merged

forgejo_admin merged 2 commits from 4-fastapi-scaffold into main 2026-03-28 22:35:47 +00:00

Conversation 2 Commits 2 Files changed 10 +367

forgejo_admin commented 2026-03-28 21:49:48 +00:00

Owner

Copy link

Summary

Scaffolds the entire FastAPI application with a GroupMe webhook callback handler, health endpoints, pydantic-settings config, multi-stage Dockerfile, Woodpecker CI pipeline, and 8 passing tests. The callback validates group allowlist, ignores bot messages, logs user messages, and returns 200. No AI logic yet -- that is wired in ticket #6.

Changes

• app/__init__.py -- empty package init
• app/config.py -- pydantic BaseSettings with all env vars (GroupMe, Anthropic, Keycloak, basketball-api)
• app/main.py -- FastAPI app with lifespan, includes health and groupme routers
• app/groupme.py -- POST /groupme/callback: parse body, validate group_id against allowlist, ignore bot sender_type, log sender_name + text
• app/health.py -- GET /healthz (liveness, always 200), GET /health/ready (readiness, checks ANTHROPIC_API_KEY)
• Dockerfile -- multi-stage python:3.12-slim, gunicorn+uvicorn workers, EXPOSE 8000
• .woodpecker.yaml -- test on push/PR, build+push to Harbor on main, kustomize tag update
• requirements.txt -- fastapi, uvicorn, gunicorn, httpx, anthropic, pydantic-settings, pytest, pytest-asyncio
• tests/__init__.py -- empty package init
• tests/test_groupme.py -- 8 tests covering valid message, bot ignore, wrong group, malformed payload, invalid JSON, healthz, readiness with/without key

Test Plan

• pytest tests/ -v -- all 8 tests pass
• Docker image builds with docker build .
• POST /groupme/callback with valid payload returns 200
• Bot messages and wrong group_id silently return 200
• GET /healthz returns 200
• GET /health/ready returns ready/not_ready based on ANTHROPIC_API_KEY

Review Checklist

• All 8 tests pass
• No unrelated changes
• Dockerfile follows basketball-api multi-stage pattern
• CI pipeline uses internal Harbor URL
• GroupMe callback never crashes on malformed input

Related

• Closes #4

Related Notes

• project-westside-ai-assistant -- parent project
• arch-domain-westside-ai-assistant -- A1: GroupMe Webhook Handler

## Summary Scaffolds the entire FastAPI application with a GroupMe webhook callback handler, health endpoints, pydantic-settings config, multi-stage Dockerfile, Woodpecker CI pipeline, and 8 passing tests. The callback validates group allowlist, ignores bot messages, logs user messages, and returns 200. No AI logic yet -- that is wired in ticket #6. ## Changes - `app/__init__.py` -- empty package init - `app/config.py` -- pydantic BaseSettings with all env vars (GroupMe, Anthropic, Keycloak, basketball-api) - `app/main.py` -- FastAPI app with lifespan, includes health and groupme routers - `app/groupme.py` -- POST /groupme/callback: parse body, validate group_id against allowlist, ignore bot sender_type, log sender_name + text - `app/health.py` -- GET /healthz (liveness, always 200), GET /health/ready (readiness, checks ANTHROPIC_API_KEY) - `Dockerfile` -- multi-stage python:3.12-slim, gunicorn+uvicorn workers, EXPOSE 8000 - `.woodpecker.yaml` -- test on push/PR, build+push to Harbor on main, kustomize tag update - `requirements.txt` -- fastapi, uvicorn, gunicorn, httpx, anthropic, pydantic-settings, pytest, pytest-asyncio - `tests/__init__.py` -- empty package init - `tests/test_groupme.py` -- 8 tests covering valid message, bot ignore, wrong group, malformed payload, invalid JSON, healthz, readiness with/without key ## Test Plan - `pytest tests/ -v` -- all 8 tests pass - Docker image builds with `docker build .` - POST /groupme/callback with valid payload returns 200 - Bot messages and wrong group_id silently return 200 - GET /healthz returns 200 - GET /health/ready returns ready/not_ready based on ANTHROPIC_API_KEY ## Review Checklist - [x] All 8 tests pass - [x] No unrelated changes - [x] Dockerfile follows basketball-api multi-stage pattern - [x] CI pipeline uses internal Harbor URL - [x] GroupMe callback never crashes on malformed input ## Related - Closes #4 ## Related Notes - `project-westside-ai-assistant` -- parent project - `arch-domain-westside-ai-assistant` -- A1: GroupMe Webhook Handler

forgejo_admin added 1 commit 2026-03-28 21:49:48 +00:00

feat: FastAPI scaffold with GroupMe webhook handler and CI pipeline ... c60dd64dd0

Implements the full application skeleton: GroupMe callback endpoint that
validates group allowlist, ignores bot messages, and logs user messages.
Includes health/readiness probes, pydantic-settings config, multi-stage
Dockerfile, Woodpecker CI pipeline, and 8 passing tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-03-28 21:50:55 +00:00

Author

Owner

Copy link

QA Review -- PR #10 (Parent Issue #4)

Spec Compliance

Checked every file target and acceptance criterion from issue #4:

Criterion	Status
POST /groupme/callback valid payload returns 200, logs message	PASS
Bot messages (sender_type=="bot") silently ignored	PASS
Non-allowlisted group_id silently ignored	PASS
GET /healthz returns 200	PASS
GET /health/ready returns 200 when config loaded	PASS
Docker image builds	PASS
All tests pass (8/8)	PASS
All 10 file targets created	PASS
No unrelated changes	PASS

Findings

Nit 1: Unused import os in tests/test_groupme.py (line 3)
import os is never used. Should be removed.

Nit 2: Readiness probe returns 200 even when not ready
GET /health/ready returns HTTP 200 with {"status": "not_ready"} when ANTHROPIC_API_KEY is missing. Kubernetes readiness probes typically expect a non-2xx status (e.g., 503) to mark the pod as not ready. The current implementation means k8s will consider the pod ready even when config is incomplete. Consider returning 503 for the not-ready case. This is a nit for this ticket since the issue spec says "check that config loaded" without specifying the HTTP status for the failure path -- but it will need to be addressed before k8s deployment.

Verification

• pytest tests/ -v -- 8/8 passed
• docker build . -- image built successfully
• Diff reviewed against all acceptance criteria and test expectations from issue #4

VERDICT: APPROVED with 2 nits

## QA Review -- PR #10 (Parent Issue #4) ### Spec Compliance Checked every file target and acceptance criterion from issue #4: | Criterion | Status | |-----------|--------| | `POST /groupme/callback` valid payload returns 200, logs message | PASS | | Bot messages (sender_type=="bot") silently ignored | PASS | | Non-allowlisted group_id silently ignored | PASS | | `GET /healthz` returns 200 | PASS | | `GET /health/ready` returns 200 when config loaded | PASS | | Docker image builds | PASS | | All tests pass (8/8) | PASS | | All 10 file targets created | PASS | | No unrelated changes | PASS | ### Findings **Nit 1: Unused `import os` in tests/test_groupme.py (line 3)** `import os` is never used. Should be removed. **Nit 2: Readiness probe returns 200 even when not ready** `GET /health/ready` returns HTTP 200 with `{"status": "not_ready"}` when ANTHROPIC_API_KEY is missing. Kubernetes readiness probes typically expect a non-2xx status (e.g., 503) to mark the pod as not ready. The current implementation means k8s will consider the pod ready even when config is incomplete. Consider returning 503 for the not-ready case. This is a nit for this ticket since the issue spec says "check that config loaded" without specifying the HTTP status for the failure path -- but it will need to be addressed before k8s deployment. ### Verification - `pytest tests/ -v` -- 8/8 passed - `docker build .` -- image built successfully - Diff reviewed against all acceptance criteria and test expectations from issue #4 VERDICT: APPROVED with 2 nits

forgejo_admin added 1 commit 2026-03-28 21:51:31 +00:00

fix: QA nits -- remove unused import, return 503 on readiness failure ... fcc65f0e9b

Readiness probe now returns HTTP 503 when ANTHROPIC_API_KEY is not set,
which is the correct signal for Kubernetes to mark the pod as not ready.
Also removed unused `import os` from test file.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-03-28 22:24:52 +00:00

Author

Owner

Copy link

PR #10 Review

DOMAIN REVIEW

Tech stack: Python 3.12 / FastAPI / Pydantic Settings / Gunicorn+Uvicorn / Docker multi-stage / Woodpecker CI

app/config.py -- Clean pydantic-settings usage. allowed_group_ids property correctly parses comma-separated env var with whitespace stripping and empty-string filtering. No hardcoded secrets; all sensitive values sourced from environment.

app/groupme.py -- Webhook handler correctly returns 200 on all code paths (GroupMe expects this -- non-200 causes retries and eventual callback deregistration). Manual request.json() parse with blanket except is appropriate here since we must never crash on malformed GroupMe payloads. Group allowlist validation and bot-message filtering are clean.

app/health.py -- Standard liveness/readiness probe pattern. Liveness always 200, readiness gates on ANTHROPIC_API_KEY presence. Correct use of JSONResponse with explicit 503 for not-ready state.

app/main.py -- Lifespan context manager, structured logging config, router includes. The settings import with noqa: F401 ensures config validation runs at startup (fails fast on bad env). Clean.

Dockerfile -- Multi-stage build correctly separates dependency install from runtime. Copies site-packages and bin from builder stage. Gunicorn with UvicornWorker class and 2 workers is a solid production default.

tests/test_groupme.py -- 8 tests covering: valid message, bot ignore, wrong group, empty payload, invalid JSON, liveness, readiness (with and without key). The importlib.reload pattern in the client fixture is necessary to pick up monkeypatched env vars since settings is a module-level singleton. Coverage is solid for a scaffold.

.woodpecker.yaml -- Clone uses internal Forgejo service URL. Test step installs deps and runs pytest. Build uses kaniko with Harbor secrets from Woodpecker secret store. Kustomize tag update downloads the shared script from pal-e-platform with retry logic. Pipeline structure follows established patterns.

BLOCKERS

None.

• Test coverage: 8 tests cover all endpoints and edge cases. Not zero-coverage.
• Input validation: Webhook handler gracefully handles malformed/missing JSON. No SQL, no HTML rendering, no path traversal surface.
• Secrets: All sensitive config via env vars, no hardcoded credentials.
• DRY/auth: No auth logic in this scaffold (deferred to tickets #5/#6).

NITS

1. Test/prod dependencies mixed (requirements.txt L7-8): pytest and pytest-asyncio are bundled with production dependencies. The Docker image ships with test frameworks installed. Consider splitting into requirements.txt (prod) and requirements-dev.txt (test) so the runtime image stays lean.

2. Permissive-by-default allowlist (app/groupme.py L33): When GROUPME_ALLOWED_GROUP_IDS is empty (the default), allowed_group_ids returns [], and the if allowed and group_id not in allowed check passes all groups through. This is likely intentional for dev convenience, but a code comment documenting "empty allowlist = accept all" would prevent future confusion.

3. basketball_api_url default port conflict (app/config.py L9): Defaults to http://localhost:8000, which is the same port gunicorn binds to. Not a runtime issue (will be overridden in k8s), but the default is misleading for local development.

4. Readiness probe scope (app/health.py L20): Only gates on ANTHROPIC_API_KEY. Other required config for the app to function (e.g., GROUPME_BOT_ID, GROUPME_ALLOWED_GROUP_IDS) is not checked. Acceptable for scaffold, but as more features land (ticket #6, #7), readiness should validate all critical config.

5. Module-level singleton reload fragility (tests/test_groupme.py): The importlib.reload chain (config -> groupme -> health -> main) is necessary but order-sensitive. If a future module imports settings at the top level, it must be added to the reload chain or tests will use stale config. A brief docstring on the client fixture explaining the reload order requirement would help future contributors.

SOP COMPLIANCE

• Branch named after issue: 4-fastapi-scaffold follows {issue-number}-{kebab-case-purpose}
• PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present
• Related references project notes: project-westside-ai-assistant, arch-domain-westside-ai-assistant
• Tests exist: 8 tests covering all endpoints
• No secrets committed: All config via env vars, secrets via Woodpecker from_secret
• No scope creep: All files directly serve the scaffold purpose described in issue #4

PROCESS OBSERVATIONS

• Clean scaffold with good separation of concerns (config, routes, health, main). Sets up a solid foundation for the AI engine (ticket #6) and basketball-api client (ticket #5) to plug into.
• The Woodpecker pipeline is production-ready from day one -- test, build, push, kustomize tag update. Good deployment frequency enablement.
• The importlib.reload test pattern works but will accumulate friction as the module graph grows. Worth considering a fixture that constructs Settings directly (bypassing the singleton) for future test files.

VERDICT: APPROVED

## PR #10 Review ### DOMAIN REVIEW **Tech stack**: Python 3.12 / FastAPI / Pydantic Settings / Gunicorn+Uvicorn / Docker multi-stage / Woodpecker CI **app/config.py** -- Clean pydantic-settings usage. `allowed_group_ids` property correctly parses comma-separated env var with whitespace stripping and empty-string filtering. No hardcoded secrets; all sensitive values sourced from environment. **app/groupme.py** -- Webhook handler correctly returns 200 on all code paths (GroupMe expects this -- non-200 causes retries and eventual callback deregistration). Manual `request.json()` parse with blanket `except` is appropriate here since we must never crash on malformed GroupMe payloads. Group allowlist validation and bot-message filtering are clean. **app/health.py** -- Standard liveness/readiness probe pattern. Liveness always 200, readiness gates on `ANTHROPIC_API_KEY` presence. Correct use of `JSONResponse` with explicit 503 for not-ready state. **app/main.py** -- Lifespan context manager, structured logging config, router includes. The `settings` import with `noqa: F401` ensures config validation runs at startup (fails fast on bad env). Clean. **Dockerfile** -- Multi-stage build correctly separates dependency install from runtime. Copies `site-packages` and `bin` from builder stage. Gunicorn with UvicornWorker class and 2 workers is a solid production default. **tests/test_groupme.py** -- 8 tests covering: valid message, bot ignore, wrong group, empty payload, invalid JSON, liveness, readiness (with and without key). The `importlib.reload` pattern in the `client` fixture is necessary to pick up monkeypatched env vars since `settings` is a module-level singleton. Coverage is solid for a scaffold. **.woodpecker.yaml** -- Clone uses internal Forgejo service URL. Test step installs deps and runs pytest. Build uses kaniko with Harbor secrets from Woodpecker secret store. Kustomize tag update downloads the shared script from pal-e-platform with retry logic. Pipeline structure follows established patterns. ### BLOCKERS None. - Test coverage: 8 tests cover all endpoints and edge cases. Not zero-coverage. - Input validation: Webhook handler gracefully handles malformed/missing JSON. No SQL, no HTML rendering, no path traversal surface. - Secrets: All sensitive config via env vars, no hardcoded credentials. - DRY/auth: No auth logic in this scaffold (deferred to tickets #5/#6). ### NITS 1. **Test/prod dependencies mixed** (`requirements.txt` L7-8): `pytest` and `pytest-asyncio` are bundled with production dependencies. The Docker image ships with test frameworks installed. Consider splitting into `requirements.txt` (prod) and `requirements-dev.txt` (test) so the runtime image stays lean. 2. **Permissive-by-default allowlist** (`app/groupme.py` L33): When `GROUPME_ALLOWED_GROUP_IDS` is empty (the default), `allowed_group_ids` returns `[]`, and the `if allowed and group_id not in allowed` check passes all groups through. This is likely intentional for dev convenience, but a code comment documenting "empty allowlist = accept all" would prevent future confusion. 3. **`basketball_api_url` default port conflict** (`app/config.py` L9): Defaults to `http://localhost:8000`, which is the same port gunicorn binds to. Not a runtime issue (will be overridden in k8s), but the default is misleading for local development. 4. **Readiness probe scope** (`app/health.py` L20): Only gates on `ANTHROPIC_API_KEY`. Other required config for the app to function (e.g., `GROUPME_BOT_ID`, `GROUPME_ALLOWED_GROUP_IDS`) is not checked. Acceptable for scaffold, but as more features land (ticket #6, #7), readiness should validate all critical config. 5. **Module-level singleton reload fragility** (`tests/test_groupme.py`): The `importlib.reload` chain (config -> groupme -> health -> main) is necessary but order-sensitive. If a future module imports `settings` at the top level, it must be added to the reload chain or tests will use stale config. A brief docstring on the `client` fixture explaining the reload order requirement would help future contributors. ### SOP COMPLIANCE - [x] Branch named after issue: `4-fastapi-scaffold` follows `{issue-number}-{kebab-case-purpose}` - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present - [x] Related references project notes: `project-westside-ai-assistant`, `arch-domain-westside-ai-assistant` - [x] Tests exist: 8 tests covering all endpoints - [x] No secrets committed: All config via env vars, secrets via Woodpecker `from_secret` - [x] No scope creep: All files directly serve the scaffold purpose described in issue #4 ### PROCESS OBSERVATIONS - Clean scaffold with good separation of concerns (config, routes, health, main). Sets up a solid foundation for the AI engine (ticket #6) and basketball-api client (ticket #5) to plug into. - The Woodpecker pipeline is production-ready from day one -- test, build, push, kustomize tag update. Good deployment frequency enablement. - The `importlib.reload` test pattern works but will accumulate friction as the module graph grows. Worth considering a fixture that constructs `Settings` directly (bypassing the singleton) for future test files. ### VERDICT: APPROVED

forgejo_admin merged commit dcacbcfea0 into main 2026-03-28 22:35:47 +00:00

forgejo_admin deleted branch 4-fastapi-scaffold 2026-03-28 22:35:47 +00:00

forgejo_admin referenced this pull request from a commit 2026-03-28 22:35:49 +00:00

feat: FastAPI scaffold + GroupMe webhook handler (#10)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo_admin/westside-ai-assistant!10

No description provided.