Conversation
There was a problem hiding this comment.
Pull request overview
This PR completes the backend foundation for hardened authentication flows and a production PowerSync sync contract limited to CRUD on books, including server-enforced ownership and timestamp control, plus local bootstrap and documentation updates.
Changes:
- Adds a
booksdomain table/model + PowerSync stream, and introduces a books-only PowerSync upload endpoint backed by a dedicated service layer. - Hardens auth flows (rate limiting, OAuth redirect allowlisting, password reset link behavior) and expands regression coverage.
- Improves local developer ergonomics (bootstrap script, Docker health checks) and updates docs accordingly.
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_models.py | Asserts new books table/model is registered in SQLAlchemy metadata. |
| tests/test_env_files.py | Adds a drift check for local .env keys vs .env.example (skips if .env missing). |
| tests/services/test_sync.py | Adds service-level tests for books-only PowerSync upload behavior. |
| tests/services/test_auth.py | Adds a regression test for allowed app public base redirect behavior in Google OAuth URL building. |
| tests/core/test_security.py | Adds coverage for PowerSync JWKS key rotation and blank inline key handling. |
| tests/conftest.py | Sets base URLs for tests and adds an autouse rate limiter reset fixture. |
| tests/api/routes/test_sync.py | Replaces legacy sync endpoint tests with PowerSync upload contract tests. |
| tests/api/routes/test_health.py | Updates index page expectations to return absolute public URLs. |
| tests/api/routes/test_auth.py | Adds login rate-limit test and expands OAuth + password reset behavior tests. |
| scripts/setup_local_powersync.sh | Expands publication and grants to include books. |
| scripts/bootstrap_local.sh | Adds an idempotent local bootstrap script for keys, Docker services, migrations, and PowerSync setup. |
| README.md | Updates project overview and local setup to use the new bootstrap flow and focused guides. |
| powersync/sync-config.yaml | Adds a books stream filtered by auth.user_id() for per-user isolation. |
| papyrus/services/sync.py | Introduces the books-only PowerSync upload apply/validation service. |
| papyrus/services/auth/google.py | Adds redirect URI allowlisting logic and enforces it for login/link flows. |
| papyrus/services/auth/email_flows.py | Requires APP_PUBLIC_BASE_URL for password reset emails (to avoid linkless messages). |
| papyrus/services/auth/_core.py | Adds app URL builder and updates password reset email body to use app link. |
| papyrus/schemas/sync.py | Replaces legacy sync schemas with PowerSync upload request/response and mutation schema. |
| papyrus/models/sync.py | Adds SyncBook SQLAlchemy model for books. |
| papyrus/models/init.py | Exports SyncBook for metadata/Alembic visibility. |
| papyrus/main.py | Centralizes limiter import and wires it into app state/exception handling. |
| papyrus/core/security.py | Adds “previous” PowerSync public key support and JWKS multi-key output for rotation. |
| papyrus/core/rate_limit.py | Introduces a shared SlowAPI limiter singleton. |
| papyrus/config.py | Adds OAuth allowlist settings, APP_PUBLIC_BASE_URL, and “previous key” PowerSync settings + normalization. |
| papyrus/api/routes/sync.py | Replaces legacy sync routes with POST /v1/sync/powersync-upload. |
| papyrus/api/routes/auth.py | Adds auth endpoint rate limiting and adjusts handler signatures to accept Request + payload separately. |
| docs/powersync-sandbox.md | Updates sandbox runbook to use the bootstrap flow and clarifies validation steps. |
| docs/flutter-auth-integration.md | Refactors/condenses guidance covering auth + PowerSync flows for Flutter. |
| docs/auth-testing.md | Updates local auth testing runbook, including app base URL and OAuth redirect allowlists. |
| Dockerfile | Copies README during build stage (likely for packaging/metadata). |
| docker-compose.yml | Pins PowerSync image version and adds health checks for server and powersync. |
| alembic/versions/a1d7c2f4e8b9_add_powersync_domain_tables.py | Adds migration creating the books table + index. |
| .gitignore | Ignores .codex and .agents directories. |
| .github/workflows/ci.yml | Adds a push-branch filter for CI workflow triggers. |
| .env.example | Adds app base URL + OAuth allowlist env vars and PowerSync “previous key” settings; adjusts local CORS example. |
| .codex/config.toml | Removes Codex configuration file. |
| .codex/agents/backend-reviewer.toml | Removes Codex agent configuration file. |
| .agents/skills/postgres-sqlalchemy-debugging/SKILL.md | Removes local skill doc (deprecated by PR cleanup). |
| .agents/skills/fastapi-endpoints/SKILL.md | Removes local skill doc (deprecated by PR cleanup). |
| .agents/skills/backend-review/SKILL.md | Removes local skill doc (deprecated by PR cleanup). |
| .agents/skills/alembic-migrations/SKILL.md | Removes local skill doc (deprecated by PR cleanup). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
4
to
+5
| push: | ||
| branches: [master] |
Comment on lines
+102
to
+110
| def _optional_bool(payload: dict[str, object], key: str, default: bool = False) -> bool: | ||
| if key not in payload: | ||
| return default | ||
| value = payload[key] | ||
| if isinstance(value, bool): | ||
| return value | ||
| if isinstance(value, str): | ||
| return value.strip().lower() in {"1", "true", "yes", "on"} | ||
| return bool(value) |
Comment on lines
+49
to
+58
| user = await _create_user(session, "sync@example.com") | ||
| book_id = str(uuid4()) | ||
| applied_count = await sync_service.apply_powersync_upload_batch( | ||
| session, | ||
| user.user_id, | ||
| [ | ||
| PowerSyncCrudMutation( | ||
| type="books", | ||
| op="PUT", | ||
| id=book_id, |
Comment on lines
+262
to
+265
| @pytest.fixture(autouse=True) | ||
| def reset_rate_limiter() -> None: | ||
| """Keep per-IP rate-limit state isolated between tests.""" | ||
| limiter._storage.reset() |
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
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
Validation
.venv/bin/pytest -q— 179 passed, 2 skipped.venv/bin/ruff check papyrus tests alembic— passed.venv/bin/mypy papyrus— passed.venv/bin/pytest tests/services/test_auth.py tests/api/routes/test_auth.py -q— 44 passed