fix: ensure npm cache directory has correct permissions #473
Conversation
Fix EACCES permission error when running npx commands (e.g., MCP servers) inside the Docker container. Error that was occurring: npm error code EACCES npm error syscall mkdir npm error path /home/automaker/.npm/_cacache/index-v5/1f/fc npm error errno EACCES npm error Your cache folder contains root-owned files, due to a bug in npm error previous versions of npm which has since been addressed. The fix ensures the /home/automaker/.npm directory exists and has correct ownership before switching to the automaker user in the entrypoint script.
📝 WalkthroughWalkthroughThis pull request introduces a unified provider abstraction layer for AI queries, refactors multiple server routes to use this new abstraction instead of provider-specific branching, enhances the auto-mode service with pipeline-resume capabilities, adds OpenCode CLI support to Docker builds, and implements defaultFeatureModel settings throughout the UI with associated store and persistence logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Route as Route Handler
participant SQS as SimpleQueryService
participant PF as Provider Factory
participant Provider as Selected Provider
participant SDK as AI SDK
Route->>SQS: simpleQuery/streamingQuery(options)
SQS->>SQS: Determine provider from options.model
SQS->>SQS: Build provider options (systemPrompt, outputFormat, etc.)
SQS->>PF: Get provider instance
PF->>Provider: Instantiate provider
SQS->>Provider: executeQuery(messages)
Provider->>SDK: Send request with outputFormat/thinkingLevel
SDK-->>Provider: Stream/return messages (text blocks, structured_output)
Provider-->>SQS: Yield/return ProviderMessage[]
alt Streaming
SQS->>SQS: For each message, trigger callbacks (onText, onToolUse, onThinking)
SQS-->>Route: Emit progress via callbacks
end
SQS->>SQS: Accumulate text blocks, extract structured_output
SQS->>SQS: Handle errors (max-turns, max-retries)
SQS-->>Route: Return SimpleQueryResult {text, structured_output?}
Route->>Route: Process result, emit completion events
sequenceDiagram
participant Route as Route (e.g., generate-spec)
participant SQS as SimpleQueryService
participant Provider as AI Provider
Route->>Route: Build prompt (model-specific if needed)
Route->>SQS: Call streamingQuery with onText, onToolUse callbacks
SQS->>Provider: Execute query with outputFormat for structured JSON
Provider-->>SQS: Stream text chunks + structured_output
SQS->>Route: Invoke onText callback for each chunk
Route->>Route: Emit progress events to client
SQS->>SQS: Accumulate response text
alt Structured Output Available
SQS-->>Route: Return result.structured_output
Route->>Route: Use structured JSON directly
else Fallback to Text Parsing
SQS-->>Route: Return result.text
Route->>Route: Parse JSON from accumulated text
end
Route->>Route: Generate final response (XML, features, etc.)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @thesobercoder, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's Docker environment and AI integration capabilities. It addresses critical permission issues, improves container user management, and introduces a streamlined, unified service for interacting with AI models. The changes also bring substantial improvements to the user interface for managing features with worktrees and configuring default AI settings, alongside more robust recovery mechanisms for automated pipelines. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new simple-query-service to centralize and simplify AI model interactions across the server, abstracting away provider-specific logic and reducing code duplication. Server-side routes for feature generation, spec generation, file/image description, prompt enhancement, title generation, issue validation, and suggestions have been refactored to utilize this new service, which supports both simple and streaming queries, structured output, and event callbacks. Docker configurations (Dockerfile, Dockerfile.dev, docker-compose files) have been updated to allow matching host user IDs for mounted volumes and to include installation and persistent storage for the OpenCode CLI. The UI has been enhanced with new features for worktree management, including bulk updating features with worktree assignments, improved pipeline resume logic for features stuck in execution, and security enhancements for opening external URLs. Minor UI adjustments include updated labels, scrollability in dialogs, and better handling of default model settings for new features, along with test updates to reflect these changes.
| # Ensure OpenCode CLI config directory exists with correct permissions | ||
| # OpenCode stores config and auth in ~/.local/share/opencode/ | ||
| if [ ! -d "/home/automaker/.local/share/opencode" ]; then | ||
| mkdir -p /home/automaker/.local/share/opencode | ||
| fi | ||
| chown -R automaker:automaker /home/automaker/.local/share/opencode | ||
| chmod -R 700 /home/automaker/.local/share/opencode | ||
|
|
||
| # OpenCode also uses ~/.config/opencode for configuration | ||
| if [ ! -d "/home/automaker/.config/opencode" ]; then | ||
| mkdir -p /home/automaker/.config/opencode | ||
| fi | ||
| chown -R automaker:automaker /home/automaker/.config/opencode | ||
| chmod -R 700 /home/automaker/.config/opencode | ||
|
|
||
| # OpenCode also uses ~/.cache/opencode for cache data (version file, etc.) | ||
| if [ ! -d "/home/automaker/.cache/opencode" ]; then | ||
| mkdir -p /home/automaker/.cache/opencode | ||
| fi | ||
| chown -R automaker:automaker /home/automaker/.cache/opencode | ||
| chmod -R 700 /home/automaker/.cache/opencode | ||
|
|
||
| # Ensure npm cache directory exists with correct permissions | ||
| # This is needed for using npx to run MCP servers | ||
| if [ ! -d "/home/automaker/.npm" ]; then | ||
| mkdir -p /home/automaker/.npm | ||
| fi | ||
| chown -R automaker:automaker /home/automaker/.npm |
There was a problem hiding this comment.
The pattern of checking for a directory's existence, creating it, and then setting its ownership and permissions is repeated multiple times for the opencode and npm directories. To improve readability and reduce code duplication, consider refactoring this into a reusable shell function.
For example:
ensure_dir_owned() {
local dir_path="$1"
local perms="$2"
if [ ! -d "$dir_path" ]; then
mkdir -p "$dir_path"
fi
chown -R automaker:automaker "$dir_path"
if [ -n "$perms" ]; then
chmod -R "$perms" "$dir_path"
fi
}
# ... then call it for each directory:
ensure_dir_owned "/home/automaker/.local/share/opencode" "700"
ensure_dir_owned "/home/automaker/.config/opencode" "700"
ensure_dir_owned "/home/automaker/.cache/opencode" "700"
ensure_dir_owned "/home/automaker/.npm" "755"| if [ ! -d "/home/automaker/.npm" ]; then | ||
| mkdir -p /home/automaker/.npm | ||
| fi | ||
| chown -R automaker:automaker /home/automaker/.npm |
There was a problem hiding this comment.
For consistency and security, it's a good practice to explicitly set permissions on this directory after changing ownership, similar to how it's done for the .cursor and .opencode directories. This ensures the directory has predictable permissions.
A chmod of 755 is generally appropriate for cache directories.
| chown -R automaker:automaker /home/automaker/.npm | |
| chown -R automaker:automaker /home/automaker/.npm | |
| chmod -R 755 /home/automaker/.npm |
| action: { | ||
| label: 'View PR', | ||
| onClick: () => window.open(result.result!.prUrl!, '_blank'), | ||
| onClick: () => window.open(result.result!.prUrl!, '_blank', 'noopener,noreferrer'), |
There was a problem hiding this comment.
Using noopener and noreferrer with target="_blank" is a good security practice to prevent tab-nabbing. It's great to see this applied consistently across the new window.open calls.
| if (devServerUrl.protocol !== 'http:' && devServerUrl.protocol !== 'https:') { | ||
| logger.error('Invalid dev server URL protocol:', devServerUrl.protocol); | ||
| toast.error('Invalid dev server URL', { | ||
| description: 'The server returned an unsupported URL protocol.', | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
This is a great security addition. Explicitly checking the protocol of the dev server URL before opening it prevents potential attacks using schemes like javascript:, data:, or file:. Well done.
![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
🤖 Fix all issues with AI agents
In @apps/server/src/routes/context/routes/describe-file.ts:
- Around line 178-179: The log message is provider-specific ("Claude") although
the code now uses the provider-agnostic simpleQuery; update the logger.warn call
in the describe-file route where description is checked (the if (!description ||
description.trim().length === 0) block) to use a generic message such as
"Received empty response from model/provider" so it no longer references
"Claude" and reflects use of simpleQuery.
In @apps/server/src/routes/context/routes/describe-image.ts:
- Around line 292-296: The current fallback for cursor models (checked via
isCursorModel(model)) wrongly attempts vision analysis by embedding the image
path into a text prompt and relying on the Read tool; change this branch to
explicitly block or warn: detect isCursorModel(model) and either (a)
short-circuit with a clear error/warning response that vision analysis is
unavailable for Cursor models (include requestId in the log/response), or (b)
return a message to the caller instructing them to select a vision-capable
model; do not attempt to use instructionText/actualPath/imageData.mimeType with
the Read tool for image binary analysis.
In @Dockerfile:
- Around line 106-110: Replace the current unpinned OpenCode install command
that pipes curl to bash with a pinned-version install using the provided VERSION
env pattern; set a specific VERSION (e.g., VERSION=0.1.48), fetch the install
script from the opencode GitHub raw URL instead of the generic URL, and run that
script (keep the post-install checks for /home/automaker/.local/bin and opencode
--version). Ensure the existing shell sequence that echoes the check, lists the
bin directory, and runs which opencode && opencode --version (or prints the
fallback message) remains after the pinned install command so the container
verifies the pinned opencode binary.
🧹 Nitpick comments (22)
apps/ui/tests/projects/board-background-persistence.spec.ts (2)
107-150: Consider extracting project definitions to avoid duplication.The project data (id, name, path, lastOpened) is defined twice: once in
setupWelcomeView(lines 109-122) and again in the route handler (lines 134-146). This creates maintenance overhead if project structure changes.♻️ Suggested refactor
+ // Define projects once for reuse + const testProjects = [ + { + id: projectAId, + name: projectAName, + path: projectAPath, + lastOpened: new Date(Date.now() - 86400000).toISOString(), + }, + { + id: projectBId, + name: projectBName, + path: projectBPath, + lastOpened: new Date(Date.now() - 172800000).toISOString(), + }, + ]; + // Set up welcome view with both projects in the list await setupWelcomeView(page, { workspaceDir: TEST_TEMP_DIR, - recentProjects: [ - { - id: projectAId, - ... - }, - ], + recentProjects: testProjects, }); // ... in route handler: - json.settings.projects = [ - { - id: projectAId, - ... - }, - ]; + json.settings.projects = testProjects;
194-204: Consider replacing hardcodedwaitForTimeoutwith more deterministic waits.While
waitForTimeout(2000)at line 196 works, it's generally a test smell in Playwright. Consider waiting for a more specific condition, such as a network request completing or a DOM element reflecting the loaded state.For example, you could wait for the settings API call to complete:
// Wait for project settings to load by checking for the API response await page.waitForResponse( (response) => response.url().includes('/api/settings/project') && response.status() === 200, { timeout: 5000 } );This would make the test less flaky and faster on fast machines while still being reliable on slower CI environments.
apps/ui/tests/features/feature-manual-review-flow.spec.ts (1)
126-136: Consider extracting sidebar expansion to a shared helper and escaping the regex.
The sidebar expansion logic is duplicated in
new-project-creation.spec.ts. Consider extracting this to a shared utility.The static analysis flagged
new RegExp(projectName)as a potential ReDoS risk. While theprojectNamehere is controlled (test-project-${Date.now()}), escaping regex metacharacters is a safer practice:♻️ Suggested improvement
+// Helper to escape regex special characters +function escapeRegExp(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} // Verify we're on the correct project (project name appears in sidebar button) -await expect(page.getByRole('button', { name: new RegExp(projectName) })).toBeVisible({ +await expect(page.getByRole('button', { name: new RegExp(escapeRegExp(projectName)) })).toBeVisible({ timeout: 10000, });apps/ui/tests/projects/new-project-creation.spec.ts (1)
72-83: Duplicate sidebar expansion pattern.This sidebar expansion logic is identical to the code in
feature-manual-review-flow.spec.ts(lines 126-136). Consider extracting to a shared helper in the test utilities to reduce duplication.Additionally, the same
new RegExp(projectName)concern applies here - while the input is controlled, escaping regex metacharacters would be safer.♻️ Example shared helper
Add to
apps/ui/tests/utils/core/interactions.ts:/** * Expand sidebar if collapsed */ export async function expandSidebarIfCollapsed(page: Page): Promise<void> { const expandSidebarButton = page.locator('button:has-text("Expand sidebar")'); if (await expandSidebarButton.isVisible()) { await expandSidebarButton.click(); await page.waitForTimeout(300); } } /** * Escape special regex characters in a string */ export function escapeRegExp(str: string): string { return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); }Then use in tests:
-const expandSidebarButton = page.locator('button:has-text("Expand sidebar")'); -if (await expandSidebarButton.isVisible()) { - await expandSidebarButton.click(); - await page.waitForTimeout(300); -} +await expandSidebarIfCollapsed(page); -await expect(page.getByRole('button', { name: new RegExp(projectName) })).toBeVisible({ +await expect(page.getByRole('button', { name: new RegExp(escapeRegExp(projectName)) })).toBeVisible({apps/ui/tests/projects/open-existing-project.spec.ts (2)
151-156: Consider avoidingwaitForTimeoutfor more reliable tests.The sidebar expansion logic is a good addition. However, using a fixed
waitForTimeout(300)is discouraged in Playwright tests as it can be flaky. Consider waiting for a specific state change instead.Suggested improvement
const expandSidebarButton = page.locator('button:has-text("Expand sidebar")'); if (await expandSidebarButton.isVisible()) { await expandSidebarButton.click(); - await page.waitForTimeout(300); + // Wait for sidebar to be expanded by checking for the collapse button + await page.locator('button:has-text("Collapse sidebar")').waitFor({ state: 'visible', timeout: 3000 }).catch(() => {}); }
159-164: Consider escaping regex special characters to prevent potential ReDoS.The static analysis tool flagged that
new RegExp(targetProjectName)constructs a regex from variable input. While the risk is low in this test context (project names come from test fixtures or DOM content), escaping special characters would be safer and prevent unexpected behavior if a project name contains regex metacharacters like(,),[,+, etc.Suggested fix
+ // Escape regex special characters in project name + const escapedName = targetProjectName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); if (targetProjectName) { - await expect(page.getByRole('button', { name: new RegExp(targetProjectName) })).toBeVisible({ + await expect(page.getByRole('button', { name: new RegExp(escapedName) })).toBeVisible({ timeout: 15000, }); }apps/ui/src/components/views/board-view/components/kanban-card/card-actions.tsx (1)
141-186: Consider extracting the button selection logic for clarity.The nested ternary works correctly, but two buttons are labeled "Verify" while calling different handlers (
onManualVerifyon line 148 vsonVerifyon line 178). This is intentional based on theskipTestsflag, but could be confusing during maintenance.♻️ Optional: Extract to a helper for readability
const renderActionButton = () => { if (feature.skipTests && onManualVerify) { return ( <Button variant="default" size="sm" className="flex-1 h-7 text-[11px]" onClick={(e) => { e.stopPropagation(); onManualVerify(); }} onPointerDown={(e) => e.stopPropagation()} data-testid={`manual-verify-${feature.id}`} > <CheckCircle2 className="w-3 h-3 mr-1" /> Verify </Button> ); } if (onResume) { return ( <Button variant="default" size="sm" className="flex-1 h-7 text-[11px] bg-[var(--status-success)] hover:bg-[var(--status-success)]/90" onClick={(e) => { e.stopPropagation(); onResume(); }} onPointerDown={(e) => e.stopPropagation()} data-testid={`resume-feature-${feature.id}`} > <RotateCcw className="w-3 h-3 mr-1" /> Resume </Button> ); } if (onVerify) { return ( <Button variant="default" size="sm" className="flex-1 h-7 text-[11px] bg-[var(--status-success)] hover:bg-[var(--status-success)]/90" onClick={(e) => { e.stopPropagation(); onVerify(); }} onPointerDown={(e) => e.stopPropagation()} data-testid={`verify-feature-${feature.id}`} > <CheckCircle2 className="w-3 h-3 mr-1" /> Verify </Button> ); } return null; };docker-entrypoint.sh (1)
50-56: Consider addingchmodfor consistency with other directories.The
.npmdirectory is created and ownership is set, which addresses the PR objective. However, unlike the OpenCode and Cursor directories above, there's nochmodapplied. While npm cache typically works with default permissions, consider addingchmod 700for consistency with the security posture of other directories in this script.Suggested change for consistency
# Ensure npm cache directory exists with correct permissions # This is needed for using npx to run MCP servers if [ ! -d "/home/automaker/.npm" ]; then mkdir -p /home/automaker/.npm fi chown -R automaker:automaker /home/automaker/.npm +chmod -R 700 /home/automaker/.npmapps/server/src/routes/enhance-prompt/routes/enhance.ts (1)
129-142: Clean refactor to provider-agnostic query.The switch to
simpleQuerysimplifies the code by delegating provider routing to the abstraction layer. A few observations:
- The comment on lines 130-131 correctly explains why
systemPromptis combined withuserPromptreadOnly: trueis appropriate since this endpoint only generates textmaxTurns: 1and emptyallowedToolsare correct for a single-turn text generation taskOne consideration: the original
systemPromptwas designed to be passed separately to providers that support it. Concatenating it into the prompt may lose benefits like caching. IfsimpleQueryaccepts a separatesystemPromptparameter (it does, per the relevant code snippet), consider using it:💡 Optional: Use separate systemPrompt parameter
const result = await simpleQuery({ - prompt: `${systemPrompt}\n\n${userPrompt}`, + prompt: userPrompt, model: resolvedModel, cwd: process.cwd(), maxTurns: 1, allowedTools: [], thinkingLevel, readOnly: true, + systemPrompt, });apps/ui/src/store/app-store.ts (1)
660-660: Consider adding server sync fordefaultFeatureModelsetter.The
setDefaultFeatureModelaction (line 3099) only updates local state without syncing to the server. Other similar settings actions (e.g.,setPhaseModel,setAutoLoadClaudeMd) include server synchronization viasyncSettingsToServer(). SincedefaultFeatureModelis already included inSETTINGS_FIELDS_TO_SYNCinuse-settings-sync.ts, the automatic debounced sync will eventually persist the change, but for consistency with explicit setters you might want to add immediate sync.Also, the type assertion
as PhaseModelEntryon line 1282 is unnecessary since{ model: 'opus' }already satisfiesPhaseModelEntry.Optional: Add server sync and remove redundant type assertion
- defaultFeatureModel: { model: 'opus' } as PhaseModelEntry, + defaultFeatureModel: { model: 'opus' },For the setter (if immediate sync is desired):
- setDefaultFeatureModel: (entry) => set({ defaultFeatureModel: entry }), + setDefaultFeatureModel: async (entry) => { + set({ defaultFeatureModel: entry }); + const { syncSettingsToServer } = await import('@/hooks/use-settings-migration'); + await syncSettingsToServer(); + },Also applies to: 1108-1108, 1282-1282, 3099-3099
apps/ui/src/routes/settings.tsx (1)
11-15: Consider runtime validation for theviewsearch parameter.The current implementation uses a type assertion (
as SettingsViewId | undefined) without validating thatsearch.viewis actually a validSettingsViewIdvalue. If an invalid string is passed via URL, it will be silently accepted and could cause unexpected behavior inuseSettingsView.💡 Suggested validation approach
+const VALID_SETTINGS_VIEWS: SettingsViewId[] = ['general', 'models', 'providers', /* ... other valid IDs */]; + export const Route = createFileRoute('/settings')({ component: SettingsView, validateSearch: (search: Record<string, unknown>): SettingsSearchParams => { + const view = search.view; + const isValidView = typeof view === 'string' && VALID_SETTINGS_VIEWS.includes(view as SettingsViewId); return { - view: search.view as SettingsViewId | undefined, + view: isValidView ? (view as SettingsViewId) : undefined, }; }, });apps/server/src/providers/simple-query-service.ts (2)
97-156: Extract shared message-handling logic to reduce duplication.Both
simpleQueryandstreamingQuerycontain nearly identical message-handling loops (error handling, text extraction, result processing). The only differences are the callbacks instreamingQueryand different default values formaxTurnsandallowedTools.💡 Suggested refactor approach
Consider extracting a shared internal function:
async function executeQueryInternal( options: StreamingQueryOptions, defaults: { maxTurns: number; allowedTools: string[] } ): Promise<SimpleQueryResult> { const model = options.model || DEFAULT_MODEL; const provider = ProviderFactory.getProviderForModel(model); // ... shared logic with optional callback invocation } export async function simpleQuery(options: SimpleQueryOptions): Promise<SimpleQueryResult> { return executeQueryInternal(options, { maxTurns: 1, allowedTools: [] }); } export async function streamingQuery(options: StreamingQueryOptions): Promise<SimpleQueryResult> { return executeQueryInternal(options, { maxTurns: 250, allowedTools: ['Read', 'Glob', 'Grep'] }); }Also applies to: 176-240
189-190: DefaultallowedToolsmay be too permissive for some callers.
streamingQuerydefaults to['Read', 'Glob', 'Grep']forallowedTools. Callers expecting a simple text response (no tool usage) might be surprised if the AI starts reading files. Consider documenting this prominently or defaulting to an empty array likesimpleQuery.apps/server/src/routes/features/routes/generate-title.ts (1)
66-73: Consider using thesystemPromptoption instead of concatenation.
SimpleQueryOptionshas a dedicatedsystemPromptfield, but the code concatenates the system prompt with the user prompt. Using the separate field provides cleaner separation and may allow providers to handle system prompts optimally.💡 Suggested change
const result = await simpleQuery({ - prompt: `${SYSTEM_PROMPT}\n\n${userPrompt}`, + prompt: userPrompt, model: CLAUDE_MODEL_MAP.haiku, cwd: process.cwd(), + systemPrompt: SYSTEM_PROMPT, maxTurns: 1, allowedTools: [], });apps/server/src/routes/context/routes/describe-image.ts (1)
288-311: Prompt type definition could be more precise.The
promptvariable is typed asstring | Array<{ type: string; text?: string; source?: object }>, which is a loose type. ThesimpleQueryfunction likely expects a more specific type for the content blocks. Consider using a more precise type that matches the expected API contract.💡 Suggested type improvement
- let prompt: string | Array<{ type: string; text?: string; source?: object }>; + // Use the provider's expected content block types or define a union type + let prompt: string | Array< + | { type: 'text'; text: string } + | { type: 'image'; source: { type: 'base64'; media_type: string; data: string } } + >;apps/server/src/routes/suggestions/generate-suggestions.ts (1)
254-269: Type assertion on structured_output could mask type errors.The
result.structured_outputis cast directly to{ suggestions: Array<Record<string, unknown>> }without validation. If the AI returns malformed data that doesn't match this structure, the code may fail later when accessingstructuredOutput.suggestions.Consider adding runtime validation or using the schema to validate the structured output before using it.
💡 Suggested validation
if (result.structured_output) { - structuredOutput = result.structured_output as { - suggestions: Array<Record<string, unknown>>; - }; + const output = result.structured_output; + if (output && typeof output === 'object' && 'suggestions' in output && Array.isArray(output.suggestions)) { + structuredOutput = output as { suggestions: Array<Record<string, unknown>> }; + } else { + logger.warn('Structured output does not match expected schema, falling back to text parsing'); + } logger.debug('Received structured output:', structuredOutput); - } else if (responseText) { + } + + if (!structuredOutput && responseText) {apps/server/src/routes/app-spec/generate-features-from-spec.ts (1)
115-134: Missing structured output support unlike sibling routes.This file uses
streamingQuerybut doesn't specify anoutputFormatfor structured JSON output, unlikegenerate-suggestions.ts,generate-spec.ts, andvalidate-issue.tswhich conditionally useoutputFormat: { type: 'json_schema', schema: ... }for Claude models.This means feature generation always relies on text parsing via
parseAndCreateFeatures, even when Claude models support structured output. Consider adding structured output support for consistency and reliability.💡 Suggested enhancement
+import { isCursorModel } from '@automaker/types'; + // Determine if we should use structured output (Claude supports it, Cursor doesn't) + const useStructuredOutput = !isCursorModel(model); // Use streamingQuery with event callbacks const result = await streamingQuery({ prompt, model, cwd: projectPath, maxTurns: 250, allowedTools: ['Read', 'Glob', 'Grep'], abortController, thinkingLevel, readOnly: true, settingSources: autoLoadClaudeMd ? ['user', 'project', 'local'] : undefined, + outputFormat: useStructuredOutput + ? { + type: 'json_schema', + schema: featuresSchema, // Define schema for features JSON + } + : undefined, onText: (text) => {apps/server/src/routes/github/routes/validate-issue.ts (1)
185-194: Unsafe type assertion on structured_output.Similar to
generate-suggestions.ts, theresult.structured_outputis cast throughunknowntoIssueValidationResultwithout validation. This could lead to runtime errors if the AI returns unexpected data.💡 Suggested validation
if (result.structured_output) { - validationResult = result.structured_output as unknown as IssueValidationResult; + // Basic shape validation before casting + const output = result.structured_output; + if (output && typeof output === 'object' && 'verdict' in output) { + validationResult = output as unknown as IssueValidationResult; + } else { + logger.warn('Structured output does not match expected schema'); + } logger.debug('Received structured output:', validationResult); - } else if (responseText) { + } + + if (!validationResult && responseText) { // Parse JSON from response text validationResult = extractJson<IssueValidationResult>(responseText, { logger }); }apps/server/src/routes/app-spec/generate-spec.ts (1)
166-174: Type casting on structured_output should have validation.The
result.structured_outputis cast toSpecOutputwithout validation. While this file does have a fallback path (lines 171-174) that parses JSON from response text for non-Claude providers, adding validation would make the code more robust.💡 Suggested improvement
if (result.structured_output) { - structuredOutput = result.structured_output as unknown as SpecOutput; + const output = result.structured_output; + // Basic shape validation + if (output && typeof output === 'object' && 'name' in output) { + structuredOutput = output as unknown as SpecOutput; + } else { + logger.warn('Structured output does not match expected SpecOutput schema'); + } logger.info('✅ Received structured output');apps/server/src/services/auto-mode-service.ts (1)
1006-1008: Inconsistent logging: Useloggerinstead ofconsole.log/warn.The rest of this file uses the
loggerinstance (defined at line 34), but this method usesconsole.logandconsole.warn. For consistency and proper log level control:♻️ Suggested fix
- console.log( + logger.info( `[AutoMode] Resuming feature ${featureId} from pipeline step ${pipelineInfo.stepId}` );- console.warn( + logger.warn( `[AutoMode] No context found for pipeline feature ${featureId}, restarting from beginning` );- console.warn( + logger.warn( `[AutoMode] Step ${pipelineInfo.stepId} no longer exists in pipeline, completing feature without pipeline` );Apply similar changes to lines 1110-1112, 1137-1141, 1194, 1213.
Also applies to: 1024-1026, 1036-1038
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (1)
171-171: Initial state doesn't usedefaultFeatureModel.The initial state for
modelEntryis hardcoded to{ model: 'opus' }, but theuseEffect(Line 213) andresetForm(Line 325) both usedefaultFeatureModelfrom the store. While the effect will override this when the dialog opens, initializing with a hardcoded value creates an inconsistency.Consider initializing with the store value for consistency, though this is low-impact since the effect immediately corrects it on open.
🔧 Suggested fix
This would require either:
- Moving
defaultFeatureModeldestructuring above the state initialization, or- Using a lazy initializer that reads from the store
Since the current implementation works (effect overrides on open), this is optional.
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
185-199: Consider validating branch name before applying in custom mode.When
workMode === 'custom', there's no validation thatbranchNameis non-empty before proceeding. TheWorkModeSelectorshows a visual error, buthandleApplydoesn't block submission. This could result in applying an empty branch name unintentionally.Suggested validation
+ // Validate custom branch mode has a branch name + const isCustomModeValid = workMode !== 'custom' || branchName.trim(); + const handleApply = async () => { + if (applyState.branchName && !isCustomModeValid) { + return; // Don't proceed with invalid custom branch + } + const updates: Partial<Feature> = {};Alternatively, disable the Apply button when custom mode is selected but branch name is empty:
- disabled={!hasAnyApply || isApplying} + disabled={!hasAnyApply || isApplying || (applyState.branchName && workMode === 'custom' && !branchName.trim())}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (56)
DockerfileDockerfile.devapps/server/src/lib/sdk-options.tsapps/server/src/providers/claude-provider.tsapps/server/src/providers/index.tsapps/server/src/providers/simple-query-service.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/routes/context/routes/describe-file.tsapps/server/src/routes/context/routes/describe-image.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/routes/features/routes/generate-title.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/services/auto-mode-service.tsapps/ui/eslint.config.mjsapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/board-view/components/kanban-card/card-actions.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsxapps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsxapps/ui/src/components/views/board-view/hooks/use-board-effects.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-servers.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsxapps/ui/src/components/views/settings-view/mcp-servers/dialogs/add-edit-server-dialog.tsxapps/ui/src/components/views/spec-view/dialogs/create-spec-dialog.tsxapps/ui/src/components/views/spec-view/dialogs/regenerate-spec-dialog.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/routes/settings.tsxapps/ui/src/store/app-store.tsapps/ui/tests/features/feature-manual-review-flow.spec.tsapps/ui/tests/projects/board-background-persistence.spec.tsapps/ui/tests/projects/new-project-creation.spec.tsapps/ui/tests/projects/open-existing-project.spec.tsapps/ui/tests/settings/settings-startup-sync-race.spec.tsapps/ui/tests/utils/core/constants.tsapps/ui/tests/utils/core/interactions.tsapps/ui/tests/utils/navigation/views.tsdocker-compose.dev-server.ymldocker-compose.dev.ymldocker-compose.override.yml.exampledocker-compose.ymldocker-entrypoint.shdocs/docker-isolation.mdlibs/types/src/provider.tslibs/types/src/settings.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
libs/types/src/settings.tsapps/ui/tests/utils/navigation/views.tsapps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsxapps/ui/src/components/views/board-view/hooks/use-board-effects.tsapps/server/src/providers/index.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxlibs/types/src/provider.tsapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/tests/features/feature-manual-review-flow.spec.tsapps/server/src/lib/sdk-options.tsapps/server/src/routes/github/routes/validate-issue.tsapps/ui/tests/settings/settings-startup-sync-race.spec.tsapps/server/src/providers/claude-provider.tsapps/ui/tests/utils/core/interactions.tsapps/ui/src/components/views/settings-view/mcp-servers/dialogs/add-edit-server-dialog.tsxapps/ui/tests/utils/core/constants.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-servers.tsapps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsxapps/ui/src/hooks/use-settings-sync.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/ui/src/components/views/board-view/board-header.tsxapps/server/src/routes/features/routes/generate-title.tsapps/ui/src/components/views/spec-view/dialogs/create-spec-dialog.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsxapps/ui/src/components/views/board-view/components/kanban-card/card-actions.tsxapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/spec-view/dialogs/regenerate-spec-dialog.tsxapps/ui/tests/projects/new-project-creation.spec.tsapps/ui/src/routes/settings.tsxapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/providers/simple-query-service.tsapps/ui/src/store/app-store.tsapps/ui/tests/projects/board-background-persistence.spec.tsapps/ui/tests/projects/open-existing-project.spec.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsxapps/server/src/routes/context/routes/describe-image.tsapps/server/src/routes/context/routes/describe-file.tsapps/server/src/services/auto-mode-service.tsapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
libs/types/src/settings.tsapps/ui/tests/utils/navigation/views.tsapps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsxapps/ui/src/components/views/board-view/hooks/use-board-effects.tsapps/server/src/providers/index.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxlibs/types/src/provider.tsapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/tests/features/feature-manual-review-flow.spec.tsapps/server/src/lib/sdk-options.tsapps/server/src/routes/github/routes/validate-issue.tsapps/ui/tests/settings/settings-startup-sync-race.spec.tsapps/server/src/providers/claude-provider.tsapps/ui/tests/utils/core/interactions.tsapps/ui/src/components/views/settings-view/mcp-servers/dialogs/add-edit-server-dialog.tsxapps/ui/tests/utils/core/constants.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-servers.tsapps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsxapps/ui/src/hooks/use-settings-sync.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/ui/src/components/views/board-view/board-header.tsxapps/server/src/routes/features/routes/generate-title.tsapps/ui/src/components/views/spec-view/dialogs/create-spec-dialog.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsxapps/ui/src/components/views/board-view/components/kanban-card/card-actions.tsxapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/spec-view/dialogs/regenerate-spec-dialog.tsxapps/ui/tests/projects/new-project-creation.spec.tsapps/ui/src/routes/settings.tsxapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/providers/simple-query-service.tsapps/ui/src/store/app-store.tsapps/ui/tests/projects/board-background-persistence.spec.tsapps/ui/tests/projects/open-existing-project.spec.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsxapps/server/src/routes/context/routes/describe-image.tsapps/server/src/routes/context/routes/describe-file.tsapps/server/src/services/auto-mode-service.tsapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/providers/index.tsapps/server/src/lib/sdk-options.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/providers/claude-provider.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/routes/features/routes/generate-title.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/providers/simple-query-service.tsapps/server/src/routes/context/routes/describe-image.tsapps/server/src/routes/context/routes/describe-file.tsapps/server/src/services/auto-mode-service.ts
🧠 Learnings (5)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/src/components/views/settings-view/mcp-servers/dialogs/add-edit-server-dialog.tsxapps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/spec-view/dialogs/create-spec-dialog.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsxapps/ui/src/components/views/board-view/components/kanban-card/card-actions.tsxapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/spec-view/dialogs/regenerate-spec-dialog.tsxapps/ui/src/routes/settings.tsxapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx
📚 Learning: 2026-01-07T15:20:53.127Z
Learnt from: webdevcody
Repo: AutoMaker-Org/automaker PR: 378
File: apps/server/src/lib/sdk-options.ts:141-148
Timestamp: 2026-01-07T15:20:53.127Z
Learning: In apps/server/src/lib/sdk-options.ts, the code intentionally sets permissionMode to 'bypassPermissions' and allowDangerouslySkipPermissions to true for fully autonomous operation. This is an architectural choice and should be documented with rationale, threat model, and security controls (e.g., Docker as boundary). Reviewers should verify that this pattern is intentional and not a regression; ensure access to this option is restricted and add explicit comments explaining risk and when to revoke it. Consider adding a feature flag or runtime guard, and ensure tests cover the security boundary behavior.
Applied to files:
apps/server/src/lib/sdk-options.ts
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Applies to **/*.{ts,tsx} : Use `resolveModelString()` from automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Applied to files:
apps/server/src/routes/github/routes/validate-issue.tsapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Each feature executes in an isolated git worktree created via automaker/git-utils to protect the main branch during AI agent execution
Applied to files:
apps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsxapps/server/src/services/auto-mode-service.ts
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Frontend UI must use TanStack Router for file-based routing, organize components in components/views/, implement stores with Zustand, and use custom hooks in hooks/ directory
Applied to files:
apps/ui/src/components/views/settings-view.tsx
🧬 Code graph analysis (18)
libs/types/src/settings.ts (2)
libs/types/src/index.ts (1)
PhaseModelEntry(115-115)apps/server/src/types/settings.ts (1)
PhaseModelEntry(25-25)
apps/ui/tests/features/feature-manual-review-flow.spec.ts (1)
scripts/launcher-utils.mjs (1)
projectName(742-742)
apps/server/src/routes/github/routes/validate-issue.ts (8)
apps/server/src/routes/github/routes/validation-schema.ts (2)
buildValidationPrompt(187-239)ISSUE_VALIDATION_SYSTEM_PROMPT(89-153)libs/types/src/provider-utils.ts (1)
isCursorModel(27-42)libs/types/src/index.ts (3)
isCursorModel(194-194)DEFAULT_PHASE_MODELS(132-132)IssueValidationResult(164-164)apps/server/src/lib/settings-helpers.ts (1)
getAutoLoadClaudeMdSetting(27-56)libs/model-resolver/src/resolver.ts (1)
resolvePhaseModel(169-205)libs/model-resolver/src/index.ts (1)
resolvePhaseModel(19-19)apps/server/src/providers/simple-query-service.ts (1)
streamingQuery(176-240)apps/server/src/lib/json-extractor.ts (1)
extractJson(49-177)
apps/server/src/routes/suggestions/generate-suggestions.ts (4)
libs/types/src/index.ts (1)
isCursorModel(194-194)apps/server/src/providers/index.ts (1)
streamingQuery(35-35)apps/server/src/providers/simple-query-service.ts (1)
streamingQuery(176-240)apps/server/src/lib/json-extractor.ts (1)
extractJsonWithArray(205-211)
apps/server/src/routes/enhance-prompt/routes/enhance.ts (2)
apps/server/src/providers/index.ts (1)
simpleQuery(35-35)apps/server/src/providers/simple-query-service.ts (1)
simpleQuery(97-156)
apps/server/src/routes/features/routes/generate-title.ts (2)
apps/server/src/providers/index.ts (1)
simpleQuery(35-35)apps/server/src/providers/simple-query-service.ts (1)
simpleQuery(97-156)
apps/ui/src/components/views/board-view.tsx (3)
libs/types/src/feature.ts (1)
Feature(35-71)apps/ui/src/lib/electron.ts (1)
getElectronAPI(909-918)apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(2472-2477)
apps/ui/src/components/views/board-view/components/kanban-card/card-actions.tsx (1)
apps/ui/src/components/ui/button.tsx (1)
Button(108-108)
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
apps/ui/src/components/views/board-view/shared/work-mode-selector.tsx (2)
WorkMode(8-8)WorkModeSelector(43-163)
apps/ui/tests/projects/new-project-creation.spec.ts (2)
apps/ui/tests/utils/api/client.ts (1)
authenticateForTests(366-370)apps/ui/tests/utils/core/interactions.ts (1)
handleLoginScreenIfPresent(71-143)
apps/server/src/providers/simple-query-service.ts (3)
apps/server/src/providers/index.ts (6)
SimpleQueryOptions(37-37)SimpleQueryResult(38-38)StreamingQueryOptions(39-39)simpleQuery(35-35)ProviderFactory(32-32)streamingQuery(35-35)scripts/launcher-utils.mjs (1)
input(449-449)apps/ui/src/lib/agent-context-parser.ts (1)
DEFAULT_MODEL(30-30)
apps/ui/src/store/app-store.ts (2)
libs/types/src/settings.ts (1)
PhaseModelEntry(116-123)apps/server/src/types/settings.ts (1)
PhaseModelEntry(25-25)
apps/ui/tests/projects/board-background-persistence.spec.ts (2)
apps/ui/tests/utils/api/client.ts (1)
authenticateForTests(366-370)apps/ui/tests/utils/core/interactions.ts (1)
handleLoginScreenIfPresent(71-143)
apps/ui/tests/projects/open-existing-project.spec.ts (1)
apps/ui/tests/utils/core/interactions.ts (1)
handleLoginScreenIfPresent(71-143)
apps/ui/src/components/views/settings-view.tsx (1)
apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts (1)
useSettingsView(28-39)
apps/server/src/routes/context/routes/describe-image.ts (3)
libs/types/src/index.ts (1)
isCursorModel(194-194)apps/server/src/providers/index.ts (1)
simpleQuery(35-35)apps/server/src/providers/simple-query-service.ts (1)
simpleQuery(97-156)
apps/server/src/routes/context/routes/describe-file.ts (2)
apps/server/src/providers/index.ts (1)
simpleQuery(35-35)apps/server/src/providers/simple-query-service.ts (1)
simpleQuery(97-156)
apps/server/src/services/auto-mode-service.ts (7)
libs/types/src/index.ts (4)
PipelineStep(208-208)PipelineConfig(209-209)FeatureStatusWithPipeline(211-211)Feature(49-49)libs/types/src/pipeline.ts (3)
PipelineStep(5-13)PipelineConfig(15-18)FeatureStatusWithPipeline(22-28)libs/platform/src/paths.ts (1)
getFeatureDir(49-51)apps/server/src/lib/sdk-options.ts (1)
validateWorkingDirectory(107-119)apps/server/src/lib/settings-helpers.ts (1)
getAutoLoadClaudeMdSetting(27-56)apps/server/src/lib/error-handler.ts (1)
classifyError(180-216)apps/server/src/services/pipeline-service.ts (1)
pipelineService(320-320)
🪛 ast-grep (0.40.5)
apps/ui/tests/features/feature-manual-review-flow.spec.ts
[warning] 133-133: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(projectName)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
apps/ui/tests/projects/new-project-creation.spec.ts
[warning] 80-80: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(projectName)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
apps/ui/tests/projects/board-background-persistence.spec.ts
[warning] 189-189: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(projectAName)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
apps/ui/tests/projects/open-existing-project.spec.ts
[warning] 160-160: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(targetProjectName)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
| if (!description || description.trim().length === 0) { | ||
| logger.warn('Received empty response from Claude'); |
There was a problem hiding this comment.
Update log message to be provider-agnostic.
The code now uses simpleQuery (provider-agnostic), but the log message still references "Claude" specifically. This should be updated for consistency.
🔧 Suggested fix
if (!description || description.trim().length === 0) {
- logger.warn('Received empty response from Claude');
+ logger.warn('Received empty response from AI');
const response: DescribeFileErrorResponse = {📝 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.
| if (!description || description.trim().length === 0) { | |
| logger.warn('Received empty response from Claude'); | |
| if (!description || description.trim().length === 0) { | |
| logger.warn('Received empty response from AI'); | |
| const response: DescribeFileErrorResponse = { |
🤖 Prompt for AI Agents
In @apps/server/src/routes/context/routes/describe-file.ts around lines 178 -
179, The log message is provider-specific ("Claude") although the code now uses
the provider-agnostic simpleQuery; update the logger.warn call in the
describe-file route where description is checked (the if (!description ||
description.trim().length === 0) block) to use a generic message such as
"Received empty response from model/provider" so it no longer references
"Claude" and reflects use of simpleQuery.
| if (isCursorModel(model)) { | ||
| // Use Cursor provider for Cursor models | ||
| // Note: Cursor may have limited support for image content blocks | ||
| logger.info(`[${requestId}] Using Cursor provider for model: ${model}`); | ||
|
|
||
| const provider = ProviderFactory.getProviderForModel(model); | ||
| // Strip provider prefix - providers expect bare model IDs | ||
| const bareModel = stripProviderPrefix(model); | ||
|
|
||
| // Build prompt with image reference for Cursor | ||
| // Note: Cursor CLI may not support base64 image blocks directly, | ||
| // so we include the image path as context | ||
| const cursorPrompt = `${instructionText}\n\nImage file: ${actualPath}\nMIME type: ${imageData.mimeType}`; | ||
|
|
||
| let responseText = ''; | ||
| const queryStart = Date.now(); | ||
| for await (const msg of provider.executeQuery({ | ||
| prompt: cursorPrompt, | ||
| model: bareModel, | ||
| cwd, | ||
| maxTurns: 1, | ||
| allowedTools: ['Read'], // Allow Read tool so Cursor can read the image if needed | ||
| readOnly: true, // Image description only reads, doesn't write | ||
| })) { | ||
| if (msg.type === 'assistant' && msg.message?.content) { | ||
| for (const block of msg.message.content) { | ||
| if (block.type === 'text' && block.text) { | ||
| responseText += block.text; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| logger.info(`[${requestId}] Cursor query completed in ${Date.now() - queryStart}ms`); | ||
| description = responseText; | ||
| // Cursor may not support base64 image blocks directly | ||
| // Use text prompt with image path reference | ||
| logger.info(`[${requestId}] Using text prompt for Cursor model`); | ||
| prompt = `${instructionText}\n\nImage file: ${actualPath}\nMIME type: ${imageData.mimeType}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for Cursor model references and capabilities in the codebase
rg -n -i "cursor" --type=ts --type=tsx -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Search for the Read tool definition and implementation
rg -n "Read.*tool|tool.*Read" --type=ts --type=tsx -A 3Repository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Look at the file in question to understand the broader context
head -350 apps/server/src/routes/context/routes/describe-image.ts | tail -100Repository: AutoMaker-Org/automaker
Length of output: 4189
🏁 Script executed:
# Search for allowedTools definition and usage
rg -n "allowedTools" --type=ts --type=tsx -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Search for Read tool definition
rg -n "Read" --type=ts -B 2 -A 5 | head -100Repository: AutoMaker-Org/automaker
Length of output: 6166
🏁 Script executed:
# Search for tool definitions and where Read is defined
find . -type f -name "*.ts" | xargs grep -l "Read.*tool\|tool.*Read" | head -20Repository: AutoMaker-Org/automaker
Length of output: 589
🏁 Script executed:
# Look for simpleQuery function to understand tool handling
rg -n "simpleQuery" --type=ts -B 3 -A 10 | head -150Repository: AutoMaker-Org/automaker
Length of output: 13638
🏁 Script executed:
# Search for isCursorModel function definition
rg -n "isCursorModel\|function.*isCursorModel" --type=ts -B 2 -A 5Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Search for isCursorModel function definition
rg -n "isCursorModel" --type=ts -B 5 -A 10Repository: AutoMaker-Org/automaker
Length of output: 31699
🏁 Script executed:
# Look at cursor-provider.ts to understand Cursor model capabilities
head -200 apps/server/src/providers/cursor-provider.tsRepository: AutoMaker-Org/automaker
Length of output: 6664
🏁 Script executed:
# Search for Cursor model vision or image capabilities references
rg -n -i "cursor.*vision|cursor.*image" --type=ts -B 3 -A 3Repository: AutoMaker-Org/automaker
Length of output: 2640
🏁 Script executed:
# Search for Read tool limitations or binary file handling
rg -n -i "read.*tool|binary.*file|image.*read" --type=ts -B 3 -A 3 | head -100Repository: AutoMaker-Org/automaker
Length of output: 6676
🏁 Script executed:
# Check if there are tests or documentation about Cursor and image handling
rg -n -i "describe.*image|cursor.*image" --type=ts -B 2 -A 5Repository: AutoMaker-Org/automaker
Length of output: 34512
🏁 Script executed:
# Check cursor-models.ts for all vision-related entries
cat libs/types/src/cursor-models.ts | grep -A 5 -B 5 "supportsVision"Repository: AutoMaker-Org/automaker
Length of output: 3960
Cursor models cannot provide actual image analysis via the fallback strategy.
Cursor models explicitly do not support vision through the CLI (supportsVision: false on all Cursor models). The current fallback—providing a text prompt with the image file path and allowing the Read tool—won't enable actual image analysis. The Read tool is designed for text file content extraction, not binary image data processing.
Users selecting a Cursor model for image description will receive an empty or generic response without knowing the model cannot analyze images. Add a check to either prevent Cursor model selection for this task or provide a clear warning that vision analysis is unavailable.
🤖 Prompt for AI Agents
In @apps/server/src/routes/context/routes/describe-image.ts around lines 292 -
296, The current fallback for cursor models (checked via isCursorModel(model))
wrongly attempts vision analysis by embedding the image path into a text prompt
and relying on the Read tool; change this branch to explicitly block or warn:
detect isCursorModel(model) and either (a) short-circuit with a clear
error/warning response that vision analysis is unavailable for Cursor models
(include requestId in the log/response), or (b) return a message to the caller
instructing them to select a vision-capable model; do not attempt to use
instructionText/actualPath/imageData.mimeType with the Read tool for image
binary analysis.
| // Add to running features immediately | ||
| const abortController = new AbortController(); | ||
| this.runningFeatures.set(featureId, { | ||
| featureId, | ||
| projectPath, | ||
| worktreePath: null, // Will be set below | ||
| branchName: feature.branchName ?? null, | ||
| abortController, | ||
| isAutoMode: false, | ||
| startTime: Date.now(), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all calls to resumeFeature to understand the context
rg -n -B 2 -A 3 'resumeFeature\(' --type tsRepository: AutoMaker-Org/automaker
Length of output: 3059
🏁 Script executed:
# Check context around line 3517 (resumeInterruptedFeatures call)
sed -n '3500,3530p' apps/server/src/services/auto-mode-service.ts
# Check context around line 524
sed -n '510,535p' apps/server/src/services/auto-mode-service.ts
# Check the resumeFeature method implementation to see how it handles isAutoMode
sed -n '938,1000p' apps/server/src/services/auto-mode-service.tsRepository: AutoMaker-Org/automaker
Length of output: 4909
🏁 Script executed:
# Find resumePipelineFeature method definition
rg -n -A 30 'private async resumePipelineFeature' --type ts
# Check what resumeFromPipelineStep receives as parameters
sed -n '1091,1140p' apps/server/src/services/auto-mode-service.tsRepository: AutoMaker-Org/automaker
Length of output: 4372
🏁 Script executed:
# Find resumeInterruptedFeatures method to confirm it's auto-loop related
rg -n -B 5 -A 2 'resumeInterruptedFeatures' --type ts | head -40
# Search for all usages of isAutoMode to see how it's used
rg -n 'isAutoMode' --type ts | head -30Repository: AutoMaker-Org/automaker
Length of output: 4005
🏁 Script executed:
# Check what happens at line 506 when isAutoMode is true
sed -n '475,515p' apps/server/src/services/auto-mode-service.ts
# Check line 1329 to see the other hardcoded isAutoMode
sed -n '1320,1340p' apps/server/src/services/auto-mode-service.ts
# See how isAutoMode is used in the rest of the code
rg -n -B 2 -A 2 'isAutoMode' apps/server/src/services/auto-mode-service.ts | grep -A 2 -B 2 'if.*isAutoMode\|isAutoMode.*)'Repository: AutoMaker-Org/automaker
Length of output: 2096
🏁 Script executed:
# Find saveExecutionState to understand what state is being lost
rg -n -A 10 'async saveExecutionState' --type ts
# Check if there are any other resume paths that might set isAutoMode correctly
rg -n -B 3 'isAutoMode: true' apps/server/src/services/auto-mode-service.tsRepository: AutoMaker-Org/automaker
Length of output: 1113
Propagate isAutoMode context through the resume call chain to preserve auto-loop execution state.
When resumeInterruptedFeatures() resumes features after server restart (auto-loop recovery), the isAutoMode flag is hardcoded to false at lines 1122 and 1329. This prevents saveExecutionState(projectPath) from being called (line 506), losing critical auto-loop state tracking (autoLoopWasRunning, runningFeatureIds).
The issue stems from the call chain lacking a mechanism to pass isAutoMode context:
resumeFeature()has noisAutoModeparameterresumePipelineFeature()has noisAutoModeparameterresumeFromPipelineStep()hardcodesisAutoMode: falsewithout context
Add isAutoMode as a parameter to these methods so resumeInterruptedFeatures() can pass true for auto-loop recovery scenarios, ensuring execution state is properly persisted.
| # Install OpenCode CLI (for multi-provider AI model access) | ||
| RUN curl -fsSL https://opencode.ai/install | bash && \ | ||
| echo "=== Checking OpenCode CLI installation ===" && \ | ||
| ls -la /home/automaker/.local/bin/ && \ | ||
| (which opencode && opencode --version) || echo "opencode installed (may need auth setup)" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenCode CLI installation script
💡 Result:
Use the official install script or a package manager.
-
Install script (recommended):
curl -fsSL https://opencode.ai/install | bash. [1] -
npm:
npm install -g opencode-ai. [1][2] -
Homebrew (macOS / Linux):
brew install sst/tap/opencode. [2] -
Verify:
opencode --version. [1]
Sources:
[1] OpenCode docs — Install. https://opencode.ai/docs.
[2] OpenCode CLI docs / repository (package manager options). https://opencode.ai/docs/cli and GitHub.
🌐 Web query:
OpenCode CLI version pinning latest release
💡 Result:
You can pin the OpenCode CLI to a specific release by installing that exact version. Options from the official docs / repo:
-
Install a specific version with the installer script:
VERSION=0.1.48 curl -fsSL https://raw.githubusercontent.com/opencode-ai/opencode/refs/heads/main/install | bash. [1][2] -
Install via npm pinned to a version:
npm i -g [email protected]. [1][3] -
Install via Go (pin a version):
go install github.com/opencode-ai/[email protected]. [1]
Replace 0.1.48 above with the exact release you want. To discover the latest release tag, check the project's Releases page (GitHub) or the CLI docs/releases before choosing a version. [1][2]
Sources:
[1] OpenCode docs (CLI upgrade/install info).
[2] opencode-ai GitHub repository (install script & examples).
[3] npm/opencode-ai package info.
Pin OpenCode CLI to a specific version to reduce supply chain risk.
The curl | bash installation pattern introduces supply chain risk as the script could change at any time. OpenCode supports version pinning via environment variable: VERSION=0.1.48 curl -fsSL https://raw.githubusercontent.com/opencode-ai/opencode/refs/heads/main/install | bash (replace version as needed). Consider using a pinned version similar to the GitHub CLI approach in this Dockerfile.
🤖 Prompt for AI Agents
In @Dockerfile around lines 106 - 110, Replace the current unpinned OpenCode
install command that pipes curl to bash with a pinned-version install using the
provided VERSION env pattern; set a specific VERSION (e.g., VERSION=0.1.48),
fetch the install script from the opencode GitHub raw URL instead of the generic
URL, and run that script (keep the post-install checks for
/home/automaker/.local/bin and opencode --version). Ensure the existing shell
sequence that echoes the check, lists the bin directory, and runs which opencode
&& opencode --version (or prints the fallback message) remains after the pinned
install command so the container verifies the pinned opencode binary.
2 participants
Summary
Fix EACCES permission error when running npx commands (e.g., MCP servers) inside the Docker container.
Problem
When running
npxcommands inside the container (such as starting MCP servers), the following error occurred:Solution
Added handling in
docker-entrypoint.shto ensure the/home/automaker/.npmdirectory exists and has correct ownership before switching to the automaker user.Changes
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.