Soundcheck, pt1 - #1825
Conversation
First real feature after the scaffold. Answers "should we cut a release
today?" by showing, per env pair, what production is missing vs. staging —
with PR counts by category and the top 8 commits.
Implementation:
- soundcheck/src/lib/github.ts: thin REST wrapper. Two calls per tile —
GET /repos/{owner}/{repo}/deployments?environment=... (per env, 1 record)
and GET /compare/{prod}...{staging}. Uses `next: { revalidate: 30 }` so
tiles refresh every 30s without hammering GitHub.
- soundcheck/src/components/deploy-diff.tsx: async server component that
fetches both envs in parallel, categorizes commits by conventional-commit
prefix, and renders the tile. Handles missing deployments, sync state,
and fetch errors without crashing the page.
- soundcheck/src/app/page.tsx: two tiles, one per repo (inspector +
backend), wrapped in Suspense so each renders independently.
Data source rationale: GitHub Environments records every `environment:`
clause as a Deployment, so current SHA per env is exactly one API call.
No Railway or Convex API needed for this slice.
Requires: GITHUB_PAT in the Railway env for mcpjam-soundcheck service,
fine-grained and scoped read-only to MCPJam/inspector and
MCPJam/mcpjam-backend (Contents, Deployments, Metadata).
dosubot
Bot
added
the
size:L
GitHub Actions
Inactive
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Internal previewPreview URL: https://mcp-inspector-pr-1825.up.railway.app |
WalkthroughAdds deployment-diff UI and GitHub API support. Introduces a new 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: 4
🧹 Nitpick comments (3)
soundcheck/src/app/page.tsx (1)
25-66: LGTM — independent Suspense boundaries per tile is the right call.Wrapping each
DeployDiffin its own<Suspense>means a slow (or failing) GitHub response on one repo doesn't hold the other tile hostage, and pairs nicely withrevalidate: 30in the fetch layer. One small nudge: consider anErrorBoundaryaround each Suspense so an unexpected throw in the server component doesn't blow up the whole page — todayDeployDiffcatches its own fetch errors, but anything thrown outside thosetryblocks (e.g., a future refactor) would bubble up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@soundcheck/src/app/page.tsx` around lines 25 - 66, Add an ErrorBoundary around each Suspense + DeployDiff tile so an unexpected throw inside the server component won’t crash the whole page; locate the Suspense blocks that render DeployDiff (the Suspense with fallback DeployDiffSkeleton and the DeployDiff props for "Inspector" and "Backend") and wrap each Suspense in your ErrorBoundary component (or create one if missing) so errors render a safe fallback per tile while keeping existing DeployDiff error handling intact.soundcheck/src/components/deploy-diff.tsx (2)
76-90: TDZ risk if you ever readstaging/productioninside thecatch.
let staging, production;without initializers plus destructuring assignment insidetrymeans that if either GitHub call rejects, both bindings stayundefined. Today you immediatelyreturnfrom the catch so it's fine, but it's a footgun for the next person who adds a log line likeconsole.error({ staging, production, err })outside the catch. A one-liner makes intent explicit:♻️ Optional tightening
- let staging, production; + let staging: Awaited<ReturnType<typeof getLatestEnvironmentDeployment>>; + let production: typeof staging; try { [staging, production] = await Promise.all([🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@soundcheck/src/components/deploy-diff.tsx` around lines 76 - 90, Declare and initialize the bindings to avoid a TDZ when the destructuring in the try block fails: change the top declaration from "let staging, production;" to an explicit initializer such as "let staging = null, production = null;" (or undefined if you prefer), then keep the await Promise.all([...getLatestEnvironmentDeployment...]) inside the try as-is; this ensures staging and production are always defined (null) if either getLatestEnvironmentDeployment call rejects and prevents accidental reads in the catch/logging code.
29-37: Conventional-commit regex misses the bare-colon case without paren/bang, and merge/revert commits fall through toother.
/^feat(\(|:|!)/matchesfeat:,feat(scope):,feat!:— all good. ButMerge pull request …,Revert "…", and squash-merge titles likeSomething (#123)all land inother, which inflates that bucket and hides the fact that the diff is mostly noise. Not a bug per se, just something to be aware of when reading tiles — you may want amerge/revertshort-circuit so they're excluded from the headline count.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@soundcheck/src/components/deploy-diff.tsx` around lines 29 - 37, Update the categorize function to short-circuit merge and revert titles and to correctly match conventional commit forms that include the bare-colon case; specifically, inside categorize add checks for /^merge/i and /^revert/i to return "chore" (or an excluded category) before other tests, and broaden the conventional-commit regexes (the /^feat.../ and /^fix.../ patterns used in categorize) to accept "type:", "type(scope):", and "type!" variations (i.e., allow a colon without needing a parenthesis or bang) so conventional commits are classified as "feat" or "fix" instead of falling through to "other".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@soundcheck/src/components/deploy-diff.tsx`:
- Around line 39-49: The describeCategories function leaves counts.other
unpluralized; update the branch that builds the parts array for counts.other so
it includes a noun and pluralizes like the others (e.g. use counts.other with
"other" + conditional "s"), mirroring the pattern used for counts.feat,
counts.fix, and counts.chore so the string becomes "1 other" vs "3 others"
rather than "3 other".
- Around line 155-197: The overflow count currently uses diff.commits.length
which is truncated by GitHub; compute overflow using diff.aheadBy minus the
number shown: const shown = Math.min(diff.commits.length, 8); const overflow =
Math.max(0, diff.aheadBy - shown); then replace the existing conditional
(diff.commits.length > 8) and the "+ N more" text with a check for overflow > 0
and render "+ {overflow} more"; use the same symbols (diff.aheadBy,
diff.commits, the slice(0,8) display) so the UI still shows up to 8 commits but
reports the true aheadBy overflow.
In `@soundcheck/src/lib/github.ts`:
- Around line 31-37: githubFetch currently includes up to 200 chars of the raw
response body in the thrown Error (which deploy-diff.tsx renders); instead, in
githubFetch inspect res.headers.get('content-type') and only include a body
snippet when the content-type is a safe textual type (e.g., startsWith('text/')
or 'application/json'), otherwise log the full res.text() server-side (via your
server logger) and throw a sanitized error like `GitHub API ${res.status} for
${path}`; if content-type is JSON you can parse it and include only a safe field
(e.g., parsed.message) rather than the raw blob.
- Around line 53-75: getLatestEnvironmentDeployment currently picks the newest
deployment regardless of status; change it to return the most recent deployment
that has a successful status. Modify getLatestEnvironmentDeployment to request a
larger page (or page through results) from the deployments list, include the
deployment id from each item, then for each candidate call the statuses endpoint
(use githubFetch on /repos/{owner}/{repo}/deployments/{deployment_id}/statuses)
and inspect the returned statuses for state === "success"; return the first
deployment whose statuses contain a success state (preserving sha, created_at,
creator?.login, environment, url in the DeploymentInfo) and continue searching
if none are successful. Ensure you reference githubFetch and DeploymentInfo and
handle empty/no-success results by returning null.
---
Nitpick comments:
In `@soundcheck/src/app/page.tsx`:
- Around line 25-66: Add an ErrorBoundary around each Suspense + DeployDiff tile
so an unexpected throw inside the server component won’t crash the whole page;
locate the Suspense blocks that render DeployDiff (the Suspense with fallback
DeployDiffSkeleton and the DeployDiff props for "Inspector" and "Backend") and
wrap each Suspense in your ErrorBoundary component (or create one if missing) so
errors render a safe fallback per tile while keeping existing DeployDiff error
handling intact.
In `@soundcheck/src/components/deploy-diff.tsx`:
- Around line 76-90: Declare and initialize the bindings to avoid a TDZ when the
destructuring in the try block fails: change the top declaration from "let
staging, production;" to an explicit initializer such as "let staging = null,
production = null;" (or undefined if you prefer), then keep the await
Promise.all([...getLatestEnvironmentDeployment...]) inside the try as-is; this
ensures staging and production are always defined (null) if either
getLatestEnvironmentDeployment call rejects and prevents accidental reads in the
catch/logging code.
- Around line 29-37: Update the categorize function to short-circuit merge and
revert titles and to correctly match conventional commit forms that include the
bare-colon case; specifically, inside categorize add checks for /^merge/i and
/^revert/i to return "chore" (or an excluded category) before other tests, and
broaden the conventional-commit regexes (the /^feat.../ and /^fix.../ patterns
used in categorize) to accept "type:", "type(scope):", and "type!" variations
(i.e., allow a colon without needing a parenthesis or bang) so conventional
commits are classified as "feat" or "fix" instead of falling through to "other".
🪄 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: CHILL
Plan: Pro
Run ID: 93793541-90e4-4b8a-945a-427cfc984f02
📒 Files selected for processing (3)
soundcheck/src/app/page.tsxsoundcheck/src/components/deploy-diff.tsxsoundcheck/src/lib/github.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Five fixes from PR #1825 review (CodeRabbit + Cursor Bugbot): 1. MAJOR: getLatestEnvironmentDeployment no longer reports mid-flight or failed deploys as current. It now fetches the last 10 deployments and walks back until it finds one whose latest status is `success` or `inactive` (superseded but previously successful — still represents what was actually live if no newer deploy has succeeded yet). Adds a second API call per candidate (GET /deployments/{id}/statuses?per_page=1). 2. MEDIUM: Commit preview now shows newest-first, not oldest-first. GitHub Compare returns commits chronologically (oldest first); slice(0, 8) was hiding the most recent work behind "+ N more". Switched to slice(-8).reverse(). Matters because "should we cut a release?" is answered by the most recent changes. 3. MINOR: Overflow count ("+ N earlier commits") now uses aheadBy - preview.length instead of commits.length - 8. The compare endpoint caps `commits` at 250, so for wide diffs the previous calculation under-reported by up to (aheadBy - 250). 4. MINOR: githubFetch no longer dumps raw upstream response body into the thrown Error (which deploy-diff.tsx then rendered to the page). Full body is now console.error'd server-side for debugging; the user-facing error surfaces only the parsed JSON `message` field when present, or just the status + path otherwise. 5. MINOR: describeCategories pluralizes "other" ("3 other commits" instead of "3 other"). No API surface change; DeployDiff props and CompareResult shape are unchanged. Tile error messages reworded slightly to say "No successful deployment" since the filter now excludes failed/in-flight records.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
soundcheck/src/components/deploy-diff.tsx (1)
78-83: Givestagingandproductionan explicit type.
let staging, production;leaves both bindings asanyuntil the destructuring assignment narrows them, which undermines the niceDeploymentInfo | nullcontractgetLatestEnvironmentDeploymentreturns and would tripnoImplicitAnyif this project ever tightens its compiler options. A one-line annotation keeps the types honest through thecatchbranch too.♻️ Typed declaration
- let staging, production; + let staging: Awaited<ReturnType<typeof getLatestEnvironmentDeployment>>; + let production: Awaited<ReturnType<typeof getLatestEnvironmentDeployment>>;Or simply import and reuse
DeploymentInfo:-import { - compareCommits, - getLatestEnvironmentDeployment -} from "@/lib/github"; +import { + compareCommits, + getLatestEnvironmentDeployment, + type DeploymentInfo +} from "@/lib/github"; ... - let staging, production; + let staging: DeploymentInfo | null; + let production: DeploymentInfo | null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@soundcheck/src/components/deploy-diff.tsx` around lines 78 - 83, Declare staging and production with an explicit type (e.g., DeploymentInfo | null) instead of using "let staging, production;" so their types are known before the destructuring and in the catch branch; update the binding where you currently call getLatestEnvironmentDeployment(owner, repo, stagingEnvironment) and getLatestEnvironmentDeployment(owner, repo, productionEnvironment) to use the typed variables (import or reuse DeploymentInfo) so both staging and production are typed as DeploymentInfo | null.soundcheck/src/lib/github.ts (1)
102-113: Sequential status lookups — fine today, noisy tomorrow.Each iteration awaits one
/statusescall before starting the next, so a tile whose latest 9 deployments all failed pays ~10 serial round-trips (one list + up to 10 statuses) before resolving. At the current deploy cadence this is invisible, but since the list is already bounded to 10 you could fire the status lookups in parallel and pick the firstsuccess/inactivein original order. Not blocking — flagging in case dashboard latency ever grows teeth.♻️ Possible parallelization
- for (const d of data) { - const state = await getLatestDeploymentStatus(owner, repo, d.id); - if (state === "success" || state === "inactive") { - return { - sha: d.sha, - createdAt: d.created_at, - creator: d.creator?.login ?? null, - environment: d.environment, - url: d.url - }; - } - } - return null; + const states = await Promise.all( + data.map((d) => getLatestDeploymentStatus(owner, repo, d.id)) + ); + const idx = states.findIndex((s) => s === "success" || s === "inactive"); + if (idx === -1) return null; + const d = data[idx]; + return { + sha: d.sha, + createdAt: d.created_at, + creator: d.creator?.login ?? null, + environment: d.environment, + url: d.url + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@soundcheck/src/lib/github.ts` around lines 102 - 113, The loop is awaiting getLatestDeploymentStatus serially causing unnecessary latency; instead map the current data array to a list of promises by calling getLatestDeploymentStatus(owner, repo, d.id) for each d (preserving the same index order), await them in parallel with Promise.all, then iterate the resolved results in original order to find the first status === "success" || "inactive" and return the corresponding d's sha/created_at/creator/environment/url; update the code that currently references the sequential for..of and getLatestDeploymentStatus to use this parallelized pattern while keeping the original data list and result shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@soundcheck/src/lib/github.ts`:
- Around line 148-163: The commits mapping can throw when commit.author is null;
update the map in the block that transforms data.commits (the arrow function
mapping each c) to use optional chaining and fallbacks for author and date
(e.g., use c.commit.author?.name ?? c.commit.committer?.name ?? "Unknown" and
c.commit.author?.date ?? c.commit.committer?.date ?? c.commit?.author?.date ??
null or an ISO fallback) and keep the message extraction safe
(c.commit.message?.split("\n")[0] ?? ""). Ensure the resulting object still
supplies sha, message, author, date, and url with sensible default values
instead of assuming commit.author exists.
---
Nitpick comments:
In `@soundcheck/src/components/deploy-diff.tsx`:
- Around line 78-83: Declare staging and production with an explicit type (e.g.,
DeploymentInfo | null) instead of using "let staging, production;" so their
types are known before the destructuring and in the catch branch; update the
binding where you currently call getLatestEnvironmentDeployment(owner, repo,
stagingEnvironment) and getLatestEnvironmentDeployment(owner, repo,
productionEnvironment) to use the typed variables (import or reuse
DeploymentInfo) so both staging and production are typed as DeploymentInfo |
null.
In `@soundcheck/src/lib/github.ts`:
- Around line 102-113: The loop is awaiting getLatestDeploymentStatus serially
causing unnecessary latency; instead map the current data array to a list of
promises by calling getLatestDeploymentStatus(owner, repo, d.id) for each d
(preserving the same index order), await them in parallel with Promise.all, then
iterate the resolved results in original order to find the first status ===
"success" || "inactive" and return the corresponding d's
sha/created_at/creator/environment/url; update the code that currently
references the sequential for..of and getLatestDeploymentStatus to use this
parallelized pattern while keeping the original data list and result shape.
🪄 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: CHILL
Plan: Pro
Run ID: 4b5b6d6f-5b7d-45be-be8c-a83d51ca7097
📒 Files selected for processing (2)
soundcheck/src/components/deploy-diff.tsxsoundcheck/src/lib/github.ts
| commits: Array<{ | ||
| sha: string; | ||
| commit: { message: string; author: { name: string; date: string } }; | ||
| html_url: string; | ||
| }>; | ||
| }; | ||
| return { | ||
| aheadBy: data.ahead_by, | ||
| behindBy: data.behind_by, | ||
| commits: data.commits.map((c) => ({ | ||
| sha: c.sha, | ||
| message: c.commit.message.split("\n")[0], | ||
| author: c.commit.author.name, | ||
| date: c.commit.author.date, | ||
| url: c.html_url | ||
| })) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is the commit.author field nullable in the GitHub REST compare commits response?
💡 Result:
Yes, the commit.author field is nullable in the GitHub REST API compare commits response.
Citations:
- 1: https://docs.github.com/rest/commits/commits
- 2: Null Fields on commits returned when using Compare API octokit/octokit.net#1433
- 3: https://docs.github.com/v3/repos/commits
- 4: https://www.rookierise.org
- 5: https://docs.github.com/en/rest/commits/commits?apiVersion=2026-03-10
Handle null values in commit.author field.
The GitHub REST API returns null for commit.author when a commit cannot be matched to a GitHub user. The current .map will crash with a TypeError if this occurs. Guard against it with optional chaining and sensible fallbacks:
Defensive fallback
commits: Array<{
sha: string;
- commit: { message: string; author: { name: string; date: string } };
+ commit: {
+ message: string;
+ author: { name: string; date: string } | null;
+ };
html_url: string;
}>;
};
return {
aheadBy: data.ahead_by,
behindBy: data.behind_by,
commits: data.commits.map((c) => ({
sha: c.sha,
message: c.commit.message.split("\n")[0],
- author: c.commit.author.name,
- date: c.commit.author.date,
+ author: c.commit.author?.name ?? "unknown",
+ date: c.commit.author?.date ?? "",
url: c.html_url
}))
};📝 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.
| commits: Array<{ | |
| sha: string; | |
| commit: { message: string; author: { name: string; date: string } }; | |
| html_url: string; | |
| }>; | |
| }; | |
| return { | |
| aheadBy: data.ahead_by, | |
| behindBy: data.behind_by, | |
| commits: data.commits.map((c) => ({ | |
| sha: c.sha, | |
| message: c.commit.message.split("\n")[0], | |
| author: c.commit.author.name, | |
| date: c.commit.author.date, | |
| url: c.html_url | |
| })) | |
| commits: Array<{ | |
| sha: string; | |
| commit: { | |
| message: string; | |
| author: { name: string; date: string } | null; | |
| }; | |
| html_url: string; | |
| }>; | |
| }; | |
| return { | |
| aheadBy: data.ahead_by, | |
| behindBy: data.behind_by, | |
| commits: data.commits.map((c) => ({ | |
| sha: c.sha, | |
| message: c.commit.message.split("\n")[0], | |
| author: c.commit.author?.name ?? "unknown", | |
| date: c.commit.author?.date ?? "", | |
| url: c.html_url | |
| })) | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@soundcheck/src/lib/github.ts` around lines 148 - 163, The commits mapping can
throw when commit.author is null; update the map in the block that transforms
data.commits (the arrow function mapping each c) to use optional chaining and
fallbacks for author and date (e.g., use c.commit.author?.name ??
c.commit.committer?.name ?? "Unknown" and c.commit.author?.date ??
c.commit.committer?.date ?? c.commit?.author?.date ?? null or an ISO fallback)
and keep the message extraction safe (c.commit.message?.split("\n")[0] ?? "").
Ensure the resulting object still supplies sha, message, author, date, and url
with sensible default values instead of assuming commit.author exists.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4108987. Configure here.
| )} | ||
| </ul> | ||
| </Tile> | ||
| ); |
There was a problem hiding this comment.
Misleading display when production is ahead of staging
Medium Severity
When staging.sha !== production.sha but production is actually ahead of staging (e.g. a hotfix deployed straight to prod), the compare returns aheadBy: 0 and an empty commits array. The component unconditionally renders "Production is 0 commits behind staging (0 commits)" with an empty list — actively misleading. The behindBy value is available in CompareResult but is never inspected, so this state is silently presented as if staging has nothing new. A guard for the aheadBy === 0 (or diverged) case is missing.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4108987. Configure here.
| sha: c.sha, | ||
| message: c.commit.message.split("\n")[0], | ||
| author: c.commit.author.name, | ||
| date: c.commit.author.date, |
There was a problem hiding this comment.
Null git author crashes entire compare mapping
Medium Severity
The GitHub compare API's commit.author field is explicitly nullable in the API schema (typed as nullable-git-user), and this occurs in practice when a commit author's email isn't linked to a GitHub account. Accessing c.commit.author.name and c.commit.author.date without a null guard throws a TypeError, which causes the entire .map() in compareCommits to fail. Because the caller catches this as a generic error, a single commit with a null author makes the whole tile show "Failed to compare commits" — losing all useful diff information.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4108987. Configure here.
1 participant
First real feature after the scaffold. Answers "should we cut a release today?" by showing, per env pair, what production is missing vs. staging — with PR counts by category and the top 8 commits.
Implementation:
next: { revalidate: 30 }so tiles refresh every 30s without hammering GitHub.Data source rationale: GitHub Environments records every
environment:clause as a Deployment, so current SHA per env is exactly one API call. No Railway or Convex API needed for this slice.Requires: GITHUB_PAT in the Railway env for mcpjam-soundcheck service, fine-grained and scoped read-only to MCPJam/inspector and MCPJam/mcpjam-backend (Contents, Deployments, Metadata).
Note
Medium Risk
Introduces new server-side calls to the GitHub API using a PAT and deployment/compare logic; misconfiguration or API/rate-limit issues could impact dashboard availability and error handling.
Overview
Adds a Deploy Diff dashboard to the Soundcheck home page, showing whether production is behind staging for the
inspectorandmcpjam-backendrepos, with independent loading viaSuspense.Introduces an async
DeployDiffserver component that fetches latest successful deployments per environment, compares SHAs, categorizes commit messages (feat/fix/chore/other), and renders a short commit preview with links plus clear empty/sync/error states.Adds a small
lib/github.tsREST wrapper that readsGITHUB_PAT, fetches GitHub deployments/compare data with 30s revalidation, and sanitizes upstream errors before surfacing messages to the UI.Reviewed by Cursor Bugbot for commit 4108987. Bugbot is set up for automated code reviews on this repo. Configure here.