RFC Review: Direct Send API — Proactive WhatsApp Utility Messaging Without Templates

Executive Summary

• Overall Score: 8.0/10
• Rating: Strong
• RFC Type: backend
• Sub-Type: new-feature
• Assessment Confidence: Medium
• Applied Caps/Gates: none
• Implementation Readiness Verdict: PROCEED with notes — fix advisory lock bug (DIC), migration inconsistencies (DMS), and specify recovery job before final merge
• Report Path: direct-send-api-v2.1-review.md
• RFC Author: edi.prakoso@mekari.com | Reviewed: 2026-05-29

This RFC is one of the best-structured in this codebase: 14 numbered ADRs with options tables and rejection rationale, field-level API schemas with real JSON examples, 6 mermaid sequence diagrams, chunk-ordered execution plan with assertable acceptance criteria, and 14 pinpointed code anchors. An agent can implement most of this correctly on a first pass. Two implementation-correctness gaps block full confidence: (1) the pg_advisory_xact_lock call is placed outside ApplicationRecord.transaction, which means the lock is released immediately and the TOCTOU race it claims to prevent is not actually prevented; (2) the DDL spec in § 4 and the migration code in § 7 disagree on column types and constraints for source (VARCHAR 50 vs limit: 255) and header/footer (VARCHAR 60 vs unspecified :string). Both are silent spec violations — an agent will generate incorrect code by following the spec literally. A third gap — the orphaned-message recovery job — is described in ADR-05 but has no implementation chunk, no schedule spec, and no acceptance criteria. Fix these three and the RFC is ready for agentic execution.

---

Quick Verdict

Why this RFC can be implemented agentically:

• 14 ADRs with alternatives tables, rejection rationale, and code snippets — no major architectural choice is left open
• API contracts specify request/response schemas field-by-field with nullability, example payloads, and cursor format differences documented
• Pattern alignment is exceptional: 14 specific file anchors with reading order and source-verification table

Why this RFC will cause agent guessing or rework:

