fix: harden siza production blockers and audit gating#481
fix: harden siza production blockers and audit gating#481LucasSantana-Dev merged 2 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review WalkthroughThis PR introduces production E2E testing infrastructure with Playwright configuration and orchestration scripts, refactors API error handling for generations endpoints with a centralized error handler, adds API authentication contract tests, and updates roadmap UI components with layout and overflow fixes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.16.4)apps/web/src/app/api/generations/error-handler.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m apps/web/src/app/api/generations/__tests__/auth-contract.test.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m apps/web/src/app/api/generations/[id]/route.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m
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 |
Project Scorecard |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
apps/web/scripts/e2e-production-audit.sh (4)
42-82: Apply the same[[conditional pattern to remaining functions.These functions also use
[conditionals that should be updated to[[for consistency and safety.♻️ Proposed fixes
resolve_service_role_key() { - if [ -n "${SUPABASE_SERVICE_ROLE_KEY:-}" ]; then + if [[ -n "${SUPABASE_SERVICE_ROLE_KEY:-}" ]]; then return fi local url="${NEXT_PUBLIC_SUPABASE_URL:-}" - if [ -z "${url}" ]; then + if [[ -z "${url}" ]]; then return fi local project_ref project_ref="$(echo "${url}" | sed -E 's#https?://([^.]+).*#\1#')" - if [ -z "${project_ref}" ]; then + if [[ -z "${project_ref}" ]]; then return fi local resolved resolved="$( supabase projects api-keys --project-ref "${project_ref}" --output json 2>/dev/null \ | jq -r '.[] | select(.name=="service_role") | .api_key // empty' \ | head -n 1 )" - if [ -n "${resolved}" ]; then + if [[ -n "${resolved}" ]]; then export SUPABASE_SERVICE_ROLE_KEY="${resolved}" fi } resolve_vercel_env_dir() { local common_git_dir common_git_dir="$(git -C "${REPO_DIR}" rev-parse --path-format=absolute --git-common-dir 2>/dev/null || true)" - if [ -n "${common_git_dir}" ]; then + if [[ -n "${common_git_dir}" ]]; then local canonical_repo_dir canonical_repo_dir="$(cd "${common_git_dir}/.." && pwd)" - if [ -f "${canonical_repo_dir}/.vercel/project.json" ]; then + if [[ -f "${canonical_repo_dir}/.vercel/project.json" ]]; then echo "${canonical_repo_dir}" return fi fi echo "${REPO_DIR}" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/e2e-production-audit.sh` around lines 42 - 82, Update the conditional tests in resolve_service_role_key and resolve_vercel_env_dir to use the Bash [[ ... ]] syntax instead of [ ... ] for safer, more consistent conditionals: change all occurrences like if [ -n "${VAR}" ]; then and if [ -z "${VAR}" ]; then to if [[ -n "${VAR}" ]]; then / if [[ -z "${VAR}" ]]; then, and change the file test if [ -f "${canonical_repo_dir}/.vercel/project.json" ]; then to if [[ -f "${canonical_repo_dir}/.vercel/project.json" ]]; then; keep the existing logic and variable names (SUPABASE_SERVICE_ROLE_KEY, NEXT_PUBLIC_SUPABASE_URL, project_ref, resolved, common_git_dir, canonical_repo_dir, REPO_DIR, resolve_service_role_key, resolve_vercel_env_dir) unchanged.
270-278: Apply[[conditionals and stderr redirect to run_pack function.♻️ Proposed fixes
local playwright_bin="${WEB_DIR}/node_modules/.bin/playwright" - if [ ! -x "${playwright_bin}" ]; then + if [[ ! -x "${playwright_bin}" ]]; then playwright_bin="${REPO_DIR}/node_modules/.bin/playwright" fi - if [ ! -x "${playwright_bin}" ]; then - echo "[ERROR] Playwright binary not found." + if [[ ! -x "${playwright_bin}" ]]; then + echo "[ERROR] Playwright binary not found." >&2 exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/e2e-production-audit.sh` around lines 270 - 278, In the run_pack function update the file-existence checks to use Bash [[ ]] conditionals for portability and safety (replace if [ ! -x "${playwright_bin}" ]; then ... fi with if [[ ! -x "${playwright_bin}" ]]; then ... fi) and ensure error messages go to stderr by redirecting echoes (use echo "[ERROR] Playwright binary not found." >&2) and exits remain the same; apply these changes around the playwright_bin resolution logic inside run_pack so the checks use [[ ]] and error output is redirected to stderr.
18-40: Consider using[[for conditional tests and redirecting errors to stderr.Static analysis flagged multiple instances where
[is used instead of[[for conditional tests, and error messages should be redirected to stderr. While functional,[[is safer in bash (handles word splitting and globbing better), and stderr is the appropriate channel for error output.♻️ Proposed fixes for shell best practices
require_env() { local name="$1" - if [ -z "${!name:-}" ]; then - echo "[ERROR] Missing required env: ${name}" + if [[ -z "${!name:-}" ]]; then + echo "[ERROR] Missing required env: ${name}" >&2 exit 1 fi + return 0 } ensure_non_local_supabase() { local url="${NEXT_PUBLIC_SUPABASE_URL:-}" if [[ "${url}" == *"localhost"* || "${url}" == *"127.0.0.1"* ]]; then - echo "[ERROR] NEXT_PUBLIC_SUPABASE_URL must be non-local for production audit." + echo "[ERROR] NEXT_PUBLIC_SUPABASE_URL must be non-local for production audit." >&2 exit 1 fi + return 0 } ensure_generation_backend() { - if [ -n "${MCP_GATEWAY_URL:-}" ] || [ -n "${GEMINI_API_KEY:-}" ] || [ -n "${OPENAI_API_KEY:-}" ] || [ -n "${ANTHROPIC_API_KEY:-}" ]; then + if [[ -n "${MCP_GATEWAY_URL:-}" ]] || [[ -n "${GEMINI_API_KEY:-}" ]] || [[ -n "${OPENAI_API_KEY:-}" ]] || [[ -n "${ANTHROPIC_API_KEY:-}" ]]; then return fi - echo "[ERROR] Missing generation backend (set MCP_GATEWAY_URL or one AI provider key)." + echo "[ERROR] Missing generation backend (set MCP_GATEWAY_URL or one AI provider key)." >&2 exit 1 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/e2e-production-audit.sh` around lines 18 - 40, Update the three helper functions to use bash [[ ]] conditionals and send error messages to stderr: in require_env replace the [ -z "${!name:-}" ] test with [[ -z "${!name:-}" ]] and write the echo error to >&2 before exiting; in ensure_non_local_supabase use [[ "${url}" == *localhost* || "${url}" == *127.0.0.1* ]] and echo the error to >&2; in ensure_generation_backend use [[ -n "${MCP_GATEWAY_URL:-}" || -n "${GEMINI_API_KEY:-}" || -n "${OPENAI_API_KEY:-}" || -n "${ANTHROPIC_API_KEY:-}" ]] and echo its error to >&2 — keep the same exit codes and preserve variable quoting when referencing NEXT_PUBLIC_SUPABASE_URL and parameter expansion like ${!name:-}.
334-337: Redirect error message to stderr.♻️ Proposed fix
*) - echo "[ERROR] Invalid PROD_E2E_SCOPE: ${SCOPE} (use all|public|auth)" + echo "[ERROR] Invalid PROD_E2E_SCOPE: ${SCOPE} (use all|public|auth)" >&2 exit 1 ;;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/e2e-production-audit.sh` around lines 334 - 337, The error message in the default case currently prints to stdout; change the echo in the case '*' block that references SCOPE/PROD_E2E_SCOPE to write to stderr instead (e.g., use redirection so the "[ERROR] Invalid PROD_E2E_SCOPE: ${SCOPE} (use all|public|auth)" message goes to stderr) and keep the exit 1 unchanged; update the case '*' branch where SCOPE is checked so the error output is redirected to stderr.
🤖 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/web/scripts/e2e-production-audit.sh`:
- Around line 42-82: Update the conditional tests in resolve_service_role_key
and resolve_vercel_env_dir to use the Bash [[ ... ]] syntax instead of [ ... ]
for safer, more consistent conditionals: change all occurrences like if [ -n
"${VAR}" ]; then and if [ -z "${VAR}" ]; then to if [[ -n "${VAR}" ]]; then / if
[[ -z "${VAR}" ]]; then, and change the file test if [ -f
"${canonical_repo_dir}/.vercel/project.json" ]; then to if [[ -f
"${canonical_repo_dir}/.vercel/project.json" ]]; then; keep the existing logic
and variable names (SUPABASE_SERVICE_ROLE_KEY, NEXT_PUBLIC_SUPABASE_URL,
project_ref, resolved, common_git_dir, canonical_repo_dir, REPO_DIR,
resolve_service_role_key, resolve_vercel_env_dir) unchanged.
- Around line 270-278: In the run_pack function update the file-existence checks
to use Bash [[ ]] conditionals for portability and safety (replace if [ ! -x
"${playwright_bin}" ]; then ... fi with if [[ ! -x "${playwright_bin}" ]]; then
... fi) and ensure error messages go to stderr by redirecting echoes (use echo
"[ERROR] Playwright binary not found." >&2) and exits remain the same; apply
these changes around the playwright_bin resolution logic inside run_pack so the
checks use [[ ]] and error output is redirected to stderr.
- Around line 18-40: Update the three helper functions to use bash [[ ]]
conditionals and send error messages to stderr: in require_env replace the [ -z
"${!name:-}" ] test with [[ -z "${!name:-}" ]] and write the echo error to >&2
before exiting; in ensure_non_local_supabase use [[ "${url}" == *localhost* ||
"${url}" == *127.0.0.1* ]] and echo the error to >&2; in
ensure_generation_backend use [[ -n "${MCP_GATEWAY_URL:-}" || -n
"${GEMINI_API_KEY:-}" || -n "${OPENAI_API_KEY:-}" || -n "${ANTHROPIC_API_KEY:-}"
]] and echo its error to >&2 — keep the same exit codes and preserve variable
quoting when referencing NEXT_PUBLIC_SUPABASE_URL and parameter expansion like
${!name:-}.
- Around line 334-337: The error message in the default case currently prints to
stdout; change the echo in the case '*' block that references
SCOPE/PROD_E2E_SCOPE to write to stderr instead (e.g., use redirection so the
"[ERROR] Invalid PROD_E2E_SCOPE: ${SCOPE} (use all|public|auth)" message goes to
stderr) and keep the exit 1 unchanged; update the case '*' branch where SCOPE is
checked so the error output is redirected to stderr.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a988fa9e-1b3c-46af-a64a-b7ed54b3cba1
⛔ Files ignored due to path filters (2)
CHANGELOG.mdis excluded by!**/*.mdREADME.mdis excluded by!**/*.md
📒 Files selected for processing (13)
apps/web/e2e/lead-readiness.spec.tsapps/web/e2e/production-public-smoke.spec.tsapps/web/package.jsonapps/web/playwright.production.config.tsapps/web/scripts/e2e-production-audit.shapps/web/src/app/(marketing)/roadmap/roadmap-client.tsxapps/web/src/app/api/generations/[id]/__tests__/route.test.tsapps/web/src/app/api/generations/[id]/route.tsapps/web/src/app/api/generations/__tests__/route.test.tsapps/web/src/app/api/generations/history/__tests__/route.test.tsapps/web/src/app/api/generations/history/route.tsapps/web/src/app/api/generations/route.tsapps/web/src/components/roadmap/PhaseCard.tsx
|



Summary
/api/generations,/api/generations/history, and/api/generations/[id]overflow-x-hiddenBilling is not enabledwhen disabled)test:e2e:prod, production Playwright config, public smoke spec)e2e-production-audit.shto use real runtime probes only and map fix targets correctlyValidation
npm run lint(apps/web) ✅npm run test -- src/app/api/generations/__tests__/route.test.ts src/app/api/generations/history/__tests__/route.test.ts✅npm run test -- --runTestsByPath 'src/app/api/generations/[id]/__tests__/route.test.ts'✅PROD_E2E_SCOPE=public npm run test:e2e:prod✅ harness execution + issues-map generation (expected fail on current prod roadmap/auth until deploy)Notes
/api/generations-> 500/api/generations/history-> 500/api/generations/nonexistent-id-> 500/roadmapmobile overflow test failsSummary by CodeRabbit
Release Notes
Bug Fixes
Tests