feat: Slack notification on new user signup#5
Conversation
Add Firebase Cloud Function (Auth onCreate trigger) that posts to #operation-hired Slack channel when a new user signs up on Perception. Uses SLACK_WEBHOOK_URL Firebase secret (already configured). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @jeremylongshore, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a new feature to enhance real-time operational awareness by automatically notifying a designated Slack channel whenever a new user registers. This is achieved by deploying a Firebase Cloud Function that listens for new user signups and then securely dispatches relevant user details to Slack, ensuring the team is immediately informed of new platform adoption. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a Firebase Functions configuration and a Cloud Function (Node.js 22) that triggers on Auth user creation to post masked signup details to a Slack webhook; also adds functions package manifest and .gitignore entry, and updates project issue tracking with new epics/tasks for ingestion, observability, and notifications. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Auth as Firebase Auth
participant CF as Cloud Function
participant Slack as Slack Webhook
User->>Auth: Sign up / create account
Auth->>CF: onCreate trigger (user data)
CF->>CF: Mask email, build payload, read secret
CF->>Slack: POST payload to webhook (with timeout)
Slack-->>CF: HTTP 200 / error response
CF-->>CF: Log success or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a Firebase function to send a Slack notification on new user signups. The implementation is sound, but I've identified a couple of areas for improvement in functions/index.js. Specifically, I've suggested removing some unused code related to v2 functions and refactoring the HTTP request logic to use the modern fetch API, which is available in the nodejs22 runtime. This will make the code cleaner, more robust, and easier to maintain. The rest of the changes look good.
functions/index.js
Outdated
| const https = require("https"); | ||
|
|
||
| exports.onNewUser = functions | ||
| .runWith({ secrets: ["SLACK_WEBHOOK_URL"] }) | ||
| .auth.user() | ||
| .onCreate(async (user) => { | ||
| const webhookUrl = process.env.SLACK_WEBHOOK_URL; | ||
| if (!webhookUrl) { | ||
| logger.error("SLACK_WEBHOOK_URL secret not configured"); | ||
| return; | ||
| } | ||
|
|
||
| const createdAt = user.metadata.creationTime || new Date().toISOString(); | ||
| const payload = JSON.stringify({ | ||
| text: [ | ||
| ":new: *New Perception user signup*", | ||
| `*Email:* ${user.email || "no email"}`, | ||
| `*UID:* \`${user.uid}\``, | ||
| `*Provider:* ${user.providerData?.[0]?.providerId || "email"}`, | ||
| `*Time:* ${createdAt}`, | ||
| ].join("\n"), | ||
| }); | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
| const url = new URL(webhookUrl); | ||
| const req = https.request( | ||
| { | ||
| hostname: url.hostname, | ||
| path: url.pathname, | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| "Content-Length": Buffer.byteLength(payload), | ||
| }, | ||
| }, | ||
| (res) => { | ||
| let data = ""; | ||
| res.on("data", (chunk) => (data += chunk)); | ||
| res.on("end", () => { | ||
| if (res.statusCode === 200) { | ||
| logger.info(`Slack notified for new user: ${user.email}`); | ||
| resolve(data); | ||
| } else { | ||
| logger.error(`Slack webhook failed: ${res.statusCode} ${data}`); | ||
| reject(new Error(`Slack webhook failed: ${res.statusCode}`)); | ||
| } | ||
| }); | ||
| } | ||
| ); | ||
| req.on("error", reject); | ||
| req.write(payload); | ||
| req.end(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The current implementation for sending the Slack notification uses the native https module wrapped in a Promise. This is verbose and contains an anti-pattern where an async function returns a new Promise. Since the function runtime is nodejs22, we can leverage the built-in fetch API to greatly simplify this logic, make it more robust, and align with modern JavaScript practices. This change also makes the https module import unnecessary.
exports.onNewUser = functions
.runWith({ secrets: ["SLACK_WEBHOOK_URL"] })
.auth.user()
.onCreate(async (user) => {
const webhookUrl = process.env.SLACK_WEBHOOK_URL;
if (!webhookUrl) {
logger.error("SLACK_WEBHOOK_URL secret not configured");
return;
}
const createdAt = user.metadata.creationTime || new Date().toISOString();
const payload = {
text: [
":new: *New Perception user signup*",
`*Email:* ${user.email || "no email"}`,
`*UID:* \`${user.uid}\``,
`*Provider:* ${user.providerData?.[0]?.providerId || "email"}`,
`*Time:* ${createdAt}`,
].join("\n"),
};
try {
const response = await fetch(webhookUrl, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify(payload),
});
if (response.ok) {
logger.info(`Slack notified for new user: ${user.email}`);
} else {
const errorText = await response.text();
logger.error(
`Slack webhook failed: ${response.status} ${response.statusText}`,
{ error: errorText }
);
}
} catch (error) {
logger.error("Error sending Slack notification", { error });
}
});
functions/index.js
Outdated
| const { onDocumentCreated } = require("firebase-functions/v2/firestore"); | ||
| const { defineSecret } = require("firebase-functions/params"); | ||
| const { logger } = require("firebase-functions"); | ||
|
|
||
| const slackWebhookUrl = defineSecret("SLACK_WEBHOOK_URL"); |
There was a problem hiding this comment.
These imports and variable definitions appear to be from an attempt to use v2 Cloud Functions. Since the implementation uses a v1 Auth trigger, onDocumentCreated, defineSecret, and the slackWebhookUrl constant are unused. They should be removed to improve clarity and reduce code clutter.
| const { onDocumentCreated } = require("firebase-functions/v2/firestore"); | |
| const { defineSecret } = require("firebase-functions/params"); | |
| const { logger } = require("firebase-functions"); | |
| const slackWebhookUrl = defineSecret("SLACK_WEBHOOK_URL"); | |
| const { logger } = require("firebase-functions"); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@functions/index.js`:
- Around line 31-38: The Slack payload construction (variable payload in
functions/index.js) currently embeds PII (user.email and user.uid) into the
message and function logs; remove or mask these values before sending or
logging—replace user.email with a non-PII placeholder or masked string (e.g.,
omit domain or use "<redacted>") and avoid printing raw user.uid (use a
truncated/hash form) wherever payload is built or logged (also apply the same
change to the other payload/log locations around the code noted at lines 58-59).
Ensure any processLogger or console.log calls do not emit user.email or user.uid
and update the code paths that createPayload/sendSlackNotification to accept
already-redacted identity fields.
- Around line 41-70: The Slack webhook Promise currently creates an
https.request without a timeout, which can hang; add a timeout on the request
(use the returned request object `req`) — call `req.setTimeout(timeoutMs, ...)`
or attach a `'timeout'` handler to abort the request and reject the Promise with
a clear timeout Error, ensure you also remove/avoid double-rejects by cleaning
up listeners and calling `req.destroy()`/`req.abort()` as needed; reference the
existing symbols `webhookUrl`, `https.request`, `req`, `payload`, and `logger`
so the change is applied inside the same Promise block that resolves/rejects
based on `res.statusCode`.
In `@functions/package.json`:
- Around line 4-10: The package.json currently lists incompatible dependency
versions: "firebase-functions": "^6.3.0" and "firebase-admin": "^13.0.0"; update
the dependency pair so peer constraints match — either downgrade
"firebase-admin" to a ^12.x version or (preferred) bump "firebase-functions" to
a v7-compatible range (e.g. "^7.0.0") so it supports "firebase-admin@13"; edit
the "dependencies" section in package.json (look for the "firebase-functions"
and "firebase-admin" entries) to make this change and then run your install to
verify resolution.
🧹 Nitpick comments (2)
functions/index.js (2)
7-14: Doc comment doesn’t match behavior.The comment describes a Firestore fallback, but only the Auth
onCreatetrigger is implemented. Update the comment or add the Firestore trigger to avoid confusion.✏️ Suggested comment fix
- * Firebase Auth triggers aren't available in v2 yet for all runtimes, - * so we watch the /users/{userId} Firestore collection instead. - * If no Firestore doc is created on signup, we fall back to an Auth - * onCreate trigger (v1). + * Firebase Auth triggers use the v1 API here, so we rely on the Auth + * onCreate trigger for signup notifications.
20-25: Remove unuseddefineSecretfor clarity.The
slackWebhookUrlvariable defined on line 5 is never used. The function binds the secret viarunWith({ secrets: ["SLACK_WEBHOOK_URL"] })and reads it viaprocess.env.SLACK_WEBHOOK_URLon line 24, making thedefineSecretcall redundant. Remove it to avoid confusion:Remove defineSecret
-const { defineSecret } = require("firebase-functions/params"); -const slackWebhookUrl = defineSecret("SLACK_WEBHOOK_URL");
- Replace https module with native fetch API (Node.js 22) with 10s timeout - Mask PII in Slack payload and logs (email redacted, UID truncated to 8 chars) - Remove unused v2 imports (defineSecret, onDocumentCreated, slackWebhookUrl) - Fix misleading doc comment about Firestore fallback - Downgrade firebase-admin to ^12.7.0 for peer dep compatibility with functions ^6.3.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.beads/issues.jsonl:
- Around line 35-38: The issue description for perception-s9t.1 incorrectly says
"Firestore onCreate trigger" — update the description and acceptance criteria to
reference the Firebase Auth onCreate trigger used by PR `#5` (e.g., replace
"Firestore onCreate" with "Firebase Auth onCreate") so reviewers and deploy/test
tasks clearly target the Auth user-creation event; ensure the description
mentions PR `#5` and that acceptance verifies the Auth onCreate Cloud Function is
merged and triggers on new user signup.
🧹 Nitpick comments (2)
functions/package.json (1)
1-12: Consider adding"private": trueto prevent accidental publishing.Since this package is internal to your Firebase project, adding
"private": trueguards against accidentalnpm publish.📦 Suggested addition
{ "name": "perception-functions", "description": "Firebase Cloud Functions for Perception", + "private": true, "engines": { "node": "22" },functions/index.js (1)
1-10: Consolidate duplicate imports from the same module.Both
logger(line 1) andfunctions(line 10) are imported fromfirebase-functions. Consider combining them into a single import statement.♻️ Suggested consolidation
-const { logger } = require("firebase-functions"); - -/** - * Notify Slack when a new user signs up. - * - * Uses the v1 Auth onCreate trigger which fires on every - * Firebase Auth signup event. - */ - -const functions = require("firebase-functions"); +const functions = require("firebase-functions"); +const { logger } = functions; + +/** + * Notify Slack when a new user signs up. + * + * Uses the v1 Auth onCreate trigger which fires on every + * Firebase Auth signup event. + */
Change "Firestore onCreate trigger" to "Firebase Auth onCreate trigger" to match the actual implementation in PR #5. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- perception-s9t.1: update description to past tense (PR #5 was merged) - perception-s9t: clarify PR #5 already merged in epic description - perception-pkm, perception-t89: refresh updated_at timestamps - perception-a27.2: fix file path (routers/trigger.py not routers/rss.py) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… content (#6) * chore(beads): create 4 epics with tasks for ingestion, observability, notifications, and content Set up structured task tracking with beads for identified issues: - PERC-INGEST (P1): Automated daily ingestion pipeline (4 tasks, chained deps) - PERC-OBSERVABILITY (P2): OpenTelemetry + Cloud Trace (3 tasks, chained) - PERC-NOTIFY (P2): User signup Slack notifications (3 tasks, chained) - PERC-CONTENT (P2): Brief & summary generation (3 tasks, cross-epic dep on INGEST) Reparented existing tasks (generate_brief, store_articles) under PERC-CONTENT epic. Installed updated git hooks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(beads): use absolute date and refresh stale updated_at timestamps - Replace "7+ days stale" with "Last ingestion: 2026-02-06" in PERC-INGEST epic - Bump updated_at on perception-pkm and perception-t89 after reparenting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(beads): update PERC-NOTIFY epic to reflect PR #5 merged Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(beads): sync JSONL after rebase and PR #5 merge Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(beads): address Gemini and CodeRabbit review comments - perception-s9t.1: update description to past tense (PR #5 was merged) - perception-s9t: clarify PR #5 already merged in epic description - perception-pkm, perception-t89: refresh updated_at timestamps - perception-a27.2: fix file path (routers/trigger.py not routers/rss.py) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: jeremylongshore <jeremylongshore@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
onNewUser) that fires on every Firebase Auth signup#operation-hiredSlack channel with user email, UID, provider, and timestampSLACK_WEBHOOK_URLFirebase secret (already configured viafirebase functions:secrets:set)functions/directory withfirebase-functionsv1 Auth triggerFiles added
functions/index.js— Cloud Function with AuthonCreatetriggerfunctions/package.json— Dependencies (firebase-admin, firebase-functions)functions/.gitignore— Excludes node_modulesfirebase.json— Updated to include functions configDeploy steps (after merge)
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores