Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .ilana/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -501,3 +501,11 @@ Redis queue polling (200ms..2s), not blocking primitives (multi-condition wake-u
- Enforcement points: `admitSend` (daily UTC-day message count), `domainHandler.handleCreate` (non-deleted domains), `humanauth.InviteToOrganization` (early) + `AcceptOrgInvitationForExistingHuman`/`WithSignup` (authoritative, tenant row `FOR UPDATE`), `broadcastHandler.handleCreate`, `webhookHandler.handleCreate`, retention purge default.
- Paystack: `POST /v1/billing/checkout` (owner-only, body `{tenant_id, plan}`, Initialize Transaction in USD cents with metadata `{tenant_id, plan}`, returns `authorization_url`); `GET /v1/billing/subscription?tenant_id=` (any member); `POST /v1/billing/webhook` (public, x-paystack-signature HMAC-SHA512 verified, charge.success applies plan for 30 days, `billing_payments.reference` PK makes it idempotent). `plan-lapse` hourly component downgrades expired paid plans to free/lapsed; no auto-renewal (RSK-044).
- Config: `MAILX_PAYSTACK_SECRET_KEY` (enables billing + enforcement), `MAILX_PAYSTACK_PUBLIC_KEY` (frontend only, unused server-side), optional `MAILX_PAYSTACK_CALLBACK_URL`, optional `MAILX_PAYSTACK_BASE_URL` (tests/tooling).

## Dashboard backend (v0.47 phase 3a; design decisions DEC-228..230)

- Human-JWT routes (`humanAuthMiddleware`, registered only when humanAuth is configured; `internal/api/dashboard_handler.go`): `GET/PATCH /v1/me` (profile + orgs; name/avatar_url only), `GET /v1/orgs/{id}` (name, logo_url, plan/plan_status/plan_current_period_end, created_at; no slug column exists), `PATCH /v1/orgs/{id}` (owner), `GET /v1/orgs/{id}/members` (any member), `DELETE /v1/orgs/{id}/members/{humanId}` (owner; 204), `GET /v1/orgs/{id}/invites` (owner; pending only), `DELETE /v1/orgs/{id}/invites/{inviteId}` (owner; idempotent 204), `GET /v1/orgs/{id}/analytics/overview|timeseries` (any member; delegates to the API-key analytics handlers via `withTenant`).
- AuthZ: non-member -> 404 `organization_not_found`; member non-owner on owner-only route -> 403 `not_org_owner` (DEC-228).
- Invariant: an org always has >= 1 owner; removals are serialized by the tenant row lock (DEC-229). Owners cannot remove themselves (409 `cannot_remove_self`).
- DB: `internal/database/dashboard.go` (`UpdateHumanProfile`, `UpdateOrganization`, `ListTenantMembers`, `RemoveTenantMember`, `ListPendingOrgInvitations`, `RevokeOrgInvitation`). No migration.
- Limitations: no leave-org / transfer-ownership / role-change flow; no email change; no org slug; `/v1/orgs/*` dashboard routes are not in `TestOpenAPIRoutesMatchRuntime` (its mux has no humanAuth), covered by `dashboard_handler_test.go` instead.
3 changes: 3 additions & 0 deletions .ilana/decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,6 @@ Process decisions above (DEC-001..DEC-007) belong to the v0.23 FLEET run and sta
- DEC-225 [v0.47 phase 2 billing]: Paystack integration is Initialize Transaction + webhook only (no Paystack Subscriptions/plan codes). `POST /v1/billing/webhook` is public and authenticated solely by `x-paystack-signature` = hex HMAC-SHA512(secret key, raw body), verified with `hmac.Equal` before any parsing; failure is 401. A verified `charge.success` is applied only if status=success, currency=USD, amount >= plan price, metadata names a tenant and a paid plan; it sets plan/active/period_end = now+30d. Replays are neutralized by `billing_payments.reference` PRIMARY KEY inserted in the same transaction as the plan update (duplicate -> rollback, 200 `already_applied`). Unknown events and signed-but-unusable payloads get 200 (no Paystack retry); only our storage errors are 5xx. Checkout/subscription take the org as `tenant_id` (body/query) because a human may own several orgs; checkout is owner-only, subscription any member (non-member gets 404, no enumeration).
- DEC-226 [org invitations, CI caught what Greptile's trial limit ended before finding]: Greptile's free-trial credit limit was exhausted after PR #23 merged (no more automated review available going forward - manual review is now this project's primary line of defense, alongside CI). CI's own `-race` run on PR #24 caught a genuine, deeper bug in DEC-219/220's own fix: `TestConcurrentResendsNeverInvalidateBothLinks` failed intermittently with 2 survivors instead of 1, reproducing locally at roughly 50% under `go test -count=25`. Root cause: the `(created_at, id)` tuple comparison in `SupersedeOtherPendingOrgInvitations` assumes a smaller tuple means "already committed, therefore visible to a later query" - true only if inserts for the same address are serialized, which they were not. PostgreSQL's `now()` is fixed at a transaction's BEGIN, not its commit, so two genuinely concurrent autocommit INSERTs can commit in a DIFFERENT order than their `created_at` values suggest; a row with a small timestamp can still become visible to a later query AFTER that query's snapshot was already taken, meaning nothing ever supersedes it. Fixed properly this time with two changes together (verified each is independently necessary): (1) `CreateOrgInvitation` now holds `pg_advisory_xact_lock(hashtextextended(tenant_id||':'||normalized_email, 0))` for the duration of its own transaction, fully serializing concurrent inserts for the same address (the lock is scoped to a short transaction that commits well before the slower `SendSystemEmail` network call that follows - never held across it, preserving DEC-218's fix). (2) `created_at` is now set explicitly via `clock_timestamp()` in the INSERT rather than left to the column's `now()` default - critical, because a transaction that waited on the advisory lock would otherwise still capture an EARLIER timestamp (from its own BEGIN) than one that acquired the lock and committed first, silently reintroducing the exact ordering violation the lock exists to prevent. The advisory lock alone was proven insufficient by testing: re-ran the stress test 25x after adding only the lock and it still failed roughly half the time; only after also switching to `clock_timestamp()` did 40/40 stress runs (and the full `-race` suite) pass cleanly. This is the fifth fix-on-a-fix in this feature's review history (DEC-217→218→219→220→226), and the first one Greptile never got to review - a reminder that "manual review = read the diff and reason about it" is not equivalent to "manual review = actually stress-test the concurrency claim under `-count=N` before trusting it," especially now that automated review isn't available as a backstop. Verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean, `TestConcurrentResendsNeverInvalidateBothLinks` run 40x consecutively with `-race` with zero failures, Docker rebuild+boot smoke clean.
- DEC-227 [billing plans, CodeRabbit review pass]: Greptile's trial is exhausted (see DEC-226); CodeRabbit is now active on this repo and reviewed PR #24, catching 3 real findings, 2 of them data-integrity-critical. (1) **Data loss on billing enablement or plan lapse** (the most severe): with plan enforcement on, a NULL `tenants.retention_days` falls back to the tenant's PLAN window (Free = 7 days) instead of the flat `DefaultRetentionDays` (90). An operator turning on `MAILX_PAYSTACK_SECRET_KEY` for the first time on an existing deployment would silently shrink every pre-existing tenant's retention window, and the next hourly `retention-purge` run would irreversibly hard-delete any terminal message between 7 and 90 days old; the same happens to a Plus/Pro tenant the instant `plan-lapse` downgrades it (a renewal running even one hour late triggers it). Fixed two ways: migration 000031 now backfills `retention_days = 90` for every tenant that has no explicit value BEFORE the plan columns' semantics can apply (pins pre-existing tenants at today's effective window; a brand-new tenant created after billing is enabled has no messages yet, so its plan's window applying from day one is correct, not a regression) - and `DowngradeLapsedPlans` now pins `retention_days` to the lapsing plan's own window (`COALESCE(retention_days, CASE plan WHEN 'plus' THEN 30 WHEN 'pro' THEN 90 END)`) before clearing `plan`, so a lapse only ever changes billing state, never retention behavior. (2) **Renewal loses remaining paid days** (major): `ApplyPlanPayment` overwrote `plan_current_period_end` to `now + 30 days` regardless of any remaining time on the current period, so an owner renewing 5 days early paid for 30 days but only received 25. Fixed by changing the parameter from an absolute `periodEnd` to a `period time.Duration`, and extending from the LATER of `now()` and the existing `plan_current_period_end` when the tenant is renewing the SAME plan while still active; a plan CHANGE (upgrade/downgrade) or a renewal after the plan had already lapsed still starts a fresh period from now, since carrying over time priced under a different plan has no well-defined meaning. (3) **Signed-but-unusable webhook payloads answered 400 instead of 200** (minor): Paystack can send non-object `metadata` (e.g. `0` or `""`) for a transaction MailX's own checkout never created (a payment page or another integration on the same Paystack account); `Metadata`'s plain-struct JSON decoding failed on those, and `handleWebhook`'s contract requires unrecognized/unusable-but-validly-signed payloads to be acknowledged 200 (Paystack retries forever on anything else). Fixed with a custom `Metadata.UnmarshalJSON` that treats a decode failure as an empty (not erroring) `Metadata` - the handler already ignores an empty `TenantID`. All three fixes covered by new/updated tests (`TestParseEventToleratesNonObjectMetadata`; `TestApplyPlanPaymentAndReplay` extended with same-plan-extends and different-plan-fresh-period cases; `TestDowngradeLapsedPlans` extended to assert the pinned retention window). Verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean, migration 000031 round-trip re-validated with the new backfill statement, Docker rebuild+boot smoke clean.
- DEC-228 [v0.47 phase 3a dashboard backend]: one authorization posture for every `/v1/orgs/{id}/*` dashboard route, implemented once in `dashboardHandler.orgAccess` (`internal/api/dashboard_handler.go`): `IsTenantMember` first (non-member or nonexistent org -> 404 `organization_not_found`, no enumeration, same as `/v1/billing/subscription`), then `IsTenantOwner` for owner-only routes (member non-owner -> 403 `not_org_owner`). Owner-only: PATCH org, DELETE member, GET/DELETE invites (listing reveals non-members' emails). No new membership/ownership queries were written. PATCH bodies: omitted field = unchanged, empty URL = clear; URLs must be absolute http(s) <= 2048 chars (blocks `javascript:` etc.), names trimmed 1..200. Email change is out of scope (needs re-verification).
- DEC-229 [v0.47 phase 3a]: "every org keeps >= 1 owner" is enforced in `database.RemoveTenantMember` under the lockMemberCap pattern: `SELECT 1 FROM tenants WHERE id=$1 FOR UPDATE`, then under that lock re-check the ACTOR is still an owner, the target is a member, and the owner count. Self-removal is refused before the tx (`ErrCannotRemoveSelf`, 409; leave/transfer-ownership flow deferred). Because self-removal is blocked and the actor must be an owner, the only way to reach zero owners is two owners removing each other concurrently; the lock serializes them and the loser gets `ErrNotTenantOwner` (403). `ErrLastOwner` (409) stays as a defensive last line. Proven by `TestRemoveTenantMemberConcurrentMutualRemovalKeepsAnOwner` (20 rounds x 2 racers with a start barrier, asserts exactly 1 ok / 1 denied / 1 owner left); verified the test FAILS with `FOR UPDATE` removed. Invitation revoke reuses the supersede mechanism (`expires_at = now` on a pending row), idempotent for expired/accepted rows, 404 only if the ID does not belong to the org.
- DEC-230 [v0.47 phase 3a]: org analytics routes do not duplicate analytics code: `dashboardHandler.orgAnalytics` runs the membership check, attaches the org via `withTenant` (the single tenant-identity entry point) and calls the SAME `analyticsHandler.handleOverview`/`handleTimeseries` the API-key routes use, so validation, caps and response shape are identical. Org detail omits `slug`: `tenants` has no slug column (create-org already accepts and discards it); adding one needs a migration + uniqueness design, deferred rather than invented.
3 changes: 3 additions & 0 deletions .ilana/ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,3 +326,6 @@ Greptile's free trial hit its credit limit after PR #23, so PR #24 (billing) got

## 2026-09-25 | billing plans: CodeRabbit review pass (first reviewer since Greptile's trial ran out) | GATE PASS
Greptile's free-trial credit limit was hit right after PR #23 merged - it can no longer review this repo. CodeRabbit turned out to be configured on the repo too and reviewed PR #24 (billing) in Greptile's place, catching 3 real findings the earlier manual review had missed, 2 of them serious data-integrity bugs: enabling billing on an existing deployment (or a paid plan simply lapsing) could silently shrink a tenant's retention window from 90 days down to Free's 7, and the very next hourly retention-purge run would irreversibly hard-delete anything in that gap - fixed by backfilling retention_days for pre-existing tenants in the migration itself and by pinning the lapsing plan's window explicitly before a lapse clears the plan column, so billing state and retention behavior are now fully decoupled. Also fixed: a plan renewal was overwriting the remaining paid period instead of extending it (an owner renewing early lost days they'd already paid for), and a webhook whose metadata wasn't a JSON object (a real Paystack scenario for transactions MailX didn't create) was answered 400 instead of the required 200, which would make Paystack retry forever. This confirms the plan from the last session: without Greptile, review discipline has to come from us directly, and it's working - CodeRabbit filled the gap this time, but the org-invitation race from the previous entry proves CI's own -race run is just as important a backstop as any external reviewer. Verified: gofmt/go vet/go build clean, full go test ./... and go test -race ./... clean, migration round-trip re-validated, Docker rebuild+boot smoke clean. Ilana updated: decisions.md (DEC-227), state.json (DEC/RSK counters).

## 2026-09-25 | v0.47 phase 3a: dashboard backend proper | GATE PASS
Built the human-JWT dashboard surface: /v1/me (GET/PATCH), /v1/orgs/{id} (GET/PATCH), members list/remove, pending invites list/revoke, and org-scoped analytics that reuse the API-key analytics handlers via withTenant. Last-owner invariant enforced under the tenant row lock; the concurrent mutual-removal test was checked to fail with the lock removed. No migration; slug omitted (no column). Verified against real Postgres+Redis: gofmt/go vet/go build clean, full go test -race ./... clean, OpenAPI tests pass, Docker rebuild+boot smoke. Ilana updated: decisions.md (DEC-228..230), architecture.md, milestones.md, state.json.
5 changes: 5 additions & 0 deletions .ilana/milestones.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,8 @@ explicitly deferred to a later phase — v0.47 as a whole is NOT complete.
- Routes (only when `MAILX_PAYSTACK_SECRET_KEY` set): POST /v1/billing/checkout, GET /v1/billing/subscription, POST /v1/billing/webhook; `plan-lapse` hourly component.
- Enforcement wired into admitSend (daily volume), domain create, invite send + both accept transactions (member cap, row-locked), broadcast create, webhook create, retention purge default. All inert when billing is not configured (DEC-221..225).
- Tests: `internal/billing/billing_test.go`, `internal/database/plans_test.go` (incl. concurrent-accept race), `internal/api/billing_handler_test.go`, `internal/humanauth` member-cap test. Full `go test -race ./...` clean against real Postgres+Redis (0 skips in database/api/humanauth); migration down/up round-trip on a disposable DB; Docker rebuild+boot clean.

## v0.47 phase 3a — Dashboard backend proper (profile, org detail, members, invites, org analytics) — COMPLETE (scope per DEC-228..230)
- 10 human-JWT routes in `internal/api/dashboard_handler.go`, DB layer `internal/database/dashboard.go`, OpenAPI documented; no migration.
- Tests: `internal/database/dashboard_test.go` (rules, concurrent mutual-removal race, invite list/revoke idempotency, profile/org update), `internal/api/dashboard_handler_test.go` (every endpoint: happy path, non-member 404, non-owner 403, validation).
- Deferred: leave org / transfer ownership / role changes, email change, org slug column. Plan auto-renewal and OAuth/MFA are separate parallel work, not part of 3a.
8 changes: 4 additions & 4 deletions .ilana/state.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"DEF": 27,
"CR": 24,
"RSK": 45,
"DEC": 227,
"DEC": 230,
"MET": 72,
"ETH": 1
},
Expand All @@ -34,10 +34,10 @@
"G8": 1
},
"mailx": {
"last_completed_milestone": "v0.47 phase 2 (billing & plans MVP)",
"ilana_current_through": "v0.47 phase 2",
"last_completed_milestone": "v0.47 phase 3a (dashboard backend: profile/org/members/invites/org analytics)",
"ilana_current_through": "v0.47 phase 3a",
"current_milestone": "v0.47 Human Accounts & Organizations",
"current_milestone_status": "v0.47 phase 1 (human auth, orgs, password reset, invitations) and phase 2 (Free/Plus/Pro plans + Paystack one-off checkout, enforcement gated on MAILX_PAYSTACK_SECRET_KEY) complete; auto-renewal (RSK-044), OAuth, MFA deferred",
"current_milestone_status": "v0.47 phase 1 (human auth, orgs, password reset, invitations) and phase 2 (Free/Plus/Pro plans + Paystack one-off checkout, enforcement gated on MAILX_PAYSTACK_SECRET_KEY) complete; auto-renewal (RSK-044), OAuth, MFA deferred; phase 3a dashboard backend (DEC-228..230) complete; leave-org/transfer-ownership, email change, org slug deferred",
"next_milestone": "v0.47 phase 3 (plan auto-renewal, dashboard backend) or v0.48 (TBD)"
}
}
Loading
Loading