feat(brand-claim): signup-domain → claim suggestion (closes #4744)#4790
Merged
Conversation
bokelley
added a commit
that referenced
this pull request
May 19, 2026
Code-reviewer + security-reviewer findings on #4790: - Slack mrkdwn injection in notifyBrandClaimOpportunity: WorkOS-signup fields (first_name, last_name, email) and external-research fields (brand_name, verified_owner_org_name) flowed raw into mrkdwn blocks. Same vector as #4754. Apply sanitizeMrkdwn to all interpolated user- controlled fields; introduce sanitizeMrkdwnLinkLabel for fields used inside <url|LABEL> link syntax (strips |, <, > on top of standard escaping). 4 new sanitization tests pin the behavior. - Dismiss endpoint accepted any string: an authed caller could plant arbitrary nudge_key rows by passing junk domains. Add a parseDomainParam helper that caps raw length at 253, canonicalizes, and asserts the result via assertValidBrandDomain. Same validator now used on the ?domain= GET branch. Two new 400-path tests. - Webhook double-emits on WorkOS retries: user.created is at-least-once with no idempotency. Record a signup_notified:<domain> marker in user_dismissed_nudges after a successful Slack send and skip when present. Reusing the table is documented in user-nudges-db.ts as a dual-use convention (dismissals + system fire-once markers). - Free-email gate inconsistency: webhook used isFreeEmailDomain(domain), service used getCompanyDomain(email). Drift risk. Webhook now also uses getCompanyDomain so both surfaces share one predicate. - Dedicated KYC Slack channel: added SIGNUP_OPS_CHANNEL_ID with fallback to REGISTRY_EDITS_CHANNEL_ID so the signup notification doesn't mix with wiki-edit reviewers but ships without extra env wiring in dev. - getSuggestionForDomain defensively canonicalizes the requestedDomain param so the service is safe to call from new surfaces without each caller having to canonicalize first. - nudgeKey lowercases the domain so a caller passing "Scope3.com" still hits the canonical "scope3.com" cooldown row. - getUserEmailById moved from the service file to db/users-db.ts next to resolvePrimaryOrganization — service was reaching into query() directly, which is the wrong layering. - Test name/assertion mismatch fixed: the "400s a scoped query with a malformed domain" test was asserting 200. With the new validator it now correctly returns 400. 26 unit tests pass across the two relevant suites. Typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 19, 2026
Per the design resolution on #4765: when a user signs up with a verified email domain that matches a brand in the registry, nudge them to claim it through three surfaces. 1. Dashboard banner — soft, dismissible, on /dashboard. 30-day cooldown on dismissal via a new generic user_dismissed_nudges table (migration 485). 2. Just-in-time prompt on /brand/view/{domain} when the viewer's email domain equals the brand being viewed. Highest-intent surface. 3. Slack notify to ops on every signup whose domain matches a registry brand. No threshold (low volume; throttle later). Suppression rules: - Free email domains never suggest. - Brand verified by caller's own org — already done, skip. - Brand verified by another org — claim would collision-fail, skip. - Dismissal within 30 days returns suggestion as inactive; older re-activates. Delegation (#4747) deliberately stays orthogonal — this flow handles "you control this domain" only. Cross-org management is the delegated-grant problem. Endpoints: - GET /api/me/brand-claim-suggestion (optional ?domain= for JIT) - POST /api/me/brand-claim-suggestion/dismiss 15 unit tests cover the suppression matrix, cooldown semantics, and the endpoint canonicalization + dismiss roundtrip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-reviewer + security-reviewer findings on #4790: - Slack mrkdwn injection in notifyBrandClaimOpportunity: WorkOS-signup fields (first_name, last_name, email) and external-research fields (brand_name, verified_owner_org_name) flowed raw into mrkdwn blocks. Same vector as #4754. Apply sanitizeMrkdwn to all interpolated user- controlled fields; introduce sanitizeMrkdwnLinkLabel for fields used inside <url|LABEL> link syntax (strips |, <, > on top of standard escaping). 4 new sanitization tests pin the behavior. - Dismiss endpoint accepted any string: an authed caller could plant arbitrary nudge_key rows by passing junk domains. Add a parseDomainParam helper that caps raw length at 253, canonicalizes, and asserts the result via assertValidBrandDomain. Same validator now used on the ?domain= GET branch. Two new 400-path tests. - Webhook double-emits on WorkOS retries: user.created is at-least-once with no idempotency. Record a signup_notified:<domain> marker in user_dismissed_nudges after a successful Slack send and skip when present. Reusing the table is documented in user-nudges-db.ts as a dual-use convention (dismissals + system fire-once markers). - Free-email gate inconsistency: webhook used isFreeEmailDomain(domain), service used getCompanyDomain(email). Drift risk. Webhook now also uses getCompanyDomain so both surfaces share one predicate. - Dedicated KYC Slack channel: added SIGNUP_OPS_CHANNEL_ID with fallback to REGISTRY_EDITS_CHANNEL_ID so the signup notification doesn't mix with wiki-edit reviewers but ships without extra env wiring in dev. - getSuggestionForDomain defensively canonicalizes the requestedDomain param so the service is safe to call from new surfaces without each caller having to canonicalize first. - nudgeKey lowercases the domain so a caller passing "Scope3.com" still hits the canonical "scope3.com" cooldown row. - getUserEmailById moved from the service file to db/users-db.ts next to resolvePrimaryOrganization — service was reaching into query() directly, which is the wrong layering. - Test name/assertion mismatch fixed: the "400s a scoped query with a malformed domain" test was asserting 200. With the new validator it now correctly returns 400. 26 unit tests pass across the two relevant suites. Typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
force-pushed
the
bokelley/4744-signup-claim-suggestion
branch
from
b1c3258 to
1fac74c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the resolution from the #4765 design discussion. When a user signs up with a verified email domain that matches a brand in the registry, nudge them to claim it.
Three surfaces
1. Dashboard banner — soft, dismissible, on `/dashboard`. Links to `/brand/builder?domain=…` (the claim flow from #4742). Dismissal records a 30-day cooldown in a new generic `user_dismissed_nudges` table.
2. Just-in-time prompt on `/brand/view/{domain}` — same banner, scoped to the brand being viewed. Fires only when the brand's domain equals the viewer's verified email domain. Highest-intent moment.
3. Slack notify to ops on every signup whose verified email domain matches a registry brand. No threshold (low volume; throttle later when it gets noisy). Fires from the existing `user.created` WorkOS webhook handler.
Suppression rules (per #4765 design)
Delegation is orthogonal
Per Brian's reframe in #4765, we deliberately don't handle holding-co / agency / consultant scenarios here. Those go through #4747's delegated-grant flow (planned) or the brand-owning org adding the user to their WorkOS org. This flow stays narrow: "you control this domain, want to claim it?"
New surfaces
Tests
15 unit tests pin:
TypeScript clean.
Test plan
Out of scope (followups)
🤖 Generated with Claude Code