Repository navigation
P2: Complete organization lifecycle and PostgreSQL release acceptance #253
Description
Current Priority Decision
Owner direction, 2026-09-13: complete Church/Basketball business workflows and day-to-day operations for every role first; billing and platform readiness later.
Execution lane: phase: platform-later. Current business milestone: #289.
This section overrides older priority, start conditions, broad dependency order and launch estimates below. Keep the detailed technical recommendations where compatible. Execute only the NOW slice; do not expand a mixed issue into its whole production scope. Record completed slice receipts in #289; keep this issue open while retained later work remains.
NOW
No new destructive organization-lifecycle project. Preserve the already-implemented org/admin/audit guards in all current work; #254/#255 test access needed for onboarding and ordinary use.
LATER
Complete PostgreSQL cascade/delete/restore/retention acceptance before real production lifecycle operations. A newly discovered reachable authorization regression is immediately current, not deferred by this label.
Validation stays local. No CI checks, hosted reviewers or Ollama code review. Preserve tenant isolation, real member responses and atomic roster changes. A deferred feature is not permission to expose an unfixed vulnerability. No deployment, paid-provider activation or production sign-off is authorized by this reprioritization.
Before implementation, read #252 and #289; finish one role/work package with tests, local review and affected docs/assets. The earlier completion receipt below covers whole-issue closure, not a requirement to finish every deferred package before the business milestone.
Current Implementation Handoff
Prepared 2026-09-13 for a lower-cost builder at source 21a4a804aa57580451edded04736b1f51aef7e48.
No CI checks. All implementation validation and code review run locally.
Use the shared builder contract and this issue's work packages; no xhigh model or automatic model upgrade is required. This is a detailed recommendation, not a claim that a smaller model cannot make mistakes or that tests have passed.
Risk/review focus: High: tenant deletion and transactional audit.
Start condition: DEFERRED. Do not start implementation from this issue's older prompt or package list until the owner resumes this track after #289, except a confirmed still-reachable security regression.
Source of Truth and Current State
api/routers/organizations.pyapi/models.pyapi/utils/audit_logger.pyalembic/versions/tests/api/test_organization_authorization.pytests/api/test_org_lifecycle.pytests/api/test_org_soft_delete.py
_get_scoped_organization and _commit_organization_change already implement tenant/admin guards and commit=False audit recording in the mutation transaction. Preserve them. Remaining work is complete regression coverage and PostgreSQL cascade/restore evidence, not replacing already-fixed guards.
This handoff supersedes stale implementation statements in the background below. Preserve existing successful behavior and tests. Recheck the current branch before editing; the baseline is a source pointer, not permission to discard newer changes.
Dependencies and Ownership
- Current execution prerequisite: owner reactivates this deferred track after P1: Accept complete Church and Basketball day-to-day business flows #289. Original provider/platform dependencies are retained as future acceptance, not blockers for the current business flow.
- Coordinate, but do not wait for entire tickets to close merely to inspect/write isolated tests: P0: Prevent privileged signup and enforce organization membership #255, P2: Prove production recovery and retention after business acceptance #268, P2: Reconcile platform assurances with later release evidence #284.
- The final commands below require P1: Repair local test entry points, coverage and isolation #281's safe local environment or an equivalently verified isolated setup. A missing dependency blocks only the slice that needs it; document that state.
- Shared source/ORM/migration changes are serialized per the master roadmap. Do not start concurrent edits to the same ownership area.
Recommended Decisions
- Keep the current public empty-organization exception until P0: Prevent privileged signup and enforce organization membership #255 changes bootstrap deliberately. Never broaden list visibility beyond the caller's tenant.
- Use the existing audit helper with commit=False; commit exactly once at the lifecycle boundary and rollback both state and audit on failure.
- Review every Organization relationship before changing cascade behavior. Do not cascade-delete the audit trail merely to make a foreign-key error disappear; distinguish the live audit storage policy from entity cleanup.
Small Work Packages
Each item is one reviewable slice, not permission for one giant PR. Add the failing regression first; finish code, tests and affected docs for that slice together. Leave this issue open until all packages and original acceptance criteria are satisfied or explicitly revised by the owner.
- 253.1: Record an operation/actor matrix for read/list/update/cancel/restore/delete and compare it with existing API tests; add missing denial and zero-write cases first.
- 253.2: Run the same lifecycle fixtures through P1: Prove local database correctness; defer production parity #260's migrated PostgreSQL database; identify the first actual FK/audit failure and fix only the responsible relationship/migration.
- 253.3: Add cancel -> default-list hidden -> include-cancelled visible -> restore transitions, then a synthetic hard-delete/drill test.
- 253.4: Reconcile remaining state in this issue and P2: Reconcile platform assurances with later release evidence #284, retaining implemented guards and documenting retention decisions from P2: Prove production recovery and retention after business acceptance #268.
Required Regression Cases
These are specifications for tests to add/retain, not claimed execution results. Each new negative case must assert unchanged unauthorized state and zero forbidden side effects.
- T253-01: Foreign admin update/cancel/restore/delete -> 403 with unchanged tenant data and no successful-mutation audit; volunteer mutations -> 403.
- T253-02: Own admin cancel -> one committed mutation/audit, omitted from default list; restore -> flags cleared and visible again.
- T253-03: Injected audit flush/commit failure -> lifecycle state unchanged after rollback and no orphan audit transaction.
- T253-04: Migrated PostgreSQL hard delete -> defined child cleanup and retained/redacted audit evidence, without touching another tenant.
Local Commands and Evidence
Existing targeted commands (paths checked against the audit source; run only after the stated safe preflight):
poetry run pytest tests/api/test_organization_authorization.py tests/api/test_org_lifecycle.py tests/api/test_org_soft_delete.py -q
poetry run pytest tests/integration/test_organizations.py tests/integration/test_audit.py -qAlso run the shared formatting/lint/touched-type/unit/full-suite and local review protocol from #252 for the final pushed revision. Add new targeted tests to these commands when implemented. Run API and browser tiers in separate processes. Native, PostgreSQL, image, provider and operator drills require their explicit environment; an unavailable tool/target is blocked/not run, never a pass.
Schema and Compatibility
Only add an Alembic migration if an observed FK/cascade change requires one. Test upgrade with existing data; do not rewrite the initial migration.
Stop Conditions
Audit retention or tenant purge requirements are unresolved under #268: stop that destructive slice, not the already-safe regression work. No deletion of real organizations is authorized.
After two failed focused repair attempts without new diagnostic evidence, stop the affected package and post the exact failure, commands, suspected boundary and needed decision. Do not silently broaden scope, weaken tests or upgrade models. A fresh local reviewer checks: Review relationship cascades, audit commit ordering and foreign-tenant negative tests against PostgreSQL, not only SQLite.
Completion Receipt
- Work-package and regression IDs above map to changed files and actual results.
- Commands, versions, dates, pass/fail/skip/not-run counts, logs/screenshots and tested head/base SHAs are linked.
- A separate local review records findings and resolution; self-review is labeled if used and is not misrepresented as independent review.
- Affected docs/README/playbooks/screenshots and dependency/roadmap status are reconciled, not left as unnamed follow-ups.
- If implementation is authorized through PR/merge, GitHub reports mergeable and the shared local-evidence requirements are met; reviewer agents never merge.
- No hosted CI check, status attestation, Ollama reviewer, live provider action, deployment, real-data purge or store submission was introduced by implication.
Copyable Builder Prompt
First read this issue's Current Priority Decision and #289. Run only its NOW slice.
If this issue is deferred, report that state instead of starting the older package list.
Implement the next ready work package in tomqwu/SignUpFlow issue #253.
Read its Current Implementation Handoff and #252 Builder Handoff Contract first.
Inspect current source and preserve newer/unrelated changes. Start with the
package's failing regression, then complete code, local tests, local review and
affected docs/assets together. Do not skip acceptance or invent passing evidence.
No CI checks or Ollama code review. Do not deploy, activate providers, purge real
data or submit to stores. Stop and report unmet prerequisites or policy decisions.
Record the package/test IDs and exact reviewed/tested source SHAs before claiming done.
Earlier Audit and Acceptance Context
Current policy (2026-09-13)
No CI checks. Everything is validated locally. This includes code review,
formatting, lint, type checks, migrations, all test tiers, security scans,
artifact checks and mobile validation. Do not add hosted jobs, required CI
statuses, synthetic success checks or an Ollama reviewer. GitHub is for source,
PRs, issues and publication, not validation.
Record commands, environment, results, limitations and reviewed head/base SHAs.
Builders merge only with completed local evidence and GitHub mergeability;
reviewer agents never merge. Real staging/provider/device acceptance remains
required where applicable, driven by authorized local operator tools.
Historical evidence and older comments do not override this policy.
Progress reconciliation (2026-09-13)
Organization membership/admin guards and transactional audit changes are merged in PR #272. Revalidate regressions locally and complete PostgreSQL cascade/release evidence. Older draft/unmerged comments are stale; do not redo implemented guards.
Parent roadmap: #252
Priority: P0, blocks core production pilot. Phase: A. Suggested owner: Backend/security. Original estimate (superseded; re-estimate remaining work): 2-3 engineering days.
Historical audit evidence (recheck against current source)
api/routers/organizations.py:59 exposes organization list/get/update/delete without authenticated dependencies. The router is mounted in api/main.py. A disposable in-memory TestClient request changed a synthetic organization without credentials (200). Delete lacks the same guard by source inspection; no live organization deletion was attempted.
Baseline: GitHub main 214e3f3f17a582d5f9b2063be6872ea2b1d25714, audited 2026-09-09. Findings concern synthetic reproduction or source inspection, not a claim of live exploitation.
Implementation plan
- Write failing real-JWT tests for anonymous callers, volunteers, another tenant's admin, and the owning admin across read/update/delete/cancel/restore.
- Require get_current_user/get_current_admin_user and verify_org_member; constrain Organization.id to the authenticated tenant before lookup. Decide whether any minimal organization discovery is deliberately public; do not expose config as onboarding metadata.
- Keep initial organization creation as a narrowly defined onboarding operation, coordinated with the signup issue. Separate destructive deletion from cancellation and validate ownership before any cascade.
- Update internal web handler calls and test seed helpers to use explicit service boundaries or authenticated requests, so fixes cannot be bypassed by another entry point.
Acceptance criteria
- Unauthorized access returns 401/403; cross-tenant IDs never reveal or modify another organization, including config and cancellation state.
- Owning admins retain the documented lifecycle; volunteers cannot change or delete organization settings.
- Deletion/cancellation regression tests use synthetic data and verify related rows, rollback on failure, and audit records.
- API schema and route policy describe every intentionally public exception.
Dependencies
None; start immediately.
Validation
Run the new real-JWT tests in tests/api, affected tests/web, make test-unit, and make test-all. Attach the anonymous/other-tenant denial matrix to the PR. Include PostgreSQL cascade tests before release.
Whole-repository audit scope (2026-09-13)
Baseline: 21a4a804aa57580451edded04736b1f51aef7e48. This addendum assigns full-scope follow-through; it is not a new test pass or production sign-off. No CI checks; all review and validation runs locally.
Preserve the authorization/audit fixes already merged in #272. Remaining acceptance is PostgreSQL deletion/cascade/restore behavior and tenant lifecycle visibility, including canceled organizations and audit failures. Attach engine-specific negative tests under #260 and recovery evidence under #268; do not redo completed guards or close solely on SQLite tests. Align membership policy with #255 and security claims with #284.
Keep evidence and disposition synchronized with master roadmap #252 and documentation ledger #277. Close only after the remaining acceptance criteria have linked local results; a planning/audit note is not completion.
Activity
Require tenant authorization for organization lifecycle
tomqwu commentedon Sep 9, 2026
Implementation pushed in draft PR #272, commit 0bd9cf7.
Validation: 56 focused authorization/search/cancellation/settings tests passed; 16 organization HTTP integration tests passed. Full local suite passed: 351 unit (21 skipped), 442 API, 16 CLI, 325 integration; web/contract 221 passed. Black, Ruff, and strict mypy passed. GitHub CI on c736236 is pending.
Keep this issue open pending independent review/merge and PostgreSQL cascade tests before release. Signup/membership hardening remains #255; no GitHub settings changes or merge performed.
tomqwu commentedon Sep 9, 2026
Validation update for draft PR #272, head c736236: GitHub CI passed, including backend suites, web/contract tests, Playwright, strict mypy, and the PostgreSQL Alembic migration check. Full mypy api remains advisory with existing typing debt.
Codex review was skipped because OPENAI_API_KEY is missing; this is not review approval. The PR remains draft and unmerged. Previously documented release follow-ups remain open, including PostgreSQL runtime/cascade coverage and staging delivery verification where applicable. No protection/ruleset/settings changes were made.
Harden tenant authorization and repair password-reset delivery (#272)
[-]P0: Require tenant admin authorization for organization lifecycle[/-][+]P2: Complete organization lifecycle and PostgreSQL release acceptance[/+]tomqwu commentedon Sep 15, 2026
Completion is implemented in PR #328. Package/test mapping and SHA-bound local evidence are recorded at #328 (comment). This closes the issue’s executable local acceptance: actor matrix, rollback, PostgreSQL cleanup, retained audit, foreign-tenant survival, and current docs. Production retention/purge policy and any real operation remain under #268 and are not implied by this closure.
Prove PostgreSQL organization cleanup