Merged
Conversation
WalkthroughAdds scoped rate limiting (100 req/min) around the deployments API in Fastify, introduces a getBearerToken utility, updates the deployments router and unit tests to use it, and applies large documentation and brand/content changes across the docs site (many new, updated, and deleted MDX files and site config). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 41 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/fastify.ts">
<violation number="1" location="apps/api/src/fastify.ts:56">
P1: Using the unvalidated `Authorization` header as the rate-limit key lets callers bypass the new limit by rotating fake tokens.</violation>
</file>
<file name="apps/docs/platform/deployments.mdx">
<violation number="1" location="apps/docs/platform/deployments.mdx:12">
P2: This image path does not exist in the repository, so the Demo section will render a broken image.</violation>
</file>
<file name="apps/docs/automations/incident-detection.mdx">
<violation number="1" location="apps/docs/automations/incident-detection.mdx:40">
P2: The branch regex example is incorrect: `hotfix/*` will not match branches like `hotfix/foo`. Use a pattern such as `^hotfix/.*` so the documented rule actually detects hotfix branches.</violation>
</file>
<file name="apps/docs/metrics-and-insights/dora.mdx">
<violation number="1" location="apps/docs/metrics-and-insights/dora.mdx:12">
P2: The demo image points to `/images/dora.png`, but that asset is not present in the repository, so this page will show a broken image.</violation>
</file>
<file name="apps/docs/platform/environments.mdx">
<violation number="1" location="apps/docs/platform/environments.mdx:29">
P2: These example names will not be treated as production environments, so deployments to `production-us`/`production-eu` are excluded from default DORA metrics.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/docs/metrics-and-insights/team/code-review-distribution.mdx (1)
48-48: Consider a small wording polish for readability.“Assert your team members have…” reads a bit stiff here; “ensure communication stays healthy” is clearer.
✍️ Suggested copy tweak
-- Make sure your team feels safe giving feedback. Go through your team's <a href="/metrics-and-insights/pull-requests">Pull Requests</a> to assert your team members have a healthy communication style. +- Make sure your team feels safe giving feedback. Go through your team's <a href="/metrics-and-insights/pull-requests">Pull Requests</a> to ensure communication stays respectful and healthy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/metrics-and-insights/team/code-review-distribution.mdx` at line 48, Replace the stiff phrase "assert your team members have a healthy communication style" with clearer wording like "ensure communication stays healthy" in the sentence that links to /metrics-and-insights/pull-requests (the line containing "Make sure your team feels safe giving feedback. Go through your team's <a href=\"/metrics-and-insights/pull-requests\">Pull Requests</a>..."); update the copy so the full sentence reads smoothly (for example: "Make sure your team feels safe giving feedback. Go through your team's Pull Requests to ensure communication stays healthy.") and keep the existing link target unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/fastify.ts`:
- Around line 53-56: The rate-limiting keyGenerator currently uses the raw
request.headers.authorization which fragments buckets; update the keyGenerator
in the rateLimit registration to normalize the authorization value: read
request.headers.authorization, trim whitespace, remove a case-insensitive
"Bearer" prefix if present, then use that normalized token (or fallback to
request.ip) as the key; optionally hash the normalized token before returning to
avoid storing raw credentials in memory. This change should be made inside the
existing keyGenerator function used when registering rateLimit.
In `@apps/docs/api-reference/deployments/create-deployment.mdx`:
- Around line 56-58: Update the curl examples in create-deployment.mdx so the
Authorization header sends the raw API key string (e.g., "Authorization:
YOUR_API_KEY") instead of using the "Bearer " prefix; locate the curl request
block that currently has -H "Authorization: Bearer YOUR_API_KEY" (the example
used for the /v1/deployments POST) and replace the header value to remove
"Bearer " in both occurrences (the example near the first curl block and the
repeated example later) so the server-side findApiKeyOrThrow receives the raw
key to hash and validate.
In `@apps/docs/automations/incident-detection.mdx`:
- Around line 35-40: Replace the glob example in the "Branch regex" table row
(the **Branch regex** entry) with a proper regular expression: use an anchored
pattern that begins with "hotfix/" and matches one or more characters after the
slash (escape the "/" if your renderer requires it) so branch names like
"hotfix/fix-login" are matched instead of the glob "hotfix/*".
In `@apps/docs/metrics-and-insights/team/time-to-merge.mdx`:
- Line 24: Fix the singular/plural mismatch in the sentence "Ideally, most small
Pull Requests should be merged within an hour of it being approved." by
replacing "it being approved" with a plural form such as "their approval" or
rephrasing to "they are approved" so the pronoun agrees with "Pull Requests".
In `@apps/docs/platform/environments.mdx`:
- Around line 26-29: Update the paragraph that lists "production-us" and
"production-eu" to explicitly state that only exact names "production" or "prod"
are auto-detected for default DORA metrics; for regional production environments
like "production-us" or "production-eu" you must explicitly set the production
flag so they will be counted. Reference the existing wording about
auto-detection (the sentence that currently says only exact `production`/`prod`
names are auto-detected) and add a short caveat after the bullet about multiple
production environments clarifying the need to set the production flag for
regional names.
---
Nitpick comments:
In `@apps/docs/metrics-and-insights/team/code-review-distribution.mdx`:
- Line 48: Replace the stiff phrase "assert your team members have a healthy
communication style" with clearer wording like "ensure communication stays
healthy" in the sentence that links to /metrics-and-insights/pull-requests (the
line containing "Make sure your team feels safe giving feedback. Go through your
team's <a href=\"/metrics-and-insights/pull-requests\">Pull Requests</a>...");
update the copy so the full sentence reads smoothly (for example: "Make sure
your team feels safe giving feedback. Go through your team's Pull Requests to
ensure communication stays healthy.") and keep the existing link target
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c445e0ae-972b-44b9-b4a5-1e611d6b2baa
📒 Files selected for processing (41)
apps/api/src/fastify.tsapps/docs/about/data-privacy-and-security.mdxapps/docs/about/open-source.mdxapps/docs/about/principles.mdxapps/docs/api-reference/authentication.mdxapps/docs/api-reference/deployments/create-deployment.mdxapps/docs/api-reference/introduction.mdxapps/docs/api-reference/rate-limit.mdxapps/docs/automations/alerts.mdxapps/docs/automations/digests.mdxapps/docs/automations/incident-detection.mdxapps/docs/automations/pr-size-labeler.mdxapps/docs/automations/pr-title-check.mdxapps/docs/docs.jsonapps/docs/features/automations/intro.mdxapps/docs/features/deployments.mdxapps/docs/features/environments.mdxapps/docs/get-started/get-started.mdxapps/docs/get-started/intro.mdxapps/docs/get-started/self-host.mdxapps/docs/metrics-and-insights/code-reviews.mdxapps/docs/metrics-and-insights/dora.mdxapps/docs/metrics-and-insights/pull-requests.mdxapps/docs/metrics-and-insights/team/code-review-distribution.mdxapps/docs/metrics-and-insights/team/cycle-time.mdxapps/docs/metrics-and-insights/team/intro.mdxapps/docs/metrics-and-insights/team/pr-size-distribution.mdxapps/docs/metrics-and-insights/team/time-to-approve.mdxapps/docs/metrics-and-insights/team/time-to-first-review.mdxapps/docs/metrics-and-insights/team/time-to-merge.mdxapps/docs/metrics-and-insights/wip.mdxapps/docs/metrics-and-insights/work-log.mdxapps/docs/mint.jsonapps/docs/platform/applications.mdxapps/docs/platform/deployments.mdxapps/docs/platform/environments.mdxapps/docs/platform/incidents.mdxapps/docs/platform/integrations.mdxapps/docs/platform/people.mdxapps/docs/platform/teams.mdxapps/docs/snippets/pr-badge.mdx
💤 Files with no reviewable changes (6)
- apps/docs/snippets/pr-badge.mdx
- apps/docs/features/automations/intro.mdx
- apps/docs/mint.json
- apps/docs/features/environments.mdx
- apps/docs/features/deployments.mdx
- apps/docs/get-started/get-started.mdx
Contributor
There was a problem hiding this comment.
4 issues found across 40 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/docs/metrics-and-insights/team/cycle-time.mdx">
<violation number="1" location="apps/docs/metrics-and-insights/team/cycle-time.mdx:40">
P3: This new definition conflicts with the page description, which still says cycle time starts when the PR opens. That mismatch can mislead readers about why the product shows a longer cycle time when work started before PR creation.</violation>
</file>
<file name="apps/docs/metrics-and-insights/team/time-to-approve.mdx">
<violation number="1" location="apps/docs/metrics-and-insights/team/time-to-approve.mdx:59">
P3: Document that the "ready" start only uses reviewer-requested/ready-for-review timestamps when they occur before the approval.</violation>
</file>
<file name="apps/docs/api-reference/deployments/create-deployment.mdx">
<violation number="1" location="apps/docs/api-reference/deployments/create-deployment.mdx:58">
P2: Document validation failures as `422`, not `400`, to match the endpoint's actual response.</violation>
</file>
<file name=".cursor/skills/sweetr-brand-voice/references/WEBSITE.md">
<violation number="1" location=".cursor/skills/sweetr-brand-voice/references/WEBSITE.md:28">
P3: Replace the em dash here so the reference copy matches the brand-voice rule that forbids em dashes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
apps/docs/automations/incident-detection.mdx (1)
31-35:⚠️ Potential issue | 🟡 MinorUse an actual regex in the branch example.
Line 34 uses
hotfix/*, which is glob syntax and misleading in a regex field.✏️ Suggested fix
-| **Branch regex** | Match branch names (e.g., `hotfix/*`). | +| **Branch regex** | Match branch names (e.g., `^hotfix/.*$`). |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/automations/incident-detection.mdx` around lines 31 - 35, The table row under "Branch regex" currently shows the glob-style example `hotfix/*` which is misleading for a regex field; update the example to a proper regular expression (for example `^hotfix\/.*$`) in the "Branch regex" row so it uses regex syntax instead of glob syntax, ensuring the cell that currently contains `hotfix/*` (the "Branch regex" entry) is replaced with a valid regex example.
🧹 Nitpick comments (6)
apps/docs/about/principles.mdx (1)
21-22: Tighten the sentence flow in this paragraph.Line 21-22 reads as a fragment. Consider merging into one continuous sentence for smoother readability.
✍️ Suggested copy edit
- We surface only what matters. Not to overwhelm you. Our tools are simple, - intuitive, and focused on what's actionable. + We surface only what matters so we don't overwhelm you. Our tools are + simple, intuitive, and focused on what's actionable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/about/principles.mdx` around lines 21 - 22, The two-line fragment "We surface only what matters. Not to overwhelm you. Our tools are simple, intuitive, and focused on what's actionable." in apps/docs/about/principles.mdx should be tightened into a single continuous sentence for smoother flow—merge the first two fragments into one sentence (e.g., "We surface only what matters so as not to overwhelm you.") and ensure the full paragraph reads cohesively with the following clause ("Our tools are simple, intuitive, and focused on what's actionable.")..cursor/skills/sweetr-brand-voice/references/BRAND_KIT.md (1)
13-13: Reduce legal/messaging ambiguity around “open platform.”Since the kit strictly distinguishes “fair source” from “open source,” add a short reusable note near the tagline that “open platform” is about openness/transparency, not OSI licensing, so this nuance is preserved when snippets are copied out of context.
Also applies to: 84-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.cursor/skills/sweetr-brand-voice/references/BRAND_KIT.md at line 13, Add a short reusable clarification next to the tagline "The open platform for Engineering Performance++" stating that "open platform" refers to openness/transparency and not OSI-style open-source licensing (for example: "(refers to openness/transparency, not OSI open-source licensing)"); insert the same concise note wherever the exact phrase "The open platform for Engineering Performance++" appears (including the other occurrence mentioned) and ensure it is formatted as a reusable snippet or parenthetical that can be copied with the tagline.apps/docs/metrics-and-insights/team/time-to-approve.mdx (1)
57-62: Minor inconsistency with time-to-first-review.mdx.The calculation section looks good overall. However, time-to-first-review.mdx includes the clarification "Only timestamps before the review are considered" in its Start definition (line 39), but this page omits a similar note about timestamps before approval. Consider adding this for consistency if it applies to this metric as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/metrics-and-insights/team/time-to-approve.mdx` around lines 57 - 62, Update the Start ("ready") definition in time-to-approve.mdx to match time-to-first-review.mdx by adding a clarifying sentence that only timestamps occurring before the approval are considered; specifically, after the current Start bullet ("The earliest available timestamp...") add a note like "Only timestamps before the first approval are considered" so it’s clear which candidate timestamps (first reviewer requested date, ready-for-review date, PR creation date) are valid. This ensures consistency with the Start definition behavior used in time-to-first-review.mdx.apps/docs/automations/digests.mdx (1)
43-45: Consider hyphenating "work-in-progress" when used as a compound modifier.When "work in progress" modifies a noun (e.g., "work-in-progress digest"), it's grammatically preferable to hyphenate it. This is a minor style consideration.
📝 Optional fix
-The work in progress digest includes a summary of all the team's open Pull Requests found in [Work in Progress](/metrics-and-insights/wip). +The work-in-progress digest includes a summary of all the team's open Pull Requests found in [Work in Progress](/metrics-and-insights/wip).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/automations/digests.mdx` around lines 43 - 45, Update the phrase "work in progress digest" to use a hyphenated compound modifier: change "The work in progress digest includes a summary..." to "The work-in-progress digest includes a summary..." (locate the string "work in progress digest" in the docs/automations/digests.mdx content); leave the link text "[Work in Progress]" unchanged unless you also want the linked page title hyphenated to "Work-in-Progress".apps/docs/metrics-and-insights/work-log.mdx (1)
13-15: Add alt text to the demo image.Line 14 is missing an
altattribute, so screen-reader users lose context for the screenshot.♿ Proposed tweak
<Frame> - <img src="/images/work-log.png" /> + <img + src="/images/work-log.png" + alt="Work Log weekly view showing opened PRs, merged PRs, and code reviews by day" + /> </Frame>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/metrics-and-insights/work-log.mdx` around lines 13 - 15, The <img> tag inside the <Frame> component is missing an alt attribute which prevents screen-reader users from getting context; add a concise, descriptive alt attribute to the <img> element (e.g., alt="Work log demo showing metrics and insights dashboard") so the screenshot is accessible, leaving the <Frame> and src unchanged and ensuring the alt text describes the image's content/purpose.apps/docs/metrics-and-insights/dora.mdx (1)
58-58: Addnoopener noreferrerto the external link opened in a new tab.Line 58: The link to dora.dev uses
target="_blank"but only includesrel="nofollow". Addnoopener noreferrerto prevent tabnabbing attacks.Suggested fix
-See the <a href="https://dora.dev/guides/dora-metrics-four-keys/" target="_blank" rel="nofollow">official DORA guides</a> for the latest thresholds. +See the <a href="https://dora.dev/guides/dora-metrics-four-keys/" target="_blank" rel="nofollow noopener noreferrer">official DORA guides</a> for the latest thresholds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/metrics-and-insights/dora.mdx` at line 58, The external anchor element that uses target="_blank" needs to include rel="noopener noreferrer" to prevent tabnabbing; update the anchor (the <a href="https://dora.dev/guides/dora-metrics-four-keys/" target="_blank" rel="nofollow"> link) to include noopener and noreferrer in its rel attribute (e.g., rel="nofollow noopener noreferrer") so the new tab cannot access the opener.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/skills/sweetr-brand-voice/references/WEBSITE.md:
- Line 351: Remove or replace every bare placeholder link token "[]" in
WEBSITE.md (these blank link entries are producing broken docs); search for
literal "[]" occurrences and either delete them or replace with a valid Markdown
link form [link text](URL) or plain text, ensuring each replaced token includes
meaningful link text and a target URL or is removed if not needed.
- Line 22: Replace the ambiguous expiration phrase "Until April 30th: Use
"SWEETDEAL" to get 50% off your first six months" with a concrete date including
the year (for example: "Until April 30, 2026: Use "SWEETDEAL" to get 50% off
your first six months") so the promo expiration is unambiguous and won't become
stale; update the exact string in the document where that line appears.
- Line 487: The sentence "moment.Read our privacy policy." has a missing space
after the period; locate the string "moment.Read" and change it to "moment.
Read" (or rephrase the sentence to preserve flow and punctuation) so the
sentence spacing and readability are corrected.
- Around line 93-94: The concatenated feature labels string "DORA MetricsWork In
ProgressWork LogPull RequestsCode ReviewsCode Review DistributionPull Request
Size" should be split into readable items; update the section in WEBSITE.md so
each label is separated (e.g., by newlines or commas) or converted into a
bullet/list format so items like "DORA Metrics", "Work In Progress", "Work Log",
"Pull Requests", "Code Reviews", "Code Review Distribution", and "Pull Request
Size" appear as distinct entries.
- Line 28: The sentence "Insights and automations your dev team will actually
trust. Built in the open — no surveillance or $50/seat price tag." uses an em
dash (—); replace that em dash with the brand-approved punctuation (e.g., a
spaced hyphen " - " or an en dash if your brand allows) throughout the file
(also where the same pattern appears at lines 355-356), updating the string
"Built in the open — no surveillance or $50/seat price tag." to "Built in the
open - no surveillance or $50/seat price tag." (or "Built in the open – no
surveillance..." per brand spec) so no em dash characters remain.
In @.cursor/skills/sweetr-brand-voice/SKILL.md:
- Line 14: The summary line for "**[BRAND_KIT.md](references/BRAND_KIT.md)** —
Complete brand identity..." uses an em dash which contradicts the guide's "no em
dash" rule; update that bullet to remove or replace the em dash (e.g., use a
colon, parentheses, or simple punctuation:
"**[BRAND_KIT.md](references/BRAND_KIT.md)**: Complete brand identity...") so
the wording in the BRAND_KIT.md reference is consistent with the rule enforced
elsewhere (see the no-em-dash rule in the same document).
In `@apps/docs/about/data-privacy-and-security.mdx`:
- Line 8: Update the copy for grammar and clarity: replace the sentence
containing "Your data security and privacy come first. We follow strict security
best practices to protect our servers and application." with a smoother, correct
form such as "Your data security and privacy come first; we follow strict
security best practices to protect our servers and applications." Also find the
line that says "provide service to you" and change it to "provide services to
you" (or "provide a service to you" if singular intent) so number agreement is
consistent.
- Line 28: The external anchor using target="_blank" (the link to
https://github.com/sweetr-dev/sweetr.dev) must include rel="noopener noreferrer"
to prevent tabnabbing and isolate the opener; update the anchor in the
about/data-privacy-and-security.mdx content (the <a
href="https://github.com/sweetr-dev/sweetr.dev" target="_blank">…</a> link) to
add rel="noopener noreferrer".
In `@apps/docs/about/open-source.mdx`:
- Line 10: The external anchor in the MDX content uses target="_blank" but only
rel="nofollow"; update the anchor's rel attribute to include noopener and
noreferrer as well (e.g., rel="nofollow noopener noreferrer") so the link opened
in a new tab cannot access window.opener and to prevent tabnabbing; modify the
anchor in the about/open-source.mdx content where the <a ... target="_blank"> is
defined.
- Around line 6-10: Update the ambiguous claim "Self-hosting is free, forever."
by qualifying the permitted scope consistent with the FSL paragraph: change the
statement under the "License" heading so it specifies that self-hosting is free
for personal, internal, or non‑commercial/non‑competing use and remains subject
to the Functional Source License (FSL) restrictions (the FSL prevents
commercial/competing use and converts to Apache 2.0 after two years); ensure the
revised sentence explicitly references the FSL limitations so readers understand
that "free" does not permit commercial competing usage.
---
Duplicate comments:
In `@apps/docs/automations/incident-detection.mdx`:
- Around line 31-35: The table row under "Branch regex" currently shows the
glob-style example `hotfix/*` which is misleading for a regex field; update the
example to a proper regular expression (for example `^hotfix\/.*$`) in the
"Branch regex" row so it uses regex syntax instead of glob syntax, ensuring the
cell that currently contains `hotfix/*` (the "Branch regex" entry) is replaced
with a valid regex example.
---
Nitpick comments:
In @.cursor/skills/sweetr-brand-voice/references/BRAND_KIT.md:
- Line 13: Add a short reusable clarification next to the tagline "The open
platform for Engineering Performance++" stating that "open platform" refers to
openness/transparency and not OSI-style open-source licensing (for example:
"(refers to openness/transparency, not OSI open-source licensing)"); insert the
same concise note wherever the exact phrase "The open platform for Engineering
Performance++" appears (including the other occurrence mentioned) and ensure it
is formatted as a reusable snippet or parenthetical that can be copied with the
tagline.
In `@apps/docs/about/principles.mdx`:
- Around line 21-22: The two-line fragment "We surface only what matters. Not to
overwhelm you. Our tools are simple, intuitive, and focused on what's
actionable." in apps/docs/about/principles.mdx should be tightened into a single
continuous sentence for smoother flow—merge the first two fragments into one
sentence (e.g., "We surface only what matters so as not to overwhelm you.") and
ensure the full paragraph reads cohesively with the following clause ("Our tools
are simple, intuitive, and focused on what's actionable.").
In `@apps/docs/automations/digests.mdx`:
- Around line 43-45: Update the phrase "work in progress digest" to use a
hyphenated compound modifier: change "The work in progress digest includes a
summary..." to "The work-in-progress digest includes a summary..." (locate the
string "work in progress digest" in the docs/automations/digests.mdx content);
leave the link text "[Work in Progress]" unchanged unless you also want the
linked page title hyphenated to "Work-in-Progress".
In `@apps/docs/metrics-and-insights/dora.mdx`:
- Line 58: The external anchor element that uses target="_blank" needs to
include rel="noopener noreferrer" to prevent tabnabbing; update the anchor (the
<a href="https://dora.dev/guides/dora-metrics-four-keys/" target="_blank"
rel="nofollow"> link) to include noopener and noreferrer in its rel attribute
(e.g., rel="nofollow noopener noreferrer") so the new tab cannot access the
opener.
In `@apps/docs/metrics-and-insights/team/time-to-approve.mdx`:
- Around line 57-62: Update the Start ("ready") definition in
time-to-approve.mdx to match time-to-first-review.mdx by adding a clarifying
sentence that only timestamps occurring before the approval are considered;
specifically, after the current Start bullet ("The earliest available
timestamp...") add a note like "Only timestamps before the first approval are
considered" so it’s clear which candidate timestamps (first reviewer requested
date, ready-for-review date, PR creation date) are valid. This ensures
consistency with the Start definition behavior used in time-to-first-review.mdx.
In `@apps/docs/metrics-and-insights/work-log.mdx`:
- Around line 13-15: The <img> tag inside the <Frame> component is missing an
alt attribute which prevents screen-reader users from getting context; add a
concise, descriptive alt attribute to the <img> element (e.g., alt="Work log
demo showing metrics and insights dashboard") so the screenshot is accessible,
leaving the <Frame> and src unchanged and ensuring the alt text describes the
image's content/purpose.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63bea5f6-6fea-42de-8941-f70f6a8c6958
⛔ Files ignored due to path filters (8)
apps/docs/images/applications.pngis excluded by!**/*.pngapps/docs/images/deployments.pngis excluded by!**/*.pngapps/docs/images/dora.pngis excluded by!**/*.pngapps/docs/images/environments.pngis excluded by!**/*.pngapps/docs/images/incidents.pngis excluded by!**/*.pngapps/docs/images/people.pngis excluded by!**/*.pngapps/docs/images/team-list.pngis excluded by!**/*.pngapps/docs/images/team-members.pngis excluded by!**/*.png
📒 Files selected for processing (32)
.cursor/skills/sweetr-brand-voice/SKILL.md.cursor/skills/sweetr-brand-voice/references/BRAND_KIT.md.cursor/skills/sweetr-brand-voice/references/WEBSITE.mdapps/docs/about/data-privacy-and-security.mdxapps/docs/about/open-source.mdxapps/docs/about/principles.mdxapps/docs/api-reference/authentication.mdxapps/docs/api-reference/deployments/create-deployment.mdxapps/docs/api-reference/rate-limit.mdxapps/docs/automations/alerts.mdxapps/docs/automations/digests.mdxapps/docs/automations/incident-detection.mdxapps/docs/automations/pr-title-check.mdxapps/docs/get-started/intro.mdxapps/docs/metrics-and-insights/code-reviews.mdxapps/docs/metrics-and-insights/dora.mdxapps/docs/metrics-and-insights/pull-requests.mdxapps/docs/metrics-and-insights/team/code-review-distribution.mdxapps/docs/metrics-and-insights/team/cycle-time.mdxapps/docs/metrics-and-insights/team/intro.mdxapps/docs/metrics-and-insights/team/pr-size-distribution.mdxapps/docs/metrics-and-insights/team/time-to-approve.mdxapps/docs/metrics-and-insights/team/time-to-first-review.mdxapps/docs/metrics-and-insights/team/time-to-merge.mdxapps/docs/metrics-and-insights/wip.mdxapps/docs/metrics-and-insights/work-log.mdxapps/docs/platform/applications.mdxapps/docs/platform/deployments.mdxapps/docs/platform/environments.mdxapps/docs/platform/people.mdxapps/docs/platform/teams.mdxapps/docs/snippets/cta-get-started.mdx
✅ Files skipped from review due to trivial changes (2)
- apps/docs/metrics-and-insights/code-reviews.mdx
- apps/docs/metrics-and-insights/team/pr-size-distribution.mdx
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/docs/platform/environments.mdx
- apps/docs/platform/applications.mdx
- apps/docs/platform/deployments.mdx
- apps/docs/api-reference/authentication.mdx
- apps/docs/automations/pr-title-check.mdx
- apps/docs/automations/alerts.mdx
- apps/docs/api-reference/rate-limit.mdx
- apps/docs/metrics-and-insights/team/time-to-merge.mdx
- apps/docs/metrics-and-insights/team/intro.mdx
- apps/docs/api-reference/deployments/create-deployment.mdx
Contributor
There was a problem hiding this comment.
2 issues found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/docs/platform/environments.mdx">
<violation number="1" location="apps/docs/platform/environments.mdx:35">
P2: This line documents a manual `isProduction` override that the product does not currently expose, so it will mislead teams using names like `production-us` into thinking they can mark those environments as production.</violation>
</file>
<file name="apps/api/src/app/deployment/deployments.router.unit.test.ts">
<violation number="1" location="apps/api/src/app/deployment/deployments.router.unit.test.ts:67">
P2: `vi.restoreAllMocks()` does not clear these `vi.mock(...)` module mocks, so call history leaks between tests and makes later assertions order-dependent.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| For most teams, the default `production` environment is all you need. Multiple environments are useful when: | ||
|
|
||
| - **You want to measure metrics for non-production environments**, for example, tracking deployment frequency to staging. | ||
| - **You have multiple production environments**, for example, regional deployments (`production-us`, `production-eu`). Since only exact names `production` or `prod` are auto-detected, you must explicitly set the production flag for regional environment names. |
Contributor
There was a problem hiding this comment.
P2: This line documents a manual isProduction override that the product does not currently expose, so it will mislead teams using names like production-us into thinking they can mark those environments as production.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/platform/environments.mdx, line 35:
<comment>This line documents a manual `isProduction` override that the product does not currently expose, so it will mislead teams using names like `production-us` into thinking they can mark those environments as production.</comment>
<file context>
@@ -32,7 +32,7 @@ Each environment has an `isProduction` flag. This matters because DORA metrics d
- **You want to measure metrics for non-production environments**, for example, tracking deployment frequency to staging.
-- **You have multiple production environments**, for example, regional deployments (`production-us`, `production-eu`).
+- **You have multiple production environments**, for example, regional deployments (`production-us`, `production-eu`). Since only exact names `production` or `prod` are auto-detected, you must explicitly set the production flag for regional environment names.
## Archiving
</file context>
Suggested change
| - **You have multiple production environments**, for example, regional deployments (`production-us`, `production-eu`). Since only exact names `production` or `prod` are auto-detected, you must explicitly set the production flag for regional environment names. | |
| - **You have multiple production environments**, for example, regional deployments (`production-us`, `production-eu`). At the moment, only exact names `production` or `prod` are auto-detected as production environments. |
| }); | ||
|
|
||
| afterEach(async () => { | ||
| vi.restoreAllMocks(); |
Contributor
There was a problem hiding this comment.
P2: vi.restoreAllMocks() does not clear these vi.mock(...) module mocks, so call history leaks between tests and makes later assertions order-dependent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/app/deployment/deployments.router.unit.test.ts, line 67:
<comment>`vi.restoreAllMocks()` does not clear these `vi.mock(...)` module mocks, so call history leaks between tests and makes later assertions order-dependent.</comment>
<file context>
@@ -0,0 +1,237 @@
+ });
+
+ afterEach(async () => {
+ vi.restoreAllMocks();
+ await app.close();
+ });
</file context>
| @@ -1,18 +1,18 @@ | |||
| --- | |||
| title: Open Source | |||
Contributor
There was a problem hiding this comment.
Page title contradicts the brand guidelines added in this same PR
The body of this page (correctly) now says Sweetr is "fair source, not open source", and the new BRAND_KIT.md explicitly states: "Never call Sweetr 'open source.' Use 'fair source', 'source-available', or 'built in the open' instead."
However, the page title (which also becomes the sidebar navigation label) still reads Open Source, which directly contradicts that rule.
Consider renaming to something like Fair Source or Source & License to stay consistent with the brand voice:
Suggested change
| title: Open Source | |
| title: Fair Source |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/docs/metrics-and-insights/team/code-review-distribution.mdx (1)
48-48: Optional wording polish for readability and consistency.Line 48 can read a bit smoother with “providing feedback” and a Markdown link instead of raw HTML.
Suggested edit
-- Make sure your team feels safe giving feedback. Go through your team's <a href="/metrics-and-insights/pull-requests">Pull Requests</a> to ensure communication stays healthy. +- Make sure your team feels safe providing feedback. Go through your team's [Pull Requests](/metrics-and-insights/pull-requests) to ensure communication stays healthy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/metrics-and-insights/team/code-review-distribution.mdx` at line 48, Replace the HTML link and wording on the line containing "Make sure your team feels safe giving feedback. Go through your team's <a href="/metrics-and-insights/pull-requests">Pull Requests</a> to ensure communication stays healthy." with smoother phrasing using "providing feedback" and a Markdown link; specifically locate the line containing that exact sentence and change it to something like: "Make sure your team feels safe providing feedback. Go through your team's [Pull Requests](/metrics-and-insights/pull-requests) to ensure communication stays healthy." so the wording and link format are consistent with the rest of the document.apps/docs/automations/incident-detection.mdx (1)
23-23: Use consistent arrow notation in the rollback sequence example.Line 23 mixes
→and->in a single sequence; standardizing improves readability.✏️ Suggested tweak
-For example, given deployment versions `A → B → C → D -> B²`: +For example, given deployment versions `A → B → C → D → B²`:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/automations/incident-detection.mdx` at line 23, The rollback sequence example contains mixed arrow characters "A → B → C → D -> B²"; update it so the arrows are consistent (either all "→" or all "->")—replace the mismatched "->" before "B²" to match the chosen arrow style and ensure the example string "A → B → C → D -> B²" is updated accordingly..cursor/skills/sweetr-brand-voice/SKILL.md (1)
48-48: Consider reformatting the banned words list for readability.The current format packs ~50 banned words into a single 1000+ character line, which makes it harder to scan, maintain, or reference specific words. A bulleted list would improve usability when writers need to check if a word is banned.
♻️ Suggested refactoring
-- **Sound human, not like AI.** Avoid fancy/flowery words that real developers would never say. Banned words include: erode, delve, elevate, harness, foster, empower, unleash, bolster, spearhead, holistic, pivotal, transformative, streamline, paradigm, synergy, encompass, multifaceted, nuanced, underscore, facilitate, robust, scalable (unless literally about infrastructure), seamless, cutting-edge, groundbreaking, unparalleled, fortify, commencing, aforementioned, culminate, discern, elucidate, endeavor, intricate, myriad, paramount, perpetuate, proliferate, quintessential, resonate, subsequently, traverse, utilize (use "use"), whilst, navigate (as metaphor), landscape (as metaphor), ecosystem (unless literal), moreover, furthermore, additionally, comprehensive, meticulous, realm, testament, embark, journey (as metaphor), pave the way, shed light on, at its core, notably, interestingly, crucial, it's worth noting that, in today's [anything] world. If a word feels like it belongs in a college essay, pick a simpler one. +- **Sound human, not like AI.** Avoid fancy/flowery words that real developers would never say. +- **Banned words** (if a word feels like it belongs in a college essay, pick a simpler one): + - erode, delve, elevate, harness, foster, empower, unleash, bolster, spearhead + - holistic, pivotal, transformative, streamline, paradigm, synergy, encompass, multifaceted, nuanced + - underscore, facilitate, seamless, cutting-edge, groundbreaking, unparalleled, fortify + - commencing, aforementioned, culminate, discern, elucidate, endeavor, intricate, myriad, paramount + - perpetuate, proliferate, quintessential, resonate, subsequently, traverse + - whilst, realm, testament, embark, moreover, furthermore, additionally, comprehensive, meticulous + - crucial, notably, interestingly + - robust, scalable (unless literally about infrastructure) + - navigate, landscape, ecosystem, journey (unless used literally, not as metaphors) + - utilize (say "use" instead) + - Phrases: "pave the way", "shed light on", "at its core", "it's worth noting that", "in today's [anything] world"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.cursor/skills/sweetr-brand-voice/SKILL.md at line 48, The long inline banned-words sentence under the "**Sound human, not like AI.**" guideline is hard to scan; split that single ~50-word comma-separated line into a readable bulleted list (one banned word or short phrase per line) beneath the same heading, preserving the exact banned words and any parenthetical notes (e.g., "scalable (unless literally about infrastructure)") and keep the preceding guidance sentence unchanged so writers can easily reference specific banned terms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/deployment/deployments.router.unit.test.ts`:
- Around line 56-60: Current unit test registers deploymentsRouter directly (in
beforeEach using app = Fastify(); app.register(deploymentsRouter)), so it
bypasses the scoped `@fastify/rate-limit` middleware used in production; replace
or add an integration test that boots the full app via buildApp() (which applies
the rate-limit plugin and trustProxy settings from fastify.ts), then exercise
the deployments endpoint to assert a 429 after exceeding the 100 requests/minute
threshold, test key generation precedence by sending a Bearer token in
Authorization and verifying the rate-limit key uses the token instead of client
IP, and verify proxy behavior by enabling trustProxy and asserting IP-based
keying changes when requests come via X-Forwarded-For.
In `@apps/docs/automations/incident-detection.mdx`:
- Around line 30-37: Update the incident-detection docs to clarify that the
regex fields are optional: change the table entries for "PR title regex",
"Branch regex", and "PR label regex" to indicate each can be left empty to
disable that matcher, and add one short sentence under the table stating "Leave
any regex field blank to disable that matcher" (these correspond to the hotfix
regex fields defined in types.ts). Ensure the wording matches the implementation
(empty string allowed) so readers know leaving the field empty disables the
matcher.
---
Nitpick comments:
In @.cursor/skills/sweetr-brand-voice/SKILL.md:
- Line 48: The long inline banned-words sentence under the "**Sound human, not
like AI.**" guideline is hard to scan; split that single ~50-word
comma-separated line into a readable bulleted list (one banned word or short
phrase per line) beneath the same heading, preserving the exact banned words and
any parenthetical notes (e.g., "scalable (unless literally about
infrastructure)") and keep the preceding guidance sentence unchanged so writers
can easily reference specific banned terms.
In `@apps/docs/automations/incident-detection.mdx`:
- Line 23: The rollback sequence example contains mixed arrow characters "A → B
→ C → D -> B²"; update it so the arrows are consistent (either all "→" or all
"->")—replace the mismatched "->" before "B²" to match the chosen arrow style
and ensure the example string "A → B → C → D -> B²" is updated accordingly.
In `@apps/docs/metrics-and-insights/team/code-review-distribution.mdx`:
- Line 48: Replace the HTML link and wording on the line containing "Make sure
your team feels safe giving feedback. Go through your team's <a
href="/metrics-and-insights/pull-requests">Pull Requests</a> to ensure
communication stays healthy." with smoother phrasing using "providing feedback"
and a Markdown link; specifically locate the line containing that exact sentence
and change it to something like: "Make sure your team feels safe providing
feedback. Go through your team's [Pull
Requests](/metrics-and-insights/pull-requests) to ensure communication stays
healthy." so the wording and link format are consistent with the rest of the
document.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f4cac2c-4f35-4575-bd4c-d02159081a4b
📒 Files selected for processing (12)
.cursor/skills/sweetr-brand-voice/SKILL.mdapps/api/src/app/api-keys/services/api-keys.service.tsapps/api/src/app/deployment/deployments.router.tsapps/api/src/app/deployment/deployments.router.unit.test.tsapps/api/src/fastify.tsapps/docs/about/data-privacy-and-security.mdxapps/docs/about/open-source.mdxapps/docs/api-reference/deployments/create-deployment.mdxapps/docs/automations/incident-detection.mdxapps/docs/metrics-and-insights/team/code-review-distribution.mdxapps/docs/metrics-and-insights/team/cycle-time.mdxapps/docs/platform/environments.mdx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/docs/about/open-source.mdx
- apps/docs/about/data-privacy-and-security.mdx
- apps/api/src/fastify.ts
- apps/docs/platform/environments.mdx
- apps/docs/metrics-and-insights/team/cycle-time.mdx
Comment on lines
+56
to
+60
| beforeEach(async () => { | ||
| app = Fastify(); | ||
| app.setErrorHandler(errorHandler); | ||
| await app.register(deploymentsRouter); | ||
| await app.ready(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Inspect rate-limit wiring and key generation ==="
rg -n -C3 --type=ts 'trustProxy|@fastify/rate-limit|rateLimit|keyGenerator|request\.ip|getBearerToken|deploymentsRouter' apps/api/src
echo "=== Inspect current tests for rate-limit/proxy assertions ==="
rg -n -C2 --type=ts '429|rate.?limit|x-forwarded-for|forwarded|trustProxy' apps/api/srcRepository: sweetr-dev/sweetr.dev
Length of output: 28766
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check full test file structure for deployments.router.unit.test.ts ==="
wc -l apps/api/src/app/deployment/deployments.router.unit.test.ts
echo "=== Look for existing integration tests for rate-limit behavior ==="
find apps/api/src -name "*.integration.test.ts" -o -name "*.e2e.test.ts" | head -20
echo "=== Search for any 429/rate-limit tests in the codebase ==="
rg -l '429|\.status.*429|statusCode.*429' apps/api/src --type=ts
echo "=== Check if there are any tests that test through fastify.ts integration ==="
rg -n 'buildApp|fastify.ts' apps/api/src --type=ts -l | grep testRepository: sweetr-dev/sweetr.dev
Length of output: 509
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read deployments test file to understand current coverage ==="
cat -n apps/api/src/app/deployment/deployments.router.unit.test.ts
echo "=== Check if buildApp is used in any test files ==="
rg -n 'buildApp' apps/api/src --type=tsRepository: sweetr-dev/sweetr.dev
Length of output: 8613
Add integration test for rate-limit enforcement in deployments endpoint.
This unit test registers deploymentsRouter directly, bypassing the scoped @fastify/rate-limit middleware that wraps it in production (fastify.ts lines 54–61). Add an integration test using buildApp() to verify:
429response when rate limit (100 requests/minute) is exceeded- Key generation fallback: Bearer token (from Authorization header) takes precedence over client IP
- Proxy settings (
trustProxy: true) affect IP detection correctly
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/deployment/deployments.router.unit.test.ts` around lines 56
- 60, Current unit test registers deploymentsRouter directly (in beforeEach
using app = Fastify(); app.register(deploymentsRouter)), so it bypasses the
scoped `@fastify/rate-limit` middleware used in production; replace or add an
integration test that boots the full app via buildApp() (which applies the
rate-limit plugin and trustProxy settings from fastify.ts), then exercise the
deployments endpoint to assert a 429 after exceeding the 100 requests/minute
threshold, test key generation precedence by sending a Bearer token in
Authorization and verifying the rate-limit key uses the token instead of client
IP, and verify proxy behavior by enabling trustProxy and asserting IP-based
keying changes when requests come via X-Forwarded-For.
Comment on lines
+30
to
+37
| Detects incidents based on patterns in pull request titles, branch names, or labels. Configure regular expressions to match your team's conventions. | ||
|
|
||
| | Pattern | Description | | ||
| | ------------------ | -------------------------------------------------------- | | ||
| | **PR title regex** | Match pull request titles (e.g., `hotfix`, `emergency`). | | ||
| | **Branch regex** | Match branch names (e.g., `^hotfix/.+`). | | ||
| | **PR label regex** | Match pull request labels. | | ||
|
|
There was a problem hiding this comment.
Clarify that hotfix regex fields are optional.
At Line 30 and the table (Lines 32-36), this reads as if all regexes must be configured. Please add that each field can be left empty to disable that matcher.
✏️ Suggested doc patch
-Detects incidents based on patterns in pull request titles, branch names, or labels. Configure regular expressions to match your team's conventions.
+Detects incidents based on patterns in pull request titles, branch names, or labels. Configure regular expressions to match your team's conventions. Each regex is optional—leave it empty to skip that matcher.Based on learnings: in apps/web/src/app/automations/settings/incident-detection/types.ts, empty strings are intentionally allowed for optional hotfix regex fields.
📝 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
| Detects incidents based on patterns in pull request titles, branch names, or labels. Configure regular expressions to match your team's conventions. | |
| | Pattern | Description | | |
| | ------------------ | -------------------------------------------------------- | | |
| | **PR title regex** | Match pull request titles (e.g., `hotfix`, `emergency`). | | |
| | **Branch regex** | Match branch names (e.g., `^hotfix/.+`). | | |
| | **PR label regex** | Match pull request labels. | | |
| Detects incidents based on patterns in pull request titles, branch names, or labels. Configure regular expressions to match your team's conventions. Each regex is optional—leave it empty to skip that matcher. | |
| | Pattern | Description | | |
| | ------------------ | -------------------------------------------------------- | | |
| | **PR title regex** | Match pull request titles (e.g., `hotfix`, `emergency`). | | |
| | **Branch regex** | Match branch names (e.g., `^hotfix/.+`). | | |
| | **PR label regex** | Match pull request labels. | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docs/automations/incident-detection.mdx` around lines 30 - 37, Update
the incident-detection docs to clarify that the regex fields are optional:
change the table entries for "PR title regex", "Branch regex", and "PR label
regex" to indicate each can be left empty to disable that matcher, and add one
short sentence under the table stating "Leave any regex field blank to disable
that matcher" (these correspond to the hotfix regex fields defined in types.ts).
Ensure the wording matches the implementation (empty string allowed) so readers
know leaving the field empty disables the matcher.
sweetrdev
approved these changes
Mar 14, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/src/fastify.ts (1)
52-62: Rate limiting implementation looks correct.The
keyGeneratornow usesgetBearerTokento normalize the Authorization header (stripping the "Bearer " prefix), which addresses the previous concern about bucketing fragmentation. The fallback torequest.iphandles unauthenticated requests appropriately.One consideration for production:
@fastify/rate-limituses in-memory storage by default, meaning rate limits won't persist across process restarts or be shared across multiple instances. If this service runs in a clustered/multi-instance deployment, consider configuring a Redis store.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/fastify.ts` around lines 52 - 62, The current rate limiter registration (inside the async scope passed to app.register) uses the in-memory default for `@fastify/rate-limit`; for production/multi-instance deployments replace or extend that registration to use a shared Redis store by configuring the rateLimit plugin with a Redis backend (keep the existing options: max, timeWindow, keyGenerator which uses getBearerToken and request.ip) so limits persist across restarts and are shared between instances; update the rateLimit registration call (the one that registers rateLimit in the scope) to use the Redis store option when running in production.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api/src/fastify.ts`:
- Around line 52-62: The current rate limiter registration (inside the async
scope passed to app.register) uses the in-memory default for
`@fastify/rate-limit`; for production/multi-instance deployments replace or extend
that registration to use a shared Redis store by configuring the rateLimit
plugin with a Redis backend (keep the existing options: max, timeWindow,
keyGenerator which uses getBearerToken and request.ip) so limits persist across
restarts and are shared between instances; update the rateLimit registration
call (the one that registers rateLimit in the scope) to use the Redis store
option when running in production.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f90723e-aca2-4130-a1d9-8997f46738db
📒 Files selected for processing (1)
apps/api/src/fastify.ts
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
2 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.
Greptile Summary
This PR is a large documentation overhaul paired with two small but meaningful API improvements: extracting a
getBearerTokenhelper to strip theBearerprefix before API key lookup, and adding@fastify/rate-limitscoped to user-facing routes (100 req/min keyed by API key with an IP fallback).Key changes:
getBearerTokenutility (api-keys.service.ts) cleanly separates header parsing from key validation, and is exercised by a new unit test suite for the deployments router.deploymentsRouter; authenticated requests are correctly bucketed per API key.mint.jsonreplaced bydocs.json(Mintlify v2 format);features/pages reorganised intometrics-and-insights/,automations/, andplatform/sections; entirely new pages added for DORA metrics, deployments, environments, incidents, applications, and the REST API reference.BRAND_KIT.mdrules (fair source vs. open source, anti-surveillance framing, short punchy sentences, no AI-sounding words).getBearerTokenis re-implemented inline in the Vitest mock instead of usingvi.importActual, so future changes to the real function won't be caught by these tests.Confidence Score: 4/5
getBearerTokenextraction and rate limiting) are straightforward and covered by new unit tests. The only code-quality concern is the duplicatedgetBearerTokenimplementation in the test mock, which is a maintainability risk rather than a correctness bug. The documentation restructuring is extensive but does not affect runtime behaviour. Two previously-flagged items (theContents: Read-onlyGitHub App permission removal and theabout/open-sourcepage title) remain open.getBearerTokenmock duplication. apps/docs/get-started/self-host.mdx and apps/docs/about/open-source.mdx — open items from previous review threads.Important Files Changed
getBearerTokenutility that strips theBearerprefix from the Authorization header before key lookup. Simple pure function with correct regex and nullish-coalescing guard.getBearerTokento strip theBearerprefix before passing the key tofindApiKeyOrThrow, fixing the case where the raw header value (including prefix) would fail DB lookup.getBearerTokenis inlined in the mock rather than usingvi.importActual, creating a duplication risk.@fastify/rate-limitscoped to user-facing routes (100 req/min per API key).slackRouterandbullBoardRouteralso reordered before thestripeRouterconditional. IP fallback inkeyGeneratorfor unauthenticated requests may resolve to a proxy IP (previously flagged).you-web-app→your-web-app) and corrects "adresses" → "addresses". Removes theContents: Read-onlyGitHub App permission — impact on deployment PR-linking was flagged in a previous review thread.mint.jsonwith new Mintlifydocs.jsonformat, restructuring navigation into Sweetr and API Reference tabs with updated paths and icon library switched to Tabler.Sequence Diagram
sequenceDiagram participant CI as CI/CD Pipeline participant API as Fastify API participant AKS as api-keys.service participant Q as BullMQ Queue participant W as Deployment Worker CI->>API: POST /v1/deployments Authorization: Bearer key API->>AKS: getBearerToken(header) returns rawKey API->>AKS: findApiKeyOrThrow(rawKey) alt Invalid or missing key AKS-->>API: AuthorizationException API-->>CI: 401 Unauthorized else Valid key AKS-->>API: workspaceId API->>API: validateInputOrThrow(body) alt Validation fails API-->>CI: 422 Unprocessable Entity else Valid payload API->>Q: addJob(DEPLOYMENT_TRIGGERED_BY_API) API-->>CI: 202 Accepted Q->>W: process deployment job async end endComments Outside Diff (1)
apps/api/src/app/deployment/deployments.router.unit.test.ts, line 1016-1020 (link)getBearerTokenimplementation duplicated in mockThe mock for
getBearerTokeninlines the real implementation verbatim instead of delegating to the actual module. If the realgetBearerTokenlogic changes (e.g., the stripping regex is updated), these tests will continue to pass against the stale copy and silently miss the regression.The safer pattern is to spy only on
findApiKeyOrThrowand let the realgetBearerTokenrun through:This keeps the real
getBearerTokenin the execution path, so the tests in "strips Bearer prefix" and "works without Bearer prefix" verify the full chain rather than a duplicated copy.Last reviewed commit: c5f6867