Conversation
* fix(server): update roast controller, repository, routes and service * fix(firebase): update firebase-admin server initialization * fix(services): update roast client service and firestore subscription hook * fix(hooks): update useRoastProfile and useProfilesRealtime hooks * fix(ui): add delete confirm modal and update roast and profile card components * test: add unit tests for roast controller, service and client hooks * chore: update package-lock.json * fix: update import
* feat: criação busca e cadastro de skill * feat: alteração permissão para criar skill
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR adds two major features: (1) roast generation converted from request/response JSON to SSE streaming with a new delete endpoint wired end-to-end through server, client service, hook, and UI; (2) a Skills domain with entity, Firestore repository, service, security rules, and onboarding UI components that allow users to create and tag custom skills per category stored in ChangesRoast SSE Streaming and Delete
Skills Domain Feature
Sequence Diagram(s)sequenceDiagram
participant DiscoverPage
participant useRoastProfile
participant roastServiceClient as roast.service (client)
participant roastController as postRoast (server)
participant generateRoastStream
participant GeminiAPI
DiscoverPage->>useRoastProfile: executeRoast(persona)
useRoastProfile->>roastServiceClient: requestRoast(payload, onChunk)
roastServiceClient->>roastController: POST /roast (fetch SSE)
roastController->>generateRoastStream: iterate
generateRoastStream->>GeminiAPI: generateContentStream
GeminiAPI-->>generateRoastStream: chunk
generateRoastStream-->>roastController: yield chunk
roastController-->>roastServiceClient: data:{chunk}\n\n
roastServiceClient-->>useRoastProfile: onChunk(text)
useRoastProfile-->>DiscoverPage: streamingText updated
roastController-->>roastServiceClient: data:[DONE]\n\n
roastServiceClient-->>useRoastProfile: {roast: fullText}
useRoastProfile-->>DiscoverPage: selectedProfile roast field updated
sequenceDiagram
participant Onboarding
participant TagCategoryCard
participant useOnboardingForm
participant SkillService
participant FirebaseSkillRepository
participant Firestore
Onboarding->>TagCategoryCard: onCreateSkill(name, categoryKey)
TagCategoryCard->>useOnboardingForm: handleCreateSkillInCategory(name, categoryKey)
useOnboardingForm->>SkillService: createOrGetSkill(rawName, category)
SkillService->>FirebaseSkillRepository: getSkillById(normalizedName)
FirebaseSkillRepository->>Firestore: getDoc(skills/normalizedName)
Firestore-->>FirebaseSkillRepository: null (not found)
FirebaseSkillRepository-->>SkillService: null
SkillService->>FirebaseSkillRepository: createSkill(skill)
FirebaseSkillRepository->>Firestore: setDoc(skills/normalizedName)
SkillService-->>useOnboardingForm: Skill
useOnboardingForm->>useOnboardingForm: append tag to extraTagsByCategory[categoryKey]
useOnboardingForm-->>TagCategoryCard: updated extraTags
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@firestore.rules`:
- Around line 62-63: The validation for extraTagsByCategory in the Firestore
rules is too permissive because it only checks that the field is a map. Update
the rule around the extraTagsByCategory constraint to also verify that each
category value is an array of strings (user-created skill names), not arbitrary
nested data. Keep the check localized to the existing profile validation logic
so malformed onboarding data cannot be stored.
- Around line 146-154: The profile rules currently whitelist visibility in both
the create and update paths, but isValidProfile() does not validate that field.
Update the firestore.rules profile validation so isValidProfile() explicitly
constrains visibility to the expected type/allowed values, then keep the
visibility key only if that validator enforces it; otherwise remove visibility
from the incoming().keys().hasOnly and diff().affectedKeys().hasOnly checks.
- Around line 83-91: Lock down isValidSkill() before enabling client writes: it
currently only checks a required subset and still allows extra fields,
unvalidated createdAt, and a potentially non-canonical id. Update the validation
in firestore.rules so isValidSkill enforces an exact / skills document shape,
validates createdAt as a proper timestamp, and ensures id matches the canonical
skill identifier expected by FirebaseSkillRepository. Also apply the same
stricter checks to the related skill update/create rule block referenced in the
review.
In `@get-rules.ts`:
- Around line 4-9: The Firebase setup currently parses FIREBASE_SERVICE_ACCOUNT
and initializes admin before the try/catch can handle failures, so move the env
parsing and admin.initializeApp setup inside the existing try block in
get-rules.ts. Keep the JSON.parse(process.env.FIREBASE_SERVICE_ACCOUNT!) and
admin.credential.cert(serviceAccount) usage together with the rest of the
initialization, and let the catch report missing or malformed configuration
through the existing error handling path.
- Around line 11-12: The run() flow in get-rules.ts creates the securityRules
service but never uses it, so the script does not actually fetch or display the
Firestore rules. Update run() to call the appropriate ruleset method on the
service returned by admin.securityRules(), then print or otherwise output the
retrieved rules so the script performs the intended action.
In `@package.json`:
- Line 63: The firebase-admin package is being used by runtime server modules,
so it must remain in dependencies rather than devDependencies. Update the
package metadata to keep firebase-admin under dependencies, and verify the
runtime imports in firebase-admin.server, roast.repository, and firebase.admin
can still resolve after production installs that prune dev deps.
In `@src/application/services/SkillService.ts`:
- Around line 6-16: The SkillService.createOrGetSkill flow is re-implementing
skill name normalization locally via normalizeSkillName instead of using the
shared helper, which risks drift. Update SkillService to import and use the
shared normalizeSkillName from the shared utility module for the normalizedName
calculation, keeping the existing createOrGetSkill and repository lookup
behavior unchanged.
In `@src/features/discover/hooks/__tests__/useRoastProfile.test.tsx`:
- Around line 53-73: The current `useRoastProfile` test only checks the final
cleared state and never proves `streamingText` updates while chunks arrive.
Update the `requestRoast` mock in `accumulates streamingText as onChunk is
called` to keep the promise pending after the first `onChunk`, then assert the
intermediate `streamingText` value (for example, after `openProfile` and
`executeRoast`) before resolving. Keep the final success assertion too, so the
test validates both chunk accumulation and the post-success reset in
`useRoastProfile`.
In `@src/features/discover/hooks/useRoastProfile.ts`:
- Around line 20-31: Scope the roast stream to the profile that initiated it in
useRoastProfile: the current global streamingText and onSuccess
setSelectedProfile logic can apply late SSE chunks or completions to a different
selection. Track the in-flight memberId (or cancel the previous request) inside
roastMutation/requestRoast and ignore chunk updates plus success handling once
the selected profile changes, so only the active profile’s modal/text is
updated.
- Around line 43-46: The delete success callback in useRoastProfile is clearing
the roast field on whichever selectedProfile is current, which can wipe the
wrong member if selection changes before the request completes. Update the
onSuccess handler for the delete mutation to use the mutation’s memberId and
only call setSelectedProfile when prev.id matches that memberId, keeping the
existing persona-to-field logic for roastBrutal and roastMild and leaving
onDeleteSuccess unchanged.
In `@src/features/onboarding/components/GuildPassport.tsx`:
- Around line 71-75: The tooltip on GuildPassport’s name field should match the
rendered fallback, since the current title expression in the component leaves it
empty when the UI shows “Aguardando...”. Update the title in GuildPassport to
reuse the same fallback logic as the displayed text so both the visible label
and tooltip stay consistent.
In `@src/features/onboarding/components/SkillAutocomplete.tsx`:
- Around line 149-182: The autocomplete actions in SkillAutocomplete are only
wired through onMouseDown, which makes the result items and “Adicionar” button
unusable via keyboard. Keep the blur-prevention behavior in onMouseDown if
needed, but move the actual handleSelect(skill) and handleCreate() triggers to
onClick on those buttons so Enter/Space activation works for accessibility.
- Around line 98-111: Add a programmatic accessible name to the autocomplete
input in SkillAutocomplete by updating the input element to include an explicit
label via a <label> or an aria-label/aria-labelledby. Keep the existing input
behavior in the component, but ensure the textbox in this search/add skill
control is announced with a meaningful label for assistive technologies.
In `@src/features/onboarding/components/SkillSelectorCard.tsx`:
- Around line 41-45: The skill selector input in SkillSelectorCard is missing an
accessible name. Update the input to include a visible label or an
aria-label/aria-labelledby tied to the search/create field, and keep the
existing placeholder as supplemental text only. Use the input element in
SkillSelectorCard as the target for the accessibility fix.
In `@src/features/onboarding/components/TagCategoryCard.tsx`:
- Around line 161-195: The dropdown actions in TagCategoryCard are currently
wired only through onMouseDown, so keyboard activation on the focused buttons
won’t trigger select/create behavior. Keep the preventDefault handling on
onMouseDown to avoid blur, but move the actual tag selection and handleCreate
logic to onClick for both the tag button and the “Adicionar” button so they work
for mouse and keyboard users.
- Around line 118-131: The search input in TagCategoryCard is missing an
accessible name, so fix the onboarding control by giving the input a proper
label. Update the input in TagCategoryCard to include either a visible <label>
tied to the input or an aria-label/aria-labelledby on the input element; do not
rely on the placeholder text alone.
In `@src/features/onboarding/hooks/useOnboardingForm.ts`:
- Around line 117-124: The onboarding form state is preserving stale custom tags
when a profile/member document does not include extraTagsByCategory. Update
useOnboardingForm so the data-loading path always resets that state with
data.extraTagsByCategory ?? {} instead of only setting it when present, and also
clear it in the fallback branch alongside the existing setForm update. Focus on
the state updates around setExtraTagsByCategory and the form hydration logic.
- Around line 143-155: The skill creation flow in useOnboardingForm currently
swallows Firestore/rules errors by logging in the catch and then resolving,
which makes TagCategoryCard think onCreateSkill succeeded. Update the create
skill path around skillService.createOrGetSkill and the setExtraTagsByCategory
update to propagate failure back to the caller by re-throwing the caught error
or returning an explicit failure result that TagCategoryCard can inspect. Keep
the existing error log, but ensure the promise does not resolve successfully
when creation fails so the UI can keep the dropdown open and handle the error.
In `@src/infrastructure/firebase/skillRepository.ts`:
- Around line 15-18: Preserve the Firestore path ID when hydrating Skill in
skillRepository by making sure the canonical doc.id cannot be overwritten by
stored data. Update the mapping in the snapshot.docs map (and the similar
hydration path around the other referenced block) so the resulting Skill.id
always comes from doc.id, with any fields from doc.data() merged in a way that
excludes or overrides a persisted id value.
In `@src/server/features/roast/roast.controller.ts`:
- Around line 9-16: Validate the incoming persona before invoking
deleteProfileRoast so invalid query values do not delete the generic roast
field. In roast.controller.ts, replace the unsafe RoastPersona cast on
req.query.persona with an explicit runtime check against the allowed persona
values and return a 400 error for unknown values; apply the same validation in
the other delete handler referenced by deleteProfileRoast so both paths enforce
the same guard.
In `@src/server/features/roast/roast.service.ts`:
- Around line 23-33: The streamed roast handling in roast.service’s generator
currently always calls saveProfileRoast after the loop, which can persist an
empty string when result yields no text. Update the logic around fullText
accumulation and saveProfileRoast so that only non-empty generated content is
saved; if no text was received from result, skip persisting and let the caller
handle the empty generation case instead of overwriting the existing roast with
"".
In `@src/shared/components/ui/DeleteRoastConfirmModal.tsx`:
- Around line 23-29: The DeleteRoastConfirmModal dismissal paths still allow
closing while deletion is in progress; update the dismiss handlers in
DeleteRoastConfirmModal so the backdrop and X button both respect isDeleting and
cannot trigger onCancel during an active delete. While adjusting the X button in
the modal header, also set its type to button, and keep the existing footer
disable behavior aligned with the same isDeleting guard.
In `@src/shared/components/ui/RoastModal.tsx`:
- Around line 121-129: The delete button in RoastModal should have an explicit
button type so it does not default to submitting a parent form. Update the
button rendered under the onDeleteRoast and roastText condition to include
type="button" alongside the existing onClick={onDeleteRoast}, preserving the
current styling and disabled behavior.
In `@src/shared/lib/utils/normalizeSkillName.ts`:
- Around line 4-5: The normalizeSkillName helper still leaves characters like
"/" in the normalized value, which can break Firestore document ID usage in
FirebaseSkillRepository.createSkill. Update normalizeSkillName to sanitize the
result into a Firestore-safe document ID (for example, by replacing or removing
path-separator characters after trimming/lowercasing) while keeping the existing
normalization behavior, and make sure any code relying on skill.normalizedName
continues to use the safe value.
In `@src/shared/services/__tests__/roast.service.test.ts`:
- Around line 8-18: The current `sseStream` helper in `roast.service.test.ts`
emits all SSE data as one buffered chunk, so `requestRoast()`’s multi-read
parsing path is not exercised. Update the tests around
`sseStream`/`requestRoast` to build a fragmented `ReadableStream` that splits at
least one `data:` event and the `[DONE]` marker across separate
`controller.enqueue` calls, then assert the parser still reconstructs the stream
correctly. Add the new coverage alongside the existing `requestRoast` cases so
split-frame handling is verified even when `reader.read()` returns partial
payloads.
In `@test-rules.js`:
- Around line 12-31: The new /skills coverage in test-rules.js only exercises
the allowed paths, so add negative assertions for the Skills rules. In the same
test block around authenticatedContext, getDoc, and setDoc, verify
unauthenticated reads/creates are rejected, creating a skill with mismatched
skillId or normalizedName is denied, and update/delete attempts on the skills
document are also rejected. Use the existing Firestore test helpers
(assertFails/assertSucceeds) alongside the current alice.firestore() and
skills/react document setup to cover these deny paths.
In `@test-skill.ts`:
- Around line 18-21: The skill creation write in setDoc for skills is currently
using the client SDK without any authentication, so it will not exercise the
intended security rule. Update the test flow in test-skill.ts around the setDoc
call to sign in first with an authenticated user (using the existing auth
setup/helpers in the test) before attempting to create the skill document, and
then assert the expected authenticated-only behavior rather than an
unauthenticated client write. If this script is meant to verify rejection, keep
the write unauthenticated and assert permission-denied explicitly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cb94bd0d-025f-454f-97b0-a42cdc1acb88
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
firebase.jsonfirestore.rulesget-rules.tspackage.jsonsrc/application/factories/skillServiceFactory.tssrc/application/services/SkillService.tssrc/domain/entities/Skill.tssrc/domain/ports/ISkillRepository.tssrc/features/discover/components/RoastModal.tsxsrc/features/discover/hooks/__tests__/useRoastProfile.test.tsxsrc/features/discover/hooks/useProfilesRealtime.tssrc/features/discover/hooks/useRoastProfile.tssrc/features/discover/pages/DiscoverPage.tsxsrc/features/onboarding/components/GuildPassport.tsxsrc/features/onboarding/components/SkillAutocomplete.tsxsrc/features/onboarding/components/SkillSelectorCard.tsxsrc/features/onboarding/components/SkillSliders.tsxsrc/features/onboarding/components/TagCategoryCard.tsxsrc/features/onboarding/constants/tagCategories.tssrc/features/onboarding/hooks/useOnboardingForm.tssrc/features/onboarding/pages/Onboarding.tsxsrc/infrastructure/firebase/skillRepository.tssrc/server/features/roast/__tests__/roast.controller.test.tssrc/server/features/roast/__tests__/roast.service.test.tssrc/server/features/roast/roast.controller.tssrc/server/features/roast/roast.repository.tssrc/server/features/roast/roast.routes.tssrc/server/features/roast/roast.service.tssrc/server/shared/lib/firebase-admin.server.tssrc/shared/components/ui/DeleteRoastConfirmModal.tsxsrc/shared/components/ui/ProfileCard.tsxsrc/shared/components/ui/RoastModal.tsxsrc/shared/hooks/useFirestoreSubscription.tssrc/shared/lib/utils/normalizeSkillName.tssrc/shared/services/__tests__/roast.service.test.tssrc/shared/services/roast.service.tstest-rules.jstest-skill.ts
💤 Files with no reviewable changes (1)
- src/features/discover/hooks/useProfilesRealtime.ts
Comment on lines
+62
to
+63
| // extraTagsByCategory: mapa de categoria → lista de nomes de skills criadas pelo usuário | ||
| && (!('extraTagsByCategory' in data) || data.extraTagsByCategory is map) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the contents of extraTagsByCategory, not just that it is a map.
The PR contract here is category → list of user-created skill names, but this rule accepts any nested structure. Malformed profile data can persist numbers or objects under a category and then break the onboarding code that reads these values as string arrays.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@firestore.rules` around lines 62 - 63, The validation for extraTagsByCategory
in the Firestore rules is too permissive because it only checks that the field
is a map. Update the rule around the extraTagsByCategory constraint to also
verify that each category value is an array of strings (user-created skill
names), not arbitrary nested data. Keep the check localized to the existing
profile validation logic so malformed onboarding data cannot be stored.
Comment on lines
+83
to
+91
| function isValidSkill(data) { | ||
| return data.keys().hasAll(['id', 'name', 'normalizedName', 'status', 'usageCount', 'createdBy', 'createdAt']) | ||
| && data.id is string && data.id.size() > 0 && data.id.size() <= 128 | ||
| && data.name is string && data.name.size() > 0 && data.name.size() <= 100 | ||
| && data.normalizedName is string && data.normalizedName.size() > 0 && data.normalizedName.size() <= 100 | ||
| && data.status is string && data.status in ['pending', 'approved', 'rejected'] | ||
| && data.usageCount is number && data.usageCount >= 0 | ||
| && data.createdBy is string | ||
| && (!('category' in data) || data.category == null || (data.category is string && data.category.size() <= 50)); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Lock down the /skills schema before opening client writes.
isValidSkill() only enforces a minimum field set. Any authenticated client can still add arbitrary extra fields, send a non-canonical id, and bypass the Skill timestamp contract because createdAt is never validated. FirebaseSkillRepository then casts that payload straight back to Skill.
Also applies to: 99-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@firestore.rules` around lines 83 - 91, Lock down isValidSkill() before
enabling client writes: it currently only checks a required subset and still
allows extra fields, unvalidated createdAt, and a potentially non-canonical id.
Update the validation in firestore.rules so isValidSkill enforces an exact /
skills document shape, validates createdAt as a proper timestamp, and ensures id
matches the canonical skill identifier expected by FirebaseSkillRepository. Also
apply the same stricter checks to the related skill update/create rule block
referenced in the review.
Comment on lines
+146
to
+154
| && incoming().keys().hasOnly(['userId', 'name', 'photoURL', 'github', 'linkedin', 'bio', 'primaryRole', 'secondaryRoles', 'skills', 'canvas', 'status', 'squadId', 'eventId', 'roast', 'roastBrutal', 'roastMild', 'createdAt', 'updatedAt', 'visibility', 'extraTagsByCategory']); | ||
|
|
||
| allow update: if isSignedIn() | ||
| && isValidId(profileId) | ||
| && isValidProfile(incoming()) | ||
| && incoming().userId == existing().userId | ||
| && incoming().createdAt == existing().createdAt | ||
| && ( | ||
| (incoming().diff(existing()).affectedKeys().hasOnly(['skills', 'canvas', 'name', 'photoURL', 'github', 'linkedin', 'bio', 'primaryRole', 'secondaryRoles', 'status', 'squadId', 'eventId', 'updatedAt', 'roast', 'roastBrutal', 'roastMild', 'visibility'])) | ||
| (incoming().diff(existing()).affectedKeys().hasOnly(['skills', 'canvas', 'name', 'photoURL', 'github', 'linkedin', 'bio', 'primaryRole', 'secondaryRoles', 'status', 'squadId', 'eventId', 'updatedAt', 'roast', 'roastBrutal', 'roastMild', 'visibility', 'extraTagsByCategory'])) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Don't whitelist visibility until isValidProfile() constrains it.
These lines allow visibility on profile create/update, but the profile validator never checks its type or allowed values. Right now any authenticated client can store arbitrary data in that field.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@firestore.rules` around lines 146 - 154, The profile rules currently
whitelist visibility in both the create and update paths, but isValidProfile()
does not validate that field. Update the firestore.rules profile validation so
isValidProfile() explicitly constrains visibility to the expected type/allowed
values, then keep the visibility key only if that validator enforces it;
otherwise remove visibility from the incoming().keys().hasOnly and
diff().affectedKeys().hasOnly checks.
Comment on lines
+4
to
+9
| const serviceAccount = JSON.parse(process.env.FIREBASE_SERVICE_ACCOUNT!); | ||
| const dbId = process.env.VITE_FIREBASE_FIRESTORE_DATABASE_ID!; | ||
|
|
||
| admin.initializeApp({ | ||
| credential: admin.credential.cert(serviceAccount), | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard env parsing and app init inside the try.
A missing or malformed FIREBASE_SERVICE_ACCOUNT throws before your catch runs, so the script exits with an uncaught error instead of a clear diagnostic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@get-rules.ts` around lines 4 - 9, The Firebase setup currently parses
FIREBASE_SERVICE_ACCOUNT and initializes admin before the try/catch can handle
failures, so move the env parsing and admin.initializeApp setup inside the
existing try block in get-rules.ts. Keep the
JSON.parse(process.env.FIREBASE_SERVICE_ACCOUNT!) and
admin.credential.cert(serviceAccount) usage together with the rest of the
initialization, and let the catch report missing or malformed configuration
through the existing error handling path.
Comment on lines
+11
to
+12
| async function run() { | ||
| const securityRules = admin.securityRules(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does firebase-admin 14.1.0 expose a top-level securityRules() API for retrieving Firestore security rules?
💡 Result:
Yes, firebase-admin 14.1.0 provides a top-level securityRules API that allows you to access the SecurityRules service for managing and retrieving Firestore security rules [1][2][3]. You can access this service by calling admin.securityRules [2][3]. Once you have the SecurityRules instance, you can use the getFirestoreRuleset method to retrieve the ruleset currently applied to Cloud Firestore [4][5][6]. Example usage: const admin = require('firebase-admin'); admin.initializeApp; // Retrieve the currently applied Firestore ruleset admin.securityRules.getFirestoreRuleset.then((ruleset) => { console.log('Current Firestore ruleset:', ruleset); }).catch((error) => { console.error('Error retrieving ruleset:', error); }); This API was originally introduced in earlier versions of the SDK [1][7] and remains available in version 14.1.0 [8][9].
Citations:
- 1: https://firebase.google.com/support/release-notes/admin/node
- 2: https://firebase.google.cn/docs/reference/admin/node/firebase-admin.security-rules
- 3: https://cdn.jsdelivr.net/npm/firebase-admin@13.0.2/lib/security-rules/security-rules-namespace.d.ts
- 4: https://firebase.google.com/docs/reference/admin/node/firebase-admin.security-rules.securityrules
- 5: https://firebase.google.com/docs/rules/manage-deploy
- 6: https://cdn.jsdelivr.net/npm/firebase-admin@13.0.2/lib/security-rules/security-rules.d.ts
- 7: feat: Security Rules Management API firebase/firebase-admin-node#645
- 8: https://github.com/firebase/firebase-admin-node/releases/tag/v14.1.0
- 9: https://registry.npmjs.org/firebase-admin
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target file.
git ls-files | rg '^get-rules\.ts$|/get-rules\.ts$|^.*get-rules\.ts$' || true
if [ -f get-rules.ts ]; then
wc -l get-rules.ts
cat -n get-rules.ts
fi
# Check the firebase-admin version declared in package files, if present.
rg -n '"firebase-admin"|firebase-admin' package.json package-lock.json pnpm-lock.yaml yarn.lock --no-heading || true
# Look for other uses of securityRules in the repo.
rg -n 'securityRules\(' . --no-heading || trueRepository: MatchDock/match-tech
Length of output: 2302
🌐 Web query:
firebase-admin 14.1.0 securityRules is it a function or a property? show TypeScript definitions or docs for admin.securityRules usage
💡 Result:
In the Firebase Admin SDK, admin.securityRules() is a function, not a property [1][2]. When you call admin.securityRules(), it returns an instance of the SecurityRules class, which provides the service interface for managing Cloud Firestore and Cloud Storage security rules [3][1]. TypeScript Definition The function signature for admin.securityRules() is defined as follows: export declare function getSecurityRules(app?: App): SecurityRules; Usage You call the function to obtain the service instance, which you then use to manage your rules. Example (Basic): const securityRules = admin.securityRules; await securityRules.releaseFirestoreRulesetFromSource(source); Example (Specific App): const otherSecurityRules = admin.securityRules(otherApp); The SecurityRules class itself contains various methods (such as getFirestoreRuleset, createRuleset, and releaseFirestoreRulesetFromSource) that you use to programmatically manage your rulesets [3][4]. Note that this service is specifically for Cloud Firestore and Cloud Storage; for Realtime Database rules, you instead use the getRules and setRules methods available on the admin.database() service [5][6].
Citations:
- 1: https://firebase.google.com/docs/reference/admin/node/firebase-admin.security-rules
- 2: https://modular-admin.web.app/reference/firebase-admin.security-rules/
- 3: https://firebase.google.com/docs/reference/admin/node/firebase-admin.security-rules.securityrules
- 4: https://cdn.jsdelivr.net/npm/firebase-admin@13.0.2/lib/security-rules/security-rules.d.ts
- 5: https://firebase.google.com/docs/rules/manage-deploy
- 6: https://firebase.google.com/docs/reference/admin/node/firebase-admin.database.database
Call a ruleset method on the returned service get-rules.ts:12 The securityRules() call is valid, but the returned service is never used, so this script still doesn’t retrieve or print any Firestore rules.
🧰 Tools
🪛 ESLint
[error] 12-12: 'securityRules' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@get-rules.ts` around lines 11 - 12, The run() flow in get-rules.ts creates
the securityRules service but never uses it, so the script does not actually
fetch or display the Firestore rules. Update run() to call the appropriate
ruleset method on the service returned by admin.securityRules(), then print or
otherwise output the retrieved rules so the script performs the intended action.
Comment on lines
+121
to
+129
| {onDeleteRoast && roastText && ( | ||
| <button | ||
| className="py-4 px-5 border-[3px] border-black bg-neo-pink hover:bg-white text-white hover:text-neo-pink font-black transition-all cursor-pointer shadow-[4px_4px_0_0_#000] hover:shadow-none -translate-y-0.5 hover:translate-y-0 active:translate-y-1 flex items-center justify-center shrink-0" | ||
| disabled={isGenerating} | ||
| onClick={onDeleteRoast} | ||
| title="Apagar este veredito" | ||
| > | ||
| <Trash2 className="w-5 h-5" /> | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Give the delete button an explicit type.
Without type="button", this control will submit any surrounding form by default.
Suggested fix
{onDeleteRoast && roastText && (
<button
+ type="button"
className="py-4 px-5 border-[3px] border-black bg-neo-pink hover:bg-white text-white hover:text-neo-pink font-black transition-all cursor-pointer shadow-[4px_4px_0_0_#000] hover:shadow-none -translate-y-0.5 hover:translate-y-0 active:translate-y-1 flex items-center justify-center shrink-0"
disabled={isGenerating}
onClick={onDeleteRoast}
title="Apagar este veredito"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {onDeleteRoast && roastText && ( | |
| <button | |
| className="py-4 px-5 border-[3px] border-black bg-neo-pink hover:bg-white text-white hover:text-neo-pink font-black transition-all cursor-pointer shadow-[4px_4px_0_0_#000] hover:shadow-none -translate-y-0.5 hover:translate-y-0 active:translate-y-1 flex items-center justify-center shrink-0" | |
| disabled={isGenerating} | |
| onClick={onDeleteRoast} | |
| title="Apagar este veredito" | |
| > | |
| <Trash2 className="w-5 h-5" /> | |
| </button> | |
| {onDeleteRoast && roastText && ( | |
| <button | |
| type="button" | |
| className="py-4 px-5 border-[3px] border-black bg-neo-pink hover:bg-white text-white hover:text-neo-pink font-black transition-all cursor-pointer shadow-[4px_4px_0_0_#000] hover:shadow-none -translate-y-0.5 hover:translate-y-0 active:translate-y-1 flex items-center justify-center shrink-0" | |
| disabled={isGenerating} | |
| onClick={onDeleteRoast} | |
| title="Apagar este veredito" | |
| > | |
| <Trash2 className="w-5 h-5" /> | |
| </button> |
🧰 Tools
🪛 React Doctor (0.5.8)
[warning] 122-122: Your users can submit the form by accident because a <button> with no type defaults to submit.
Set an explicit button type so plain buttons do not submit forms by accident: type="button", "submit", or "reset".
(button-has-type)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shared/components/ui/RoastModal.tsx` around lines 121 - 129, The delete
button in RoastModal should have an explicit button type so it does not default
to submitting a parent form. Update the button rendered under the onDeleteRoast
and roastText condition to include type="button" alongside the existing
onClick={onDeleteRoast}, preserving the current styling and disabled behavior.
Source: Linters/SAST tools
Comment on lines
+4
to
+5
| export function normalizeSkillName(name: string): string { | ||
| return name.trim().toLowerCase().replace(/\s+/g, " "); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the normalized skill name safe for Firestore document IDs.
normalizeSkillName("CI/CD") still returns ci/cd, and FirebaseSkillRepository.createSkill() uses that value as doc(db, "skills", skill.normalizedName). Any skill name containing / will fail when building the document path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shared/lib/utils/normalizeSkillName.ts` around lines 4 - 5, The
normalizeSkillName helper still leaves characters like "/" in the normalized
value, which can break Firestore document ID usage in
FirebaseSkillRepository.createSkill. Update normalizeSkillName to sanitize the
result into a Firestore-safe document ID (for example, by replacing or removing
path-separator characters after trimming/lowercasing) while keeping the existing
normalization behavior, and make sure any code relying on skill.normalizedName
continues to use the safe value.
Comment on lines
+8
to
+18
| function sseStream(...events: string[]) { | ||
| const data = events.map((e) => `data: ${e}\n\n`).join(""); | ||
| const encoder = new TextEncoder(); | ||
| const bytes = encoder.encode(data); | ||
| return new ReadableStream<Uint8Array>({ | ||
| start(controller) { | ||
| controller.enqueue(bytes); | ||
| controller.close(); | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Exercise fragmented SSE frames, not a single buffered payload.
Line 8 only enqueues one Uint8Array, so the new parser path in requestRoast() that buffers across multiple reader.read() calls is never tested. A bug in split-frame handling would still pass this suite. Add at least one case where a data: event and the [DONE] marker are split across separate chunks.
Proposed test shape
function sseStream(...events: string[]) {
- const data = events.map((e) => `data: ${e}\n\n`).join("");
const encoder = new TextEncoder();
- const bytes = encoder.encode(data);
return new ReadableStream<Uint8Array>({
start(controller) {
- controller.enqueue(bytes);
+ for (const chunk of events) {
+ controller.enqueue(encoder.encode(chunk));
+ }
controller.close();
},
});
}
+it("handles SSE frames split across multiple reads", async () => {
+ mockFetch.mockResolvedValue({
+ ok: true,
+ body: sseStream(
+ 'data: {"chunk":"Olá',
+ ' "}\n\n',
+ 'data: {"chunk":"mundo"}\n',
+ '\n',
+ 'data: [DO',
+ 'NE]\n\n',
+ ),
+ });
+
+ await expect(
+ requestRoast({ memberId: "u1", memberData: {}, persona: "brutal" }),
+ ).resolves.toEqual({ roast: "Olá mundo" });
+});Also applies to: 24-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shared/services/__tests__/roast.service.test.ts` around lines 8 - 18, The
current `sseStream` helper in `roast.service.test.ts` emits all SSE data as one
buffered chunk, so `requestRoast()`’s multi-read parsing path is not exercised.
Update the tests around `sseStream`/`requestRoast` to build a fragmented
`ReadableStream` that splits at least one `data:` event and the `[DONE]` marker
across separate `controller.enqueue` calls, then assert the parser still
reconstructs the stream correctly. Add the new coverage alongside the existing
`requestRoast` cases so split-frame handling is verified even when
`reader.read()` returns partial payloads.
Comment on lines
+12
to
+31
| const alice = testEnv.authenticatedContext("alice", { email: "alice@example.com" }); | ||
|
|
||
| // Test reading skills | ||
| console.log("Testing read skill..."); | ||
| await assertSucceeds(getDoc(doc(alice.firestore(), "skills", "react"))); | ||
|
|
||
| // Test creating skill | ||
| console.log("Testing create skill..."); | ||
| await assertSucceeds( | ||
| setDoc(doc(alice.firestore(), "skills", "react"), { | ||
| id: "react", | ||
| name: "React", | ||
| normalizedName: "react", | ||
| category: "core_tech", | ||
| status: "pending", | ||
| usageCount: 1, | ||
| createdBy: "", | ||
| createdAt: serverTimestamp(), | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add deny-path assertions for the new /skills rules.
These tests only prove the happy path. A regression that permits unauthenticated creates, mismatched skillId/normalizedName, or updates/deletes would still pass unnoticed.
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 20-29: React's useState should not be directly called
Context: setDoc(doc(alice.firestore(), "skills", "react"), {
id: "react",
name: "React",
normalizedName: "react",
category: "core_tech",
status: "pending",
usageCount: 1,
createdBy: "",
createdAt: serverTimestamp(),
})
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test-rules.js` around lines 12 - 31, The new /skills coverage in
test-rules.js only exercises the allowed paths, so add negative assertions for
the Skills rules. In the same test block around authenticatedContext, getDoc,
and setDoc, verify unauthenticated reads/creates are rejected, creating a skill
with mismatched skillId or normalizedName is denied, and update/delete attempts
on the skills document are also rejected. Use the existing Firestore test
helpers (assertFails/assertSucceeds) alongside the current alice.firestore() and
skills/react document setup to cover these deny paths.
Comment on lines
+18
to
+21
| await setDoc(doc(db, "skills", skill.normalizedName), { | ||
| ...skill, | ||
| createdAt: serverTimestamp(), | ||
| }, { merge: false }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
This write path is unauthenticated, so the new rules should reject it.
skills creation is authenticated-only, but this script uses the client SDK without signing in first. As written, it won't validate the feature flow; it should fail with permission-denied.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test-skill.ts` around lines 18 - 21, The skill creation write in setDoc for
skills is currently using the client SDK without any authentication, so it will
not exercise the intended security rule. Update the test flow in test-skill.ts
around the setDoc call to sign in first with an authenticated user (using the
existing auth setup/helpers in the test) before attempting to create the skill
document, and then assert the expected authenticated-only behavior rather than
an unauthenticated client write. If this script is meant to verify rejection,
keep the write unauthenticated and assert permission-denied explicitly.
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
3 participants
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.
Título do PR:
Feat: Criação dinâmica e Busca de Skills no Onboarding (Skill Selector)
Descrição:
Qual é o objetivo deste PR?
Implementar a funcionalidade completa de seleção, busca e criação de tags/skills personalizadas durante a etapa de onboarding (ARSENAL_DE_SKILLS), permitindo que os usuários adicionem tecnologias que não estão listadas por padrão.
Mudanças Principais:
skillsutilizando Clean Architecture (SkillServiceeSkillRepository).firestore.rulespara validar a criação dinâmica com total segurança (isValidSkill).max-height(400px) eoverflow-y-autona lista de tags para evitar que a página cresça infinitamente.Testes Realizados:
Summary by CodeRabbit
New Features
Bug Fixes
Tests