Conversation
- 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>
Shironex
added
Bug
📝 WalkthroughWalkthroughServer startup and verification now detect Claude authentication method (API key, OAuth token, or CLI). Platform path checks return structured per-path results. The verify endpoint and UI surfaces expose authType; thinking-level and model metadata updated (adaptive thinking, Opus→4.6, Codex GPT‑5.3). Tests for OAuth detection added. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI
participant Server as Server
participant Platform as Platform
participant FS as Filesystem
UI->>Server: GET /verify-claude-auth
Server->>Platform: getClaudeAuthIndicators()
Platform->>FS: read settings.json, stats-cache.json, projects dir, credential files
FS-->>Platform: FileCheckResult / DirectoryCheckResult (per-path)
Platform-->>Server: ClaudeAuthIndicators (checks + cli indicators)
Server->>Server: determine authType (api_key | oauth | cli) and reason
Server-->>UI: { success, authenticated, authType, error? }
UI->>UI: set cliAuthType / render conditional success message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @Shironex, 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 refines the authentication mechanism for Claude, focusing on improving the clarity, debuggability, and user experience around credential detection. It introduces more detailed logging and error reporting for authentication checks, provides better feedback to users through specific UI messages and warning details, and expands support for OAuth tokens via environment variables. Additionally, it includes new troubleshooting guidance for Docker users and a robust set of unit tests to validate the enhanced detection logic. 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 significantly improves the OAuth credential detection and startup warnings. The changes to getClaudeAuthIndicators to return detailed check information and handle errors gracefully are excellent for debugging. The addition of comprehensive unit tests for credential detection scenarios is a major step forward in ensuring the robustness of this critical feature. The UI improvements, such as showing specific auth success messages and adding Docker troubleshooting tips, will greatly enhance the user experience. I've identified one high-severity logic bug in how multiple credential files are prioritized, which could lead to incorrect auth detection. With that addressed, this will be a very strong set of improvements.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@libs/platform/src/system-paths.ts`:
- Around line 1069-1076: The code sets settingsFileCheck.readable to true based
solely on systemPathAccess(settingsPath) which currently checks F_OK (existence)
and can be misleading; change the check to explicitly verify read permission
(use R_OK via systemPathAccess or an fs.access call with fs.constants.R_OK) when
determining settingsFileCheck.readable, keep result.hasSettingsFile true only if
existence check passes, and ensure the catch still assigns
settingsFileCheck.error; update references in this block (systemPathAccess,
settingsPath, settingsFileCheck.readable, result.hasSettingsFile) accordingly.
🧹 Nitpick comments (1)
libs/platform/tests/oauth-credential-detection.test.ts (1)
68-69: Use@automaker/platformentrypoint instead of relative module paths.The
getClaudeAuthIndicatorsfunction and related exports are available directly from the@automaker/platformpublic entrypoint. Relative imports from../src/system-pathsbypass the shared package interface. Switch to the platform barrel export to align with package architecture:const { getClaudeAuthIndicators } = await import('@automaker/platform');This applies to all similar dynamic imports in this test file.
- 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.
…tup-warning-ignores-claude-oauth-credenti-fuzx
Shironex
left a comment
Shironex
left a comment
There was a problem hiding this comment.
GitChorus AI Review
This is a well-structured PR that improves OAuth credential detection, adds helpful debugging information for Docker environments, and includes comprehensive tests. The core logic changes (continuing to check subsequent credential files when the first is empty, supporting CLAUDE_CODE_OAUTH_TOKEN env var, returning auth type in the verify response) are all sound and well-motivated by the issue it addresses.
The most significant issue is the box formatting bug where path lines in the warning message are 2 characters wider than the box border (73 vs 71 chars), which would produce visually broken ASCII art in the console — easily fixable by changing BOX_CONTENT_WIDTH - 2 to BOX_CONTENT_WIDTH - 4.
The secondary concern is that the getClaudeAuthIndicators() call in verify-claude-auth.ts is unguarded and could turn a successful auth verification into a 500 error if the file system check fails — wrapping it in try/catch would be a quick improvement.
The test coverage is thorough, covering multiple credential formats, edge cases (empty files, null values, malformed JSON), and combined scenarios. The UI changes correctly display auth-type-specific messages. Overall, this is solid work that meaningfully improves the debugging experience.
Quality Score: 7/10 ⭐⭐⭐⭐⭐⭐⭐☆☆☆
Findings Summary
| # | Severity | Category | Finding | Location |
|---|---|---|---|---|
| 1 | 🟠 Major | Logic | Warning box path lines are 2 characters too wide, breaking ASCII box alignment | index.ts:252 |
| 2 | 🟡 Minor | Logic | getClaudeAuthIndicators failure in verify handler causes 500 error despite successful auth | verify-claude-auth.ts:331 |
| 3 | 🟡 Minor | Performance | Unnecessary dynamic import for @automaker/platform in request handler | verify-claude-auth.ts:331 |
| 4 | 🔵 Nit | Style | Verbose debug logging manually reconstructs objects that could be logged directly | index.ts:159 |
| 5 | 🔵 Nit | Style | Redundant field-by-field destructuring in debug log | index.ts:151 |
| 6 | 🔵 Nit | Codebase-fit | Unnecessary truthy checks on always-populated path strings | index.ts:235 |
via GitChorus
apps/server/src/index.ts
Outdated
| pathsCheckedInfo = ` | ||
| ║ ║ | ||
| ║ ${'Paths checked:'.padEnd(BOX_CONTENT_WIDTH)}║ | ||
| ${pathsChecked.map((p) => `║ ${p.substring(0, BOX_CONTENT_WIDTH - 2).padEnd(BOX_CONTENT_WIDTH - 2)} ║`).join('\n')}`; |
There was a problem hiding this comment.
Warning
Major - Logic: Warning box path lines are 2 characters too wide, breaking ASCII box alignment
The path lines in the warning box have incorrect width calculation. The box interior is 69 characters wide (71 total - 2 for ║ borders). Each path line uses: 4 leading spaces + content.padEnd(65) + 2 trailing spaces = 71 interior characters, which is 2 more than the 69-char box interior. This causes the right border ║ to be misaligned, breaking the visual box formatting in the console warning. The 'Paths checked:' header line just above uses the correct BOX_CONTENT_WIDTH (67) with ║ prefix (2 spaces), which totals 69 interior chars correctly.
Problematic code:
${pathsChecked.map((p) => `║ ${p.substring(0, BOX_CONTENT_WIDTH - 2).padEnd(BOX_CONTENT_WIDTH - 2)} ║`).join('\n')}Suggested fix:
// Change BOX_CONTENT_WIDTH - 2 to BOX_CONTENT_WIDTH - 4 to account for
// the 4 leading spaces + 2 trailing spaces (6 chars overhead vs 4 for standard lines)
${pathsChecked.map((p) => `║ ${p.substring(0, BOX_CONTENT_WIDTH - 4).padEnd(BOX_CONTENT_WIDTH - 4)} ║`).join('\n')}| } else if (authMethod === 'cli') { | ||
| // Check if CLI auth is via OAuth (Claude Code subscription) or generic CLI | ||
| // OAuth tokens are stored in the credentials file by the Claude CLI | ||
| const { getClaudeAuthIndicators } = await import('@automaker/platform'); |
There was a problem hiding this comment.
Important
Minor - Logic: getClaudeAuthIndicators failure in verify handler causes 500 error despite successful auth
If getClaudeAuthIndicators() throws (e.g., file system error), the entire request handler's outer catch block returns a 500 error, losing the already-determined authentication success result. Since this call is purely for determining the auth sub-type (oauth vs cli) and is not critical to the auth verification itself, it should be wrapped in its own try/catch so that a failure here gracefully falls back to 'cli' auth type rather than crashing the endpoint.
Problematic code:
const { getClaudeAuthIndicators } = await import('@automaker/platform');
const indicators = await getClaudeAuthIndicators();Suggested fix:
let authType: 'oauth' | 'api_key' | 'cli' | undefined;
if (authenticated) {
if (authMethod === 'api_key') {
authType = 'api_key';
} else if (authMethod === 'cli') {
try {
const { getClaudeAuthIndicators } = await import('@automaker/platform');
const indicators = await getClaudeAuthIndicators();
authType = indicators.credentials?.hasOAuthToken ? 'oauth' : 'cli';
} catch {
// Fall back to generic CLI if credential check fails
authType = 'cli';
}
}
}| } else if (authMethod === 'cli') { | ||
| // Check if CLI auth is via OAuth (Claude Code subscription) or generic CLI | ||
| // OAuth tokens are stored in the credentials file by the Claude CLI | ||
| const { getClaudeAuthIndicators } = await import('@automaker/platform'); |
There was a problem hiding this comment.
Important
Minor - Performance: Unnecessary dynamic import for @automaker/platform in request handler
Using dynamic import() inside a request handler when the module could be statically imported at the top of the file. @automaker/platform is already imported elsewhere in the codebase and the module is always available. While the runtime cost of a cached dynamic import is minimal, static imports are the established pattern in this file (see line 10: import { ... } from '../../../lib/auth-utils.js') and make dependencies clearer.
Problematic code:
const { getClaudeAuthIndicators } = await import('@automaker/platform');Suggested fix:
// At the top of the file, add to existing imports:
import { getClaudeAuthIndicators } from '@automaker/platform';
// Then in the handler, use directly:
const indicators = await getClaudeAuthIndicators();
apps/server/src/index.ts
Outdated
| credentials: indicators.credentials, | ||
| }); | ||
|
|
||
| logger.debug('[CREDENTIAL_CHECK] File check details:', { |
There was a problem hiding this comment.
Note
Nit - Style: Verbose debug logging manually reconstructs objects that could be logged directly
The debug logging manually destructures every field from the check objects to build identical objects. Since these objects are already structured data suitable for logging, the entire indicators.checks object can be logged directly, which is more maintainable and won't drift if fields are added/removed from the interfaces.
Problematic code:
logger.debug('[CREDENTIAL_CHECK] File check details:', {
settingsFile: {
path: indicators.checks.settingsFile.path,
exists: indicators.checks.settingsFile.exists,
readable: indicators.checks.settingsFile.readable,
error: indicators.checks.settingsFile.error,
},
...Suggested fix:
// Instead of manually rebuilding each check object, log directly:
logger.debug('[CREDENTIAL_CHECK] File check details:', indicators.checks);
apps/server/src/index.ts
Outdated
| const indicators = cliAuthIndicators; | ||
|
|
||
| // Log detailed credential detection results | ||
| logger.debug('[CREDENTIAL_CHECK] Claude CLI auth indicators:', { |
There was a problem hiding this comment.
Note
Nit - Style: Redundant field-by-field destructuring in debug log
Similar to the file check details, this debug log manually picks fields from the indicators object. The top-level fields (minus checks) could be destructured more concisely. However, this is a minor style preference - the explicit listing does make it clear exactly what's logged at debug level.
Problematic code:
logger.debug('[CREDENTIAL_CHECK] Claude CLI auth indicators:', {
hasCredentialsFile: indicators.hasCredentialsFile,
hasSettingsFile: indicators.hasSettingsFile,
hasStatsCacheWithActivity: indicators.hasStatsCacheWithActivity,
hasProjectsSessions: indicators.hasProjectsSessions,
credentials: indicators.credentials,
});Suggested fix:
// Option: destructure to exclude the verbose `checks` object
const { checks, ...indicatorSummary } = indicators;
logger.debug('[CREDENTIAL_CHECK] Claude CLI auth indicators:', indicatorSummary);
apps/server/src/index.ts
Outdated
| const pathsChecked: string[] = []; | ||
|
|
||
| // Collect paths that were checked | ||
| if (cliAuthIndicators.checks.settingsFile.path) { |
There was a problem hiding this comment.
Note
Nit - Codebase-fit: Unnecessary truthy checks on always-populated path strings
The path field on FileCheckResult is typed as string (always truthy once set), and the check objects are always initialized with paths from the path helper functions. So if (path) will always be true. The conditional check is unnecessary but harmless - it just adds confusion about whether paths can be empty/undefined.
Problematic code:
if (cliAuthIndicators.checks.settingsFile.path) {
pathsChecked.push(`Settings: ${cliAuthIndicators.checks.settingsFile.path}`);
}Suggested fix:
// The paths are always set, so the if-checks are unnecessary.
// Either remove them or add a comment explaining they're defensive:
pathsChecked.push(`Settings: ${cliAuthIndicators.checks.settingsFile.path}`);
pathsChecked.push(`Stats cache: ${cliAuthIndicators.checks.statsCache.path}`);
pathsChecked.push(`Projects dir: ${cliAuthIndicators.checks.projectsDir.path}`);
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.gitignore:
- Around line 98-99: Remove the unexplained .gitignore entries for ".mcp.json"
and ".planning" because they are not generated or used by any code in this repo;
either delete those two lines from .gitignore now, or if they are intended
placeholders, move them into a dedicated follow-up PR with documentation
explaining their purpose and generation mechanism (refer to the .gitignore file
and the specific entries ".mcp.json" and ".planning" when making the change).
In `@apps/server/package.json`:
- Line 27: Add the missing peer dependency "zod" to your package.json
dependencies to satisfy `@anthropic-ai/claude-agent-sdk`@0.2.32's peer
requirement; specifically add a dependency entry for zod with version specifier
^4.0.0 so package managers will install it alongside
"@anthropic-ai/claude-agent-sdk": "0.2.32".
In `@apps/server/src/index.ts`:
- Around line 248-253: The path lines in the box are two chars wider than the
border because the template for pathsChecked uses a 4-space indent and a padEnd
that doesn't match BOX_CONTENT_WIDTH; update the construction of
pathsCheckedInfo (where pathsChecked and BOX_CONTENT_WIDTH are used) to use the
same "║ ${...}║" pattern as other lines — i.e., use a 2-space indent and
pad/truncate each path to BOX_CONTENT_WIDTH (or BOX_CONTENT_WIDTH - 0 per
surrounding spaces) so each mapped line length matches the rest of the box and
the borders align.
In `@libs/platform/src/system-paths.ts`:
- Around line 1099-1137: The code currently sets exists=false only on ENOENT;
change the error handling in the stats cache and projects dir blocks so that for
non-ENOENT permission errors (EACCES/EPERM) you set exists = true and readable =
false while still recording the error message; specifically update the catch for
systemPathReadFile(statsCachePath) to set statsCacheCheck.exists = true and
statsCacheCheck.readable = false when (err.code === 'EACCES' || err.code ===
'EPERM'), and likewise update the catch for systemPathReaddir(projectsDir) to
set projectsDirCheck.exists = true and projectsDirCheck.readable = false for
those codes (keep the current behaviour for ENOENT and other errors). Ensure you
apply the same pattern where credential file checks are implemented.
🧹 Nitpick comments (3)
apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
170-186: Consider importing from@automaker/typesinstead of maintaining a duplicate list.The
THINKING_LEVELSandTHINKING_LEVEL_LABELSdefined here are duplicated fromlibs/types/src/model-display.ts(which already exports identical arrays/records including the new'adaptive'entry). Maintaining two copies risks them drifting out of sync over time.That said, the additions here (
'adaptive') are consistent with the shared type and theThinkingLevelunion — no correctness issue.libs/types/src/settings.ts (1)
251-268: Avoid string-based adaptive thinking detection; rely on model capability metadata.
isAdaptiveThinkingModelcurrently uses string matching (includes('opus-4-6')/=== 'claude-opus'), which is brittle and bypasses the per-model capability flag pattern used elsewhere. Consider adding an explicit capability flag in the model definitions (e.g.,supportsAdaptiveThinkingormaxThinkingLevel: 'adaptive') and querying by model ID (or alias-resolved ID) insidegetThinkingLevelsForModel. This keeps adaptive thinking aligned with model metadata and future versions.Based on learnings: When implementing AI model capability flags (like thinking/reasoning support), always add per-model flags to model definitions and check the specific model being used, rather than assuming all models from a provider have the same capabilities. Follow the pattern: add flags to model definitions (e.g., supportsThinking: boolean) and look up capabilities by model ID in helper functions like profileHasThinking.
apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (1)
1299-1301: Normalize Claude-compatible provider models before callinggetThinkingLevelsForModel.Only the opus mapping is normalized; other mapped provider models pass their provider IDs into
getThinkingLevelsForModel, so any Claude-specific capability rules (current or future) won’t apply. Consider resolving anymapsToClaudeModelto a canonical Claude model ID (e.g., via a resolver/helper) and passing that togetThinkingLevelsForModel, falling back tomodel.idonly when no mapping exists. This keeps thinking-level options consistent across providers.Also applies to: 1408-1410
| .mcp.json | ||
| .planning |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for references to .mcp.json and .planning files
echo "=== Searching for .mcp.json references ==="
rg -n '\.mcp\.json|mcp\.json|"mcp"' --max-count=50
echo ""
echo "=== Searching for .planning references ==="
rg -n '\.planning|planning' --max-count=50
echo ""
echo "=== Check for file write operations that might generate these files ==="
rg -n 'writeFile|writeFileSync|createWriteStream' --type=ts --type=js --max-count=30
echo ""
echo "=== Look for configuration or initialization code ==="
rg -n 'config|Config|init|initialize' --type=ts --type=js -C2 | head -100Repository: AutoMaker-Org/automaker
Length of output: 50379
🏁 Script executed:
#!/bin/bash
# More focused search for .mcp.json and .planning files
echo "=== Checking if these files currently exist in repo ==="
find . -name ".mcp.json" -o -name ".planning" 2>/dev/null | head -20
echo ""
echo "=== Search for any code that writes/creates these specific filenames ==="
rg 'mcp\.json|\.mcp\.json' --type=ts --type=js --type=json -i
echo ""
echo "=== Search for code patterns that might create .planning ==="
rg '\\.planning|"\.planning"|\'.planning\'' --type=ts --type=js --type=json -i
echo ""
echo "=== Check .gitignore context around lines 96-99 ==="
sed -n '90,105p' .gitignoreRepository: AutoMaker-Org/automaker
Length of output: 345
🏁 Script executed:
#!/bin/bash
# Check .gitignore context around lines 98-99
echo "=== .gitignore entries around lines 90-105 ==="
sed -n '90,105p' .gitignore
echo ""
echo "=== Check if .mcp.json or .planning exist anywhere ==="
find . -type f \( -name ".mcp.json" -o -name ".planning" \) 2>/dev/null
echo ""
echo "=== Search for literal file writes to these files ==="
rg "\.mcp\.json|\.planning" --type=ts --type=js --type=json | head -20Repository: AutoMaker-Org/automaker
Length of output: 2793
Remove these entries or clarify their purpose in a separate PR.
Investigation shows .mcp.json and .planning are not generated by any code in this repository. No generation mechanism exists for these files, and they do not relate to the OAuth credential detection improvements in this PR. If these are placeholders for future work, they should be added in a dedicated PR with clear documentation of their purpose.
🤖 Prompt for AI Agents
In @.gitignore around lines 98 - 99, Remove the unexplained .gitignore entries
for ".mcp.json" and ".planning" because they are not generated or used by any
code in this repo; either delete those two lines from .gitignore now, or if they
are intended placeholders, move them into a dedicated follow-up PR with
documentation explaining their purpose and generation mechanism (refer to the
.gitignore file and the specific entries ".mcp.json" and ".planning" when making
the change).
| }, | ||
| "dependencies": { | ||
| "@anthropic-ai/claude-agent-sdk": "0.1.76", | ||
| "@anthropic-ai/claude-agent-sdk": "0.2.32", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if version 0.2.32 exists on npm
curl -s "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk/0.2.32" | jq '.'Repository: AutoMaker-Org/automaker
Length of output: 3487
🏁 Script executed:
#!/bin/bash
# Search for usage of claude-agent-sdk in the codebase
rg "@anthropic-ai/claude-agent-sdk" --type ts --type js -A 2 -B 2Repository: AutoMaker-Org/automaker
Length of output: 2485
🏁 Script executed:
#!/bin/bash
# Check the full package.json to see context of this dependency
cat apps/server/package.json | head -50Repository: AutoMaker-Org/automaker
Length of output: 1545
🏁 Script executed:
#!/bin/bash
# Check if zod is in dependencies
rg "zod" apps/server/package.jsonRepository: AutoMaker-Org/automaker
Length of output: 49
🌐 Web query:
@anthropic-ai/claude-agent-sdk 0.2.32 release notes breaking changes changelog
💡 Result:
@anthropic-ai/claude-agent-sdk v0.2.32 changelog entry is:
- “Updated to parity with Claude Code v2.1.32” (no other items, and no breaking changes called out for 0.2.32 specifically). [1]
So, any functional changes in 0.2.32 come from what shipped in Claude Code v2.1.32, whose release notes include (highlights): Opus 4.6 availability, research-preview “agent teams” (via CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1), automatic memory recording/recall, “Summarize from here”, skills loading improvements, several fixes, and some VSCode fixes. [2]
Sources: [1] [2]
Add missing peer dependency zod to package.json.
Version 0.2.32 exists and contains no breaking changes—it updates to parity with Claude Code v2.1.32 with only feature additions (Opus 4.6 support, experimental agent teams, memory improvements). However, @anthropic-ai/claude-agent-sdk@0.2.32 declares zod@^4.0.0 as a peer dependency, which is not listed in your package.json dependencies. Add it explicitly to ensure proper dependency resolution:
"zod": "^4.0.0"🤖 Prompt for AI Agents
In `@apps/server/package.json` at line 27, Add the missing peer dependency "zod"
to your package.json dependencies to satisfy
`@anthropic-ai/claude-agent-sdk`@0.2.32's peer requirement; specifically add a
dependency entry for zod with version specifier ^4.0.0 so package managers will
install it alongside "@anthropic-ai/claude-agent-sdk": "0.2.32".
| // Check stats cache for recent activity | ||
| try { | ||
| const statsContent = await systemPathReadFile(getClaudeStatsCachePath()); | ||
| const stats = JSON.parse(statsContent); | ||
| if (stats.dailyActivity && stats.dailyActivity.length > 0) { | ||
| result.hasStatsCacheWithActivity = true; | ||
| const statsContent = await systemPathReadFile(statsCachePath); | ||
| statsCacheCheck.exists = true; | ||
| statsCacheCheck.readable = true; | ||
| try { | ||
| const stats = JSON.parse(statsContent); | ||
| if (stats.dailyActivity && stats.dailyActivity.length > 0) { | ||
| statsCacheCheck.hasDailyActivity = true; | ||
| result.hasStatsCacheWithActivity = true; | ||
| } else { | ||
| statsCacheCheck.hasDailyActivity = false; | ||
| } | ||
| } catch (parseErr) { | ||
| statsCacheCheck.error = `JSON parse error: ${parseErr instanceof Error ? parseErr.message : String(parseErr)}`; | ||
| } | ||
| } catch (err) { | ||
| if ((err as NodeJS.ErrnoException).code === 'ENOENT') { | ||
| statsCacheCheck.exists = false; | ||
| } else { | ||
| statsCacheCheck.error = err instanceof Error ? err.message : String(err); | ||
| } | ||
| } catch { | ||
| // Ignore errors | ||
| } | ||
|
|
||
| // Check for sessions in projects directory | ||
| try { | ||
| const sessions = await systemPathReaddir(getClaudeProjectsDir()); | ||
| const sessions = await systemPathReaddir(projectsDir); | ||
| projectsDirCheck.exists = true; | ||
| projectsDirCheck.readable = true; | ||
| projectsDirCheck.entryCount = sessions.length; | ||
| if (sessions.length > 0) { | ||
| result.hasProjectsSessions = true; | ||
| } | ||
| } catch { | ||
| // Ignore errors | ||
| } catch (err) { | ||
| if ((err as NodeJS.ErrnoException).code === 'ENOENT') { | ||
| projectsDirCheck.exists = false; | ||
| } else { | ||
| projectsDirCheck.error = err instanceof Error ? err.message : String(err); | ||
| } |
There was a problem hiding this comment.
Preserve exists on permission errors to avoid misleading diagnostics.
When systemPathReadFile / systemPathReaddir fails with EACCES/EPERM, the check records error but leaves exists false, which makes the diagnostics look like the path is missing. Set exists = true (and readable = false) for non-ENOENT errors so the UI can distinguish “missing” vs “present but unreadable”. Apply this pattern to the stats cache, projects dir, and credential file checks.
🔧 Suggested pattern (apply similarly to projects/credentials)
- } catch (err) {
- if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
- statsCacheCheck.exists = false;
- } else {
- statsCacheCheck.error = err instanceof Error ? err.message : String(err);
- }
- }
+ } catch (err) {
+ const errno = err as NodeJS.ErrnoException;
+ if (errno.code === 'ENOENT') {
+ statsCacheCheck.exists = false;
+ } else {
+ statsCacheCheck.exists = true;
+ statsCacheCheck.readable = false;
+ statsCacheCheck.error = err instanceof Error ? err.message : String(err);
+ }
+ }Also applies to: 1140-1182
🤖 Prompt for AI Agents
In `@libs/platform/src/system-paths.ts` around lines 1099 - 1137, The code
currently sets exists=false only on ENOENT; change the error handling in the
stats cache and projects dir blocks so that for non-ENOENT permission errors
(EACCES/EPERM) you set exists = true and readable = false while still recording
the error message; specifically update the catch for
systemPathReadFile(statsCachePath) to set statsCacheCheck.exists = true and
statsCacheCheck.readable = false when (err.code === 'EACCES' || err.code ===
'EPERM'), and likewise update the catch for systemPathReaddir(projectsDir) to
set projectsDirCheck.exists = true and projectsDirCheck.readable = false for
those codes (keep the current behaviour for ENOENT and other errors). Ensure you
apply the same pattern where credential file checks are implemented.
- Fix warning box path lines being 2 chars too wide (BOX_CONTENT_WIDTH - 4) - Wrap getClaudeAuthIndicators in try/catch to prevent 500 on auth success - Convert dynamic import to static import for @automaker/platform - Simplify verbose debug logging to log objects directly - Remove unnecessary truthy checks on always-populated path strings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/server/src/routes/setup/routes/verify-claude-auth.ts`:
- Around line 324-339: When computing authType after authentication, handle the
auto-detect case by invoking getClaudeAuthIndicators whenever authenticated is
true and authMethod is not 'api_key' (i.e., when authMethod is
undefined/auto-detect) so you can set authType to 'oauth' if
indicators.credentials?.hasOAuthToken is true, or to 'cli' if not (and fall back
to 'cli' on errors); keep the existing branch that sets authType = 'api_key'
when authMethod === 'api_key' and otherwise perform the credential check (using
getClaudeAuthIndicators) to ensure authType is never left undefined.
| // Determine specific auth type for success messages | ||
| let authType: 'oauth' | 'api_key' | 'cli' | undefined; | ||
| if (authenticated) { | ||
| if (authMethod === 'api_key') { | ||
| authType = 'api_key'; | ||
| } else if (authMethod === 'cli') { | ||
| // Check if CLI auth is via OAuth (Claude Code subscription) or generic CLI | ||
| try { | ||
| const indicators = await getClaudeAuthIndicators(); | ||
| authType = indicators.credentials?.hasOAuthToken ? 'oauth' : 'cli'; | ||
| } catch { | ||
| // Fall back to generic CLI if credential check fails | ||
| authType = 'cli'; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how the UI consumes authType from the verify-claude-auth response
rg -n -C5 'authType' --type=ts --type=tsx -g '!node_modules/**' -g '!dist/**'Repository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Search for authType usage with correct rg syntax
rg -n -C5 'authType' --type=ts -g '!node_modules/**' -g '!dist/**'Repository: AutoMaker-Org/automaker
Length of output: 13252
🏁 Script executed:
#!/bin/bash
# Get the full context of the verify-claude-auth route to understand authMethod handling
cat -n apps/server/src/routes/setup/routes/verify-claude-auth.ts | head -200Repository: AutoMaker-Org/automaker
Length of output: 8378
🏁 Script executed:
#!/bin/bash
# Continue reading the verify-claude-auth.ts file to see the rest of the logic
cat -n apps/server/src/routes/setup/routes/verify-claude-auth.ts | tail -n +200Repository: AutoMaker-Org/automaker
Length of output: 7518
When authMethod is omitted (auto-detect mode), authType remains undefined in the success response, causing the UI to incorrectly default to 'cli' and display the wrong success message.
In auto-detect mode (no authMethod provided), authentication can succeed with OAuth credentials (line 157), but the code on lines 327-338 only sets authType when authMethod === 'api_key' or authMethod === 'cli'. When authMethod is undefined, authType stays undefined and is returned to the UI.
The UI handler in claude-setup-step.tsx (line 144) treats undefined as 'cli': const authType = result.authType === 'oauth' ? 'oauth' : 'cli';. If a user authenticates via auto-detect using OAuth, they'll see "Claude CLI authentication verified!" instead of "Claude Code subscription detected and verified!"
Consider checking credentials in auto-detect mode similarly to how it's done for the 'cli' branch (lines 331-333), or returning a sensible default instead of undefined.
🤖 Prompt for AI Agents
In `@apps/server/src/routes/setup/routes/verify-claude-auth.ts` around lines 324 -
339, When computing authType after authentication, handle the auto-detect case
by invoking getClaudeAuthIndicators whenever authenticated is true and
authMethod is not 'api_key' (i.e., when authMethod is undefined/auto-detect) so
you can set authType to 'oauth' if indicators.credentials?.hasOAuthToken is
true, or to 'cli' if not (and fall back to 'cli' on errors); keep the existing
branch that sets authType = 'api_key' when authMethod === 'api_key' and
otherwise perform the credential check (using getClaudeAuthIndicators) to ensure
authType is never left undefined.
Shironex
left a comment
Shironex
left a comment
There was a problem hiding this comment.
GitChorus AI Review
All 6 previous findings have been fully addressed in this revision. The major box formatting bug is fixed with the correct width calculation. The try/catch for getClaudeAuthIndicators in the verify handler, the static import conversion, the simplified debug logging, and the removal of unnecessary truthy checks are all cleanly resolved.
The incremental diff also includes substantial new features beyond the original PR scope: terminal custom configurations with theme sync, Opus 4.6 adaptive thinking support, GPT-5.3-Codex model addition, board refresh functionality, dev server URL detection, switch-branch refactoring, Cursor provider Windows support, and more. These are well-implemented with tests.
The three new findings are all minor/nit-level: an unsanitized branch name in a shell command (low risk since the value comes from git internals), a string-to-ThemeMode cast that should use proper typing, and a singleton factory with a misleading optional parameter. None of these are blocking.
Score increased from 7 → 9 because all previous findings (including the major one) were addressed cleanly, the code quality is high, comprehensive tests were added, and the remaining new findings are minor style/safety nits.
Quality Score: 9/10 ⭐⭐⭐⭐⭐⭐⭐⭐⭐☆
Findings Summary
| # | Severity | Category | Finding | Location |
|---|---|---|---|---|
| 1 | 🟡 Minor | Logic | Unsanitized branch name interpolated into shell command | auto-mode-service.ts:2686 |
| 2 | 🔵 Nit | Style | Parameter typed as string then cast to ThemeMode | terminal-service.ts:871 |
| 3 | 🔵 Nit | Codebase-fit | Singleton factory parameter silently ignored after first creation | terminal-service.ts:930 |
via GitChorus
Comments Not Placed Inline
The following findings could not be placed as inline comments (line not in diff):
apps/server/src/services/auto-mode-service.ts:2686— File "apps/server/src/services/auto-mode-service.ts" not found in diffapps/server/src/services/terminal-service.ts:871— File "apps/server/src/services/terminal-service.ts" not found in diffapps/server/src/services/terminal-service.ts:930— File "apps/server/src/services/terminal-service.ts" not found in diff
Shironex
deleted the
feature/bug-startup-warning-ignores-claude-oauth-credenti-fuzx
branch
1 participant
Closes #721
Summary by CodeRabbit
New Features
Improvements
Tests
Chores