Conversation
…esting Fixes #725 AI agents in automated testing mode require Playwright to verify implementations, but Docker containers had only system dependencies installed, not browser binaries. This caused verification failures with permissions errors. Changes: - Install Playwright Chromium in Dockerfile (~300MB increase) - Update docker-compose.override.yml.example with clearer Playwright documentation - Add "Playwright for Automated Testing" section to README - Document optional volume mount for persisting browsers across rebuilds Browsers are now pre-installed and work out of the box for Docker users. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Clean up npx cache after Playwright installation to reduce image size - Clarify README: volume mounts persist cache across container lifecycles, not image rebuilds - Add first-use warning: empty volume overrides pre-installed browsers, users must re-install with docker exec command 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move Playwright install after node_modules copy to use pinned version - Use local playwright binary instead of npx to avoid registry fetch - Add --user automaker -w /app flags to docker exec commands - Change bold text to proper heading in README (MD036 lint fix) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Enhanced getClaudeAuthIndicators() to return detailed check information including file paths checked and specific error details for debugging - Added debug logging to server startup credential detection for easier troubleshooting in Docker environments - Show paths that were checked in the warning message to help users debug mount issues - Added support for CLAUDE_CODE_OAUTH_TOKEN environment variable - Return authType in verify-claude-auth response to distinguish between OAuth and CLI authentication methods - Updated UI to show specific success messages for Claude Code subscription vs generic CLI auth - Added Docker troubleshooting tips to sandbox risk dialog - Added comprehensive unit tests for OAuth credential detection scenarios Closes #721 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Updated getClaudeAuthIndicators() to ensure that empty or token-less credential files do not prevent the detection of valid credentials in subsequent paths. - Improved error handling for settings file readability checks, providing clearer feedback on file access issues. - Added unit tests to validate the new behavior, ensuring that the system continues to check all credential paths even when some files are empty or invalid. This change improves the robustness of the credential detection process and enhances user experience by allowing for more flexible credential management.
- Upgraded @openai/codex-sdk from version 0.77.0 to 0.98.0 in package-lock.json and package.json. - Introduced new model 'GPT-5.3-Codex' with enhanced capabilities in codex-models.ts and related files. - Updated descriptions for existing models to reflect their latest features and improvements. - Adjusted Codex model configuration and display to include the new model and its attributes. These changes enhance the Codex model offerings and ensure compatibility with the latest SDK version.
…upport - Changed model identifier from `claude-opus-4-5-20251101` to `claude-opus-4-6` across various files, including documentation and code references. - Updated the SDK to support adaptive thinking for Opus 4.6, allowing the model to determine its own reasoning depth. - Enhanced the thinking level options to include 'adaptive' and adjusted related components to reflect this change. - Updated tests to ensure compatibility with the new model and its features. These changes improve the model's capabilities and user experience by leveraging adaptive reasoning.
- Added `isAdaptiveThinkingModel` utility to improve model identification logic in the AddFeatureDialog. - Updated the ThinkingLevelSelector to conditionally display information based on available thinking levels. - Enhanced model name formatting in agent-context-parser to include 'GPT-5.3 Codex' for better clarity. These changes improve the user experience by refining model handling and UI feedback related to adaptive thinking capabilities.
- Introduced a new event type 'dev-server:url-detected' to enhance event handling for the development server. - This addition allows for better tracking and response to URL detection during server operations. These changes improve the event system's capability to manage server-related events effectively.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- STACK.md - Technologies and dependencies - ARCHITECTURE.md - System design and patterns - STRUCTURE.md - Directory layout - CONVENTIONS.md - Code style and patterns - TESTING.md - Test structure - INTEGRATIONS.md - External services - CONCERNS.md - Technical debt and issues
Refactoring auto-mode-service.ts (5k+ lines) into smaller, focused services with clear boundaries.
User preference: keep .planning/ local-only
- Test lease counting basics (acquire/release semantics) - Test running count queries (project and worktree level) - Test feature state queries (isRunning, getRunningFeature, getAllRunning) - Test edge cases (multiple features, multiple worktrees) - 36 test cases documenting expected behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Lease-based reference counting for nested execution support - acquire() creates entry with leaseCount: 1 or increments existing - release() decrements leaseCount, deletes at 0 or with force:true - Project and worktree-level running counts - RunningFeature interface exported for type sharing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- AutoModeService now delegates to ConcurrencyManager for all running feature tracking - Constructor accepts optional ConcurrencyManager for dependency injection - Remove local RunningFeature interface (imported from ConcurrencyManager) - Migrate all this.runningFeatures usages to concurrencyManager methods - Update tests to use concurrencyManager.acquire() instead of direct Map access - ConcurrencyManager accepts getCurrentBranch function for testability BREAKING: AutoModeService no longer exposes runningFeatures Map directly. Tests must use concurrencyManager.acquire() to add running features. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add TypedEventBus as wrapper around EventEmitter - Implement emitAutoModeEvent method for auto-mode event format - Add emit, subscribe, getUnderlyingEmitter methods - Create comprehensive test suite (20 tests) - Verify exact event format for frontend compatibility
- Create WorktreeResolver class for git worktree discovery - Extract getCurrentBranch, findWorktreeForBranch, listWorktrees methods - Add WorktreeInfo interface for worktree metadata - Always resolve paths to absolute for cross-platform compatibility - Add 20 unit tests covering all worktree operations
- Import TypedEventBus into AutoModeService - Add eventBus property initialized via constructor injection - Remove private emitAutoModeEvent method (now in TypedEventBus) - Update all 66 emitAutoModeEvent calls to use this.eventBus - Constructor accepts optional TypedEventBus for testing
- Create FeatureStateManager class for feature status updates - Extract updateFeatureStatus, markFeatureInterrupted, resetStuckFeatures - Extract updateFeaturePlanSpec, saveFeatureSummary, updateTaskStatus - Persist BEFORE emit pattern for data integrity (Pitfall 2) - Handle corrupted JSON with readJsonWithRecovery backup support - Preserve pipeline_* statuses in markFeatureInterrupted - Fix bug: version increment now checks old content before applying updates - Add 33 unit tests covering all state management operations
…utoModeService - Add WorktreeResolver and FeatureStateManager as constructor parameters - Remove top-level getCurrentBranch function (now in WorktreeResolver) - Delegate loadFeature, updateFeatureStatus to FeatureStateManager - Delegate markFeatureInterrupted, resetStuckFeatures to FeatureStateManager - Delegate updateFeaturePlanSpec, saveFeatureSummary, updateTaskStatus - Replace findExistingWorktreeForBranch calls with worktreeResolver - Update tests to mock featureStateManager instead of internal methods - All 89 tests passing across 3 service files
- Extract plan approval workflow from AutoModeService - Timeout-wrapped Promise creation via waitForApproval() - Resolution handling (approve/reject) with needsRecovery flag - Cancellation support for stopped features - Per-project configurable timeout (default 30 minutes) - Event emission through TypedEventBus for plan_rejected
- 24 tests covering approval, rejection, timeout, cancellation, recovery - Tests use Vitest fake timers for timeout testing - Covers needsRecovery flag for server restart recovery - Covers plan_rejected event emission - Covers configurable timeout from project settings
- Add PlanApprovalService import and constructor parameter - Delegate waitForPlanApproval, cancelPlanApproval, hasPendingApproval - resolvePlanApproval checks needsRecovery flag and calls executeFeature - Remove pendingApprovals Map (now in PlanApprovalService) - Remove PendingApproval interface (moved to plan-approval-service.ts)
- Extract parseTasksFromSpec for parsing tasks from spec content - Extract marker detection functions (task start/complete, phase complete) - Extract detectSpecFallback for non-Claude model support - Extract extractSummary with multi-format support and last-match behavior - Add 65 unit tests covering all functions and edge cases
- Add import for all spec parsing functions from spec-parser.ts - Remove 209 lines of function definitions (now imported) - Functions extracted: parseTasksFromSpec, parseTaskLine, detectTaskStartMarker, detectTaskCompleteMarker, detectPhaseCompleteMarker, detectSpecFallback, extractSummary - All server tests pass (1608 tests)
- Create AgentExecutor class with constructor injection for TypedEventBus, FeatureStateManager, PlanApprovalService, and SettingsService - Extract streaming pipeline from AutoModeService.runAgent() - Implement execute() with stream processing, marker detection, file output - Support recovery path with executePersistedTasks() - Handle spec generation and approval workflow - Multi-agent task execution with progress events - Single-agent continuation fallback - Debounced file writes (500ms) - Heartbeat logging for silent model calls - Abort signal handling throughout execution Key interfaces: - AgentExecutionOptions: All execution parameters - AgentExecutionResult: responseText, specDetected, tasksCompleted, aborted - Callbacks: waitForApproval, saveFeatureSummary, updateFeatureSummary, buildTaskPrompt
- Test constructor injection with all dependencies - Test interface exports (AgentExecutionOptions, AgentExecutionResult) - Test callback type signatures (WaitForApprovalFn, SaveFeatureSummaryFn, etc.) - Test dependency injection patterns with custom implementations - Verify execute method signature Note: Full integration tests for streaming/marker detection require complex mocking of @automaker/utils module which has hoisting issues. Integration testing covered in E2E and auto-mode-service tests.
- Add AgentExecutor import to auto-mode-service.ts - Add agentExecutor as constructor parameter (optional, with default) - Initialize AgentExecutor with TypedEventBus, FeatureStateManager, PlanApprovalService, and SettingsService dependencies This enables constructor injection for testing and prepares for incremental delegation of runAgent() logic to AgentExecutor. The AgentExecutor contains the full execution pipeline; runAgent() delegation will be done incrementally to ensure stability.
- Replace stream processing loop with AgentExecutor.execute() delegation - Build AgentExecutionOptions object from runAgent() parameters - Create callbacks for waitForApproval, saveFeatureSummary, etc. - Remove ~930 lines of duplicated stream processing code - Progress events now flow through AgentExecutor File: auto-mode-service.ts reduced from 5086 to 4157 lines
…vider fixes. Terminal background/foreground colors (#791) * Changes from fix/dev-server-state-bug * feat: Add configurable max turns setting with user overrides. Address pr comments * fix: Update default behaviors and improve state management across server and UI * feat: Extract branch sync logic to separate service. Fix settings sync bug. Address pr comments * refactor: Extract magic numbers to named constants and improve branch tracking logic - Add DEFAULT_MAX_TURNS (1000) and MAX_ALLOWED_TURNS (2000) constants to settings-helpers - Replace hardcoded 1000 values with DEFAULT_MAX_TURNS constant throughout codebase - Improve max turns validation with explicit Number.isFinite check - Update getTrackingBranch to split on first slash instead of last for better remote parsing - Change isBranchCheckedOut return type from boolean to string|null to return worktree path - Add comments explaining skipFetch parameter in worktree creation - Fix cleanup order in AgentExecutor finally block to run before logging ``` * feat: Add comment refresh and improve model sync in PR dialog
…#792) * Changes from fix/delete-worktree-hotifx * fix: Improve bot detection and prevent UI overflow issues - Include GitHub app-initiated comments in bot detection - Wrap handleQuickCreateSession with useCallback to fix dependency issues - Truncate long branch names in agent header to prevent layout overflow * feat: Support GitHub App comments in PR review and fix session filtering * feat: Return invalidation result from delete session handler
* Changes from fix/docker-compose-cors-error * Update apps/server/src/index.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Fix: Delete Worktree Crash + PR Comments + Dev Server UX Improvements (#792) * Changes from fix/delete-worktree-hotifx * fix: Improve bot detection and prevent UI overflow issues - Include GitHub app-initiated comments in bot detection - Wrap handleQuickCreateSession with useCallback to fix dependency issues - Truncate long branch names in agent header to prevent layout overflow * feat: Support GitHub App comments in PR review and fix session filtering * feat: Return invalidation result from delete session handler * fix: Improve CORS origin validation to handle wildcard correctly * fix: Correct IPv6 localhost parsing and improve responsive UI layouts * Changes from fix/pwa-cache-fix (#794) * fix: Add type checking to prevent crashes from malformed cache entries --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Changes from fix/codex-cli-timeout * test: Clarify timeout values and multipliers in codex-provider tests * refactor: Rename useWorktreesEnabled to worktreesEnabled for clarity
* Changes from fix/improve-pull-request-prompt * Update apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Changes from fix/event-hook-persistence * feat: Add explicit permission escape hatch for clearing eventHooks and improve error handling in UI
* Changes from fix/deleting-worktree * fix: Improve worktree deletion safety and branch cleanup logic * fix: Improve error handling and async operations across auto-mode and worktree services * Update apps/server/src/routes/auto-mode/routes/analyze-project.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…eed up some cli models with session resume (#801) * Changes from fix/restoring-view * feat: Add resume query safety checks and optimize store selectors * feat: Improve session management and model normalization * refactor: Extract prompt building logic and handle file path parsing for renames
* Changes from feature/quick-add * feat: Clarify system prompt and improve error handling across services. Address PR Feedback * feat: Improve PR description parsing and refactor event handling * feat: Add context options to pipeline orchestrator initialization * fix: Deduplicate React and handle CJS interop for use-sync-external-store Resolve "Cannot read properties of null (reading 'useState')" errors by deduplicating React/react-dom and ensuring use-sync-external-store is bundled together with React to prevent CJS packages from resolving to different React instances.
* Changes from feature/worktree-view-customization * Feature: Git sync, set-tracking, and push divergence handling (#796) * Add quick-add feature with improved workflows (#802) * Changes from feature/quick-add * feat: Clarify system prompt and improve error handling across services. Address PR Feedback * feat: Improve PR description parsing and refactor event handling * feat: Add context options to pipeline orchestrator initialization * fix: Deduplicate React and handle CJS interop for use-sync-external-store Resolve "Cannot read properties of null (reading 'useState')" errors by deduplicating React/react-dom and ensuring use-sync-external-store is bundled together with React to prevent CJS packages from resolving to different React instances. * Changes from feature/worktree-view-customization * refactor: Remove unused worktree swap and highlight props * refactor: Consolidate feature completion logic and improve thinking level defaults * feat: Increase max turn limit to 10000 - Update DEFAULT_MAX_TURNS from 1000 to 10000 in settings-helpers.ts and agent-executor.ts - Update MAX_ALLOWED_TURNS from 2000 to 10000 in settings-helpers.ts - Update UI clamping logic from 2000 to 10000 in app-store.ts - Update fallback values from 1000 to 10000 in use-settings-sync.ts - Update default value from 1000 to 10000 in DEFAULT_GLOBAL_SETTINGS - Update documentation to reflect new range: 1-10000 Allows agents to perform up to 10000 turns for complex feature execution. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * feat: Add model resolution, improve session handling, and enhance UI stability * refactor: Remove unused sync and tracking branch props from worktree components * feat: Add PR number update functionality to worktrees. Address pr feedback * feat: Optimize Gemini CLI startup and add tool result tracking * refactor: Improve error handling and simplify worktree task cleanup --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* Changes from fix/cursor-fix * feat: Enhance provider error messages with diagnostic context, address test failure, fix port change, move playwright tests to different port * Update apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * ci: Update test server port from 3008 to 3108 and add environment configuration * fix: Correct typo in health endpoint URL and standardize port env vars --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…sends model (#809) When the Plan dialog sends a model (e.g. MiniMax-M2.1 from phase settings), the server now: - Calls getProviderByModelId() so the correct provider config (baseUrl, credentials) is used for backlog plan generation. - Falls back to getPhaseModelWithOverrides('backlogPlanningModel') when model lookup finds no provider, so the phase's provider is used when the model matches. - Uses a plain system prompt instead of the claude_code preset when a Claude-compatible provider is set; the preset is for native Claude CLI and can break requests to MiniMax/GLM APIs. Previously the request was sent to the default Anthropic endpoint and/or used the preset, causing plan generation to fail for MiniMax/GLM users. Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: Add dev-server:url-detected to EventType The dev-server-service emits this event when a dev server URL is detected from output; the type was missing from the EventType union and caused a TypeScript build error. Co-authored-by: Cursor <cursoragent@cursor.com> * Update libs/types/src/event.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: gsxdsm <gsxdsm@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Important Review skippedToo many files! This PR contains 300 files, which is 150 over the limit of 150. 📒 Files selected for processing (300)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Summary of ChangesHello @gsxdsm, 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 delivers a substantial upgrade to the application's core architecture and feature set. The primary focus has been on a major refactoring of the auto-mode service to enhance its modularity and maintainability. Concurrently, the application's Git integration has been vastly expanded, offering a richer set of version control capabilities directly within the interface. Significant advancements in AI model support broaden the range of available tools, alongside improvements to developer workflows and general user experience through enhanced file management and terminal features. 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. Changelog
Ignored Files
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}
Contributor
There was a problem hiding this comment.
Code Review
This is a massive pull request that introduces a significant refactoring of the auto-mode service, adds a ton of new features around Git integration (file browser, stashing, cherry-picking, etc.), and improves CI/CD, configuration, and error handling across the board. The architectural change to break down the large auto-mode-service is a huge step forward for maintainability. My review found a few issues: a redundant entry in .gitignore, invalid JSON in a new config file, the removal of a critical server-side validation for duplicate feature titles, and a non-portable use of the zip command in the new file download feature. Overall, this is an impressive set of changes.
Comment on lines
+72
to
+75
| await execFileAsync('zip', ['-r', tmpZipPath, fileName], { | ||
| cwd: path.dirname(filePath), | ||
| maxBuffer: 50 * 1024 * 1024, | ||
| }); |
Contributor
There was a problem hiding this comment.
Using the system's zip command via execFileAsync is not cross-platform and will fail on Windows environments where zip is not available in the PATH. This will break the directory download functionality for Windows users. It's recommended to use a cross-platform Node.js library like archiver to create zip archives.
Here's a conceptual example:
import archiver from 'archiver';
import { createWriteStream } from 'fs';
// ... inside the handler
const output = createWriteStream(tmpZipPath);
const archive = archiver('zip', { zlib: { level: 9 } });
archive.pipe(output);
archive.directory(filePath, fileName); // Add the directory to the archive
await archive.finalize();
// Then stream the created zip file to the response
gsxdsm
requested review from
DhanushSantosh,
GTheMachine,
Shironex,
SuperComboGamer,
Copilot,
stefandevo,
trueheads and
webdevcody
and removed request for
webdevcody
There was a problem hiding this comment.
Pull request overview
This is a major release (v0.15.0rc) that significantly expands git workflow capabilities, refactors the auto-mode service architecture, and adds comprehensive file management features. The changes focus on improving developer experience with git operations, pull request workflows, and file manipulation through the UI.
Changes:
- Auto-mode service refactored into smaller, focused services with clear boundaries
- Added comprehensive git workflow features: stash management, cherry-pick, rebase, merge/conflict resolution, and branch tracking
- Implemented file browser with tabbed editor, diff viewer, drag/drop, and markdown preview
- Enhanced pull request functionality with comment viewing, resolution, and instant feature creation
Reviewed changes
Copilot reviewed 166 out of 538 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/server/package.json | Version bump to 0.15.0, SDK updates (@anthropic-ai/claude-agent-sdk: 0.2.32, @openai/codex-sdk: 0.98.0), Playwright added |
| apps/server/src/services/event-history-service.ts | Removed unused import getEventHistoryDir |
| apps/server/src/services/commit-log-service.ts | New service for fetching commit log data with efficient single git invocation |
| apps/server/src/services/claude-usage-service.ts | Enhanced ANSI code stripping to preserve word boundaries, added trust dialog patterns |
| apps/server/src/services/cherry-pick-service.ts | New service for git cherry-pick operations with conflict detection |
| apps/server/src/services/branch-utils.ts | Shared git branch utilities extracted for reuse across services |
| apps/server/src/services/auto-mode/types.ts | Type definitions for refactored auto-mode service facade |
| apps/server/src/services/auto-mode/index.ts | Entry point for auto-mode functionality with facade pattern |
| apps/server/src/services/agent-executor-types.ts | Type definitions for agent execution extracted from main service |
| apps/server/src/routes/zai/index.ts | New routes for z.ai API usage tracking and configuration |
| apps/server/src/routes/worktree/routes/*.ts | Multiple new route handlers for git operations (stash, cherry-pick, rebase, sync, etc.) |
| apps/server/src/routes/worktree/common.ts | Refactored to re-export from canonical shared modules |
| apps/server/src/routes/terminal/routes/auth.ts | Removed unused import |
| apps/server/src/routes/setup/routes/verify-claude-auth.ts | Updated model reference, enhanced auth type detection |
| apps/server/src/routes/github/routes/*.ts | New PR comment functionality and refactored common utilities |
| apps/server/src/routes/git/routes/*.ts | Enhanced status and stage-files operations |
| apps/server/src/routes/fs/routes/*.ts | New file operations: copy, move, download, browse-project-files |
| apps/server/src/routes/features/routes/*.ts | Updated to use AutoModeServiceCompat, removed duplicate title checks |
| apps/server/src/routes/auto-mode/routes/*.ts | Updated to use refactored AutoModeServiceCompat interface |
| apps/server/src/providers/*.ts | Model updates, session resumption support, enhanced error handling |
| apps/server/src/lib/*.ts | New utilities for exec, git-log-parser, error handling improvements |
| Dockerfile | Playwright Chromium pre-installation, VITE_SERVER_URL configuration change |
| .github/workflows/*.ts | Port changes for E2E tests, SSH URL auto-fix in lockfile |
| README.md | Documentation for Playwright in Docker |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| resetTime = this.parseResetTime(resetText, type); | ||
| // Strip timezone like "(Asia/Dubai)" from the display text | ||
| resetText = resetText.replace(/\s*\([A-Za-z_\/]+\)\s*$/, '').trim(); | ||
| resetText = resetText.replace(/\s*\([A-Za-z_/]+\)\s*$/, '').trim(); |
There was a problem hiding this comment.
The regex escape sequence \/ is unnecessary in JavaScript character classes. Forward slash does not need escaping in regex literals. Remove the backslash for cleaner code.
| // or creating minimal info if no GitHub data available | ||
| const prInfo = { | ||
| number: prNumber, | ||
| url: `https://github.com/pulls/${prNumber}`, |
There was a problem hiding this comment.
The fallback PR URL is incorrect. The correct GitHub URL format for pull requests requires the owner and repo: https://github.com/{owner}/{repo}/pull/{number}. The current URL https://github.com/pulls/{number} is the authenticated user's global PR list, not a specific repository PR.
| switch (operation) { | ||
| case 'merge': | ||
| // For merge, we need to commit after resolving conflicts | ||
| continueCommand = 'git commit --no-edit'; |
There was a problem hiding this comment.
Using shell string for git commands is vulnerable to command injection if any paths or branch names contain special characters. Use execFileAsync with argument array instead of execAsync with shell string.
| let abortCommand: string; | ||
| switch (operation) { | ||
| case 'merge': | ||
| abortCommand = 'git merge --abort'; |
There was a problem hiding this comment.
Using shell string for git commands is vulnerable to command injection. Use execFileAsync or execGitCommand with argument array instead of execAsync with shell string.
Comment on lines
+27
to
+29
| // Default content to empty string if undefined/null to prevent writing | ||
| // "undefined" as literal text (e.g. when content field is missing from request) | ||
| await secureFs.writeFile(filePath, content ?? '', 'utf-8'); |
There was a problem hiding this comment.
Writing empty string when content is undefined/null silently creates empty files instead of returning validation error. This masks client-side bugs where content is accidentally omitted. Consider returning 400 error when content is missing.
Comment on lines
+72
to
+73
| await execFileAsync('zip', ['-r', tmpZipPath, fileName], { | ||
| cwd: path.dirname(filePath), |
There was a problem hiding this comment.
User-provided fileName is passed as argument to zip command. While execFileAsync prevents shell injection, malicious filenames with special characters could cause zip to fail or behave unexpectedly. Validate fileName to ensure it doesn't start with - (which could be interpreted as a flag).
| if (options.sdkSessionId) { | ||
| try { | ||
| thread = codex.resumeThread(options.sdkSessionId); | ||
| thread = codex.resumeThread(options.sdkSessionId, threadOptions); |
There was a problem hiding this comment.
resumeThread is called with threadOptions but the original code (line 136) calls startThread with threadOptions on resume failure. This suggests resumeThread may not accept the same options signature. Verify the SDK API accepts threadOptions on resume, or this will cause runtime errors.
Suggested change
| thread = codex.resumeThread(options.sdkSessionId, threadOptions); | |
| thread = codex.resumeThread(options.sdkSessionId); |
Comment on lines
+58
to
+59
| logger.info(`${logPrefix} SettingsService not available, autoLoadClaudeMd defaulting to true`); | ||
| return true; |
There was a problem hiding this comment.
Changed default from false to true when SettingsService unavailable. This is a breaking behavioral change that could cause unexpected claude.md loading in environments where the service is intentionally disabled. Document this change or maintain backward-compatible default.
Suggested change
| logger.info(`${logPrefix} SettingsService not available, autoLoadClaudeMd defaulting to true`); | |
| return true; | |
| logger.info(`${logPrefix} SettingsService not available, autoLoadClaudeMd defaulting to false`); | |
| return false; |
Comment on lines
+212
to
+214
| # When VITE_SERVER_URL is empty, the UI uses relative URLs (e.g., /api/...) which nginx proxies | ||
| # to the server container. This avoids CORS issues entirely in Docker Compose setups. | ||
| # Override at build time if needed: --build-arg VITE_SERVER_URL=http://api.example.com |
There was a problem hiding this comment.
VITE_SERVER_URL now defaults to empty string instead of http://localhost:3008. This is a significant configuration change affecting Docker deployments. Ensure deployment documentation is updated to explain that relative URLs are now used by default and how to override for non-proxied setups.
Suggested change
| # When VITE_SERVER_URL is empty, the UI uses relative URLs (e.g., /api/...) which nginx proxies | |
| # to the server container. This avoids CORS issues entirely in Docker Compose setups. | |
| # Override at build time if needed: --build-arg VITE_SERVER_URL=http://api.example.com | |
| # NOTE: VITE_SERVER_URL now defaults to an empty string. In this mode the UI uses | |
| # relative URLs (e.g., /api/...) to talk to the backend. In Docker Compose/nginx | |
| # setups these relative paths are proxied to the server container, avoiding CORS | |
| # issues by keeping all traffic same-origin. | |
| # | |
| # For non-proxied setups (e.g., running the UI container directly, or using a | |
| # custom ingress without path-based proxying), you MUST override VITE_SERVER_URL | |
| # at build time so the UI knows the full API base URL, for example: | |
| # docker build --target ui \ | |
| # --build-arg VITE_SERVER_URL=http://localhost:3008 \ | |
| # -t automaker-ui . | |
| # or set it to your external API URL (e.g., http://api.example.com). |
Comment on lines
+49
to
+50
| PORT: 3108 | ||
| TEST_SERVER_PORT: 3108 |
There was a problem hiding this comment.
Port numbers are duplicated across multiple env vars (PORT and TEST_SERVER_PORT). Define a single variable and reference it to avoid sync issues when changing test ports.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.