Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds an interactive Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as Init Command
participant FS as File System
participant Auth as Stack Auth
participant Cloud as Stack Cloud API
User->>CLI: run `stack init`
CLI->>User: prompt for mode (create / link-config / link-cloud)
alt create
User->>CLI: select apps
CLI->>FS: write `stack.config.ts`
FS-->>CLI: confirm write
CLI->>User: print STACK AUTH SETUP INSTRUCTIONS
else link-config
User->>CLI: provide config path
CLI->>FS: read/validate config file
FS-->>CLI: file contents / not found
CLI->>User: print linked path or error
else link-cloud
CLI->>Auth: perform login (if needed)
Auth-->>CLI: refresh token / keys
CLI->>Cloud: fetch projects
Cloud-->>CLI: project list
User->>CLI: select project
CLI->>Auth: request project keys
Auth-->>CLI: keys
CLI->>FS: create/append `.env` with keys
FS-->>CLI: confirm write
end
CLI->>User: operation complete / errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR replaces the previous Key issues found:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([stack init]) --> B{--mode flag provided?}
B -- No, TTY --> C[Interactive: select mode]
B -- No, non-TTY --> ERR1[Error: requires interactive terminal]
B -- Yes --> D{mode value}
C --> D
D -- create --> E[handleCreate]
D -- link / link-config / link-cloud --> F[handleLink]
D -- unknown --> ERR2[CliError: Unknown mode]
E --> E1{--apps flag?}
E1 -- Yes --> E2[Split comma-separated app IDs]
E1 -- No --> E3[Interactive: checkbox prompt]
E2 & E3 --> E4[Build installed object\nRecord with dynamic keys ⚠️]
E4 --> E5[JSON.stringify config]
E5 --> E6[Write stack.config.ts]
E6 --> PROMPT
F --> F1{mode?}
F1 -- link-config --> G[handleLinkFromConfigFile]
F1 -- link-cloud --> H[handleLinkFromCloud]
F1 -- link interactive --> F2[Select source prompt]
F2 --> G & H
G --> G1{--config-file flag?}
G1 -- Yes --> G2[Validate file exists]
G1 -- No --> G3[Interactive: input path]
G2 & G3 --> G4[Log: Linked to config file]
G4 --> PROMPT
H --> H1{Auth token present?}
H1 -- No, non-TTY --> ERR3[CliError: run stack login first]
H1 -- No, TTY --> H2[performLogin browser flow]
H2 --> H3[resolveSessionAuth]
H1 -- Yes --> H3
H3 --> H4[listOwnedProjects]
H4 --> H5{--select-project-id?}
H5 -- Yes --> H6[Validate project in list]
H5 -- No --> H7[Interactive: select project]
H6 & H7 --> H8[createInternalApiKey\n200-year expiry ⚠️]
H8 --> H9{.env exists?}
H9 -- No --> H10[Write new .env]
H9 -- Yes, non-TTY --> H11[Auto-append ⚠️ newline check]
H9 -- Yes, TTY --> H12[confirm prompt]
H12 -- Yes --> H11
H12 -- No --> H13[Print keys to stdout]
H10 & H11 --> PROMPT
PROMPT([createInitPrompt\nconfigPath passed but unused ⚠️])
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/stack-cli/src/commands/init.ts (1)
164-164: Replace non-null assertion with defensive check.Per coding guidelines, prefer
?? throwErr(...)over non-null assertions. While thefindis likely to succeed given the prior validation, this makes the assumption explicit.♻️ Suggested fix
- const project = projects.find((p) => p.id === projectId)!; + const project = projects.find((p) => p.id === projectId) ?? (() => { throw new CliError(`Project '${projectId}' unexpectedly not found.`); })();Or extract a helper:
const project = projects.find((p) => p.id === projectId); if (!project) { throw new CliError(`Project '${projectId}' unexpectedly not found.`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/init.ts` at line 164, The line using a non-null assertion for the result of projects.find should be made defensive: replace "const project = projects.find((p) => p.id === projectId)!" with code that checks for null/undefined and throws a clear CLI error (e.g., via CliError or the project's throwErr helper) if not found; locate the variable "project" and the call to projects.find in init.ts and change it to use a null-coalescing throw or an explicit if (!project) throw new CliError(...) so the code never relies on the non-null assertion.packages/stack-cli/src/lib/init-prompt.ts (1)
1-1: UnusedconfigPathparameter.The
configPathparameter is defined but never used in the template string. Either remove it or incorporate it into the instructions (e.g., mentioning the config file location).♻️ If unused, remove it
-export const createInitPrompt = (web: boolean, configPath?: string) => `============================= +export const createInitPrompt = (web: boolean) => `=============================🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/lib/init-prompt.ts` at line 1, The createInitPrompt function declares a configPath parameter that is never used; either remove the unused parameter from the function signature or incorporate it into the returned template string (e.g., include the config file location in the prompt text). Update the createInitPrompt(web: boolean, configPath?: string) signature accordingly and adjust any callers if you remove the parameter, or interpolate configPath into the template returned by createInitPrompt to use the 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 `@packages/stack-cli/src/commands/init.ts`:
- Around line 240-241: The code assigns opts.apps -> selectedApps without
checking values against ALL_APPS, risking invalid AppId values; update the logic
where selectedApps is created (the opts.apps split/map/filter block) to validate
each id is included in ALL_APPS, either by filtering out invalid entries and
logging/warning the user or by throwing a clear error listing invalid ids;
ensure the resulting selectedApps elements are cast/typed as AppId (or built
only from ALL_APPS entries) so the config will only contain valid AppId values.
---
Nitpick comments:
In `@packages/stack-cli/src/commands/init.ts`:
- Line 164: The line using a non-null assertion for the result of projects.find
should be made defensive: replace "const project = projects.find((p) => p.id ===
projectId)!" with code that checks for null/undefined and throws a clear CLI
error (e.g., via CliError or the project's throwErr helper) if not found; locate
the variable "project" and the call to projects.find in init.ts and change it to
use a null-coalescing throw or an explicit if (!project) throw new CliError(...)
so the code never relies on the non-null assertion.
In `@packages/stack-cli/src/lib/init-prompt.ts`:
- Line 1: The createInitPrompt function declares a configPath parameter that is
never used; either remove the unused parameter from the function signature or
incorporate it into the returned template string (e.g., include the config file
location in the prompt text). Update the createInitPrompt(web: boolean,
configPath?: string) signature accordingly and adjust any callers if you remove
the parameter, or interpolate configPath into the template returned by
createInitPrompt to use the value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7b0b680-53b2-4d91-9fee-2851291402de
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/e2e/tests/general/cli.test.tspackages/stack-cli/package.jsonpackages/stack-cli/src/commands/init.tspackages/stack-cli/src/lib/init-prompt.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/stack-cli/src/commands/init.ts (3)
251-254: Type cast may hide future stage values.The cast
as keyof typeof stageOrderassumes all non-alpha stages are covered bystageOrder. If a new stage (e.g.,"preview") is added toALL_APPS, the sort would silently fail by comparingundefinedvalues.♻️ Handle unknown stages explicitly
- const stageOrder = { stable: 0, beta: 1 } as const; + const stageOrder: Record<string, number> = { stable: 0, beta: 1 }; const appEntries = Object.entries(ALL_APPS) .filter(([, app]) => app.stage !== "alpha") - .sort((a, b) => stageOrder[a[1].stage as keyof typeof stageOrder] - stageOrder[b[1].stage as keyof typeof stageOrder]); + .sort((a, b) => (stageOrder[a[1].stage] ?? 99) - (stageOrder[b[1].stage] ?? 99));As per coding guidelines: "Do NOT use
as/any/type casts or anything else to bypass the type system."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/init.ts` around lines 251 - 254, The current sort uses a type cast on stageOrder which can hide unknown stages; update the comparison to explicitly map app.stage to a numeric rank without using "as" casts (e.g., add a helper like getStageOrder(stage: string): number or a lookup that returns a default high/explicit rank for unknown stages) and use that in the sort for appEntries so new stages (e.g., "preview") are handled deterministically; reference stageOrder, ALL_APPS, appEntries and the sort callback when making this change.
126-126: Avoid type casts; validate or type the flags properly.Multiple lines (126, 134, 212) use
flags as { projectId?: string }to bypass type checking. Per coding guidelines, type casts should be avoided. Consider either:
- Defining proper types for the flags from Commander
- Using runtime validation to extract the expected properties
♻️ Extract with explicit null/undefined checks
- sessionAuth = resolveSessionAuth(flags as { projectId?: string }); + const projectIdFlag = typeof flags.projectId === "string" ? flags.projectId : undefined; + sessionAuth = resolveSessionAuth({ projectId: projectIdFlag });Apply similarly to lines 134 and 212.
As per coding guidelines: "Do NOT use
as/any/type casts or anything else to bypass the type system unless you specifically asked the user about it."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/init.ts` at line 126, The code is bypassing TypeScript via casts like "flags as { projectId?: string }" when calling resolveSessionAuth; remove those casts and either (a) properly type the Commander flags object (declare an interface e.g. Flags { projectId?: string } and use it as the type for the parsed flags passed into init command and calls to resolveSessionAuth and other places on lines 126, 134, 212) or (b) perform runtime extraction/validation (check flags.projectId !== undefined/null and extract into a local const projectId: string | undefined before calling resolveSessionAuth) so you avoid using "as" while preserving the same behavior. Ensure resolveSessionAuth is called with the correctly typed value (or validated local) rather than a cast.
38-46: Useinstanceofcheck with correct import source forExitPromptError.The duck-typing check for
ExitPromptErrorviaerror.name === "ExitPromptError"is brittle and breaks silently if the library changes.ExitPromptErroris exported from@inquirer/core(not@inquirer/prompts), so useinstanceoffor type-safe error handling.♻️ Safer approach
+import { ExitPromptError } from "@inquirer/core"; ... } catch (error: unknown) { - if (error != null && typeof error === "object" && "name" in error && error.name === "ExitPromptError") { + if (error instanceof ExitPromptError) { console.log("\nAborted."); process.exit(0); } throw error; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/init.ts` around lines 38 - 46, Replace the brittle duck-typing error check in the catch block around runInit by importing ExitPromptError from '@inquirer/core' and using an instanceof check (e.g., error instanceof ExitPromptError) to detect the prompt abort; update the catch to log "Aborted." and call process.exit(0) when the instanceof check matches, otherwise rethrow the error. Ensure the import of ExitPromptError is added at top of the module and the symbol name matches exactly.packages/stack-cli/src/lib/init-prompt.ts (1)
1-3: UnusedconfigPathparameter may mislead callers.The
configPathparameter is declared but never used in the function body. While the TODO comment explains future intent, callers (likeinit.tsline 76) currently pass this value expecting it to affect output. Consider either removing the parameter until the emulator feature is implemented, or adding a placeholder that shows the path.♻️ Option: Remove unused parameter for now
-// TODO: Use configPath in the prompt once local emulator is set up: -// Add "npx `@stackframe/stack-cli` emulator run --config-file ${configPath}" to project dev command -export const createInitPrompt = (web: boolean, configPath?: string) => `============================= +// TODO: Add local emulator instructions once set up +export const createInitPrompt = (web: boolean) => `=============================🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/lib/init-prompt.ts` around lines 1 - 3, The createInitPrompt function declares a configPath parameter that is never used; either remove the unused parameter from createInitPrompt and update all call sites that pass configPath so signatures match, or use configPath to include a visible placeholder in the returned prompt (e.g., inject a line like "Emulator config: <configPath>" when configPath is provided) so callers see the effect; update the function signature and its callers (or the prompt string interpolation) accordingly to eliminate the misleading unused parameter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stack-cli/src/commands/init.ts`:
- Line 164: The current line uses a non-null assertion when locating the
project: const project = projects.find((p) => p.id === projectId)!; — replace
this with explicit error handling by using the nullish-coalescing pattern:
assign the result of projects.find(...) to project and immediately use ??
throwErr("clear message") so the code fails loudly with a descriptive error if
no project is found; also import or reference the existing throwErr helper (or
add one) and ensure the error message mentions the expected projectId and that
the project was not found to aid debugging.
- Around line 182-206: The code currently always appends envLines to envPath (in
the block using fs.existsSync(envPath)), which can create duplicate Stack Auth
variables; modify that branch to first read and parse the existing .env (the
existing variable), extract keys from envLines (the new keys you intend to add),
and compare them to existing keys: if none collide, append as before; if some
keys already exist, in non-interactive mode only append the missing keys and
skip existing ones, and in interactive mode prompt (confirm) whether to
overwrite existing keys or append only missing ones (use the same confirm flow
that uses confirm(), preserve isNonInteractiveEnv() behavior). If overwriting,
replace the existing key lines in the existing content and write the updated
content back to envPath; if skipping, only append missing envLines (with the
same separator handling). Update console messages to reflect whether you
appended, skipped, or overwrote keys.
---
Nitpick comments:
In `@packages/stack-cli/src/commands/init.ts`:
- Around line 251-254: The current sort uses a type cast on stageOrder which can
hide unknown stages; update the comparison to explicitly map app.stage to a
numeric rank without using "as" casts (e.g., add a helper like
getStageOrder(stage: string): number or a lookup that returns a default
high/explicit rank for unknown stages) and use that in the sort for appEntries
so new stages (e.g., "preview") are handled deterministically; reference
stageOrder, ALL_APPS, appEntries and the sort callback when making this change.
- Line 126: The code is bypassing TypeScript via casts like "flags as {
projectId?: string }" when calling resolveSessionAuth; remove those casts and
either (a) properly type the Commander flags object (declare an interface e.g.
Flags { projectId?: string } and use it as the type for the parsed flags passed
into init command and calls to resolveSessionAuth and other places on lines 126,
134, 212) or (b) perform runtime extraction/validation (check flags.projectId
!== undefined/null and extract into a local const projectId: string | undefined
before calling resolveSessionAuth) so you avoid using "as" while preserving the
same behavior. Ensure resolveSessionAuth is called with the correctly typed
value (or validated local) rather than a cast.
- Around line 38-46: Replace the brittle duck-typing error check in the catch
block around runInit by importing ExitPromptError from '@inquirer/core' and
using an instanceof check (e.g., error instanceof ExitPromptError) to detect the
prompt abort; update the catch to log "Aborted." and call process.exit(0) when
the instanceof check matches, otherwise rethrow the error. Ensure the import of
ExitPromptError is added at top of the module and the symbol name matches
exactly.
In `@packages/stack-cli/src/lib/init-prompt.ts`:
- Around line 1-3: The createInitPrompt function declares a configPath parameter
that is never used; either remove the unused parameter from createInitPrompt and
update all call sites that pass configPath so signatures match, or use
configPath to include a visible placeholder in the returned prompt (e.g., inject
a line like "Emulator config: <configPath>" when configPath is provided) so
callers see the effect; update the function signature and its callers (or the
prompt string interpolation) accordingly to eliminate the misleading unused
parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04360709-e215-4eba-8eac-67bd8fcff498
📒 Files selected for processing (2)
packages/stack-cli/src/commands/init.tspackages/stack-cli/src/lib/init-prompt.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/stack-cli/src/lib/claude-agent.ts (2)
182-194: System message handling uses unsafe property access without type guards.The code casts
messagetoRecord<string, unknown>and then accesses properties liketask_id,subtype,description, andsummarywithout proper type guards. This could fail silently or produce unexpected behavior if the SDK's message shape changes.♻️ Suggested safer approach
} else if (message.type === "system") { - // Subagent task lifecycle - const msg = message as Record<string, unknown>; - const taskId = msg.task_id as string | undefined; - - if (msg.subtype === "task_started" && taskId) { - ui.setSpinner(taskId, String(msg.description ?? "Working...")); - } else if (msg.subtype === "task_progress" && taskId) { - ui.setSpinner(taskId, String(msg.description ?? "Working...")); - } else if (msg.subtype === "task_notification" && taskId) { - ui.complete(taskId, String(msg.summary ?? msg.description ?? "Done")); - } + // Subagent task lifecycle - safely extract properties + const msg = message as Record<string, unknown>; + const taskId = typeof msg.task_id === "string" ? msg.task_id : undefined; + const subtype = typeof msg.subtype === "string" ? msg.subtype : undefined; + const description = typeof msg.description === "string" ? msg.description : "Working..."; + const summary = typeof msg.summary === "string" ? msg.summary : description; + + if (subtype === "task_started" && taskId) { + ui.setSpinner(taskId, description); + } else if (subtype === "task_progress" && taskId) { + ui.setSpinner(taskId, description); + } else if (subtype === "task_notification" && taskId) { + ui.complete(taskId, summary); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/lib/claude-agent.ts` around lines 182 - 194, In the system-message branch of claude-agent.ts (the block starting with if (message.type === "system")), replace the unsafe cast to Record<string, unknown> and direct property access with explicit type guards: validate that message is an object and that properties task_id, subtype, description, and summary exist and are the expected string types (use typeof checks or a small type-guard helper) before calling ui.setSpinner or ui.complete; ensure you only derive taskId after confirming it's a string and only pass string descriptions/summaries (fall back to "Working..." or "Done" when validation fails) so that functions like ui.setSpinner, ui.complete and the subtype checks are protected from malformed SDK messages.
144-148: Type cast may mask undefined values in environment.
process.envvalues are typed asstring | undefined, but the cast toRecord<string, string>ignores this. If the SDK expects all values to be defined strings, this could cause subtle issues.♻️ Safer alternative
function stripClaudeCodeEnv(): Record<string, string> { const env = { ...process.env }; delete env.CLAUDECODE; - return env as Record<string, string>; + // Filter out undefined values to ensure type safety + return Object.fromEntries( + Object.entries(env).filter((entry): entry is [string, string] => entry[1] !== undefined) + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/lib/claude-agent.ts` around lines 144 - 148, The function stripClaudeCodeEnv currently casts process.env to Record<string,string> which hides possible undefined values; update stripClaudeCodeEnv to remove CLAUDECODE then build and return an object that only includes env entries whose values are non-undefined strings (e.g., iterate Object.entries(process.env), skip undefined values, and accumulate into a Record<string,string>), and update the function signature accordingly so callers receive a true Record<string,string> without unsafe type casting; reference: stripClaudeCodeEnv and the CLAUDECODE deletion logic.packages/stack-cli/src/commands/init.ts (1)
96-96: Consider defining a typed interface forflagsto avoid repeated type casts.The
flags as { projectId?: string }cast appears multiple times (lines 143, 151, 229). Defining a proper type would improve type safety and reduce duplication.♻️ Suggested approach
type CliFlags = { projectId?: string; // add other global flags as needed }; async function handleLinkFromCloud(flags: CliFlags, opts: InitOptions, outputDir: string): Promise<{ configPath?: string }> { // ... }Then update the call sites to pass properly typed flags.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/init.ts` at line 96, The flags parameter in handleLink is repeatedly being cast (e.g., casts to { projectId?: string } in multiple places) — define a dedicated typed interface (e.g., type CliFlags { projectId?: string; /* other global flags */ }) and update the handleLink signature to async function handleLink(flags: CliFlags, opts: InitOptions, outputDir: string): Promise<{ configPath?: string }>, then replace all inline casts in this file (the repeated casts you saw) with the typed flags usage and adjust any helper calls (e.g., handleLinkFromCloud or other callers) to accept CliFlags so the type flows through instead of being cast at call sites.apps/backend/src/app/api/latest/integrations/ai-proxy/[[...path]]/route.ts (1)
37-39: Encode the proxied path instead of interpolating it.
params.pathis already decoded, so joining it straight into the URL can mis-handle reserved characters like#or?. Build the upstream URL withencodeURIComponent()per segment or the URL helper instead. As per coding guidelines, "UseurlString``\" orencodeURIComponent()` instead of normal string interpolation for URLs for consistency."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/integrations/ai-proxy/`[[...path]]/route.ts around lines 37 - 39, The current construction of targetUrl interpolates subpath from params.path without encoding, which can break on reserved characters; update the logic that computes subpath/targetUrl (look for params, subpath, OPENROUTER_BASE_URL and targetUrl in route.ts) to build the upstream URL by encoding each path segment (e.g., map params.path through encodeURIComponent and join with "/") or by using the URL/URLSearchParams helpers to append path segments safely, then append req.nextUrl.search; ensure the final targetUrl uses the encoded segments instead of the raw joined subpath.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/integrations/ai-proxy/`[[...path]]/route.ts:
- Around line 17-23: In sanitizeBody, JSON.parse can throw a SyntaxError which
bubbles up as a 500; wrap the decode/parse in a try/catch inside sanitizeBody
(around TextDecoder().decode(raw) and JSON.parse(text)), catch parse errors and
rethrow a StatusError(400, "Invalid JSON in request body" or include the parse
error message) so malformed JSON returns a 400; keep the existing object/array
validation and throw the same StatusError if the parsed value is not a plain
object.
- Around line 35-71: The proxy in route.ts (proxyToOpenRouter) currently
forwards requests using getApiKey and handleApiRequest without any
authentication or rate limiting, exposing the shared OpenRouter key; fix it by
gating the handler: either make the route internal-only (reject requests unless
e.g. a trusted internal header or VPC-only context is present) or add explicit
authentication/authorization and rate limiting at the start of proxyToOpenRouter
(validate the caller token/session, enforce project scoping, check permissions
and apply per-actor rate limits) and only then proceed to call getApiKey and
forward; update handleApiRequest usage or wrap proxyToOpenRouter with an auth
middleware so unauthorized callers are denied before the OpenRouter key is used.
In `@packages/stack-cli/package.json`:
- Around line 29-32: Update the dependency versions in package.json: change
"@inquirer/prompts" from "^7.0.0" to "^8.3.0" and
"@anthropic-ai/claude-agent-sdk" from "^0.2.73" to "^0.2.74"; after editing, run
the project's package manager install (npm/yarn/pnpm) and run the test/build
commands to ensure no breaking changes, and update lockfile accordingly.
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/integrations/ai-proxy/`[[...path]]/route.ts:
- Around line 37-39: The current construction of targetUrl interpolates subpath
from params.path without encoding, which can break on reserved characters;
update the logic that computes subpath/targetUrl (look for params, subpath,
OPENROUTER_BASE_URL and targetUrl in route.ts) to build the upstream URL by
encoding each path segment (e.g., map params.path through encodeURIComponent and
join with "/") or by using the URL/URLSearchParams helpers to append path
segments safely, then append req.nextUrl.search; ensure the final targetUrl uses
the encoded segments instead of the raw joined subpath.
In `@packages/stack-cli/src/commands/init.ts`:
- Line 96: The flags parameter in handleLink is repeatedly being cast (e.g.,
casts to { projectId?: string } in multiple places) — define a dedicated typed
interface (e.g., type CliFlags { projectId?: string; /* other global flags */ })
and update the handleLink signature to async function handleLink(flags:
CliFlags, opts: InitOptions, outputDir: string): Promise<{ configPath?: string
}>, then replace all inline casts in this file (the repeated casts you saw) with
the typed flags usage and adjust any helper calls (e.g., handleLinkFromCloud or
other callers) to accept CliFlags so the type flows through instead of being
cast at call sites.
In `@packages/stack-cli/src/lib/claude-agent.ts`:
- Around line 182-194: In the system-message branch of claude-agent.ts (the
block starting with if (message.type === "system")), replace the unsafe cast to
Record<string, unknown> and direct property access with explicit type guards:
validate that message is an object and that properties task_id, subtype,
description, and summary exist and are the expected string types (use typeof
checks or a small type-guard helper) before calling ui.setSpinner or
ui.complete; ensure you only derive taskId after confirming it's a string and
only pass string descriptions/summaries (fall back to "Working..." or "Done"
when validation fails) so that functions like ui.setSpinner, ui.complete and the
subtype checks are protected from malformed SDK messages.
- Around line 144-148: The function stripClaudeCodeEnv currently casts
process.env to Record<string,string> which hides possible undefined values;
update stripClaudeCodeEnv to remove CLAUDECODE then build and return an object
that only includes env entries whose values are non-undefined strings (e.g.,
iterate Object.entries(process.env), skip undefined values, and accumulate into
a Record<string,string>), and update the function signature accordingly so
callers receive a true Record<string,string> without unsafe type casting;
reference: stripClaudeCodeEnv and the CLAUDECODE deletion logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25ca5540-12ee-42c4-a981-853dc354f0ec
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/backend/src/app/api/latest/integrations/ai-proxy/[[...path]]/route.tspackages/stack-cli/package.jsonpackages/stack-cli/src/commands/init.tspackages/stack-cli/src/lib/claude-agent.tspackages/stack-cli/src/lib/init-prompt.tspackages/stack-cli/tsdown.config.tspnpm-workspace.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stack-cli/src/lib/init-prompt.ts
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
packages/stack-cli/src/commands/init.ts (2)
181-181: 🛠️ Refactor suggestion | 🟠 MajorReplace the non-null assertion when resolving
project.
projects.find(...)!can still fail and produce an unhelpful runtime crash. Fail with an explicit CLI error instead.Proposed fix
- const project = projects.find((p) => p.id === projectId)!; + const project = projects.find((p) => p.id === projectId); + if (project == null) { + throw new CliError(`Project '${projectId}' not found. This should not happen after selection.`); + }As per coding guidelines: "Code defensively. Prefer
?? throwErr(...)over non-null assertions with good error messages explicitly stating the assumption."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/init.ts` at line 181, The code uses a non-null assertion on projects.find(...) when assigning project; replace this with a defensive check that throws a clear CLI error if the project isn't found. Change the assignment of project to capture the find result (const project = projects.find(p => p.id === projectId);) and then coalesce or explicitly throw (e.g., project ?? throwErr(`Project with id ${projectId} not found`) or call the existing CLI error helper) so you fail with a readable message instead of a runtime exception; reference the symbols project, projects, and projectId in init.ts.
199-223:⚠️ Potential issue | 🟡 MinorPrevent duplicate/conflicting Stack keys when appending to existing
.env.Current append behavior can duplicate keys on repeated runs, leaving ambiguous config state. Please append only missing keys (or offer overwrite).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/init.ts` around lines 199 - 223, When appending envLines to an existing .env at envPath, avoid blindly appending duplicates: read the existing file (existing), parse it into keys, parse envLines into key/value pairs, and compute only the missing keys to append; in the interactive branch (confirm) also detect keys that already exist and prompt the user to either overwrite duplicates (replace existing key lines with new values) or skip them before writing. Update the logic around isNonInteractiveEnv(), confirm(), fs.appendFileSync/fs.writeFileSync and ensure the separator behavior remains; produce the final text to append (or the merged file content if overwriting) and write it back to envPath, logging the appropriate message (appended only missing keys / updated existing keys / created .env).apps/backend/src/app/api/latest/integrations/ai-proxy/[[...path]]/route.ts (1)
32-68:⚠️ Potential issue | 🔴 CriticalGate this proxy before using the shared OpenRouter key.
Line 33 fetches a shared API key and forwards requests, but this handler is exposed as public GET/POST (Lines 67-68) and
handleApiRequestdoes not enforce auth. That allows unauthorized callers to burn credits and bypass scope controls.Minimal guard example
async function proxyToOpenRouter(req: NextRequest, options: { params: Promise<{ path?: string[] }> }) { + const internalProxyToken = getEnvVariable("STACK_OPENROUTER_PROXY_TOKEN"); + if (req.headers.get("x-stack-internal-proxy-token") !== internalProxyToken) { + throw new StatusError(403, "Forbidden"); + } + const apiKey = getEnvVariable("STACK_OPENROUTER_API_KEY");(If this route is meant for end users, replace the header gate with explicit authz + per-actor rate limiting.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/integrations/ai-proxy/`[[...path]]/route.ts around lines 32 - 68, The proxyToOpenRouter handler currently fetches the shared OpenRouter key (getEnvVariable("STACK_OPENROUTER_API_KEY")) and forwards requests without auth; add an explicit gate at the start of proxyToOpenRouter (or inside handleApiRequest) that validates the caller (e.g., check a session, API token, or internal service header) and returns 401/403 for unauthorized callers; if this endpoint must be public, require callers to supply their own per-user API key and enforce per-actor rate limiting/quotas before using the shared key; update logic around proxyToOpenRouter, handleApiRequest, and any middleware that extracts auth to ensure only authorized requests reach the fetch to OPENROUTER_BASE_URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/integrations/ai-proxy/`[[...path]]/route.ts:
- Around line 35-36: The code builds targetUrl from raw params.path causing
unencoded path segments; update the subpath construction used in the targetUrl
assignment so each segment from params.path is encoded (e.g., map each element
with encodeURIComponent and join with "/"), then use that encoded subpath when
creating targetUrl (`OPENROUTER_BASE_URL`, `subpath`, `req.nextUrl.search`) to
ensure safe, standard URL encoding.
- Around line 11-27: The parsed JSON is currently treated as any and nested
properties like parsed.metadata.user_id are accessed without type checks; add a
type guard and explicit validations: after JSON.parse, assert parsed is a plain
object (e.g., use a helper isRecord(obj): obj is Record<string, unknown>) and
cast parsed to Record<string, unknown> before assigning parsed.model =
OPENROUTER_MODEL, then validate parsed.metadata is an object and that
metadata.user_id, if present, is a string before reading .length; if
metadata.user_id exists but is not a string, throw new StatusError(400,
"metadata.user_id must be a string"), otherwise truncate the string to 128 chars
as before; reference parsed, metadata, OPENROUTER_MODEL, and StatusError to
locate the changes.
In `@packages/stack-cli/src/commands/init.ts`:
- Line 143: Replace unsafe "as" casts by explicitly extracting and validating
properties or by changing the called functions' signatures: for the
resolveSessionAuth and resolveLoginConfig calls, stop casting the whole flags
object and instead pass { projectId: flags.projectId as string | undefined } or
update resolveSessionAuth/resolveLoginConfig to accept Record<string, unknown>
and pull/validate projectId internally; likewise remove other blanket casts on
flags. For the stage cast involving stageOrder (the a[1].stage as keyof typeof
stageOrder), enforce an exhaustive runtime check after filtering (e.g., if
(app.stage === "alpha") throw throwErr(...); or throw for any unexpected value)
rather than casting, so the type-narrowing is explicit and safe. Ensure all
fixes reference and update the functions resolveSessionAuth, resolveLoginConfig
and the code handling app.stage/stageOrder to validate inputs instead of
bypassing the type system.
---
Duplicate comments:
In `@apps/backend/src/app/api/latest/integrations/ai-proxy/`[[...path]]/route.ts:
- Around line 32-68: The proxyToOpenRouter handler currently fetches the shared
OpenRouter key (getEnvVariable("STACK_OPENROUTER_API_KEY")) and forwards
requests without auth; add an explicit gate at the start of proxyToOpenRouter
(or inside handleApiRequest) that validates the caller (e.g., check a session,
API token, or internal service header) and returns 401/403 for unauthorized
callers; if this endpoint must be public, require callers to supply their own
per-user API key and enforce per-actor rate limiting/quotas before using the
shared key; update logic around proxyToOpenRouter, handleApiRequest, and any
middleware that extracts auth to ensure only authorized requests reach the fetch
to OPENROUTER_BASE_URL.
In `@packages/stack-cli/src/commands/init.ts`:
- Line 181: The code uses a non-null assertion on projects.find(...) when
assigning project; replace this with a defensive check that throws a clear CLI
error if the project isn't found. Change the assignment of project to capture
the find result (const project = projects.find(p => p.id === projectId);) and
then coalesce or explicitly throw (e.g., project ?? throwErr(`Project with id
${projectId} not found`) or call the existing CLI error helper) so you fail with
a readable message instead of a runtime exception; reference the symbols
project, projects, and projectId in init.ts.
- Around line 199-223: When appending envLines to an existing .env at envPath,
avoid blindly appending duplicates: read the existing file (existing), parse it
into keys, parse envLines into key/value pairs, and compute only the missing
keys to append; in the interactive branch (confirm) also detect keys that
already exist and prompt the user to either overwrite duplicates (replace
existing key lines with new values) or skip them before writing. Update the
logic around isNonInteractiveEnv(), confirm(),
fs.appendFileSync/fs.writeFileSync and ensure the separator behavior remains;
produce the final text to append (or the merged file content if overwriting) and
write it back to envPath, logging the appropriate message (appended only missing
keys / updated existing keys / created .env).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3293a49a-f9f8-4982-8301-d2ebf539296f
📒 Files selected for processing (3)
apps/backend/src/app/api/latest/integrations/ai-proxy/[[...path]]/route.tspackages/stack-cli/package.jsonpackages/stack-cli/src/commands/init.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stack-cli/package.json
Summary by CodeRabbit
New Features
Tests