• The pg_advisory_xact_lock TOCTOU guard (ADR-10 and AgentSendsMessage#result) is structurally broken — the agent will faithfully reproduce a race condition
• The DDL spec (§ 4.1, § 4.2) and migration code (§ 7 Chunk 1) disagree on four column specifications; an agent cannot know which to follow
• The orphaned-message recovery job (ADR-05 / OQ-13) has no implementation chunk — an agent will skip it entirely or invent a schedule

---

PRD → RFC Traceability Matrix

The PRD is behind an authenticated Confluence URL (https://jurnal.atlassian.net/wiki/spaces/QON/pages/51065552898) and was not accessible for direct inspection. The RFC's own gate checklist (§ 10) asserts "All PRD sections extracted and covered (or marked n/a) ✅" and references user stories US-07 and US-08 by name. Scoring is on self-contained justification evidence only; the external PRD link is not treated as evidence.

Element	Present?	Quality	Assessment
Problem statement	yes	specific — "proactive outbound WA utility messages without pre-approved templates"	agent knows what's being built
Success criteria	partial	qualitative: "admin view shows auto-generated templates", "agent can send without template" — no quantitative metrics in the RFC body	agent knows the features but not the quality bar
Scope boundaries	yes	explicit Non-Goals section (§ 1.4): frontend out of scope, scheduling deferred, UU PDP full analysis deferred	agent won't over-build
Non-goals	yes	concrete: no balance refund, no language detection, no scheduled send, no frontend	agent won't scope-creep
US-07 (language detection)	partial — deferred	explicitly deferred to GA; no implementation	agent knows to skip
US-08 (admin template view)	full	ADR-02, ADR-09, § 5.1 extension, § 7 Chunk 2	agent knows which files to edit
Multi-contact send	full	ADR-12, § 5.2, § 7 Chunk 3	full
Account restriction / compliance webhook	full	ADR-07, ADR-08, § 5.3, § 7 Chunk 4	full
Template sync	full	ADR-11, § 5.3	full
Balance guard	full	ADR-13, § 7 Chunk 2	full

PRT score note: No PRD directly accessible. Scored on self-contained justification and user story references. Max possible: 8.0.

Summary: All in-scope user stories appear covered. US-07 explicitly deferred. No RFC decisions appear to lack a PRD driver. Confidence is medium because the PRD itself could not be read.

---

Scorecard

Backend Scorecard

Category	Score	Evidence-Based Rationale
PRT — PRD Traceability	6.5	PRD linked but not accessible; RFC references US-07/US-08 by name; no bidirectional matrix; § 10 gate checklist self-reports coverage
TDC — Technical Decisions	9.0	14 ADRs with options tables, rejection rationale, and consequences; all OQs resolved; one minor gap: auto-assign bypass (OQ-05/06) is resolved in prose only, no ADR
DMS — Data Model & Schema	7.0	DDL-level precision in § 4 but § 7 migration code diverges: source column is VARCHAR(50) with CHECK in § 4.1 but limit: 255 without CHECK in Migration 1; header/footer are VARCHAR(60) in § 4.2 but :string (no limit) in Migration 2
ACV — API Contract & Versioning	8.5	Field-by-field schemas with nullability, real example payloads, cursor format differences documented; gap: rate limit values never quantified beyond "429"; ttl_seconds validation not in Grape values: constraint
DIC — Data Integrity & Consistency	5.5	Transaction boundaries clear; BUT pg_advisory_xact_lock called outside ApplicationRecord.transaction — lock released immediately, TOCTOU race NOT prevented; recovery job described but not specified or chunked
FMC — Failure Mode & Retry Coverage	7.5	Sneakers retry policy specified (3×, 500ms/1500ms/4500ms), DLQ named with depth alert; Meta timeout (30s); Contact Service fail-open; poison-message handling; gap: no circuit breaker spec for Meta API; recovery job failure behavior not specified
CSS — Concurrency & Scaling	6.0	Idempotency key and TOCTOU concern acknowledged; rate limit key in Redis mentioned; gap: no specific rate limit numbers per org, no Sneakers worker count, no max contact_ids per org rationale quantified
SAS — Security & Authorization	7.0	OAuth2 scopes per endpoint, 403 feature flag guard, params[:organization_id] = me.organization_id tenancy isolation, field length limits, enum validation; gap: no threat model, ttl_seconds range not enforced in Grape values: block, no injection surface analysis
MRP — Migration & Rollout Plan	7.5	6-step rollback recipe with exact SQL, feature flag kill-switch, additive migrations; breaking change deploy order in ADR-12; gap: no rollout stages (1%/10%/100%), no go/no-go evidence gates
OBS — Observability Definition	7.0	5 named metrics with tags in § 8; gap: no SLO targets, no alert thresholds, no dashboard location, no structured log field spec
SBC — Service Boundary & Coupling	8.0	hub-service vs hub_core split clear; existing Sneakers publisher/subscriber chain reused; no new sync dependencies; Contact Service fail-open documented; gap: recovery job queue ownership undefined
CPA — Pattern Alignment	9.0	14 pinpointed file anchors with reading order (§ 2); AbstractIteractor typo documented; frozen_string_literal: true, interact_with, stub_auth_deprecation, params[:organization_id] = me.organization_id all explicitly anchored
CDG — Compliance & Data Governance	6.0	Triggered: PII in messages.body/header/footer, cross-border transfer to Meta US. Minimum Beta spec documented (§ 9 CDG): PII field table, DPA requirement, right-to-delete path, audit log fields. Gaps: no encryption-at-rest spec for new fields, no audit log storage destination, full analysis explicitly deferred to GA

Resource & Cost Advisory

This RFC adds 3 new ES queries per contact search (1 ES query per GET /contacts call), 1 DB write per contact per POST /messages call (up to 3 contacts), and inline Meta Template API calls on status webhooks when template_id not in DB. The inline sync in waba.rb adds unbounded HTTP latency to the webhook acknowledgment path — high-volume WABA accounts processing many status webhooks will see waba.rb response time increase proportionally to new Direct Send message volume. No Redis memory estimate provided for rate-limit keys.

---

Decision Closure Assessment

Decision Index

#	Decision	Status	Critical Gaps
1	pg_advisory_xact_lock TOCTOU guard (ADR-10)	Partial	Lock acquired outside ApplicationRecord.transaction; released immediately; race condition not prevented
2	Migration column specs vs migration code (§ 4 vs § 7)	Partial	source VARCHAR(50)/CHECK vs limit:255/no-CHECK; header/footer VARCHAR(60) vs :string
3	Orphaned-message recovery job (ADR-05 / OQ-13)	Partial	No schedule, no DLQ, no acceptance criteria, no chunk
4	STI room identification via Models::DirectSendRoom (ADR-03)	Resolved	—
5	Async send via existing Sneakers path (ADR-05)	Resolved	—
6	Balance deduction via webhook-driven WaDeductionWorker (ADR-13)	Resolved	—
7	Direct Send payload builder WaCloud::Builders::DirectSendMessage (ADR-14)	Resolved	—
8	Multi-contact array send (ADR-12)	Resolved	—
9	Feature flag direct_send_enabled via Services::Preference (ADR-06)	Resolved	—
10	Template sync webhook-driven in waba.rb (ADR-11)	Resolved	—
11	Rate limit values for Direct Send endpoints	Dangling	No specific req/min or req/day limit stated
12	ttl_seconds enforcement in Grape params block	Partial	API spec says 30–43200; Grape block shows optional :ttl_seconds, type: Integer with no values: constraint

Aggregate: 8 of 12 decisions Resolved, 3 Partial, 1 Dangling

---

Decision 1 — pg_advisory_xact_lock TOCTOU Guard

Status: Partial

What was decided

"Use a PostgreSQL advisory lock held for the transaction duration to serialise concurrent checks"

The chosen mechanism is pg_advisory_xact_lock, described as a transaction-scoped lock.

Alternatives considered

None documented. The RFC treats the advisory lock as the correct solution without addressing timing.

Grounding in existing code

ADR-10 cites no existing precedent for advisory lock use. This is a new pattern in this codebase.

Interface specification

Lock key: Zlib.crc32("#{organization_id}:#{local_id}"). Concern: CRC32 is 32-bit — 4 billion possible keys. For production org/batch volume this is acceptable, but a hash collision would serialize two unrelated requests and could cause spurious duplicate suppression. A 64-bit key (organization_id.hash ^ Digest::SHA256.hexdigest(local_id).to_i(16) % 2**31) would be safer.

Failure handling

NOT SPECIFIED — advisory lock waits indefinitely on contention (no lock timeout specified). A single slow request could queue all concurrent requests with the same local_id.

Challenge results

• Scale: CRC32 collision probability is negligible at expected volume.
• Reversibility: Medium — changing idempotency granularity requires careful coordination with in-flight requests.
• Consistency: THIS IS THE CRITICAL DEFECT: pg_advisory_xact_lock is a transaction-scoped lock. When called outside an explicit BEGIN…COMMIT block, PostgreSQL wraps the single statement in an implicit transaction and releases the lock immediately after. In the AgentSendsMessage#result code (§ 7 Chunk 2, also verbatim in ADR-10), the lock call appears before ApplicationRecord.transaction do. The find_by check and the INSERT both run in separate transactions. Two concurrent requests with the same local_id can both pass the find_by check simultaneously and both create rooms.
• Agent implementability: An agent implementing ADR-10 verbatim will produce code with a race condition that passes unit tests (stubs don't simulate real DB concurrency) but fails in production under load.

Gaps and suggestions

Missing:

• Advisory lock acquisition must be inside the same ApplicationRecord.transaction that contains the INSERT

Suggested resolution:

def result
  rooms_result = ApplicationRecord.transaction do
    if local_id.present?
      lock_key = Zlib.crc32("#{organization_id}:#{local_id}")
      ActiveRecord::Base.connection.execute("SELECT pg_advisory_xact_lock(#{lock_key})")
      existing = Models::Message.find_by(local_id: local_id, organization_id: organization_id)
      if existing
        next { duplicate: true, rooms: existing_rooms_for(existing) }
      end
    end

    # ... validation (outside transaction is fine for reads) ...
    rooms = []
    contact_ids.each { |cid| ... }
    { duplicate: false, rooms: rooms }
  end
  return Success(rooms_result[:rooms]) if rooms_result[:duplicate]
  # ... publish events, return Success ...
end

Note: validation steps that are read-only (validate_channel, validate_contacts, validate_balance) can stay outside the transaction to avoid holding the lock during those reads.

Open questions for the author:

• What is the acceptable lock wait timeout? Should lock_timeout be set for this advisory lock?
• Is Zlib.crc32 collision risk acceptable, or should a 64-bit hash be used?

---

Decision 2 — Migration Column Specs vs Migration Code

Status: Partial

What was decided

§ 4.1 DDL specifies source VARCHAR(50) CHECK (source IS NULL OR source = 'AUTO_GENERATED'). § 4.2 DDL specifies header VARCHAR(60) and footer VARCHAR(60) for messages.

Interface specification

The migration code in § 7 Chunk 1 contradicts both:

Column	§ 4 DDL	§ 7 Migration code
message_templates.source	VARCHAR(50) with CHECK constraint	add_column :message_templates, :source, :string, limit: 255 — no CHECK
messages.header	VARCHAR(60)	add_column :messages, :header, :string — no limit
messages.footer	VARCHAR(60)	add_column :messages, :footer, :string — no limit

An agent asked to write migrations will generate code that matches one spec but not the other. Without the CHECK constraint, the database will accept any string in source, not just 'AUTO_GENERATED' or NULL — the guard lives only in application code.

Missing:

• Reconciled migration code: add limit: 50 to source, limit: 60 to header/footer
• Add CHECK constraint: execute "ALTER TABLE message_templates ADD CONSTRAINT check_source CHECK (source IS NULL OR source = 'AUTO_GENERATED')"

Suggested resolution: Update Migration 1 and Migration 2 in § 7 to match the DDL spec in § 4 exactly.

Open questions for the author:

• Was limit: 255 on source intentional (to allow future values), or is the CHECK constraint the real enforcement?
• Should header/footer on messages be DB-enforced at 60 chars or only validated at the application layer?

---

Decision 3 — Orphaned Message Recovery Job

Status: Partial

What was decided

"Implement a new lightweight scheduled Sneakers task to republish orphaned messages in status='created'." (ADR-05 / OQ-13 resolution)

A recovery snippet is provided in ADR-05, but no implementation chunk exists.

Interface specification

NOT SPECIFIED: no schedule (how often?), no queue name, no retry policy, no DLQ, no concurrency limit, no idempotency key for the recovery job itself, no acceptance criteria, no file path.

Failure handling

NOT SPECIFIED — if the recovery job itself fails, orphaned messages remain stuck.

Missing:

• Chunk 5 (or addition to Chunk 2) for the recovery job with: schedule (e.g., every: '30s'), queue name, worker class name, file path, acceptance criteria
• DLQ for the recovery job
• Guard against republishing messages older than a configurable window (currently 30.seconds.ago — is this the right threshold for production?)

Open questions for the author:

• Should the recovery job run per-org or globally?
• What is the maximum acceptable orphan lag (currently 30 seconds)?
• Is there an existing scheduled task infrastructure in hub_core or hub-worker that this should follow?

---

Decision 11 — Rate Limit Values

Status: Dangling

Every new endpoint mentions 429 Rate limit exceeded in its error table but no rate limit values appear anywhere in the RFC (no req/min, req/day, or req/batch per org). ADR-06 mentions a Redis rate-limit key exists for Direct Send but never specifies the threshold.

Missing: Specific rate limit per org for POST /direct_send/messages (e.g., 10 req/min per org) and GET /direct_send/contacts.

Open questions for the author:

• What is the intended rate limit for Direct Send sends per org?
• Is there an existing rate limiter for similar endpoints (e.g., broadcasts/directs.rb) that should be cloned?

---

Decision 12 — ttl_seconds Validation in Grape

Status: Partial

§ ADR-14 Field Limits states ttl_seconds valid range is 30–43200. The Grape params block in § 7 Chunk 3 shows optional :ttl_seconds, type: Integer with no values: constraint. An out-of-range ttl_seconds will silently pass Grape validation and reach the Meta API, which returns error code 100 (documented in ADR-14).

Missing: values: (30..43200) in the Grape params block, or an explicit interactor-level validation that returns a 422 before reaching the builder.

---

Data Integrity Deep-Dive

Write Path	Transaction Scope	Partial Failure Behavior	Idempotency Key	Consistency Guarantee	Duplicate Handling
Room + participant + message creation (AgentSendsMessage)	ApplicationRecord.transaction — all contacts or none; COMMIT before any publish	ActiveRecord::Rollback raised → all writes rolled back; HTTP 201 not returned	messages.local_id + advisory lock (BUGGY — lock outside tx; race possible)	Strong — single DB connection	find_by(local_id) check before insert; duplicate returns existing rooms
wamid + status='sent' update (WaCloud::Repositories::Messages::Send)	Single UPDATE; no explicit transaction scope	NOT SPECIFIED — if DB write fails after Meta success, message stays status='created'; recovery job will re-publish to Meta	Idempotency check in consumer: message.status == 'created' before calling Meta	Eventual — async via Sneakers	Skip-if-not-created check prevents duplicate Meta API calls
status='failed' + system message INSERT (async failure)	NOT SPECIFIED — two writes: UPDATE + INSERT; if INSERT fails, message stays failed with no system message visible	Partial failure (UPDATE succeeds, INSERT fails) → agent sees no error message; silent failure	None	NOT SPECIFIED	None
SyncTemplateFromMeta UPSERT (template sync)	Single UPSERT; no explicit transaction	Idempotent — returns Success whether upserted or already exists	message_template_id unique index (existing)	Strong — UPSERT	Idempotent
HandleAccountRestriction (restriction write)	Single JSONB merge on channel_integrations	NOT SPECIFIED	None — JSONB merge is idempotent	Strong	Idempotent (JSONB merge)
HandleTemplateCategoryMismatch (status = "FLAGGED")	Single UPDATE	NOT SPECIFIED	None	Strong	Last-write-wins
Orphaned message recovery job	NOT SPECIFIED	NOT SPECIFIED	Existing status='created' check (same as consumer)	Eventual	Same as Sneakers consumer

Summary: The primary transaction boundary (all-or-none for multi-contact) is well-specified. Two gaps: (1) the status-update-after-Meta-success path doesn't specify behavior if the DB write fails; (2) the status='failed' + system-message INSERT is two separate writes with no explicit transaction scope.

---

Concurrency Collision Map

#	Shared Resource	Writers	Collision Scenario	Resolution Mechanism	Lock Failure Behavior	Assessment
1	messages row with same local_id + organization_id	Two concurrent AgentSendsMessage calls	Both pass find_by before either commits INSERT	pg_advisory_xact_lock — BROKEN (lock outside transaction)	No lock failure handling; could produce duplicate rooms	CRITICAL GAP
2	rooms.extra['is_locked']	AgentSendsMessage (lock on create), inbound message subscriber (unlock)	Race between new Direct Send room creation and an inbound message arriving before lock is set	Lock is set in the creation transaction; inbound handler checks is_a?(Models::DirectSendRoom) first	NOT SPECIFIED — if inbound arrives before creation commits, is_locked is not set yet	Incomplete
3	channel_integrations.settings['direct_send_restriction']	HandleAccountRestriction (webhook), GET restriction_status (read)	Two account_restriction webhooks for same org	JSONB merge is last-write-wins	N/A — idempotent	Adequate
4	message_templates row for same template_id	SyncTemplateFromMeta called from multiple concurrent webhooks	Two webhooks trigger sync for same new template_id	UPSERT with unique message_template_id index	N/A — UPSERT handles this	Adequate
5	Models::Billing::WhatsappPackage.balance	validate_balance (read), WaDeductionWorker (write)	Concurrent Direct Send sends could both pass validation with the same remaining balance	NOT SPECIFIED — accepted race condition per ADR-13	N/A	Acceptable (acknowledged in ADR-13)

---

API Contract Completeness Check

Endpoint	Request Schema	Response Schema	Error Taxonomy	Auth Spec	Idempotency	Example Payloads	Assessment
GET /direct_send/contacts	complete	complete	3 codes (403/422/429); no quantified rate limit	complete (scopes listed)	N/A — read	yes	5/6 complete
POST /direct_send/messages	complete — field types, required, validation, max sizes	complete — room list field table with nullability	8 error codes with conditions	complete (scopes listed)	yes (local_id)	yes	5.5/6 — ttl_seconds validation not in params block
GET /direct_send/restriction_status	complete	partial — example JSON only, no field-type table	2 codes	partial (scopes listed, interactor not named in endpoint spec)	N/A	yes	4/6
GET /templates/whatsapp (extended)	partial — existing undocumented, only new param described	not documented	not documented (inherited from existing)	not documented (inherited from existing)	N/A	no	1/6 — agent must inspect existing endpoint

---

Async Job / Event Consumer Spec

Job/Consumer	Trigger	Input Shape	Retry Policy	DLQ	Concurrency Limit	Idempotency Key	Timeout	Assessment
Subscribers::MessageSend (wa_cloud route)	message.send queue event	{ id: <message_uuid> } — specified	3×; 500ms/1500ms/4500ms — specified	direct_send.message_send_failed; 7d retention; depth > 50 alert — specified	Shared with existing message.send pool — unspecified count	message.status == 'created' check	35s (30s Meta + 5s buffer)	7/7 specified — strong
Recovery job (orphaned messages)	Scheduled cron	Scan messages WHERE status='created' AND created_at < 30s	NOT SPECIFIED	NOT SPECIFIED	NOT SPECIFIED	status == 'created' reuse	NOT SPECIFIED	0/7 specified — MISSING
SyncTemplateFromMeta	Inline in waba.rb statuses branch	{ template_id, organization_id }	No retry (10s timeout) — specified	None (Meta API failure → Rollbar + Failure; webhook returns 200) — specified	UPSERT idempotent	10s	4/7 specified
HandleAccountRestriction	account_update / ACCOUNT_RESTRICTION webhook	organization_id + violation/restriction_type + expiration	error_code: 200 (webhook always acks)	None — webhook failure logs to Rollbar	NOT SPECIFIED	NOT SPECIFIED	NOT SPECIFIED	2/7 specified
HandleTemplateCategoryMismatch	template_correct_category_detection webhook	NOT SPECIFIED in detail	error_code: 200	None	NOT SPECIFIED	NOT SPECIFIED	NOT SPECIFIED	1/7 specified

---

Compliance Trigger Check

Trigger	Found?	Data Location	Classification	Assessment
PII (name, email, phone, address, IP, device ID)	yes	messages.body/header/footer, contact_objects.full_name/phone_number, rooms.account_uniq_id	PII — classified in § 9 CDG table	Handled — DPA required for Beta orgs
Payment data (card, bank account)	no	N/A	N/A	N/A
Health data	no	N/A	N/A	N/A
User content with retention	yes	messages.body/header/footer — inherit existing retention	No separate Direct Send retention	Partial — retention review deferred to GA
Auth/session data	no	N/A	N/A	N/A
Cross-border data transfer	yes	messages.body/header/footer → Meta US-based Cloud API	UU PDP cross-border transfer	Disclosed in § 9 CDG; DPA required

CDG Status: Active — CDG category scored.

---

Agentic Readiness Deep-Dive

Vague Word Audit

#	Word/Phrase	Location	Impact	Concrete Replacement
1	"shared with message.send consumer pool — no additional cap per channel"	ADR-05 Sneakers Consumer Spec	Agent has no number for worker concurrency	"Inherits existing message.send Sneakers concurrency cap of N workers (read from config/sidekiq.yml)"
2	"alerts if depth > 50"	ADR-05 Sneakers Consumer Spec	No alert routing specified	"PagerDuty alert via X service if DLQ depth > 50 for 5 minutes"
3	"existing user/contact deletion flow" (right-to-delete)	§ 9 CDG	Agent doesn't know which file to extend for messages.body/header/footer scrubbing	Name the specific interactor or file path
4	"advisory only" on resource cost section	§ 8	Fine — advisory sections not scored

Total vague words in spec sections: 3 medium-impact

Dangling Alternatives

#	Alternatives	Location	Impact
1	Rate limit values for Direct Send endpoints	§ 5.2 error tables, ADR-06	Agent cannot write the Redis throttle configuration

Total dangling alternatives: 1

Task Decomposition Assessment

Chunk	Acceptance Criteria	Assessment
Chunk 1 — DB Migrations + Model Updates	8 assertable acceptance criteria (run in console / check DB)	Verifiable
Chunk 2 — New Interactors	18 assertable acceptance criteria (interactor output checks + builder output checks)	Verifiable
Chunk 3 — Grape Endpoints	11 assertable acceptance criteria (response shape checks)	Verifiable
Chunk 4 — Webhook Handlers	5 assertable acceptance criteria (interactor called, DB state changed)	Verifiable
Chunk 5 — Recovery Job	Missing	Not specified

---

Strengths

• ADR coverage (§ 3): 14 decision records with options tables and rejection rationale. ADR-13 (balance deduction) is particularly well-documented — the three blocking risks (double deduction, cross-DB rollback, no refund path) are quantified and directly grounded in the existing WaDeductionWorker path. An agent reading ADR-13 will not introduce billing bugs.
• API contract depth (§ 5.2): Both GET /contacts and POST /messages responses are documented field-by-field with types, nullability, and real example payloads verified against contact_list_response.json and room_list_response.json. The cursor format difference (base64 vs ms-integer) is proactively called out.
• Pattern alignment (§ 2): 14 specific file anchors with explicit reading order and a source-verification table resolving each one against actual line numbers. The AbstractIteractor typo is called out in § 10 — the agent will not create an orphan class by misspelling the base class.

---

Biggest Gaps

• Advisory lock outside transaction (ADR-10, § 7 Chunk 2): pg_advisory_xact_lock is released immediately when called outside ApplicationRecord.transaction. An agent implementing this spec verbatim produces code with a TOCTOU race condition that would create duplicate rooms in production under concurrent load. This is the most critical gap — it silently corrupts behavior.
• Migration code diverges from DDL spec (§ 4 vs § 7): source column is VARCHAR(50) with a CHECK constraint in § 4.1 but limit: 255 without CHECK in Migration 1. header/footer are VARCHAR(60) in § 4.2 but :string without limit in Migration 2. An agent cannot know which to follow; the DB will not enforce the length limits without the correct limit: parameter, and will not enforce the enum constraint without the CHECK.
• Recovery job is a named concept without an implementation spec (ADR-05 / OQ-13 / no chunk): OQ-13 resolves to "create a new lightweight scheduled Sneakers task" but no schedule, class name, queue, retry policy, DLQ, file path, or acceptance criteria exists. An agent will either skip this entirely or invent a design, leaving orphaned messages permanently stuck.

---

Priority Actions

Ordered by implementation-correctness impact.

1. ADR-10 / § 7 Chunk 2 — Fix advisory lock scope: Move the advisory lock acquisition, find_by check, and the ApplicationRecord.transaction block into a single database transaction. The lock must be acquired inside the transaction to remain held through the INSERT. Update the code snippets in ADR-10 and in AgentSendsMessage#result to show this corrected structure. Add a note on lock wait timeout.

2. § 4.1, § 4.2, § 7 Chunk 1 — Reconcile migration code with DDL spec: Choose one canonical spec (DDL or migration code) and make them identical. For source: either drop the VARCHAR(50)/CHECK from § 4.1 and accept app-layer-only validation, or add limit: 50 and an execute statement in Migration 1. For header/footer: add limit: 60 to both add_column calls in Migration 2.

3. ADR-05 / OQ-13 — Add Chunk 5 (Recovery Job): Specify the recovery job as a full implementation chunk: worker class name, file path, schedule (e.g., every: '30s' or via cron), queue name, DLQ, retry policy, concurrency limit, and at least 3 acceptance criteria.

4. § 5.2, § 7 Chunk 3 — Add ttl_seconds Grape validation: Add values: (30..43200) to the optional :ttl_seconds param declaration so out-of-range values return 422 before reaching the Meta API builder.

---

Backend Contract Addendum

Endpoint Contract Details

Endpoint	Method/Path	AuthZ	Request Contract	Response Contract	Error Contract	Idempotency/Versioning	Status
Contact Search	GET /api/core/v1/direct_send/contacts	OAuth2: admin/owner/supervisor/agent/has_broadcast_access/bot + direct_send_enabled flag	15 optional params; full table § 5.2	Contact list entity + cursor pagination (base64)	403/422/429 — no rate limit value	Read — no idempotency needed	Complete
Send Message	POST /api/core/v1/direct_send/messages	OAuth2: admin/owner/supervisor/agent/has_broadcast_access + direct_send_enabled flag	channel_integration_id, contact_ids[1..3], message.type/body/header/footer/cta_button/reply_buttons, ttl_seconds[30..43200], local_id[max 64]	Array of room entities + integer ms-cursor pagination	403/422 (8 conditions)/429 — no rate limit value	messages.local_id batch-level; pg_advisory_xact_lock (NEEDS FIX)	Missing: ttl_seconds Grape values constraint, rate limit value
Restriction Status	GET /api/core/v1/direct_send/restriction_status	OAuth2: admin/owner/supervisor/agent/member/bot + direct_send_enabled flag	channel_integration_id (required)	{is_restricted, violation_type, restriction_type, expires_at}	403/422	N/A	Missing: response field types table

Database Changes Details

Change	Table/Entity	DDL / Shape Diff	Data Migration Plan	Rollback Plan	Compatibility Window	Status
Add is_direct_send, source	message_templates	BOOLEAN NOT NULL DEFAULT FALSE, VARCHAR(50) CHECK (NULL OR 'AUTO_GENERATED')	No backfill needed — new rows only	ALTER TABLE REMOVE COLUMN — migration has down method implied	Existing queries unaffected (default false, nullable source)	Missing: migration code must add limit: 50 and CHECK constraint
Add header, footer, local_id	messages (partitioned)	VARCHAR(60), VARCHAR(60), VARCHAR(64)	No backfill needed	ALTER TABLE REMOVE COLUMN	Existing queries unaffected (nullable)	Missing: migration code must add limit: 60 to header/footer
Add idx_messages_local_id_org	messages	Partial index WHERE local_id IS NOT NULL on (local_id, organization_id)	N/A	DROP INDEX	N/A	Complete
New STI class Models::DirectSendRoom	rooms	No migration — STI type column already exists	N/A	UPDATE rooms SET type='Models::CustomerServiceRoom' WHERE type='Models::DirectSendRoom'; delete model file	Immediate — no migration	Complete

---

Implementation Readiness Checklist

Unblocked (agent can proceed)

All types:

• Self-contained justification sufficient (PRD linked; user stories referenced)
• All major technical decisions resolved with alternatives rejected (ADRs 01–14)
• Failure modes handled for primary async path (Sneakers retry, DLQ, poison message)
• Feature flag and rollback mechanism specified
• Pattern alignment verified (§ 2 code anchors)
• Task decomposition with acceptance criteria per chunk (Chunks 1–4)
• Zero vague words in specification sections — 3 medium-impact vague terms remain

Backend:

• Schema changes at DDL-level precision — PARTIAL (migration code diverges from § 4 DDL)
• API contracts with full request/response schemas, error taxonomy, example payloads
• Transaction boundaries and idempotency keys defined per write path — PARTIAL (advisory lock outside tx)
• Concurrency collision points listed with resolution mechanisms — PARTIAL (advisory lock bug; no rate limit values)
• Security: auth boundaries, input validation, org isolation
• Migration plan: additive, zero-downtime, rollback steps provided
• Service boundary and coupling documented
• Compliance handled — PARTIAL (Beta minimum spec; full analysis deferred)

Blocked (must fix first)

• Advisory lock in AgentSendsMessage#result must be inside ApplicationRecord.transaction (ADR-10)
• Migration code must match § 4 DDL specs (source limit + CHECK; header/footer limit)
• Recovery job must have implementation chunk with schedule, queue, DLQ, and acceptance criteria

Verdict: Fix 3 blockers first

---

Task Manifest

Chunk 5 is proposed by reviewer — RFC does not include it.

Order	Chunk	Files to Create/Modify	Acceptance Criteria	Dependencies
1	DB Migrations + Model Updates (hub_core)	db/migrate/*_add_direct_send_columns_to_message_templates.rb, db/migrate/*_add_direct_send_columns_to_messages.rb, models/message_template.rb, models/message.rb, models/room.rb, models/channel_integration.rb, models/organization.rb, models/direct_send_room.rb	db:migrate exits 0; Models::Room::TYPES.include?('Models::DirectSendRoom') true; message.local_id column exists; idx_messages_local_id_org index present	None
2	New Interactors + Builder (hub_core)	apps/direct_send/interactors/agent_sends_message.rb, agent_lists_contacts.rb, admin_gets_restriction_status.rb, handle_template_category_mismatch.rb, handle_account_restriction.rb, sync_template_from_meta.rb, wa_cloud/builders/direct_send_message.rb, wa_cloud/repositories/messages/send.rb (edit), wa_cloud/interactors/agent_send_message.rb (edit), whatsapp/templates/user_list_local_template.rb (edit)	All Chunk 2 acceptance criteria in § 7; advisory lock inside transaction; no deduct_balance call	Chunk 1
3	Grape Endpoints (hub-service)	api/core/v1/direct_send/routes.rb, resources/messages.rb, resources/contacts.rb, resources/restriction_status.rb, core_api.rb (edit), templates/resources/templates.rb (edit), spec/services/api/core/v1/direct_send/	All Chunk 3 acceptance criteria in § 7; ttl_seconds Grape values: (30..43200) present	Chunk 2
4	Webhook Handlers (hub-service)	api/webhook/resources/waba.rb (edit), spec/services/api/webhook/resources/waba_spec.rb (edit)	All Chunk 4 acceptance criteria in § 7	Chunk 2
5	Recovery Job — proposed by reviewer	hub_core/app/apps/direct_send/workers/orphaned_message_republisher_worker.rb (new), relevant schedule config	Worker scans messages WHERE status='created' AND created_at < 30s per org; publishes via Publishers::MessageSend; skips messages with status != 'created'; DLQ configured; no duplicate publishes on concurrent runs	Chunk 2

RFC DOES NOT SPECIFY Chunk 5 — proposed manifest above based on reviewer analysis. Author must specify schedule, queue, DLQ, and concurrency before agent execution of Chunk 5.

---

Dangling Decisions Log

#	Decision	Location	Owner	Deadline
1	Advisory lock must be inside ApplicationRecord.transaction	ADR-10, § 7 Chunk 2	edi.prakoso@mekari.com	Before Chunk 2 implementation
2	Reconcile source column DDL vs migration code (limit 50 vs 255; CHECK constraint)	§ 4.1 vs § 7 Migration 1	edi.prakoso@mekari.com	Before Chunk 1 implementation
3	Reconcile header/footer DDL vs migration code (limit 60 vs unspecified)	§ 4.2 vs § 7 Migration 2	edi.prakoso@mekari.com	Before Chunk 1 implementation
4	Recovery job spec: schedule, class, queue, DLQ, concurrency, acceptance criteria	ADR-05 / OQ-13	edi.prakoso@mekari.com	Before Chunk 2 implementation
5	Rate limit values for Direct Send endpoints	§ 5.2, ADR-06	edi.prakoso@mekari.com / PM	Before Chunk 3 implementation

---

Open Questions

#	Question	Category	Severity
1	Should pg_advisory_xact_lock be inside the ApplicationRecord.transaction block? (Current code places it outside — lock is released immediately.)	DIC	Blocking
2	Is source VARCHAR(50) CHECK (source IS NULL OR source = 'AUTO_GENERATED') the canonical spec, or was limit: 255 intentional?	DMS	Blocking
3	Should messages.header and messages.footer be DB-enforced at 60 chars or is application-layer validation sufficient?	DMS	Blocking
4	What is the schedule for the orphaned-message recovery job? What queue should it use?	FMC	Blocking
5	What are the rate limit values (req/min per org) for POST /direct_send/messages?	CSS	Important
6	Is there a lock wait timeout for the advisory lock? What should the client experience if the lock is contended?	DIC	Important
7	Should ttl_seconds be validated in the Grape values: constraint (422 before Meta) or only at the builder level (Meta error 100 → system message)?	ACV	Important
8	Which specific interactor/file implements the existing contact deletion PII scrubbing pass that OQ-14 extends? (The RFC says "extend existing deletion flow" but doesn't name the file.)	CDG	Important
9	What is the Sneakers worker count for the message.send consumer pool?	CSS	Nice-to-have
10	Is the HandleTemplateCategoryMismatch interactor interface fully specified? The webhook handler table in § 5.3 names it, but no contract exists.	FMC	Nice-to-have

---

Evidence Notes

• § 3 ADR-13 — best evidence section; risks of upfront deduction are enumerated with root cause and severity; agent will not introduce billing bugs
• § 2 Source Verification — strong; resolves 14 code anchors against line numbers; AbstractIteractor typo caught and documented
• § 7 Chunk 1 Migration 1 — weak; add_column :message_templates, :source, :string, limit: 255 diverges from § 4.1 DDL (VARCHAR(50) with CHECK); scored DMS down
• § ADR-10 / AgentSendsMessage#result — critical defect; pg_advisory_xact_lock call precedes ApplicationRecord.transaction; TOCTOU race not prevented; scored DIC 5.5
• § ADR-05 / OQ-13 resolution — recovery job described but no implementation chunk; scored FMC and DIC
• § 9 CDG — minimum Beta spec present; PII classified, cross-border transfer disclosed, right-to-delete path named; full analysis deferred; scored CDG 6.0
• § 5.2 POST /direct_send/messages — strongest API contract in the RFC; full room response field table with nullability, cursor format differences, response validation rules
• § 8 Observability — 5 named metrics with tags; no SLO or alert thresholds; scored OBS 7.0