feat(segments): P2/4 — static list members: add, count, read

[manas-narra]

Stacked on #1345 (P1 — the segment table). Base: feat/segments-p1-tables.

Why

A static list (canon/05-audiences.md:3-4) IS its members: an imported set stored as narrow rows. P1 created the list; this PR gives it members — the thing a merchant's uploaded list becomes, and what a broadcast will freeze. Phase 2 of 4: members in P2; dynamic segments in P3 (rule format, check, create) and P4 (compiler, evaluate, count).

What

Migration 086 — crm_segment_member (canon T21), owned by outreach

Column	Notes
segment_id, customer_id	composite PK — the set itself, so the same customer can never be on a list twice
added_at, source	when, and from where ("import diwali-vips.csv", "api: ops@…")

A pure child row with no merchant_id (canon/05-audiences.md:49-51) — every read enters through the parent segment. Because the row has no merchant to pin, its trigger checks what a tenant-pinned FK would (law 9: invariants live in the tables):

1. members belong to static lists only — a member row under a dynamic segment would be silently ignored by every read;
2. the customer belongs to the segment's merchant — no row can put merchant B's customer on merchant A's list.

Adding members — outreach/segments.py::add_members

1. Up to MAX_MEMBERS_PER_ADD (1,000) people per add, each {phone | email | external_ref}.
2. The segment is checked before anyone is resolved, so a wrong id or a dynamic segment creates no customers.
3. Every person goes through resolve() with imported evidence (ADR 0021: may attach a handle, never displace one live traffic set) — people are never stored in arrays (canon/05-audiences.md:45-47). A person with no usable handle is a skipped line with its reason.
4. Duplicates within one upload are dropped (plan_members, pure).
5. One atom: lock the segment row FOR UPDATE (two adds to one list run in turn, and the lock re-checks it is still this merchant's static list) → one INSERT … SELECT unnest($2::uuid[]) ON CONFLICT DO NOTHING RETURNING → recount from the rows and stamp member_count + computed_at. The count is exact because the transaction that changed the rows stamps it.

Reading members — segment_members: evaluate(id) one chunk at a time, keyset-paged on customer_id (the PK carries it), entered through the parent under merchant_id = $1, next_after as the cursor.

Routes

Route	Answers
POST /segments/{id}/members {merchant_id, members: [...], source?}	200 the report · 404 unknown or another merchant's segment · 409 dynamic segment · 422 no people, more than 1,000, or an unknown key ("mobile") · 403 merchant not reachable
GET /segments/{id}/members?merchant_id=&after=&limit=	one page + next_after · 404 · 409

Example

curl -X POST /segments/8c1e…/members -d '{
  "merchant_id": "flipkart", "source": "import diwali.csv",
  "members": [{"phone": "+919000000001"}, {"phone": "+919000000002"}, {"phone": "+919000000001"},
              {"phone": "abc"}, {"phone": "+919000000003"}, {}]}'
# 200 — every person sent is accounted for exactly once
# {"received": 6, "added": 2, "already_member": 1, "duplicates_in_upload": 1,
#  "skipped": [{"line": 4, "reason": "no_usable_handle"}, {"line": 6, "reason": "no_usable_handle"}],
#  "member_count": 3, "computed_at": "2026-10-13T10:00:00Z"}

curl '/segments/8c1e…/members?merchant_id=flipkart&limit=200'
# {"items": [{"customer_id": "…", "added_at": "…", "source": "import diwali.csv"}, …], "next_after": "…"}

Not in this PR

• CSV upload — a console feature on this route: the console reads the file and sends its rows in batches of MAX_MEMBERS_PER_ADD (1,000), sequentially, with a progress bar, and combines the reports (batch offset + line = CSV line). Re-sending a batch is safe — people already added are reported as already members. Consent columns are not accepted (the route takes only phone / email / external_ref); CSV-with-consent becomes a backend import once the consent ledger exists (W10, canon/05-audiences.md:47).
• Dynamic segments — P3 (rule format, check, create) and P4 (compiler, evaluate, count).
• Merges after a member was added: the row keeps the customer resolved at add time; following a later merge is the broadcast freeze's job (W8).

Tests

tests/crm/test_segment_members.py (30): the owner and 086's shape and two trigger invariants · the builders (one INSERT with ON CONFLICT + RETURNING, the lock merchant-first FOR UPDATE, the recount from rows stamped now, the page entered through the parent and keyset-paged) · plan_members duplicate table · every person sent accounted for exactly once (added / already a member / duplicate / skipped with line + reason) · resolve() called with imported evidence · the same people twice adds nobody · an unknown or dynamic segment resolves nobody · empty and oversized adds refused before any work · the atom's re-check under the lock (gone, or turned dynamic) writes nothing · an all-skipped upload still stamps the count · the page cursor · the routes (200 report, source defaults to the caller, 422 / 404 / 409 / 403, paging). test_segments.py's route walk now expects 5 routes. Three mutation checks (drop the check before resolving; drop the re-check under the lock; count duplicates as zero) turn the suite red.

Gates

black / isort / autoflake / pyrefly (0 errors) / migration guard (85, no gaps) / CRM boundary guard: pass. tests/crm: 1611 passed, 15 skipped.

After review

• Rebased onto feat(segments): P1/4 — the segment table, create and read a static list #1345's review fix (the name CHECK now refuses an untrimmed name too); nothing in this PR changed. CodeRabbit: no actionable comments.
• CodeRabbit's docstring-coverage warning (pre-merge, all four segment PRs): not chased — the repo does not gate on it (feat(telephony): configurable outbound ring timeout for Plivo and Vobiz #1288 merged at 40%, fix(crm): write the token onto a dispatching square before executing it #1281 at 69.67% with the same warning), and it counts every test function and one-line helper; these files follow the surrounding comment density.

Summary by CodeRabbit

• New Features
  • Added support for adding people to static CRM segments, with a summary of added members, existing members, duplicate entries, and people who couldn’t be matched.
  • Added paginated member lists for static segments, including the option to continue from a cursor.
  • Additions record their source when provided. Dynamic segments can’t be modified or viewed through these member operations.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0e3c3a42-c7c5-415d-a8ec-65b7d63e302b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: bb572838-928e-4a00-bf1f-bacee07dadf5

📥 Commits

Reviewing files that changed from the base of the PR and between 07301fc and f622fe7.

📒 Files selected for processing (14)

• app/crm/outreach/api.py
• app/crm/outreach/db/accessors/segment.py
• app/crm/outreach/db/accessors/segment_member.py
• app/crm/outreach/db/decoders/segment_member.py
• app/crm/outreach/db/queries/segment.py
• app/crm/outreach/db/queries/segment_member.py
• app/crm/outreach/db/queries/tables.py
• app/crm/outreach/schemas.py
• app/crm/outreach/segments.py
• app/database/migrations/085_create_crm_segment_member.sql
• docs/crm/migrations.md
• scripts/check_crm_boundaries.py
• tests/crm/test_segment_members.py
• tests/crm/test_segments.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The change adds static CRM segment membership storage, routes for adding and listing members, per-person resolution and duplicate reporting, transactional member-count updates, and cursor-based pagination.

Changes

Static Segment Membership

Layer / File(s)	Summary
Membership storage contract app/database/migrations/085_create_crm_segment_member.sql, app/crm/outreach/db/queries/tables.py, docs/crm/migrations.md, scripts/check_crm_boundaries.py, tests/crm/test_segment_members.py	Creates crm_segment_member with a composite segment/customer key and a trigger that checks segment type and customer merchant ownership. Registers the table as outreach-owned and tests the migration and ownership.
Member addition and transaction app/crm/outreach/api.py, app/crm/outreach/schemas.py, app/crm/outreach/segments.py, app/crm/outreach/db/queries/segment.py, app/crm/outreach/db/queries/segment_member.py, app/crm/outreach/db/accessors/segment.py, app/crm/outreach/db/accessors/segment_member.py, tests/crm/test_segment_members.py, tests/crm/test_segments.py	The add route accepts bounded member input and reports added, existing, duplicate, and skipped entries. The service resolves people and writes members with an exact count update in a transaction that locks and rechecks the segment.
Paginated member reads app/crm/outreach/api.py, app/crm/outreach/schemas.py, app/crm/outreach/segments.py, app/crm/outreach/db/queries/segment_member.py, app/crm/outreach/db/accessors/segment_member.py, app/crm/outreach/db/decoders/segment_member.py, tests/crm/test_segment_members.py	The listing route returns ordered member pages for static segments. A full page includes the last customer ID as its next cursor.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant OutreachAPI
  participant AddMembers
  participant IdentityResolution
  participant DbTxn
  participant CRMDatabase
  OutreachAPI->>AddMembers: Submit segment ID, people, and source
  AddMembers->>IdentityResolution: Resolve each person's supplied fields
  AddMembers->>DbTxn: Lock and recheck segment
  DbTxn->>CRMDatabase: Insert unique member IDs
  DbTxn->>CRMDatabase: Recount and stamp member count

