RFC Review: Company Token for Qontak Chat ⇄ CRM Channel Integration (Omnichannel side)
Companion review for
channel-integration-company-token.md, produced by therfc-reviewerskill. Lives beside the RFC; valid only for the RFC revision inreviewed_rfc_last_updated(same-day revisions disambiguated by the RFC Comment-log row). Current cycle: R2 — a delta re-review of revision 2; R1 findings reconciled below, priorREV-nids carried forward, never renumbered.
Scope note — RFC revision 3 (2026-07-13), post-review. After R2, engineering descoped Decision 4 (the auto-connect / system-connect internal endpoint): the frontend flow does not change and no new API is needed — an eligible org reaches the company-token branch through the existing user Connect endpoint, so new clients and re-integrating users are covered by the disconnect → reconnect flow. This does not lower any R2 score; it closes the two Decision-4-linked items: REV-7 (system-connect caller / stage-3 gate) is now moot — no cross-squad caller exists to confirm — and the Decision Closure "1 Partial" becomes 8 of 8 Resolved (Decision 4 = Resolved by removal). Execution chunks drop from 8 to 6 (former 7–8 renumbered 5–6); "chunks 1–8" references below should read "chunks 1–6". A full R3 re-score is not required for a scope reduction that removes the only open cross-squad gate. REV-9 (cohort check) is unaffected and still stands.
Executive Summary
- Overall Score:
8.5/10 - Rating:
Agentic-Ready - RFC Type:
backend - Sub-Type:
enhancement - Assessment Confidence:
High - Applied Caps/Gates:
none — no category below 7.5; 9.0 not reachable (FMC 8.0 < 8.5 gate) - Implementation Readiness Verdict:
PROCEED — chunks 1–8 are executable as written; REV-7 (CRM caller + governance) gates rollout stage 3 and status promotion, not implementation; REV-9 is a one-query pre-merge check - Report Path:
communications/channel-integration-company-token/rfcs/channel-integration-company-token-review.md - RFC Author: mochamad.rahmandanu@mekari.com | Reviewed: 2026-07-10 (R2)
Revision 2 closed every implementation blocker from R1. The silent
save rescue nil hole is now a checked save with its own failure log and spec
assertion; every chunk-4 target was opened and pinned with line evidence (including
the discovery that the builder passes settings through wholesale — turning chunk 4
from "expose fields" into the stronger "strip the ciphertext credentials blob");
actor identity moved to the interactor layer, eliminating the unverified params
plumbing. The design itself also improved on engineering direction: branch selection
is now an org-eligibility predicate (Qontak One / billing unified_app, both
verified in source) instead of hand-curated flags, and the CID check was replaced by
an audit-recording approach consistent with the SSO identity model — with the PRD
Scenario 6.1 deviation documented explicitly rather than silently. What keeps this
at 8.5 rather than 9.0: no circuit breaker on CRM calls (acknowledged), log-based
observability without a dashboards/alerting integration, and one new minor finding
(REV-9) about currently-flagged-but-ineligible orgs.
Quick Verdict
Why this RFC can be implemented agentically:
- Every file the execution plan touches has been opened and cited with line-level evidence — including all four targets R1 flagged as unverified (Source Verification, 27 rows).
- Contracts now carry example payloads for both response envelopes (
{status,data}vs{response:…}), nullability notes, and spec-enforced absence of credential material. - The write path is integrity-correct: checked save, failure logging, and acceptance criteria that assert the failure branch — an agent cannot ship the silent-success bug.
Why this RFC will cause agent guessing or rework:
- One residual minor: the flags-AND-eligibility change silently reroutes any currently flagged but ineligible org to the legacy branch on re-connect (REV-9 — one SQL/Flipper check pre-merge).
- Stage-3 outcome (auto-connect) still depends on an unconfirmed CRM caller — the endpoint ships dark by design (REV-7).
Findings Ledger (carry-forward)
| ID | Severity | Finding (one line) | RFC location | Status | First seen | Resolved in | Evidence / fix |
|---|---|---|---|---|---|---|---|
| REV-1 | major | Success path built on integration.save rescue nil — silent failed save reports success | §2 Detail 2.A · §4.C chunk 3 | fixed | R1 | R2 (rev 2) | chunk 3 replaces L152 with return failure error_messages_for(integration) unless integration.save (pattern apis.rb:119); Detail 2.A row 1 + failure sequence diagram now cover the persist-failed branch; chunk 3 acceptance asserts it |
| REV-2 | major | Chunk-4 targets (user_disconnects_channel.rb, uniq builder) never opened | §4.C chunk 4 · Detail 2.0 | fixed | R1 | R2 (rev 2) | both files (plus entities/channel_integration.rb, destroy.rb) opened and pinned in Source Verification with quoted evidence; discovery: builder passes settings wholesale → chunk 4 now also strips credentials (Decision 6) |
| REV-3 | major | Actor-identity passthrough into Qontak::Create params unverified | §4.C chunk 4 | fixed | R1 | R2 (rev 2) | Decision 3 Option A: logging wired at the interactor layer; actor fields sliced off before repo delegation — Qontak::Create params contract untouched |
| REV-4 | minor | Flag registration left as "grep for it" | §4.C chunk 5 | fixed | R1 | R2 (rev 2) | verified no seed file exists (searched db/, database/, migrations); registration pinned as ops-console step with the Services::Preference#add signature (preference_v2.rb:224) in Detail 4.A |
| REV-5 | minor | No example payloads; envelope divergence undocumented | Detail 2.4 | fixed | R1 | R2 (rev 2) | Examples A/B/C added; explicit envelope note (SuccessFormatter vs present(response:)) |
| REV-6 | minor | connected_at nullability unstated | §2.3 · Detail 2.4 | fixed | R1 | R2 (rev 2) | keys table gains "In API responses?" column marking connected_at nullable; UI Coverage + Detail 2.E state legacy-org absence + FE "—" rendering |
| REV-7 | major | Cross-squad + governance gates (system-connect caller, approvers, discussion thread, FE RFC) | §5 Q1–Q3 · §7 | open (narrowed) | R1 | — | the R1 sub-item "blank billing_cid semantics" is moot — rev 2 removes CID validation entirely (Decision 2, engineering direction 2026-07-10). Remaining: caller confirmation (stage-3 gate), approvers/discussion (status-promotion gate), FE RFC (product-outcome gate). None block chunks 1–8 |
| REV-8 | minor | CRM_API_KEY rotation policy unstated | §3 Security · Detail 4.A | fixed | R1 | R2 (rev 2) | two-step dual-accept swap procedure + owner (communications infra w/ CRM ops) stated in §3 and Detail 4.A |
| REV-9 | minor | Flags-AND-eligibility reroutes any currently flagged but ineligible org (no unified_sso_id, no unified_app) from the company-token branch to the legacy branch on its next (re)connect — a behaviour change for that cohort if it exists | §2 Decision 2 · §4.C chunk 3 | open | R2 | — | pre-merge check: enumerate orgs in :company_token_specific (Flipper) and verify each is eligible (unified_sso_id present or unified_app); if any fail, decide grandfather-in vs migrate before enabling chunk 3 in production |
Ledger summary: 2 open (0 blocker / 1 major / 1 minor), 7 fixed this cycle, 0 accepted-risk. REV-7 remains promoted in the RFC's §5 (Q1–Q3); REV-9 to be added there by the author or handled as a pre-merge checklist item.
PRD → RFC Traceability Matrix
PRD exists (Confluence, page 49856315521), not yet snapshotted in-repo; CTOK-Sxx
ids minted with an explicit adoption note (unchanged from R1 — acceptable).
| PRD Element | RFC Section | Coverage |
|---|---|---|
| P1 story 1 — company token generated for Chat (AC1.1) | Detail 1.C S01 + eligibility selection (Decision 2) | Full |
| P1 story 2 — delete email field (AC2.1.1/2.1.2) | Detail 1.C S02 (BE done, FE deferred) | Full (BE half) |
| P1 story 3 — reporter/deal-owner assignment | Detail 1.C S03, endpoint row 5 | Partial — filtering location tracked as §5 Q4 (explicitly, no longer silent) |
| P2 story 1 — auto-generation on account creation (+ ERR) | Detail 1.C S04, Decision 4, endpoint row 7 | Full (chat half; CRM half cross-squad) |
| P2 story 2 — one-click connect + details (+ NEG) | Detail 1.C S05, PRD-to-Schema rows 3–5 | Full |
| P2 story 3 — keep existing functions | Detail 1.C S06 (regression-only, stated) | Full |
| P2 story 4 — disable integration | Detail 1.C S07, endpoint row 4 (qontak-scoped logging) | Full (BE half) |
| P2 story 5 — rename menu | Detail 1.C S08 n/a — FE | Full (correctly deferred) |
| P2 story 6 — activity log (6.1 CID, 6.2 config changes) | Detail 1.C S09, Decisions 2–3, DDL | Full — with a documented deviation: 6.1's connect-time CID check replaced by by-construction guarantee + audit recording (crm_billing_cid vs org_company_id in metadata, tenant-audit query in §4.D); deviation is explicit in the Scenario 6.1 note, grounded in engineering direction |
| Release criteria — new-clients-only, feature flag, CID question | §4 Rollout, Decision 2, Scenario 6.1 note | Full |
| Constraints — no migration, HMSI exclusion | §1 Out of Scope | Full |
| Success criteria — 100% no manual token, ≥95% first-attempt | §1 Success Criteria (SQL-measurable) | Full |
crm_integration_activity_logs / eligibility service / checked save / credentials stripping / connected_at / internal endpoint / flag | — | All justified reverse-traces (Detail 1.A reverse table) — no scope creep |
Summary: 12 of 13 PRD items fully covered (one with a documented, justified deviation), 1 partial (S03 filtering — tracked), 0 missing. 0 RFC decisions without PRD justification.
Scorecard (Backend)
| Category | Score | Evidence-Based Rationale |
|---|---|---|
| PRT — PRD Traceability | 8.5 | Bidirectional matrix + per-story map + section coverage; the Scenario 6.1 deviation is documented in place with rationale and a preserved tracking mechanism — deviation-with-receipts, not silent drop. Deduction: minted ids not yet PRD-native; S03 partial |
| TDC — Technical Decisions | 8.5 | 8 ADR blocks; Decision 2 rewritten with three real options (eligibility, R1's validation, flags-only) and rationale grounded in the SSO identity model + verified GetUnified source; the R1 "grep for it" instruction eliminated (verified-absent seed file). Residual: Decision 4 caller still external |
| DMS — Data Model & Schema | 8.5 | Unchanged DDL strengths + keys table now carries response-visibility column; metadata shape enumerated (source, crm_billing_cid, org_company_id); schema unique index cited from the actual schema.rb |
| ACV — API Contract & Versioning | 8.5 | Examples A/B/C with the envelope-divergence note (verified against crms/auths.rb:24 and SuccessFormatter); nullable connected_at stated; the one breaking change (credentials-key removal) is explicit with a consumer-check note and spec enforcement |
| DIC — Data Integrity & Consistency | 8.5 | Checked save closes the R1 hole; Detail 2.A row 1 now matches the code it describes (failure-before-write AND failed-write branches both specified + diagrammed + spec-asserted); billing-cache staleness reasoned ("delays eligibility, never corrupts") |
| FMC — Failure Mode & Retry Coverage | 8.0 | Per-call timeouts, bounded retries, terminal-state handling, full error catalog incl. the two new rows (persist-failed, legacy-only CID), billing-fallback row with degrade-to-legacy behaviour. Still no circuit breaker (acknowledged in §5) — holds it below 8.5 |
| CSS — Concurrency & Scaling | 7.5 | Collision map now cites the real unique index idx_channel_integrations_webhook (webhook, deleted_at) with the honest Postgres NULLs-distinct caveat instead of relying on the code guard alone; load quantified; no rate limiting (acknowledged) |
| SAS — Security & Authorization | 8.0 | Ciphertext credentials stripped from all qontak responses (new, spec-enforced, threat-model item e); CRM_API_KEY rotation procedure + owner; internal-network assumption converted from silent to flagged (§5 Q7). SSO-trust posture is a deliberate, documented model — not a gap. Residual: no per-endpoint rate limits |
| MRP — Migration & Rollout Plan | 8.5 | Stage evidence now includes the tenant-audit query and eligibility-fallback rate; stop conditions include audit-mismatch rows; config contract complete with verified provisioning path per flag; rollback unchanged (flag-off, data-safe) |
| OBS — Observability Definition | 7.5 | duration_ms in the success mirror makes connect latency trendable from logs; named new warning events; three concrete verification/audit queries; 3am runbook extended with eligibility + tenant-audit paths. Still log-based with no named dashboard/alert tooling — correct for the codebase, but short of the rubric's alerting bar |
| SBC — Service Boundary & Coupling | 8.5 | New billing dependency is explicit everywhere it appears (topology, service map, contracts table, failure catalog) with owner and degrade behaviour; boundary matrix unchanged and correct |
| CPA — Consistency & Pattern Alignment | 8.5 | Every pattern row cites an opened file; the two new artifacts copy named precedents (GetUnified PORO shape; settings.delete('access_token') stripping); checked-save deviation is itself anchored to an in-repo pattern (apis.rb:119) |
| CDG — Compliance & Data Governance | 8.0 | Right-to-delete now concrete (anonymize-in-place SQL, keeps audit row, named runbook owner); retention automated via chunk 8; classification unchanged and complete |
Resource & Cost Advisory (non-blocking)
- Adequate: +1 low-volume table, +1 Redis GET per connect, rare billing HTTP fallback, one follow-up cron worker (§4 Detail 4.E). Nothing to route to infra planning.
Score-cap check: all categories ≥ 7.5 → no caps. 9.0 gate requires DMS/DIC/FMC all ≥ 8.5 — FMC is 8.0, so 9.0 is not reachable this cycle. Overall: 8.5.
Decision Closure Assessment
Decision Index
| # | Decision | Status | Critical Gaps |
|---|---|---|---|
| 1 | Reuse existing company-token branch | Resolved | — |
| 2 | Eligibility-based selection; no CID validation (audit-record instead) | Resolved — R1's Partial closed by engineering direction; three options weighed; nil-lookup behaviour specified; PRD deviation documented | — |
| 3 | Activity log: Postgres table, interactor-layer wiring | Resolved — R1's REV-3 plumbing risk designed out (Option C explicitly rejected) | — |
| 4 | Auto-connect via internal endpoint | Partial | caller + onboarding step unconfirmed (§5 Q1); chat-side contract fully specified, ships dark |
| 5 | Connect stays synchronous | Resolved | — |
| 6 | Storage LOCKBOX-in-settings + credentials stripped from responses | Resolved — strengthened vs R1 (response-surface minimization with in-file precedent) | — |
| 7 | No new caching (eligibility rides billing cache) | Resolved (honest no-alternative) | — |
| 8 | Multi-tenancy = org-scoped keys + SSO identity model | Resolved (honest no-alternative) | — |
Aggregate: 7 of 8 Resolved, 1 Partial, 0 Dangling.
Decision 2 — Eligibility + no CID validation (now Resolved; the R2 material change)
What was decided: branch = kill-switch flags AND
(organization.unified_sso_id.present? OR
GetUnified.new(company_id).call&.dig('unified_app')), via new
Crm::Services::CompanyTokenEligibility; no billing_cid comparison —
CRM-returned billing_cid + chat company_id recorded in success-row metadata;
GetUnified nil → not eligible → legacy branch + warning.
Grounding: get_unified.rb (opened — Redis key, billing fallback, rescue→nil);
abstract_builder.rb:470-475 (unified_app read precedent);
sync_organization_data.rb:39 (Qontak One ⇒ unified_sso_id convention);
team/apis.rb:7 (blank-SSO hard stop retained).
Failure handling: billing outage degrades to legacy (never blocks); mid-provisioning
unified_app orgs hit the existing 'Unified SSO id is blank!' 422.
Challenge results:
- Scale: one cached Redis read per connect — negligible at admin frequency.
- Reversibility: single call site; validation can be reintroduced in one file.
- Consistency: consistent with Decision 8 (SSO identity model) and the PRD's separate provisioning-cleanup workstream; the Scenario 6.1 deviation is documented with a preserved tracking mechanism (audit query).
- Agent implementability: fully — predicate, call site, nil semantics, logs, and specs all pinned. Residual judgment risk sits with the product (trusting the SSO invariant), and the RFC says so out loud in Consequences + Known Limitations.
Remaining gap: REV-9 (cohort check for flagged-but-ineligible orgs) — a pre-merge query, not a design hole.
Decision 4 — System-connect internal endpoint (Partial, unchanged)
Chat-side interface fully specified (request/response/status/idempotency/flag + failure diagram); the caller remains a CRM-squad confirmation (§5 Q1). Gates rollout stage 3 only; chunks 5–6 execute regardless.
(Decisions 1, 3, 5–8: Resolved; evidence in scorecard; abbreviated per SOP for categories ≥ 7.0.)
Data Integrity Deep-Dive
| Write Path | Transaction Scope | Partial Failure Behavior | Idempotency Key | Consistency Guarantee | Duplicate Handling |
|---|---|---|---|---|---|
| Connect (user/system) | single-row checked save; token fetch precedes writes | pre-write failure → nothing persisted; failed save → Failure + connect_failed(persist_failed) log, no success signal (R1 hole closed); post-save webhook-registration failure documented as eventual + retried on restore | org+channel webhook key via restore | strong (DB) / eventual (CRM webhook state) | 'already exsit' guard + unique index (with documented NULL caveat) |
| Disconnect | soft-delete + qontak-scoped log | log failure rescued, non-blocking | repeat → 404/422 | strong | n/a |
| Token refresh | in-place checked update (apis.rb:109-119) | old credentials retained; terminal states destroy channel | @refreshed <= 3 | strong | last-write-wins, both tokens valid |
| Activity-log insert | own INSERT | rescued, never fails parent | append-only | strong | duplicates acceptable |
Verdict: the matrix now matches the code it will produce. No unsupported claims remain.
Concurrency Collision Map
| # | Shared Resource | Writers | Collision Scenario | Resolution | Lock-Failure Behavior | Assessment |
|---|---|---|---|---|---|---|
| 1 | channel_integrations row | user connect, system connect, disconnect | simultaneous user + onboarding connect | code guard + idx_channel_integrations_webhook unique index, with the honest NULLs-distinct caveat documented | 422; caller re-reads uniq | adequate — documented pre-existing posture |
| 2 | settings['credentials'] | concurrent refreshes | double refresh on 401 | last-write-wins, bounded | extra CRM call, no corruption | adequate |
| 3 | Preference :crm_moderator | moderator refreshes | same | same | same | adequate |
API Contract Completeness Check
| Endpoint | Request | Response | Errors | Auth | Idempotency | Examples | Assessment |
|---|---|---|---|---|---|---|---|
POST /integrations/qontak | complete | complete (Example A) | complete (incl. persist-failed + legacy-CID rows) | specific | defined | yes | 6/6 |
GET /qontak/integration/uniq | complete | complete (Example A + nullability) | complete | specific | read | yes | 6/6 |
GET /integration/refresh_token/:id | complete | referenced-existing | complete | specific | stated | via A | 5.5/6 |
DELETE /integrations/:id | complete | referenced-existing | complete | specific | defined | n/a | 5.5/6 |
GET /qontak/crm/users | reused as-is | reused | reused | specific | read | n/a | 5.5/6 |
POST /internal/v1/crms/auths/tokens | complete | complete (Example B) | complete | specific | stated | yes | 6/6 |
POST /internal/v1/crms/integrations/qontak (new) | complete | complete (Example C) | complete | specific | defined | yes | 6/6 |
Async Job / Event Consumer Spec
Unchanged from R1: no new async jobs; existing workers tabled; retention cleanup (chunk 8) fully specified. Adequate.
Compliance Trigger Check
| Trigger | Found? | Location | Classification | Assessment |
|---|---|---|---|---|
| PII | yes | actor_email, settings['username'] | classified | handled — anonymize-in-place right-to-delete with named owner (improved vs R1) |
| Payment data | no | — | — | — |
| Content retention | yes (audit rows) | activity log | 24-mo + automated cleanup | handled |
| Auth/session data | yes | CRM credentials in settings | LOCKBOX + stripped from responses | handled (improved vs R1) |
| Cross-border | no evidence | — | — | not addressed; presumed in-region (nice-to-have) |
CDG Status: Active — scored (8.0).
Agentic Readiness Deep-Dive
Vague Word Audit
| # | Word/Phrase | Location | Impact | Concrete Replacement |
|---|---|---|---|---|
| 1 | "to be assigned" (approvers) | frontmatter/Metadata | none on code; gates status promotion | assign at kickoff (tracked §5 Q2, REV-7) |
| 2 | TBD-slack (discussion) | frontmatter | none on code | open thread (tracked §5 Q3, REV-7) |
Total vague words in spec sections: 0 (both residuals are declared governance placeholders, ledgered under REV-7; R1's two "grep for it" instances eliminated).
Dangling Alternatives
None. Both R1 Partials: one closed (Decision 2), one has a chosen default shipping dark (Decision 4).
Task Decomposition Assessment
All 8 chunks verifiable; chunk 3 now carries the checked-save assertion, chunk 4 carries the credentials-absence and non-qontak-no-log assertions, chunk 2 gained the eligibility spec matrix. No chunk requires the agent to discover an unpinned file.
Mermaid validity
12/12 blocks parse — re-validated with @mermaid-js/mermaid-cli (mmdc) this
cycle against revision 2 (several diagrams changed: eligibility nodes, billing
participants, checked-save failure branch). Zero failures.
Strengths
- Verification-driven design improvement — R1's REV-2 verification didn't just
pin paths; it discovered the wholesale settings passthrough and converted chunk 4
into a security fix (ciphertext credentials stripped, with the in-file
settings.delete('access_token')precedent). This is repo-as-documentation working as intended (Detail 2.0, Decision 6). - Deviation with receipts — the Scenario 6.1 CID-validation removal is grounded
in a quoted engineering direction, documented as an explicit PRD-deviation note,
and compensated by a concrete tenant-audit mechanism (
crm_billing_cidvsorg_company_id+ the §4.D zero-rows query). Reviewers can disagree with the decision, but nobody can miss it (Detail 1.A note, Decision 2). - Write-path integrity is now spec-enforced — the checked save, its failure log, the failure-branch sequence diagram, and chunk 3's acceptance criterion form a closed loop an agent cannot silently regress (Detail 2.A, §4.C chunk 3).
Biggest Gaps
- REV-9 (Decision 2 / chunk 3): if any org currently in
:company_token_specificlacks bothunified_sso_idandunified_app, revision 2 silently moves its next re-connect to the legacy branch — one enumeration query pre-merge decides grandfather vs migrate. - FMC 8.0: no circuit breaker on the four CRM calls; bounded retries + timeouts are the only containment. Acknowledged in §5, acceptable at this frequency, but it is the ceiling on the score.
- REV-7: stage-3 rollout and status promotion still wait on the CRM caller confirmation and approver assignment — external, tracked, and correctly scoped as non-implementation gates.
Priority Actions
- REV-9 (pre-merge) — run the cohort check: list orgs enabled in
:company_token_specific, verify each satisfies the eligibility predicate; grandfather or migrate any that don't, and note the result in §5. - REV-7 (parallel, non-code) — send CRM the caller-confirmation question; assign EM + infosec approvers; open the Slack thread.
- Optional hardening (post-merge) — wrap the four CRM calls in the existing Circuitbox pattern (SSO precedent) to lift FMC; add a dashboards/alerting hook if the platform adopts a metrics registry (lifts OBS).
Backend Contract Addendum
Endpoint Contract Details
| Endpoint | Method/Path | AuthZ | Request | Response | Errors | Idempotency/Versioning | Status |
|---|---|---|---|---|---|---|---|
| Connect | POST /api/core/v1/integrations/qontak | OAuth2 scopes + server-side org | complete | complete (Example A, credentials stripped) | complete | restore / additive v1 | Complete |
| Status | GET /api/core/v1/qontak/integration/uniq | 6 scopes | — | complete (nullable connected_at) | complete | read / additive | Complete |
| System connect | POST /api/internal/v1/crms/integrations/qontak | X-Chat-Api-Key + flag | complete | complete (Example C) | complete (3 rows) | restore / internal v1 | Complete |
| Others (rows 3–6) | — | quoted | reused | reused + Example B | reused | stated | Complete |
Database Changes Details
| Change | Table | DDL | Migration Plan | Rollback | Compat Window | Status |
|---|---|---|---|---|---|---|
| New table | crm_integration_activity_logs | complete | single additive | down-migration or leave-unused | n/a | Complete |
| JSON keys | settings['connected_at'] (+ credentials stripped from responses) | keys table w/ response visibility | set-on-connect | ignore key / revert builder line | indefinite | Complete |
Implementation Readiness Checklist
Unblocked (agent can proceed)
- Traceability complete (1 tracked partial; 1 documented deviation)
- All decisions resolved or shipping-dark with chosen defaults (7 Resolved, 1 Partial-external)
- Failure modes + error catalog per external call (incl. billing fallback + persist-failed)
- Configuration contract complete with verified provisioning path per flag
- Pattern alignment verified against opened files (27 Source Verification rows)
- Rollout plan with staged evidence, tenant-audit stop condition, flag rollback
- Observability: named log events, latency signal, audit queries, runbook
- Task decomposition: 8 chunks, all acceptance criteria assertable
- Schema at DDL precision; write paths integrity-correct (checked save)
- Concurrency mapped with real index evidence
- Security: tenancy model, response-surface minimization, rotation procedure
- Compliance: classification, automated retention, concrete right-to-delete
Blocked (must fix first)
- REV-9 — one-query cohort check before enabling chunk 3 in production (pre-merge, not pre-implementation)
Verdict: Ready to implement. REV-9 is a pre-merge operational check; REV-7 gates rollout stage 3 / status promotion only.
Task Manifest
RFC's own §4.C decomposition adopted as-is (verified above). Single amendment: add the REV-9 cohort check to the chunk-3 pre-merge steps.
Dangling Decisions Log
None dangling. One Partial (Decision 4 — external caller) with a chosen default shipping dark.
Open Questions
| # | Question | Category | Severity |
|---|---|---|---|
| 1 | Is any org currently flagged in :company_token_specific ineligible under the new predicate (REV-9)? | MRP/DIC | Blocking (pre-merge check only) |
| 2 | Which component calls the system-connect endpoint, and does it retry 422 during provisioning ordering? (§5 Q1) | SBC | Blocking (stage 3 only) |
| 3 | Does GET /qontak/crm/users filter deal-creation eligibility, or is it CRM-side? (§5 Q4) | PRT | Important |
| 4 | Is /api/internal/* network-restricted in production as assumed? (§5 Q7) | SAS | Important (verify at stage 0) |
Evidence Notes
- Detail 2.0 Source Verification — grew from 18 to 27 rows; all previously
subagent-only or missing anchors now opened directly (
user_disconnects_channel.rb,builders/channel_integration.rb,entities/channel_integration.rb,destroy.rb,get_unified.rb,abstract_builder.rb:470-475,preference_v2.rb:224,spec/dummy/db/schema.rbunique index, verified-absent seed file). - Decision 2 vs sources — eligibility predicate cross-checked against
get_unified.rb(nil-on-failure),sync_organization_data.rb:39(Qontak One convention),team/apis.rb:7(blank-SSO stop); the REV-9 cohort risk was found by reasoning over the AND-composition, not stated in the RFC — now ledgered. - Detail 2.4 examples — envelope shapes verified against
SuccessFormatterandcrms/auths.rb:24. - Mermaid validity: 12/12 parse (mmdc, revision 2).
- Evidence normalization: RFC Comment log and §5 discussion content excluded as scoring evidence; §5 used only to verify self-declared gaps.
Review History
| Cycle | Date | Reviewed RFC revision | Score | Verdict | Findings open → fixed | Notes |
|---|---|---|---|---|---|---|
| R1 | 2026-07-10 | last_updated: 2026-07-10 / working tree (rev 1) | 7.5 | PROCEED with notes | 8 open, 0 fixed | Initial review; 3 implementation blockers (REV-1..3), all small; cross-squad gates self-declared by RFC §7 |
| R2 | 2026-07-10 | last_updated: 2026-07-10 / working tree (rev 2 — see RFC Comment logs) | 8.5 | PROCEED | 2 open, 7 fixed | Eligibility-based design (engineering direction), CID validation removed with documented deviation + audit compensation, checked save, all anchors opened, credentials stripped from responses; new minor REV-9 (flagged-but-ineligible cohort check) |