Conversation
…port within the software
railway-app
Bot
temporarily deployed
to
applirank / applirank-pr-24
|
🚅 Deployed to the applirank-pr-24 environment in applirank
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughFeedback UI converted into a GitHub-issue creation flow: modal captures reporter flags, diagnostics, optional screenshot and bug/feature context; server schemas validate new fields and assemble richer markdown issue bodies; env/auth helpers add Railway preview-aware BETTER_AUTH_URL resolution and conditional env validation. Changes
sequenceDiagram
participant User as User
participant Modal as FeedbackModal
participant API as feedback.post API
participant Validator as Schema Validator
participant GitHub as GitHub API
User->>Modal: Fill form (title, description, type) + optional screenshot/context/flags
Modal->>Modal: Capture diagnostics (UA, language, platform, timezone, viewport, screen)
Modal->>Modal: Convert/compress image to JPEG (if screenshot)
User->>Modal: Submit
Modal->>API: POST payload (fields, diagnostics, screenshot data URL, contexts, flags)
API->>Validator: Validate payload against schemas
Validator-->>API: Validation result
API->>API: Build GitHub issue body (summary, reporter rows, diagnostics, context blocks, screenshot)
API->>GitHub: Create issue
GitHub-->>API: Return issue URL
API-->>Modal: Respond with success + GitHub link
Modal-->>User: Show success and "View on GitHub"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 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)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/api/feedback.post.ts (1)
163-164:⚠️ Potential issue | 🟡 Minor
GITHUB_FEEDBACK_REPOformat is not validated before splitting.If the env var doesn't contain a
/,ownergets the entire string andrepoisundefined, producing a malformed GitHub API URL. The try/catch will surface this as a generic 502, which is confusing to diagnose.🛡️ Suggested guard
// ── Create GitHub Issue ───────────────────── const [owner, repo] = env.GITHUB_FEEDBACK_REPO.split('/') + if (!owner || !repo) { + throw createError({ + statusCode: 503, + statusMessage: 'GITHUB_FEEDBACK_REPO must be in "owner/repo" format', + }) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/feedback.post.ts` around lines 163 - 164, The env.GITHUB_FEEDBACK_REPO value is being split without validation (const [owner, repo] = env.GITHUB_FEEDBACK_REPO.split('/')), which can leave repo undefined and produce a malformed GitHub API URL; update feedback.post.ts to validate that env.GITHUB_FEEDBACK_REPO is a non-empty string containing a single '/' (or split into exactly two parts) before destructuring, and if invalid return a clear error/response (or log and throw) explaining the expected "owner/repo" format so downstream GitHub calls don't produce ambiguous 502s.
🧹 Nitpick comments (6)
server/utils/schemas/feedback.ts (2)
10-13:screenshotDataUrlSchemaregex:jpgMIME type is non-standard but harmless.The regex includes
image/jpgas an accepted MIME type, but the standard MIME type for JPEG isimage/jpeg. Sincecanvas.toDataURL('image/jpeg', ...)always producesimage/jpeg, andFile.typefor JPEG files is alsoimage/jpeg, thejpgbranch will never match in practice. Not harmful, but you could clean it up.♻️ Optional cleanup
- .regex(/^data:image\/(png|jpeg|jpg|webp);base64,[A-Za-z0-9+/=]+$/, 'Screenshot must be a valid image data URL') + .regex(/^data:image\/(png|jpeg|webp);base64,[A-Za-z0-9+/=]+$/, 'Screenshot must be a valid image data URL')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/schemas/feedback.ts` around lines 10 - 13, The regex in screenshotDataUrlSchema currently accepts the non-standard MIME type "image/jpg"; update the pattern used by screenshotDataUrlSchema to remove "jpg" and only allow the standard "jpeg" (e.g., change the alternation from (png|jpeg|jpg|webp) to (png|jpeg|webp)), keeping the same error message and .max constraint so validation remains the same but uses the correct MIME type set.
36-49: Schema accepts mismatched context objects (e.g.,bugContextwithtype: 'feature').The schema doesn't enforce that
bugContextis only provided whentype === 'bug'orfeatureContextwhentype === 'feature'. The server ignores the mismatch, so this is a defense-in-depth suggestion rather than a bug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/schemas/feedback.ts` around lines 36 - 49, createFeedbackSchema currently allows mismatched context objects (e.g., bugContext with type 'feature'); add a validation step on createFeedbackSchema (using z.object(...).superRefine or .refine) to enforce that when type === 'bug' bugContext is present and featureContext is absent, when type === 'feature' featureContext is present and bugContext is absent, and for other types neither context is provided; emit clear validation issues via ctx.addIssue referencing the fields 'bugContext' and 'featureContext' so the server rejects inconsistent payloads.server/api/feedback.post.ts (1)
185-191: Consider logging or differentiating GitHub API error codes.Currently all GitHub API failures are mapped to 502. A 422 (validation error, e.g. body too large, invalid labels) is a different class of problem from a 401 (bad token) or 404 (repo not found). Logging
err.statusalongside the message would aid debugging.♻️ Suggested improvement
} catch (err: any) { - console.error('[feedback] Failed to create GitHub issue:', err.data ?? err.message) + console.error('[feedback] Failed to create GitHub issue:', err.status, err.data ?? err.message) throw createError({ statusCode: 502, statusMessage: 'Failed to submit feedback. Please try again later.', }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/feedback.post.ts` around lines 185 - 191, The catch block in the feedback submission (catch in feedback.post.ts where GitHub issue creation is handled) currently logs only err.data/err.message and always throws a 502; update it to log err.status alongside err.data/err.message to aid debugging and map common GitHub API statuses to appropriate responses (e.g., propagate 401 for authentication errors, 404 for repo-not-found, 422 for validation) by returning createError with the actual statusCode (fall back to 502 for unknown statuses) and a clear statusMessage; ensure the console.error includes the status, message, and any err.data to preserve payload details.app/components/FeedbackModal.vue (3)
116-149: Screenshot selection error clearssubmitErrorbefore processing.Line 131 sets
submitError.value = ''at the start of the try block, which clears any prior submission error when the user picks a new screenshot. This is reasonable UX, but note that a prior "Failed to submit feedback" message will disappear simply by selecting a file, which might be surprising.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/FeedbackModal.vue` around lines 116 - 149, The handler handleScreenshotSelect currently clears submitError.value at the start of processing which removes unrelated prior errors; instead, only clear or overwrite submitError when the new screenshot is successfully validated/assigned or when replacing file-specific errors—move the submitError.value = '' out of the beginning of the try block and set it after screenshotDataUrl.value and screenshotFileName.value are assigned (or explicitly clear only image/file-specific errors before validation), keeping other submission errors intact; refer to handleScreenshotSelect, submitError, resetScreenshot, fileToDataUrl, compressImageDataUrl, screenshotDataUrl, and screenshotFileName when making the change.
217-224: Consider adding Escape-key handling and focus trapping for the modal.The modal currently closes only via the backdrop click or explicit buttons. Adding
@keydown.escape="resetAndClose"on the modal container and trapping focus within the modal are standard accessibility patterns for dialogs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/FeedbackModal.vue` around lines 217 - 224, Add Escape-key and focus-trap behavior to the FeedbackModal component so keyboard users can close and stay within the dialog: attach an `@keydown.escape`="resetAndClose" handler on the modal container (the element currently rendering the dialog content) and implement focus trapping around the modal by moving focus into the dialog when it opens (set initial focus to a sensible element like the first button/input), capturing Tab/Shift+Tab to cycle through focusable elements inside the modal, and restoring focus on close; reference the existing resetAndClose method and the Teleport/modal container element to integrate these changes, or use a small focus-trap helper/library to encapsulate the Tab handling.
43-50:navigator.platformis unreliable and not recommended; browser support for alternatives is limited.
navigator.platformis unreliable for OS detection and flagged by browsers as a fingerprinting surface. MDN recommends feature detection where possible. Whilenavigator.userAgentData?.platformis the intended replacement, it's only supported in Chromium-based browsers (Chrome/Edge 93+) and remains experimental. Firefox and Safari don't support it, so a fallback tonavigator.platformwould still be necessary. For diagnostic telemetry like this, the current approach is acceptable, but consider the tradeoff before refactoring.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/FeedbackModal.vue` around lines 43 - 50, Diagnostics currently uses navigator.platform which is flagged as unreliable; update the diagnostics construction (where diagnostics.value is set) to prefer navigator.userAgentData?.platform when available and fall back to navigator.platform for unsupported browsers, i.e., check userAgentData?.platform first and only use navigator.platform as a fallback while keeping other fields (userAgent, language, timezone, viewport, screen) unchanged; ensure the change is applied in the block that assigns diagnostics.value so other code referencing diagnostics.value continues to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/FeedbackModal.vue`:
- Line 16: The includeDiagnostics ref is currently initialized to true and reset
to true in resetAndClose, making diagnostics opt-out; change the default
initialization of includeDiagnostics to false and update resetAndClose (the
function named resetAndClose) to reset includeDiagnostics to false so the
diagnostics checkbox is opt-in by default; ensure any places that read or bind
includeDiagnostics (the ref) continue to work with the new boolean default.
In `@server/api/feedback.post.ts`:
- Around line 99-115: The current screenshotSection embeds
body.screenshotDataUrl directly which can exceed GitHub's issue-body limits;
update feedback submission logic in the handler that builds screenshotSection
(referencing body.includeScreenshot, body.screenshotDataUrl and
body.screenshotFileName) to first check the decoded/base64 length and if it
exceeds a safe threshold (~50 KB / ~70,000 characters) either (a) upload the
image to external storage and insert the returned URL into the issue body
instead of the data URL, or (b) reject early with a clear 422-style error
indicating the screenshot is too large; additionally, catch GitHub 422
validation responses in the submit flow and map them to a user-friendly error
message about screenshot size so callers see the real cause instead of a generic
502.
In `@server/utils/schemas/feedback.ts`:
- Around line 10-13: The screenshotDataUrlSchema currently allows up to 300000
characters which can produce base64 image data URLs that exceed GitHub's ~65K
issue body limit; update the max() constraint on screenshotDataUrlSchema to a
lower safe value (for example ~60000) so generated issue bodies stay under
GitHub's limit and ensure the validation error message still describes the size
constraint.
- Around line 50-57: Update the superRefine custom issue to use the
string-literal issue code required by Zod 4.x: in the superRefine callback where
you call context.addIssue for screenshotDataUrl (inside the function passed to
superRefine), replace the enum reference z.ZodIssueCode.custom with the literal
"custom" so the context.addIssue call uses code: "custom".
---
Outside diff comments:
In `@server/api/feedback.post.ts`:
- Around line 163-164: The env.GITHUB_FEEDBACK_REPO value is being split without
validation (const [owner, repo] = env.GITHUB_FEEDBACK_REPO.split('/')), which
can leave repo undefined and produce a malformed GitHub API URL; update
feedback.post.ts to validate that env.GITHUB_FEEDBACK_REPO is a non-empty string
containing a single '/' (or split into exactly two parts) before destructuring,
and if invalid return a clear error/response (or log and throw) explaining the
expected "owner/repo" format so downstream GitHub calls don't produce ambiguous
502s.
---
Nitpick comments:
In `@app/components/FeedbackModal.vue`:
- Around line 116-149: The handler handleScreenshotSelect currently clears
submitError.value at the start of processing which removes unrelated prior
errors; instead, only clear or overwrite submitError when the new screenshot is
successfully validated/assigned or when replacing file-specific errors—move the
submitError.value = '' out of the beginning of the try block and set it after
screenshotDataUrl.value and screenshotFileName.value are assigned (or explicitly
clear only image/file-specific errors before validation), keeping other
submission errors intact; refer to handleScreenshotSelect, submitError,
resetScreenshot, fileToDataUrl, compressImageDataUrl, screenshotDataUrl, and
screenshotFileName when making the change.
- Around line 217-224: Add Escape-key and focus-trap behavior to the
FeedbackModal component so keyboard users can close and stay within the dialog:
attach an `@keydown.escape`="resetAndClose" handler on the modal container (the
element currently rendering the dialog content) and implement focus trapping
around the modal by moving focus into the dialog when it opens (set initial
focus to a sensible element like the first button/input), capturing
Tab/Shift+Tab to cycle through focusable elements inside the modal, and
restoring focus on close; reference the existing resetAndClose method and the
Teleport/modal container element to integrate these changes, or use a small
focus-trap helper/library to encapsulate the Tab handling.
- Around line 43-50: Diagnostics currently uses navigator.platform which is
flagged as unreliable; update the diagnostics construction (where
diagnostics.value is set) to prefer navigator.userAgentData?.platform when
available and fall back to navigator.platform for unsupported browsers, i.e.,
check userAgentData?.platform first and only use navigator.platform as a
fallback while keeping other fields (userAgent, language, timezone, viewport,
screen) unchanged; ensure the change is applied in the block that assigns
diagnostics.value so other code referencing diagnostics.value continues to work.
In `@server/api/feedback.post.ts`:
- Around line 185-191: The catch block in the feedback submission (catch in
feedback.post.ts where GitHub issue creation is handled) currently logs only
err.data/err.message and always throws a 502; update it to log err.status
alongside err.data/err.message to aid debugging and map common GitHub API
statuses to appropriate responses (e.g., propagate 401 for authentication
errors, 404 for repo-not-found, 422 for validation) by returning createError
with the actual statusCode (fall back to 502 for unknown statuses) and a clear
statusMessage; ensure the console.error includes the status, message, and any
err.data to preserve payload details.
In `@server/utils/schemas/feedback.ts`:
- Around line 10-13: The regex in screenshotDataUrlSchema currently accepts the
non-standard MIME type "image/jpg"; update the pattern used by
screenshotDataUrlSchema to remove "jpg" and only allow the standard "jpeg"
(e.g., change the alternation from (png|jpeg|jpg|webp) to (png|jpeg|webp)),
keeping the same error message and .max constraint so validation remains the
same but uses the correct MIME type set.
- Around line 36-49: createFeedbackSchema currently allows mismatched context
objects (e.g., bugContext with type 'feature'); add a validation step on
createFeedbackSchema (using z.object(...).superRefine or .refine) to enforce
that when type === 'bug' bugContext is present and featureContext is absent,
when type === 'feature' featureContext is present and bugContext is absent, and
for other types neither context is provided; emit clear validation issues via
ctx.addIssue referencing the fields 'bugContext' and 'featureContext' so the
server rejects inconsistent payloads.
| const includeReporterContext = ref(false) | ||
| const includeEmail = ref(false) | ||
| const includeScreenshot = ref(false) | ||
| const includeDiagnostics = ref(true) |
There was a problem hiding this comment.
Diagnostics defaults to opt-out, contradicting PR's stated "explicit opt-in" design.
includeDiagnostics defaults to true (line 16) and is reset to true on close (line 204), making it opt-out. The PR description states "Explicit opt-in checkboxes control sharing of email, name and current page, technical diagnostics, and screenshot." If the intent is opt-in, this should default to false.
🔧 Fix to align with opt-in
-const includeDiagnostics = ref(true)
+const includeDiagnostics = ref(false)And in resetAndClose:
- includeDiagnostics.value = true
+ includeDiagnostics.value = false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/FeedbackModal.vue` at line 16, The includeDiagnostics ref is
currently initialized to true and reset to true in resetAndClose, making
diagnostics opt-out; change the default initialization of includeDiagnostics to
false and update resetAndClose (the function named resetAndClose) to reset
includeDiagnostics to false so the diagnostics checkbox is opt-in by default;
ensure any places that read or bind includeDiagnostics (the ref) continue to
work with the new boolean default.
Comment on lines
+50
to
+57
| }).superRefine((value, context) => { | ||
| if (value.includeScreenshot && !value.screenshotDataUrl) { | ||
| context.addIssue({ | ||
| code: z.ZodIssueCode.custom, | ||
| message: 'Screenshot is required when sharing screenshot is enabled', | ||
| path: ['screenshotDataUrl'], | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Zod 4 ZodIssueCode custom superRefine API
💡 Result:
In Zod v4, z.ZodIssueCode is effectively replaced by string-literal issue codes. So instead of:
ctx.addIssue({ code: z.ZodIssueCode.custom, ... })use:
ctx.addIssue({
code: "custom",
message: "Something is wrong",
path: ["field"], // optional
params: { ... }, // optional
input: value, // optional (v4 supports it)
});Zod’s own docs now show ctx.addIssue being called with string codes like "too_big" and "custom" inside .superRefine(). [1] The v4 custom issue shape is literally code: "custom" (a string), not an enum member. [2]
If you’re looking at newer v4 guidance, note that Zod also documents a lower-level alternative, .check(), which can be used instead of .superRefine() in some cases. [1]
Sources: Zod docs (.superRefine, .check) [1]; Zod v4 type docs for $ZodIssueCustom [2].
🏁 Script executed:
#!/bin/bash
# Check package.json for Zod version
echo "=== Zod version in package.json ==="
rg -A 2 '"zod"' package.json | head -20
# Check for other uses of z.ZodIssueCode in the codebase
echo -e "\n=== Other uses of z.ZodIssueCode ==="
rg 'z\.ZodIssueCode' --type ts --type jsRepository: applirank/applirank
Length of output: 252
Replace z.ZodIssueCode.custom with "custom" at line 53.
Zod 4.x removed the z.ZodIssueCode enum and replaced it with string-literal issue codes. Change:
code: z.ZodIssueCode.custom,
to:
code: "custom",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/utils/schemas/feedback.ts` around lines 50 - 57, Update the
superRefine custom issue to use the string-literal issue code required by Zod
4.x: in the superRefine callback where you call context.addIssue for
screenshotDataUrl (inside the function passed to superRefine), replace the enum
reference z.ZodIssueCode.custom with the literal "custom" so the
context.addIssue call uses code: "custom".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
server/utils/auth.ts (1)
9-44: Hardcoded Railway project/service slug in PR URL pattern is fragile.Line 23 embeds
applirank-applirankdirectly in the URL template. If the Railway project or service is ever renamed, this will silently produce an incorrectbaseURL, breaking authentication in all preview environments without an obvious error. Consider deriving it from an env variable (e.g.,RAILWAY_PROJECT_NAME/RAILWAY_SERVICE_NAME) or at least fromRAILWAY_PUBLIC_DOMAINwhen available, falling through to the hardcoded pattern only as a last resort.Additionally, this duplicates validation already performed by the
superRefineinenvSchema(which throws before this code can run). That's fine as defense-in-depth, but worth being aware of.Suggested approach
const prNumber = env.RAILWAY_GIT_PR_NUMBER?.trim() if (prNumber) { - const previewUrl = `https://applirank-applirank-pr-${prNumber}.up.railway.app` + // Prefer the auto-assigned public domain if available; fall back to convention-based URL + const railwayDomain = env.RAILWAY_PUBLIC_DOMAIN?.trim() + const previewUrl = railwayDomain + ? `https://${railwayDomain}` + : `https://applirank-applirank-pr-${prNumber}.up.railway.app` console.info(`[Applirank] Using Railway PR-derived BETTER_AUTH_URL: ${previewUrl}`) return previewUrl }This way the hardcoded slug is only the last-resort fallback and
RAILWAY_PUBLIC_DOMAINis preferred when both are present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/auth.ts` around lines 9 - 44, In resolveBetterAuthUrl(), avoid the hardcoded "applirank-applirank" PR URL; instead build the PR preview URL from env variables (prefer RAILWAY_PUBLIC_DOMAIN when present, then derive a slug from RAILWAY_PROJECT_NAME and RAILWAY_SERVICE_NAME if available, and only fall back to the hardcoded slug as a last resort), preserve the existing checks for RAILWAY_GIT_PR_NUMBER and BETTER_AUTH_URL, and update the informational log messages accordingly so preview URLs are derived from RAILWAY_PUBLIC_DOMAIN or RAILWAY_PROJECT_NAME/RAILWAY_SERVICE_NAME before using the hardcoded pattern.server/utils/env.ts (1)
95-101: Error hint doesn't list new optional env vars.The helpful error message on line 100 lists
TRUSTED_PROXY_IPandDEMO_ORG_SLUGas optional but omits the newly addedGITHUB_FEEDBACK_TOKEN,GITHUB_FEEDBACK_REPO, and the Railway variables (RAILWAY_ENVIRONMENT_NAME, etc.). Developers debugging a failed boot won't see these as available knobs.Suggested update
- `Optional: S3_REGION (default: us-east-1), S3_FORCE_PATH_STYLE (default: true), TRUSTED_PROXY_IP, DEMO_ORG_SLUG\n`, + `Optional: S3_REGION (default: us-east-1), S3_FORCE_PATH_STYLE (default: true), TRUSTED_PROXY_IP, DEMO_ORG_SLUG, GITHUB_FEEDBACK_TOKEN, GITHUB_FEEDBACK_REPO\n`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/env.ts` around lines 95 - 101, Update the console.error message block in server/utils/env.ts (the console.error call that prints missing/invalid environment variables) to include the newly added optional env vars: GITHUB_FEEDBACK_TOKEN and GITHUB_FEEDBACK_REPO, and also list the Railway-specific variables (e.g., RAILWAY_ENVIRONMENT_NAME and other Railway vars your app uses) alongside the existing optional entries TRUSTED_PROXY_IP and DEMO_ORG_SLUG; modify the string literal passed to console.error so the "Optional:" section enumerates these extra variables (and note any defaults if applicable) so developers see all available knobs when boot fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/utils/env.ts`:
- Around line 17-27: The isRailwayPreviewEnvironment function incorrectly treats
"production" as a preview because name.startsWith('pr') matches "production";
update isRailwayPreviewEnvironment to detect PR previews more precisely (e.g.,
require the "pr" prefix to be followed by a digit or hyphen or use
startsWith('pr-')/a regex like /^pr[\d-]/) or explicitly exclude the exact
string "production" so production is never classified as a preview; modify the
logic inside isRailwayPreviewEnvironment (referencing the name variable and the
function isRailwayPreviewEnvironment) to implement one of these stricter checks
and keep the other includes (like 'pr-' and 'preview') as-is.
---
Nitpick comments:
In `@server/utils/auth.ts`:
- Around line 9-44: In resolveBetterAuthUrl(), avoid the hardcoded
"applirank-applirank" PR URL; instead build the PR preview URL from env
variables (prefer RAILWAY_PUBLIC_DOMAIN when present, then derive a slug from
RAILWAY_PROJECT_NAME and RAILWAY_SERVICE_NAME if available, and only fall back
to the hardcoded slug as a last resort), preserve the existing checks for
RAILWAY_GIT_PR_NUMBER and BETTER_AUTH_URL, and update the informational log
messages accordingly so preview URLs are derived from RAILWAY_PUBLIC_DOMAIN or
RAILWAY_PROJECT_NAME/RAILWAY_SERVICE_NAME before using the hardcoded pattern.
In `@server/utils/env.ts`:
- Around line 95-101: Update the console.error message block in
server/utils/env.ts (the console.error call that prints missing/invalid
environment variables) to include the newly added optional env vars:
GITHUB_FEEDBACK_TOKEN and GITHUB_FEEDBACK_REPO, and also list the
Railway-specific variables (e.g., RAILWAY_ENVIRONMENT_NAME and other Railway
vars your app uses) alongside the existing optional entries TRUSTED_PROXY_IP and
DEMO_ORG_SLUG; modify the string literal passed to console.error so the
"Optional:" section enumerates these extra variables (and note any defaults if
applicable) so developers see all available knobs when boot fails.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/components/FeedbackModal.vue (2)
345-354: Collapsible section lacksaria-expandedandaria-controlsfor screen reader accessibility.The toggle button at line 346 controls the visibility of the content panel but doesn't communicate its state to assistive technologies.
Proposed improvement
<button type="button" class="flex w-full items-center justify-between px-3 py-2 text-left" + :aria-expanded="showOptionalContext" + aria-controls="optional-context-panel" `@click`="showOptionalContext = !showOptionalContext" >And on the content
div:- <div v-if="showOptionalContext" class="space-y-3 ..."> + <div v-if="showOptionalContext" id="optional-context-panel" class="space-y-3 ...">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/FeedbackModal.vue` around lines 345 - 354, Add accessible ARIA attributes to the collapsible control: on the toggle button that binds to showOptionalContext (the button using `@click` and rendering ChevronDown/ChevronRight) add aria-expanded bound to the boolean (e.g., :aria-expanded="showOptionalContext") and an aria-controls that references a unique id; then give the collapsible content container a matching id (and role="region" or aria-hidden updated based on showOptionalContext) so the button properly announces state and controls the panel; ensure the state bindings update when showOptionalContext toggles.
26-26:MAX_SCREENSHOT_DATA_URL_CHARSis duplicated between client and server.This constant is defined both here (line 26) and in
server/utils/schemas/feedback.ts(line 10). If one changes without the other, the client could allow screenshots that the server rejects (or vice versa). Consider importing from a shared location or deriving the client limit from the schema.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/FeedbackModal.vue` at line 26, The client and server both define MAX_SCREENSHOT_DATA_URL_CHARS (used in FeedbackModal.vue and server/utils/schemas/feedback.ts) causing drift; consolidate the source of truth by extracting the numeric limit into a shared module (e.g., export const MAX_SCREENSHOT_DATA_URL_CHARS from a shared/constants or shared/validation file) and import that constant in both FeedbackModal.vue and the server schema, or alternatively derive the client limit from the server validation (export the schema's limit value and import it into FeedbackModal.vue) so both sides always use the same value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/api/feedback.post.ts`:
- Line 182: Validate env.GITHUB_FEEDBACK_REPO before destructuring: instead of
directly doing const [owner, repo] = env.GITHUB_FEEDBACK_REPO.split('/'), check
that env.GITHUB_FEEDBACK_REPO is a non-empty string containing exactly one '/'
and that split('/') yields two non-empty parts; if validation fails, log a clear
error and return/throw a controlled error (or fail fast at startup) so you never
call the GitHub API with owner or repo === undefined.
- Around line 135-151: The markdown list items for bug/feature context can
contain newlines (e.g., body.bugContext.stepsToReproduce, expectedResult,
actualResult and body.featureContext.*) which break the list; update the code
that builds the array (the ternary branch guarded by body.type === 'bug') to
either normalize those fields to single-line (like the existing
escapeMarkdownTableValue approach) or wrap each multi-line value in a block
format (e.g., a subheading + a fenced code block or paragraph) so newlines are
preserved without breaking the parent list; target the expressions that
interpolate stepsToReproduce, expectedResult, actualResult and the
featureContext fields and apply the chosen normalization/wrapping before
inserting into the template.
---
Duplicate comments:
In `@app/components/FeedbackModal.vue`:
- Line 16: The includeDiagnostics ref currently defaults to true and is
re-enabled in resetAndClose, which makes diagnostics opt-out; change
includeDiagnostics to initialize to false (const includeDiagnostics =
ref(false)) and ensure resetAndClose sets includeDiagnostics.value = false so
diagnostics are opt-in; update any related initialization/cleanup in the
FeedbackModal component (look for includeDiagnostics and resetAndClose) to
reflect the opt-in default.
In `@server/utils/schemas/feedback.ts`:
- Around line 52-59: The superRefine validator on the feedback schema uses
z.ZodIssueCode.custom which doesn't exist in Zod v4; update the call inside
superRefine (the block checking value.includeScreenshot and
value.screenshotDataUrl) to pass the string literal "custom" for the issue code
when calling context.addIssue (keep the same message and path
['screenshotDataUrl'] and otherwise unchanged).
---
Nitpick comments:
In `@app/components/FeedbackModal.vue`:
- Around line 345-354: Add accessible ARIA attributes to the collapsible
control: on the toggle button that binds to showOptionalContext (the button
using `@click` and rendering ChevronDown/ChevronRight) add aria-expanded bound to
the boolean (e.g., :aria-expanded="showOptionalContext") and an aria-controls
that references a unique id; then give the collapsible content container a
matching id (and role="region" or aria-hidden updated based on
showOptionalContext) so the button properly announces state and controls the
panel; ensure the state bindings update when showOptionalContext toggles.
- Line 26: The client and server both define MAX_SCREENSHOT_DATA_URL_CHARS (used
in FeedbackModal.vue and server/utils/schemas/feedback.ts) causing drift;
consolidate the source of truth by extracting the numeric limit into a shared
module (e.g., export const MAX_SCREENSHOT_DATA_URL_CHARS from a shared/constants
or shared/validation file) and import that constant in both FeedbackModal.vue
and the server schema, or alternatively derive the client limit from the server
validation (export the schema's limit value and import it into
FeedbackModal.vue) so both sides always use the same value.
Comment on lines
+135
to
+151
| ...(body.type === 'bug' | ||
| ? [ | ||
| '### Bug Reproduction', | ||
| '', | ||
| `- Steps to reproduce: ${body.bugContext?.stepsToReproduce?.trim() || '_not provided_'}`, | ||
| `- Expected result: ${body.bugContext?.expectedResult?.trim() || '_not provided_'}`, | ||
| `- Actual result: ${body.bugContext?.actualResult?.trim() || '_not provided_'}`, | ||
| '', | ||
| ] | ||
| : [ | ||
| '### Feature Context', | ||
| '', | ||
| `- User problem: ${body.featureContext?.userProblem?.trim() || '_not provided_'}`, | ||
| `- Desired workflow: ${body.featureContext?.desiredWorkflow?.trim() || '_not provided_'}`, | ||
| `- Expected impact: ${body.featureContext?.expectedImpact?.trim() || '_not provided_'}`, | ||
| '', | ||
| ]), |
There was a problem hiding this comment.
Multi-line user input will break markdown list formatting.
stepsToReproduce, expectedResult, actualResult, and the feature context fields can contain newlines (up to 1500 chars). When embedded directly in - Steps to reproduce: ..., any newline will break the list item into a separate paragraph, producing malformed markdown in the GitHub issue.
Either normalize these to single-line (like escapeMarkdownTableValue does for table cells) or switch to a block format (e.g., a sub-heading + paragraph or a fenced block) that accommodates multi-line content.
Proposed fix — use block format for multi-line fields
...(body.type === 'bug'
? [
'### Bug Reproduction',
'',
- `- Steps to reproduce: ${body.bugContext?.stepsToReproduce?.trim() || '_not provided_'}`,
- `- Expected result: ${body.bugContext?.expectedResult?.trim() || '_not provided_'}`,
- `- Actual result: ${body.bugContext?.actualResult?.trim() || '_not provided_'}`,
+ '**Steps to reproduce:**',
+ body.bugContext?.stepsToReproduce?.trim() || '_not provided_',
+ '',
+ '**Expected result:**',
+ body.bugContext?.expectedResult?.trim() || '_not provided_',
+ '',
+ '**Actual result:**',
+ body.bugContext?.actualResult?.trim() || '_not provided_',
'',
]
: [
'### Feature Context',
'',
- `- User problem: ${body.featureContext?.userProblem?.trim() || '_not provided_'}`,
- `- Desired workflow: ${body.featureContext?.desiredWorkflow?.trim() || '_not provided_'}`,
- `- Expected impact: ${body.featureContext?.expectedImpact?.trim() || '_not provided_'}`,
+ '**User problem:**',
+ body.featureContext?.userProblem?.trim() || '_not provided_',
+ '',
+ '**Desired workflow:**',
+ body.featureContext?.desiredWorkflow?.trim() || '_not provided_',
+ '',
+ '**Expected impact:**',
+ body.featureContext?.expectedImpact?.trim() || '_not provided_',
'',
]),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/api/feedback.post.ts` around lines 135 - 151, The markdown list items
for bug/feature context can contain newlines (e.g.,
body.bugContext.stepsToReproduce, expectedResult, actualResult and
body.featureContext.*) which break the list; update the code that builds the
array (the ternary branch guarded by body.type === 'bug') to either normalize
those fields to single-line (like the existing escapeMarkdownTableValue
approach) or wrap each multi-line value in a block format (e.g., a subheading +
a fenced code block or paragraph) so newlines are preserved without breaking the
parent list; target the expressions that interpolate stepsToReproduce,
expectedResult, actualResult and the featureContext fields and apply the chosen
normalization/wrapping before inserting into the template.
| } | ||
|
|
||
| // ── Create GitHub Issue ───────────────────── | ||
| const [owner, repo] = env.GITHUB_FEEDBACK_REPO.split('/') |
There was a problem hiding this comment.
GITHUB_FEEDBACK_REPO.split('/') has no format validation — repo may be undefined.
If the env var is set to a malformed value (e.g., "myrepo" without a /), repo will be undefined, producing a GitHub API URL like .../repos/myrepo/undefined/issues which will fail with a confusing error swallowed into the generic 502.
Consider validating the format at startup or before use.
Proposed guard
const [owner, repo] = env.GITHUB_FEEDBACK_REPO.split('/')
+ if (!owner || !repo) {
+ throw createError({
+ statusCode: 503,
+ statusMessage: 'GITHUB_FEEDBACK_REPO must be in "owner/repo" format',
+ })
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/api/feedback.post.ts` at line 182, Validate env.GITHUB_FEEDBACK_REPO
before destructuring: instead of directly doing const [owner, repo] =
env.GITHUB_FEEDBACK_REPO.split('/'), check that env.GITHUB_FEEDBACK_REPO is a
non-empty string containing exactly one '/' and that split('/') yields two
non-empty parts; if validation fails, log a clear error and return/throw a
controlled error (or fail fast at startup) so you never call the GitHub API with
owner or repo === undefined.
… and adding checks for empty and production names
railway-app
Bot
temporarily deployed
to
applirank / applirank-pr-24
railway-app
Bot
temporarily deployed
to
applirank / applirank-pr-24
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
.github/workflows/pr-validation.yml (3)
42-43:npm auditas a hard CI gate can permanently block PRs due to uncontrollable transitive vulnerabilities.Transitive dependencies updated upstream can introduce high-severity advisories at any moment, stalling every PR until a fix or override is applied. Consider two mitigations:
- Downgrade to
--audit-level=critical— still blocks on truly critical findings while tolerating high-severity transitive noise.- Make it advisory-only — emit the report without failing the job:
⚙️ Advisory-only audit step
- name: Audit dependencies (high severity+) - run: npm audit --audit-level=high + name: Audit dependencies (high severity+) + run: npm audit --audit-level=high || true + # Remove `|| true` once a process is in place to triage advisories🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml around lines 42 - 43, The CI step named "Audit dependencies (high severity+)" currently runs "npm audit --audit-level=high", which can block PRs due to transitive high-severity advisories; change this step to either run "npm audit --audit-level=critical" to only fail on critical issues or make it advisory-only by running "npm audit --json" (or "npm audit --audit-level=high --json") and writing the output to a file/artifact while not failing the job (ensure the step does not use a failing exit code), updating the step configuration for "Audit dependencies (high severity+)" accordingly.
18-28: Pinubuntu-latestandnode-versionfor reproducible builds.
ubuntu-latestandnode-version: 20are floating references; a GitHub infrastructure update or a new Node 20 minor release can silently change build behaviour between runs.📌 Pin to specific versions
- runs-on: ubuntu-latest + runs-on: ubuntu-24.04 - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: 20 + node-version-file: .nvmrc # or node-version: '20.x' pinned to a specific minor cache: npmIf the project doesn't have an
.nvmrc, an explicit patch version (e.g.,'20.18.0') achieves the same goal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml around lines 18 - 28, Replace the floating runner and Node version with pinned values to ensure reproducible builds: change runs-on: ubuntu-latest to a specific runner like runs-on: ubuntu-22.04 and change the actions/setup-node@v4 with node-version: 20 to an explicit patch version (e.g., node-version: '20.18.0') in the workflow step that uses actions/setup-node; alternatively, add or reference an .nvmrc and set node-version to the exact version from .nvmrc so the Setup Node.js step uses a fixed Node patch release.
16-18: Consider skipping CI on draft PRs to avoid burning runner minutes on work-in-progress.The
openedandsynchronizetypes fire unconditionally for drafts too.ready_for_reviewis additive (converts draft → ready) but does not suppress the earlier types. Adding a job-level condition gates the expensive steps:💡 Add a draft guard
validate: name: Build, typecheck, and test runs-on: ubuntu-latest + if: github.event.pull_request.draft == false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml around lines 16 - 18, The CI job "validate" (name "Build, typecheck, and test") should skip running for draft PRs to avoid wasted runner minutes; add a job-level guard that checks the PR draft flag (e.g., use the GitHub Actions expression that tests github.event.pull_request.draft is false) so the expensive steps only run when the PR is not a draft (apply this condition to the "validate" job).app/components/FeedbackModal.vue (2)
47-47:navigator.platformis deprecated — prefer User-Agent Client Hints with a fallback.
Navigator.platformis theoretically useful for detecting the browser environment, but MDN notes it "is unreliable and is not recommended" for general use.navigator.userAgentData.platformis the recommended modern alternative, thoughnavigator.userAgentData.platformis experimental and currently only supported by Chromium-based browsers.Since this field is purely informational context on a GitHub issue, the standard fallback pattern works well here:
♻️ Proposed refactor
- platform: window.navigator.platform, + platform: (window.navigator as any).userAgentData?.platform ?? window.navigator.platform,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/FeedbackModal.vue` at line 47, In FeedbackModal.vue replace direct use of window.navigator.platform with a feature-detection fallback that prefers User-Agent Client Hints: read navigator.userAgentData?.platform if available, otherwise fall back to navigator.platform and finally to an empty string; update the object key currently set as platform: window.navigator.platform to platform: (navigator.userAgentData?.platform || navigator.platform || '') and ensure you check for undefined navigator/userAgentData before accessing properties so no runtime errors occur.
47-47: Migratenavigator.platformto User-Agent Client Hints for future browser compatibility.
Navigator.platformis deprecated and unreliable in modern browsers; Chromium has frozen it to coarse values (e.g.,Win32for all Windows). Replace withnavigator.userAgentData?.platform, falling back to the current API for older browsers:platform: navigator.userAgentData?.platform ?? window.navigator.platform,For more detailed platform information if needed, use
navigator.userAgentData?.getHighEntropyValues(['platformVersion', 'architecture']).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/FeedbackModal.vue` at line 47, Replace the deprecated window.navigator.platform usage in the FeedbackModal's platform assignment with the User-Agent Client Hints API: use navigator.userAgentData?.platform with a fallback to window.navigator.platform (i.e., set platform to navigator.userAgentData?.platform ?? window.navigator.platform) and, if you need higher-entropy details, call navigator.userAgentData?.getHighEntropyValues(['platformVersion','architecture']) and include those values in the payload; update the code at the property where platform: window.navigator.platform is set (in the FeedbackModal component) accordingly.server/utils/env.ts (2)
60-62: Co-validateGITHUB_FEEDBACK_TOKENandGITHUB_FEEDBACK_REPOin.superRefine().Both fields are optional in isolation, but they're mutually required — providing only one silently disables or breaks the feedback feature at runtime with no startup-time signal. Add a cross-field check alongside the existing
BETTER_AUTH_URLvalidation:♻️ Proposed addition to superRefine
.superRefine((data, ctx) => { const isPreview = isRailwayPreviewEnvironment(data.RAILWAY_ENVIRONMENT_NAME) if (!isPreview && !data.BETTER_AUTH_URL) { ctx.addIssue({ code: z.ZodIssueCode.custom, path: ['BETTER_AUTH_URL'], message: 'BETTER_AUTH_URL is required outside Railway PR/preview environments', }) } + + const hasToken = !!data.GITHUB_FEEDBACK_TOKEN + const hasRepo = !!data.GITHUB_FEEDBACK_REPO + if (hasToken !== hasRepo) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [hasToken ? 'GITHUB_FEEDBACK_REPO' : 'GITHUB_FEEDBACK_TOKEN'], + message: 'GITHUB_FEEDBACK_TOKEN and GITHUB_FEEDBACK_REPO must both be set to enable in-app feedback', + }) + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/env.ts` around lines 60 - 62, Add a cross-field validation inside the same zod schema.superRefine() that already checks BETTER_AUTH_URL: verify that GITHUB_FEEDBACK_TOKEN and GITHUB_FEEDBACK_REPO are either both present or both absent, and if only one is provided call ctx.addIssue() with a clear message and set path to the missing field (use 'GITHUB_FEEDBACK_TOKEN' or 'GITHUB_FEEDBACK_REPO' accordingly); reference the existing superRefine block so the new check runs at startup and prevents silent misconfiguration of the feedback feature.
103-105: Error message is missing newly added optional env vars.
GITHUB_FEEDBACK_TOKEN,GITHUB_FEEDBACK_REPO, and the Railway metadata fields (RAILWAY_ENVIRONMENT_NAME,RAILWAY_GIT_PR_NUMBER,RAILWAY_PUBLIC_DOMAIN) are not mentioned in the startup hint. Operators won't know these exist when troubleshooting.📝 Proposed update
- `Optional: S3_REGION (default: us-east-1), S3_FORCE_PATH_STYLE (default: true), TRUSTED_PROXY_IP, DEMO_ORG_SLUG\n`, + `Optional: S3_REGION (default: us-east-1), S3_FORCE_PATH_STYLE (default: true), TRUSTED_PROXY_IP, DEMO_ORG_SLUG\n` + + `Optional (feedback): GITHUB_FEEDBACK_TOKEN, GITHUB_FEEDBACK_REPO (both required together)\n` + + `Optional (Railway metadata): RAILWAY_ENVIRONMENT_NAME, RAILWAY_GIT_PR_NUMBER, RAILWAY_PUBLIC_DOMAIN\n`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/env.ts` around lines 103 - 105, The startup hint string in server/utils/env.ts is missing recently added optional env vars; update the multi-line template string (the message built where the backticked block is defined—e.g., the startup hint / help message constant) to list GITHUB_FEEDBACK_TOKEN and GITHUB_FEEDBACK_REPO as optional env vars and add the Railway metadata vars RAILWAY_ENVIRONMENT_NAME, RAILWAY_GIT_PR_NUMBER, and RAILWAY_PUBLIC_DOMAIN to the message (mark them as optional/Railway-specific), keeping the existing formatting and any default notes (e.g., indicate defaults only where applicable).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/FeedbackModal.vue`:
- Line 13: The includeReporterContext ref is currently defaulting to true and
being reset to true in resetAndClose, which violates the explicit opt-in
requirement; change the initialization of includeReporterContext to false and
update resetAndClose (the resetAndClose function) to reset
includeReporterContext back to false so name+URL are only shared when the user
actively opts in via the checkbox.
- Line 13: The includeReporterContext ref in FeedbackModal.vue is currently
defaulted to true and reset to true in resetAndClose, which leaks name/URL;
change the default initialization of includeReporterContext to false and update
resetAndClose (the reset logic) to set includeReporterContext.value = false so
the reporter context is opt-in, ensuring the checkbox bound to
includeReporterContext still toggles the value as before.
- Around line 346-354: The toggle button for "Add More Context (Optional)" is
missing an accessibility state; update the button (the element with `@click`
toggling showOptionalContext and rendering ChevronDown/ChevronRight) to include
an aria-expanded attribute bound to showOptionalContext (e.g.,
:aria-expanded="showOptionalContext") and add an aria-controls that references
the id of the collapsible content panel; ensure the collapsible panel element
has that matching id so assistive tech can announce the expanded/collapsed
state.
- Around line 346-354: The toggle button that flips showOptionalContext (the
button rendering ChevronDown/ChevronRight) is missing accessible state; bind
aria-expanded to showOptionalContext on that button and add aria-controls
pointing to the collapsible container's id, then ensure the collapsible <div>
that shows/hides the optional context has a matching id and appropriate hidden
handling (e.g., v-show/v-if or aria-hidden) so screen readers can determine the
expanded/collapsed state for the showOptionalContext toggle and its content.
- Around line 117-150: The handler handleScreenshotSelect currently calls
resetScreenshot() on error but does not clear the underlying HTMLInputElement
value, so reselecting the same file won't retrigger change; update either
resetScreenshot() or handleScreenshotSelect to set input.value = '' (or cast and
clear the element stored by the file input) whenever you call resetScreenshot on
error paths (non-image type, too large, catch block) so the input's .value is
cleared and the same file can be selected again; reference the input variable in
handleScreenshotSelect and/or the resetScreenshot function to implement this
clear.
- Around line 117-150: The file input's DOM value isn't cleared, so selecting
the same file after an error doesn't re-trigger change; update
handleScreenshotSelect to clear the actual input element when an error occurs
(or change resetScreenshot to accept an HTMLInputElement and clear its
value/files). Concretely: in handleScreenshotSelect, after each error return
(non-image type, oversized, or catch) set input.value = '' (or input.files =
null), or refactor resetScreenshot(someInput) to perform input.value = '' and
call that instead, ensuring the `@change` will fire for the same file next time.
In `@server/utils/env.ts`:
- Line 40: The empty-string preprocessing currently uses emptyToUndefined
defined as z.preprocess(fn, z.string()), which rejects the undefined returned by
the preprocess (breaking RAILWAY_ENVIRONMENT_NAME and RAILWAY_PUBLIC_DOMAIN).
Update the emptyToUndefined definition so its inner schema is optional (e.g.,
z.preprocess(fn, z.string().optional())) so the preprocess can return undefined
without failing validation, and keep using emptyToUndefined.optional() for those
env vars (apply the same change where emptyToUndefined is used).
---
Duplicate comments:
In `@app/components/FeedbackModal.vue`:
- Line 16: The includeDiagnostics reactive flag currently defaults to true and
is re-enabled in resetAndClose, preserving an opt-out UX; change its initial
value (const includeDiagnostics = ref(...)) to false and update resetAndClose to
set includeDiagnostics.value = false so the modal defaults to opt-in for
diagnostics and remains off after closing.
- Line 16: The includeDiagnostics ref is defaulting to true and resetAndClose is
reassigning it to true, preserving the unwanted opt-in-by-default behavior;
change the initialization of includeDiagnostics to false and update
resetAndClose to set includeDiagnostics.value = false (referencing the
includeDiagnostics ref and the resetAndClose method) so the modal defaults and
resets to opt-out.
In `@server/utils/env.ts`:
- Around line 17-32: The isRailwayPreviewEnvironment function has been corrected
(see isRailwayPreviewEnvironment and its use of
environmentName?.toLowerCase().trim(), the empty-string guard, and the
production/prod early-return) and no further changes are required—leave this
function as-is since the prior misclassification issue is resolved and the
defensive checks and regex are appropriate.
---
Nitpick comments:
In @.github/workflows/pr-validation.yml:
- Around line 42-43: The CI step named "Audit dependencies (high severity+)"
currently runs "npm audit --audit-level=high", which can block PRs due to
transitive high-severity advisories; change this step to either run "npm audit
--audit-level=critical" to only fail on critical issues or make it advisory-only
by running "npm audit --json" (or "npm audit --audit-level=high --json") and
writing the output to a file/artifact while not failing the job (ensure the step
does not use a failing exit code), updating the step configuration for "Audit
dependencies (high severity+)" accordingly.
- Around line 18-28: Replace the floating runner and Node version with pinned
values to ensure reproducible builds: change runs-on: ubuntu-latest to a
specific runner like runs-on: ubuntu-22.04 and change the actions/setup-node@v4
with node-version: 20 to an explicit patch version (e.g., node-version:
'20.18.0') in the workflow step that uses actions/setup-node; alternatively, add
or reference an .nvmrc and set node-version to the exact version from .nvmrc so
the Setup Node.js step uses a fixed Node patch release.
- Around line 16-18: The CI job "validate" (name "Build, typecheck, and test")
should skip running for draft PRs to avoid wasted runner minutes; add a
job-level guard that checks the PR draft flag (e.g., use the GitHub Actions
expression that tests github.event.pull_request.draft is false) so the expensive
steps only run when the PR is not a draft (apply this condition to the
"validate" job).
In `@app/components/FeedbackModal.vue`:
- Line 47: In FeedbackModal.vue replace direct use of window.navigator.platform
with a feature-detection fallback that prefers User-Agent Client Hints: read
navigator.userAgentData?.platform if available, otherwise fall back to
navigator.platform and finally to an empty string; update the object key
currently set as platform: window.navigator.platform to platform:
(navigator.userAgentData?.platform || navigator.platform || '') and ensure you
check for undefined navigator/userAgentData before accessing properties so no
runtime errors occur.
- Line 47: Replace the deprecated window.navigator.platform usage in the
FeedbackModal's platform assignment with the User-Agent Client Hints API: use
navigator.userAgentData?.platform with a fallback to window.navigator.platform
(i.e., set platform to navigator.userAgentData?.platform ??
window.navigator.platform) and, if you need higher-entropy details, call
navigator.userAgentData?.getHighEntropyValues(['platformVersion','architecture'])
and include those values in the payload; update the code at the property where
platform: window.navigator.platform is set (in the FeedbackModal component)
accordingly.
In `@server/utils/env.ts`:
- Around line 60-62: Add a cross-field validation inside the same zod
schema.superRefine() that already checks BETTER_AUTH_URL: verify that
GITHUB_FEEDBACK_TOKEN and GITHUB_FEEDBACK_REPO are either both present or both
absent, and if only one is provided call ctx.addIssue() with a clear message and
set path to the missing field (use 'GITHUB_FEEDBACK_TOKEN' or
'GITHUB_FEEDBACK_REPO' accordingly); reference the existing superRefine block so
the new check runs at startup and prevents silent misconfiguration of the
feedback feature.
- Around line 103-105: The startup hint string in server/utils/env.ts is missing
recently added optional env vars; update the multi-line template string (the
message built where the backticked block is defined—e.g., the startup hint /
help message constant) to list GITHUB_FEEDBACK_TOKEN and GITHUB_FEEDBACK_REPO as
optional env vars and add the Railway metadata vars RAILWAY_ENVIRONMENT_NAME,
RAILWAY_GIT_PR_NUMBER, and RAILWAY_PUBLIC_DOMAIN to the message (mark them as
optional/Railway-specific), keeping the existing formatting and any default
notes (e.g., indicate defaults only where applicable).
| const title = ref('') | ||
| const description = ref('') | ||
| const currentUrl = ref('') | ||
| const includeReporterContext = ref(true) |
There was a problem hiding this comment.
includeReporterContext defaults to opt-out, sharing name + URL without explicit consent.
includeReporterContext is initialised to true (Line 13) and reset to true on close (Line 202), meaning the user's name and current page URL are included in every GitHub issue unless they manually uncheck. This contradicts the PR's stated "explicit opt-in" design and silently shares PII (name, URL) without affirmative action from the user.
🔧 Proposed fix to align with opt-in
-const includeReporterContext = ref(true)
+const includeReporterContext = ref(false)In resetAndClose:
- includeReporterContext.value = true
+ includeReporterContext.value = falseAlso applies to: 202-202
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/FeedbackModal.vue` at line 13, The includeReporterContext ref
is currently defaulting to true and being reset to true in resetAndClose, which
violates the explicit opt-in requirement; change the initialization of
includeReporterContext to false and update resetAndClose (the resetAndClose
function) to reset includeReporterContext back to false so name+URL are only
shared when the user actively opts in via the checkbox.
includeReporterContext defaults to opt-out, leaking name + URL without explicit consent.
includeReporterContext is initialised to true (Line 13) and reset to true on close (Line 202), meaning the user's name and current page URL are included in every GitHub issue unless they manually uncheck. This contradicts the PR's stated "explicit opt-in" design and shares PII (name, URL) without affirmative action by the user.
🔧 Proposed fix to align with opt-in
-const includeReporterContext = ref(true)
+const includeReporterContext = ref(false)In resetAndClose:
- includeReporterContext.value = true
+ includeReporterContext.value = falseAlso applies to: 202-202
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/FeedbackModal.vue` at line 13, The includeReporterContext ref
in FeedbackModal.vue is currently defaulted to true and reset to true in
resetAndClose, which leaks name/URL; change the default initialization of
includeReporterContext to false and update resetAndClose (the reset logic) to
set includeReporterContext.value = false so the reporter context is opt-in,
ensuring the checkbox bound to includeReporterContext still toggles the value as
before.
Comment on lines
+117
to
+150
| async function handleScreenshotSelect(event: Event) { | ||
| const input = event.target as HTMLInputElement | ||
| const file = input.files?.[0] | ||
|
|
||
| if (!file) { | ||
| resetScreenshot() | ||
| return | ||
| } | ||
|
|
||
| if (!file.type.startsWith('image/')) { | ||
| submitError.value = 'Screenshot must be an image file.' | ||
| resetScreenshot() | ||
| return | ||
| } | ||
|
|
||
| submitError.value = '' | ||
|
|
||
| try { | ||
| const sourceDataUrl = await fileToDataUrl(file) | ||
| const compressedDataUrl = await compressImageDataUrl(sourceDataUrl) | ||
|
|
||
| if (compressedDataUrl.length > MAX_SCREENSHOT_DATA_URL_CHARS) { | ||
| submitError.value = 'Screenshot is too large for GitHub issue body. Please use a smaller image.' | ||
| resetScreenshot() | ||
| return | ||
| } | ||
|
|
||
| screenshotDataUrl.value = compressedDataUrl | ||
| screenshotFileName.value = file.name | ||
| } catch { | ||
| submitError.value = 'Failed to process screenshot. Please try another file.' | ||
| resetScreenshot() | ||
| } | ||
| } |
There was a problem hiding this comment.
File input value not cleared on error — same file cannot be reselected.
resetScreenshot() clears the reactive refs but leaves the HTMLInputElement's own .value intact. After any error path (non-image type, oversized, processing failure), if the user tries to pick the same file again, the @change event does not fire because the browser sees no value change.
🛠️ Proposed fix
async function handleScreenshotSelect(event: Event) {
const input = event.target as HTMLInputElement
const file = input.files?.[0]
if (!file) {
resetScreenshot()
return
}
if (!file.type.startsWith('image/')) {
submitError.value = 'Screenshot must be an image file.'
resetScreenshot()
+ input.value = ''
return
}
submitError.value = ''
try {
const sourceDataUrl = await fileToDataUrl(file)
const compressedDataUrl = await compressImageDataUrl(sourceDataUrl)
if (compressedDataUrl.length > MAX_SCREENSHOT_DATA_URL_CHARS) {
submitError.value = 'Screenshot is too large for GitHub issue body. Please use a smaller image.'
resetScreenshot()
+ input.value = ''
return
}
screenshotDataUrl.value = compressedDataUrl
screenshotFileName.value = file.name
} catch {
submitError.value = 'Failed to process screenshot. Please try another file.'
resetScreenshot()
+ input.value = ''
}
}📝 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
| async function handleScreenshotSelect(event: Event) { | |
| const input = event.target as HTMLInputElement | |
| const file = input.files?.[0] | |
| if (!file) { | |
| resetScreenshot() | |
| return | |
| } | |
| if (!file.type.startsWith('image/')) { | |
| submitError.value = 'Screenshot must be an image file.' | |
| resetScreenshot() | |
| return | |
| } | |
| submitError.value = '' | |
| try { | |
| const sourceDataUrl = await fileToDataUrl(file) | |
| const compressedDataUrl = await compressImageDataUrl(sourceDataUrl) | |
| if (compressedDataUrl.length > MAX_SCREENSHOT_DATA_URL_CHARS) { | |
| submitError.value = 'Screenshot is too large for GitHub issue body. Please use a smaller image.' | |
| resetScreenshot() | |
| return | |
| } | |
| screenshotDataUrl.value = compressedDataUrl | |
| screenshotFileName.value = file.name | |
| } catch { | |
| submitError.value = 'Failed to process screenshot. Please try another file.' | |
| resetScreenshot() | |
| } | |
| } | |
| async function handleScreenshotSelect(event: Event) { | |
| const input = event.target as HTMLInputElement | |
| const file = input.files?.[0] | |
| if (!file) { | |
| resetScreenshot() | |
| return | |
| } | |
| if (!file.type.startsWith('image/')) { | |
| submitError.value = 'Screenshot must be an image file.' | |
| resetScreenshot() | |
| input.value = '' | |
| return | |
| } | |
| submitError.value = '' | |
| try { | |
| const sourceDataUrl = await fileToDataUrl(file) | |
| const compressedDataUrl = await compressImageDataUrl(sourceDataUrl) | |
| if (compressedDataUrl.length > MAX_SCREENSHOT_DATA_URL_CHARS) { | |
| submitError.value = 'Screenshot is too large for GitHub issue body. Please use a smaller image.' | |
| resetScreenshot() | |
| input.value = '' | |
| return | |
| } | |
| screenshotDataUrl.value = compressedDataUrl | |
| screenshotFileName.value = file.name | |
| } catch { | |
| submitError.value = 'Failed to process screenshot. Please try another file.' | |
| resetScreenshot() | |
| input.value = '' | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/FeedbackModal.vue` around lines 117 - 150, The handler
handleScreenshotSelect currently calls resetScreenshot() on error but does not
clear the underlying HTMLInputElement value, so reselecting the same file won't
retrigger change; update either resetScreenshot() or handleScreenshotSelect to
set input.value = '' (or cast and clear the element stored by the file input)
whenever you call resetScreenshot on error paths (non-image type, too large,
catch block) so the input's .value is cleared and the same file can be selected
again; reference the input variable in handleScreenshotSelect and/or the
resetScreenshot function to implement this clear.
File input value not cleared on error — same file cannot be reselected.
resetScreenshot() clears the reactive refs but leaves HTMLInputElement.value intact. After any of the three error paths (non-image type, oversized, processing failure), if the user tries to pick the same file again the @change event will not fire because the browser sees no value change.
🛠️ Proposed fix
if (!file.type.startsWith('image/')) {
submitError.value = 'Screenshot must be an image file.'
resetScreenshot()
+ input.value = ''
return
}
...
if (compressedDataUrl.length > MAX_SCREENSHOT_DATA_URL_CHARS) {
submitError.value = 'Screenshot is too large for GitHub issue body. Please use a smaller image.'
resetScreenshot()
+ input.value = ''
return
}
...
} catch {
submitError.value = 'Failed to process screenshot. Please try another file.'
resetScreenshot()
+ input.value = ''
}📝 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
| async function handleScreenshotSelect(event: Event) { | |
| const input = event.target as HTMLInputElement | |
| const file = input.files?.[0] | |
| if (!file) { | |
| resetScreenshot() | |
| return | |
| } | |
| if (!file.type.startsWith('image/')) { | |
| submitError.value = 'Screenshot must be an image file.' | |
| resetScreenshot() | |
| return | |
| } | |
| submitError.value = '' | |
| try { | |
| const sourceDataUrl = await fileToDataUrl(file) | |
| const compressedDataUrl = await compressImageDataUrl(sourceDataUrl) | |
| if (compressedDataUrl.length > MAX_SCREENSHOT_DATA_URL_CHARS) { | |
| submitError.value = 'Screenshot is too large for GitHub issue body. Please use a smaller image.' | |
| resetScreenshot() | |
| return | |
| } | |
| screenshotDataUrl.value = compressedDataUrl | |
| screenshotFileName.value = file.name | |
| } catch { | |
| submitError.value = 'Failed to process screenshot. Please try another file.' | |
| resetScreenshot() | |
| } | |
| } | |
| async function handleScreenshotSelect(event: Event) { | |
| const input = event.target as HTMLInputElement | |
| const file = input.files?.[0] | |
| if (!file) { | |
| resetScreenshot() | |
| return | |
| } | |
| if (!file.type.startsWith('image/')) { | |
| submitError.value = 'Screenshot must be an image file.' | |
| resetScreenshot() | |
| input.value = '' | |
| return | |
| } | |
| submitError.value = '' | |
| try { | |
| const sourceDataUrl = await fileToDataUrl(file) | |
| const compressedDataUrl = await compressImageDataUrl(sourceDataUrl) | |
| if (compressedDataUrl.length > MAX_SCREENSHOT_DATA_URL_CHARS) { | |
| submitError.value = 'Screenshot is too large for GitHub issue body. Please use a smaller image.' | |
| resetScreenshot() | |
| input.value = '' | |
| return | |
| } | |
| screenshotDataUrl.value = compressedDataUrl | |
| screenshotFileName.value = file.name | |
| } catch { | |
| submitError.value = 'Failed to process screenshot. Please try another file.' | |
| resetScreenshot() | |
| input.value = '' | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/FeedbackModal.vue` around lines 117 - 150, The file input's
DOM value isn't cleared, so selecting the same file after an error doesn't
re-trigger change; update handleScreenshotSelect to clear the actual input
element when an error occurs (or change resetScreenshot to accept an
HTMLInputElement and clear its value/files). Concretely: in
handleScreenshotSelect, after each error return (non-image type, oversized, or
catch) set input.value = '' (or input.files = null), or refactor
resetScreenshot(someInput) to perform input.value = '' and call that instead,
ensuring the `@change` will fire for the same file next time.
Comment on lines
+346
to
+354
| <button | ||
| type="button" | ||
| class="flex w-full items-center justify-between px-3 py-2 text-left" | ||
| @click="showOptionalContext = !showOptionalContext" | ||
| > | ||
| <span class="text-xs font-medium uppercase tracking-wide text-surface-500 dark:text-surface-400"> | ||
| Add More Context (Optional) | ||
| </span> | ||
| <component :is="showOptionalContext ? ChevronDown : ChevronRight" class="size-4 text-surface-500 dark:text-surface-400" /> |
There was a problem hiding this comment.
Missing aria-expanded on the collapsible toggle button.
Screen readers cannot announce the expand/collapse state of the "Add More Context" section without aria-expanded.
♿ Proposed fix
<button
type="button"
class="flex w-full items-center justify-between px-3 py-2 text-left"
+ :aria-expanded="showOptionalContext"
`@click`="showOptionalContext = !showOptionalContext"
>📝 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
| <button | |
| type="button" | |
| class="flex w-full items-center justify-between px-3 py-2 text-left" | |
| @click="showOptionalContext = !showOptionalContext" | |
| > | |
| <span class="text-xs font-medium uppercase tracking-wide text-surface-500 dark:text-surface-400"> | |
| Add More Context (Optional) | |
| </span> | |
| <component :is="showOptionalContext ? ChevronDown : ChevronRight" class="size-4 text-surface-500 dark:text-surface-400" /> | |
| <button | |
| type="button" | |
| class="flex w-full items-center justify-between px-3 py-2 text-left" | |
| :aria-expanded="showOptionalContext" | |
| `@click`="showOptionalContext = !showOptionalContext" | |
| > | |
| <span class="text-xs font-medium uppercase tracking-wide text-surface-500 dark:text-surface-400"> | |
| Add More Context (Optional) | |
| </span> | |
| <component :is="showOptionalContext ? ChevronDown : ChevronRight" class="size-4 text-surface-500 dark:text-surface-400" /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/FeedbackModal.vue` around lines 346 - 354, The toggle button
for "Add More Context (Optional)" is missing an accessibility state; update the
button (the element with `@click` toggling showOptionalContext and rendering
ChevronDown/ChevronRight) to include an aria-expanded attribute bound to
showOptionalContext (e.g., :aria-expanded="showOptionalContext") and add an
aria-controls that references the id of the collapsible content panel; ensure
the collapsible panel element has that matching id so assistive tech can
announce the expanded/collapsed state.
Missing aria-expanded on the collapsible toggle — expand/collapse state invisible to screen readers.
The chevron icon communicates open/closed state visually, but without aria-expanded, assistive technologies have no way to announce this state.
♿ Proposed fix
<button
type="button"
class="flex w-full items-center justify-between px-3 py-2 text-left"
+ :aria-expanded="showOptionalContext"
+ aria-controls="optional-context-panel"
`@click`="showOptionalContext = !showOptionalContext"
>And on the collapsible <div>:
-<div v-if="showOptionalContext" class="space-y-3 border-t border-surface-200 dark:border-surface-700 p-3">
+<div v-if="showOptionalContext" id="optional-context-panel" class="space-y-3 border-t border-surface-200 dark:border-surface-700 p-3">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/FeedbackModal.vue` around lines 346 - 354, The toggle button
that flips showOptionalContext (the button rendering ChevronDown/ChevronRight)
is missing accessible state; bind aria-expanded to showOptionalContext on that
button and add aria-controls pointing to the collapsible container's id, then
ensure the collapsible <div> that shows/hides the optional context has a
matching id and appropriate hidden handling (e.g., v-show/v-if or aria-hidden)
so screen readers can determine the expanded/collapsed state for the
showOptionalContext toggle and its content.
| BETTER_AUTH_SECRET: emptyToUndefined.pipe(z.string().min(32, 'BETTER_AUTH_SECRET must be at least 32 characters')), | ||
| BETTER_AUTH_URL: emptyToUndefined.pipe(z.url()).optional(), | ||
| /** Railway environment metadata for PR/preview detection. */ | ||
| RAILWAY_ENVIRONMENT_NAME: emptyToUndefined.optional(), |
There was a problem hiding this comment.
emptyToUndefined.optional() silently fails for empty-string inputs.
emptyToUndefined is z.preprocess(fn, z.string()) — the inner schema is a non-optional z.string(). When the input is "":
ZodOptionalsees""(notundefined) → passes through to the preprocess.fn("")returnsundefined.z.string()receivesundefined→ rejects with "Expected string, received undefined".
So a Railway deployment that injects RAILWAY_ENVIRONMENT_NAME="" or RAILWAY_PUBLIC_DOMAIN="" — the exact case the docblock says this preprocessor is designed for — will blow up schema validation entirely.
The fix is to make the preprocess inner schema optional so that the undefined produced by fn is accepted:
🛠️ Proposed fix
const emptyToUndefined = z.preprocess(
(val) => (typeof val === 'string' && val.trim() === '' ? undefined : val),
- z.string(),
+ z.string().optional(),
)This change means emptyToUndefined itself outputs string | undefined, so downstream .pipe() chains and .optional() wrappers receive the correctly normalised value.
Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/utils/env.ts` at line 40, The empty-string preprocessing currently
uses emptyToUndefined defined as z.preprocess(fn, z.string()), which rejects the
undefined returned by the preprocess (breaking RAILWAY_ENVIRONMENT_NAME and
RAILWAY_PUBLIC_DOMAIN). Update the emptyToUndefined definition so its inner
schema is optional (e.g., z.preprocess(fn, z.string().optional())) so the
preprocess can return undefined without failing validation, and keep using
emptyToUndefined.optional() for those env vars (apply the same change where
emptyToUndefined is used).
This was referenced Feb 21, 2026
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
1 participant
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.
Summary:
This PR adds a full in-app feedback flow that creates GitHub issues directly from the dashboard sidebar. Users can submit bug reports and feature requests without leaving the product, with optional context to make issues actionable for maintainers.
What’s included:
Privacy behavior:
Known limitation:
Summary by CodeRabbit
New Features
UX Improvements