Loading

Suggested reviewers: rahul-2903-juspay

Merge Risk: ⚪ Minimal · up to f622f

Membership ownership checks and null-handle handling appear sound. No identified issue blocks merging after normal checks.

Security architecture risk: 🟡 Moderate · up to f622f

The new member-writing endpoint checks merchant access but not write permission, allowing read-only accounts to change audience membership and customer identities. Normal operations are tenant-scoped and membership updates are transactional, limiting exposure. The database ownership guard also needs stronger protection against later ownership changes.

Retained concerns

• High · security · observed: The new member POST accepts legitimately issued merchant-scoped user credentials without enforcing write permission. Token policy grants role=user only read and analytics permissions, while merchant_scope checks tenant reachability alone. Such an account can now persist audience members and invoke customer creation or merging. This extends a pre-existing authorization convention to new sensitive write outcomes; it does not require access to another tenant.
• Medium · security · inferred: The new database guard checks customer/segment merchant equality and static-list kind only when a member row is inserted or updated. The inspected schema permits subsequent changes to segment merchant, segment definition, or customer merchant without revalidating existing memberships. Parent-scoped reads can therefore retain invalid ownership relationships after privileged reference-row updates. Ordinary member API operations are protected; no end-user route capable of these reference-row changes was established.

Security review details

Security Blast Radius

• inferred — The attackable scope is the merchants carried by an accepted credential, not an arbitrary merchant selected in the request. A scoped read-only account can affect membership and identities throughout its assigned merchants; existing wildcard credentials can have broader reach. The 1,000-person bound limits one request, not cumulative changes. No cross-tenant credential bypass, infrastructure authority, or automatic sending was established.

Security Findings and Attack Paths

• observed — Account creation supports role=user with merchant assignments, and login issues those assignments in a token. That role has no write permission. Nevertheless, the new POST checks only merchant reachability before calling add_members. Attacker-selected handle combinations can consequently reach customer creation or multi-owner merging, followed by durable membership insertion.

Trust Boundaries and Controls

• observed — Request-selected merchant identity is checked against authenticated scope before service execution. Resolver probes constrain customers by merchant. Membership reads join through the merchant-owned parent, and the final write transaction rechecks parent ownership and static kind. These controls prevent the ordinary member route from selecting another tenant’s segment or resolving another tenant’s customers.

Resilience and Maintainability Implications

• observed — Identity resolution and membership do not share one fate. Interruption or failure after a successful resolution can leave customer creation or merges committed without corresponding membership. The final parent recheck contains membership corruption, but membership rollback does not undo identity effects. Segment deletion cascades member cleanup, while the customer foreign key restricts customer deletion.

Hardening Proposals

• proposed — Separate tenant reachability from mutation authority on the member POST. Require the appropriate write capability before any identity resolution, preserving merchant, reseller, and administrative scope semantics.
• proposed — Preserve membership ownership across the entire reference-row lifecycle: make tenant ownership and populated-list kind immutable, or enforce corresponding parent/customer update checks with suitable concurrency protection.
• proposed — Define imported multi-owner resolution explicitly: reject conflicting owners or require separate merge authority rather than treating list co-occurrence alone as trusted merge evidence. Document that failed additions can retain previously committed identity changes.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.87% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 62 functions across 12 files. (2 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly identifies the phase and the main changes: adding, counting, and reading static segment members.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.87% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 62 functions across 12 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit sorts each name in rows,
Then hops through lists where paging flows.
New members join with counts in place,
Duplicate names keep their trace.
The burrow stores each segment’s case.}

_{Comment @coderabbitai help to get the list of available commands.}