Conversation
…toggle button - Add isExpanded and hasOverflow state for expand/collapse functionality - Add useLayoutEffect to detect content overflow (scrollHeight > clientHeight) - Apply max-h-[200px] with overflow-hidden when collapsed - Add gradient overlay at bottom when content is truncated - Add centered ghost button with ChevronDown/ChevronUp icons - Add i18n translations for showMore/showLess in en and fr Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reset files that were not related to the expand button feature back to their develop branch state: - .gitignore - apps/backend/agents/ (base.py, coder.py, planner.py, session.py) - apps/backend/core/ (client.py, simple_client.py) - apps/frontend/src/renderer/App.tsx - apps/frontend/src/renderer/components/AuthStatusIndicator.tsx - apps/frontend/src/renderer/components/KanbanBoard.tsx - apps/frontend/src/renderer/stores/task-store.ts - apps/frontend/src/shared/i18n/locales/*/common.json - tests/test_auth.py - tests/test_issue_884_plan_schema.py The expand button feature implementation remains intact. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR refactors frontend authentication to remove Claude OAuth profile integration and consolidate to API-only profiles, improves task description UX with collapsible overflow handling, simplifies task queue processing logic, enhances diagnostic logging, removes legacy orchestrator code, and updates i18n strings to support new UI elements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 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 @AndyMik90, 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 introduces a user-facing enhancement to improve the readability of lengthy task descriptions by providing an interactive expand/collapse feature. Concurrently, it refines the backend agent's operational robustness by simplifying its error handling mechanisms and streamlining the OAuth token management. The changes also include optimizations to frontend authentication and task queue processing, ensuring a more focused and efficient codebase. Highlights
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 useful expand/collapse feature for long task descriptions, which is well-implemented. However, it also includes a significant and undocumented refactoring of backend agent error handling and frontend authentication logic. While these refactorings appear to be simplifications, they should have been in a separate, well-described pull request to maintain clarity and ease of review. Please update the PR description to reflect all changes. I've added a couple of specific comments on the code.
| /shared_docs | ||
| logs/security/ | ||
| Agents.md | ||
| packages/ |
There was a problem hiding this comment.
The removal of packages/ from the .gitignore file is a significant change that could have unintended consequences, such as committing node modules or other dependencies into the repository. This change seems unrelated to the PR's goal of adding an expand button. Could you please clarify the reason for this change or revert it if it was unintentional?
apps/backend/core/simple_client.py
Outdated
|
|
||
| # Ensure SDK can access it via its expected env var | ||
| # This is required because the SDK doesn't know about per-profile Keychain naming | ||
| import os |
There was a problem hiding this comment.
According to PEP 8, imports should typically be at the top of the file. Moving import os to the top-level imports would improve code style consistency. It seems it was removed from the top in this same PR.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx (1)
58-86:⚠️ Potential issue | 🟡 MinorReset expansion state when switching tasks to keep descriptions collapsed by default.
When TaskMetadata stays mounted while receiving a different
taskprop, the previousisExpandedstate persists. This causes long descriptions to remain expanded when switching to a new task, breaking the intended UX of showing collapsed descriptions by default. Add auseEffectto resetisExpandedtofalsewhenever the task changes.Suggested fix
-import { useState, useRef, useLayoutEffect } from 'react'; +import { useState, useRef, useLayoutEffect, useEffect } from 'react'; ... const [isExpanded, setIsExpanded] = useState(false); const [hasOverflow, setHasOverflow] = useState(false); const contentRef = useRef<HTMLDivElement>(null); + useEffect(() => { + setIsExpanded(false); + }, [task.id]);
🤖 Fix all issues with AI agents
In @.gitignore:
- Line 176: The .gitignore shows new entries under the "# Auto Claude generated
files" section (e.g., Agents.md, .security-key, /shared_docs, logs/security/)
that contradict the commit message claiming a reset; either remove these added
lines to truly revert unrelated changes or explicitly document and justify them
in the commit message. Locate the "# Auto Claude generated files" block and
delete the specific entries (Agents.md, .security-key, /shared_docs,
logs/security/) if they were unintended, or update the commit message to explain
why these ignores are required and ensure any referenced files exist or are
intentionally omitted.
In `@apps/frontend/src/renderer/App.tsx`:
- Line 60: The claudeProfiles store is never loaded on app mount causing
hasOAuthConfigured to be false; call loadClaudeProfiles() during App.tsx
initialization inside the same useEffect that calls loadSettings() and
loadProfiles() (the useEffect where app bootstrap occurs) so profiles are
hydrated from the main process on mount; locate useClaudeProfileStore and its
loadClaudeProfiles function and add the call alongside loadSettings() and
loadProfiles() in the existing useEffect to ensure onboarding logic
(hasOAuthConfigured) sees persisted tokens.
In `@apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx`:
- Around line 179-222: The toggle Button currently lacks accessibility attrs and
type; update the Button in the hasOverflow block to include type="button",
aria-expanded={isExpanded}, and aria-controls pointing to the content
container's id; add a stable id to the content container (either via React's
useId() or derive from contentRef) and set that id on the div referenced by
contentRef (the ReactMarkdown container that renders displayDescription) so
aria-controls targets it; keep existing onClick={() =>
setIsExpanded(!isExpanded)} and ensure the aria-expanded value reflects
isExpanded.
.gitignore
Outdated
| logs/security/ | ||
| Agents.md | ||
| packages/ | ||
| Agents.md No newline at end of file |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
# Check git history for .gitignore to see what happened with Agents.md
git log --oneline -20 -- .gitignoreRepository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 0
stdout:
1205444 fix: remove unrelated changes from branch (qa-requested)
Script executed:
# Show the diff for .gitignore to see the exact change
git show HEAD:.gitignore | grep -n -A 2 -B 2 "Agents.md"Repository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 0
stdout:
174-/shared_docs
175-logs/security/
176:Agents.md
Script executed:
# Search for any references to Agents.md in the codebase
rg "Agents\.md" --type-listRepository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 2
stderr:
error: The argument '<PATTERN>' cannot be used with '--type-list'
USAGE:
rg [OPTIONS] PATTERN [PATH ...]
rg [OPTIONS] -e PATTERN ... [PATH ...]
rg [OPTIONS] -f PATTERNFILE ... [PATH ...]
rg [OPTIONS] --files [PATH ...]
rg [OPTIONS] --type-list
command | rg [OPTIONS] PATTERN
rg [OPTIONS] --help
rg [OPTIONS] --version
For more information try --help
Script executed:
# Check if Agents.md exists in the repository
find . -name "Agents.md" -type f 2>/dev/nullRepository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 0
Script executed:
# Search for any references to "Agents" in the codebase
rg "Agents" --type-listRepository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 2
stderr:
error: The argument '<PATTERN>' cannot be used with '--type-list'
USAGE:
rg [OPTIONS] PATTERN [PATH ...]
rg [OPTIONS] -e PATTERN ... [PATH ...]
rg [OPTIONS] -f PATTERNFILE ... [PATH ...]
rg [OPTIONS] --files [PATH ...]
rg [OPTIONS] --type-list
command | rg [OPTIONS] PATTERN
rg [OPTIONS] --help
rg [OPTIONS] --version
For more information try --help
Script executed:
# Correct search for Agents references
rg "Agents"Repository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 0
stdout:
README.md:5. **Watch it work** - Agents plan, code, and validate autonomously
README.md:| **Memory Layer** | Agents retain insights across sessions for smarter builds |
README.md:
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx: const [expandedAgents, setExpandedAgents] = useState<Set<string>>(new Set());
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx: setExpandedAgents(prev => {
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx: expandedAgents={expandedAgents}
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx: expandedAgents: Set<string>;
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx:function PhaseLogSection({ phase, phaseLog, isExpanded, onToggle, isStreaming = false, expandedAgents, onToggleAgent }: PhaseLogSectionProps) {
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx: expandedAgents={expandedAgents}
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx: expandedAgents: Set<string>;
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx:function GroupedLogEntries({ entries, phase, expandedAgents, onToggleAgent }: GroupedLogEntriesProps) {
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx: isExpanded={expandedAgents.has(`${phase}-orchestrator-activity`)}
apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx: isExpanded={expandedAgents.has(`${phase}-${group.agentName}`)}
apps/backend/security/git_validators.py: Agents should not set user.name, user.email, etc. as this:
apps/backend/runners/github/services/parallel_followup_reviewer.py: Specialist Agents:
apps/backend/runners/github/services/parallel_followup_reviewer.py: f"[ParallelFollowup] Session complete. Agents invoked: {final_agents}"
apps/backend/runners/github/services/parallel_followup_reviewer.py: f"[ParallelFollowup] Complete. Agents invoked: {final_agents}",
apps/backend/runners/github/services/parallel_followup_reviewer.py:Agents invoked: {agents_str}
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py: f"[ParallelOrchestrator] Session complete. Agents invoked: {final_agents}"
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py: f"[ParallelOrchestrator] Complete. Agents invoked: {final_agents}",
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py: # Agents used
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py: lines.append(f"**Specialist Agents Invoked:** {', '.join(agents_invoked)}")
apps/backend/runners/github/services/sdk_utils.py: self._write(f"Agents invoked: {summary.get('agents_invoked', [])}")
apps/backend/prompts/github/pr_followup_orchestrator.md:## Available Specialist Agents
apps/backend/prompts/github/pr_followup_orchestrator.md:### Phase 2: Delegate to Agents (USE TASK TOOL)
apps/backend/prompts/github/pr_parallel_orchestrator.md:## Available Specialist Agents
apps/backend/prompts/github/pr_parallel_orchestrator.md:## CRITICAL: How to Invoke Specialist Agents
apps/backend/prompts_pkg/prompt_generator.py: # Agents no longer need to call Linear MCP tools directly
apps/backend/agents/tools_pkg/__init__.py:Custom MCP Tools for Auto-Claude Agents
apps/backend/agents/__init__.py:Agents Module
apps/backend/agents/README.md:# Agents Module
Script executed:
# Check git diff to see the actual change made to .gitignore in this PR
git diff HEAD~1 .gitignoreRepository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 128
stderr:
fatal: ambiguous argument 'HEAD~1': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
Script executed:
# Check the git history more carefully - see commits affecting .gitignore
git log --oneline -- .gitignore | head -10Repository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 0
stdout:
1205444 fix: remove unrelated changes from branch (qa-requested)
Script executed:
# Get more context about the current .gitignore - check if Agents.md is a new addition
git diff --no-index /dev/null .gitignore 2>&1 | grep -A 5 -B 5 "Agents.md" || cat .gitignore | tail -20Repository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 0
stdout:
+
+# Auto Claude generated files
+.security-key
+/shared_docs
+logs/security/
+Agents.md
\ No newline at end of file
Script executed:
# Check what files are in the PR - look at all changes in this commit
git diff --name-only HEADRepository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 0
Script executed:
# Check the commit message for more context
git log -1 --format=%BRepository: AndyMik90/Auto-Claude
Repository: AndyMik90/Auto-Claude
Exit code: 0
stdout:
fix: remove unrelated changes from branch (qa-requested)
Reset files that were not related to the expand button feature back to
their develop branch state:
- .gitignore
- apps/backend/agents/ (base.py, coder.py, planner.py, session.py)
- apps/backend/core/ (client.py, simple_client.py)
- apps/frontend/src/renderer/App.tsx
- apps/frontend/src/renderer/components/AuthStatusIndicator.tsx
- apps/frontend/src/renderer/components/KanbanBoard.tsx
- apps/frontend/src/renderer/stores/task-store.ts
- apps/frontend/src/shared/i18n/locales/*/common.json
- tests/test_auth.py
- tests/test_issue_884_plan_schema.py
The expand button feature implementation remains intact.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contradiction: commit claims to reset .gitignore but actually adds new entries.
The commit message states that .gitignore was reset to remove unrelated changes from the branch, yet the diff shows new entries being added (Agents.md, .security-key, /shared_docs, logs/security/) under a "# Auto Claude generated files" section. Since Agents.md does not exist in the repository and is unrelated to the expand button feature, clarify whether these additions are intentional or should be reverted to match the commit's stated objective of removing unrelated changes.
🤖 Prompt for AI Agents
In @.gitignore at line 176, The .gitignore shows new entries under the "# Auto
Claude generated files" section (e.g., Agents.md, .security-key, /shared_docs,
logs/security/) that contradict the commit message claiming a reset; either
remove these added lines to truly revert unrelated changes or explicitly
document and justify them in the commit message. Locate the "# Auto Claude
generated files" block and delete the specific entries (Agents.md,
.security-key, /shared_docs, logs/security/) if they were unintended, or update
the commit message to explain why these ignores are required and ensure any
referenced files exist or are intentionally omitted.
| {/* Content container with conditional max-height */} | ||
| <div className="relative"> | ||
| <div | ||
| ref={contentRef} | ||
| className={cn( | ||
| 'prose prose-sm dark:prose-invert max-w-none overflow-hidden prose-p:text-foreground/90 prose-p:leading-relaxed prose-headings:text-foreground prose-strong:text-foreground prose-li:text-foreground/90 prose-ul:my-2 prose-li:my-0.5 prose-a:break-all prose-pre:overflow-x-auto prose-img:max-w-full [&_img]:!max-w-full [&_img]:h-auto [&_code]:break-all [&_code]:whitespace-pre-wrap [&_*]:max-w-full', | ||
| !isExpanded && hasOverflow && 'max-h-[200px]' | ||
| )} | ||
| style={{ wordBreak: 'break-word', overflowWrap: 'anywhere' }} | ||
| > | ||
| <ReactMarkdown remarkPlugins={[remarkGfm]}> | ||
| {displayDescription} | ||
| </ReactMarkdown> | ||
| </div> | ||
|
|
||
| {/* Gradient overlay when collapsed and has overflow */} | ||
| {!isExpanded && hasOverflow && ( | ||
| <div className="absolute bottom-0 left-0 right-0 h-16 bg-gradient-to-t from-muted/80 to-transparent pointer-events-none" /> | ||
| )} | ||
| </div> | ||
|
|
||
| {/* Expand/Collapse button */} | ||
| {hasOverflow && ( | ||
| <div className="flex justify-center mt-2"> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| onClick={() => setIsExpanded(!isExpanded)} | ||
| className="text-muted-foreground hover:text-foreground" | ||
| > | ||
| {isExpanded ? ( | ||
| <> | ||
| <ChevronUp className="h-4 w-4 mr-1" /> | ||
| {t('tasks:metadata.showLess')} | ||
| </> | ||
| ) : ( | ||
| <> | ||
| <ChevronDown className="h-4 w-4 mr-1" /> | ||
| {t('tasks:metadata.showMore')} | ||
| </> | ||
| )} | ||
| </Button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add aria-expanded/controls (and type="button") for the toggle.
This improves screen-reader clarity and avoids accidental form submission if the component is ever rendered inside a form.
♿ Suggested tweak
<div
ref={contentRef}
+ id={`task-description-${task.id}`}
className={cn(
'prose prose-sm dark:prose-invert max-w-none overflow-hidden prose-p:text-foreground/90 prose-p:leading-relaxed prose-headings:text-foreground prose-strong:text-foreground prose-li:text-foreground/90 prose-ul:my-2 prose-li:my-0.5 prose-a:break-all prose-pre:overflow-x-auto prose-img:max-w-full [&_img]:!max-w-full [&_img]:h-auto [&_code]:break-all [&_code]:whitespace-pre-wrap [&_*]:max-w-full',
!isExpanded && hasOverflow && 'max-h-[200px]'
)}
...
<Button
+ type="button"
+ aria-expanded={isExpanded}
+ aria-controls={`task-description-${task.id}`}
variant="ghost"
size="sm"
onClick={() => setIsExpanded(!isExpanded)}
className="text-muted-foreground hover:text-foreground"
>📝 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.
| {/* Content container with conditional max-height */} | |
| <div className="relative"> | |
| <div | |
| ref={contentRef} | |
| className={cn( | |
| 'prose prose-sm dark:prose-invert max-w-none overflow-hidden prose-p:text-foreground/90 prose-p:leading-relaxed prose-headings:text-foreground prose-strong:text-foreground prose-li:text-foreground/90 prose-ul:my-2 prose-li:my-0.5 prose-a:break-all prose-pre:overflow-x-auto prose-img:max-w-full [&_img]:!max-w-full [&_img]:h-auto [&_code]:break-all [&_code]:whitespace-pre-wrap [&_*]:max-w-full', | |
| !isExpanded && hasOverflow && 'max-h-[200px]' | |
| )} | |
| style={{ wordBreak: 'break-word', overflowWrap: 'anywhere' }} | |
| > | |
| <ReactMarkdown remarkPlugins={[remarkGfm]}> | |
| {displayDescription} | |
| </ReactMarkdown> | |
| </div> | |
| {/* Gradient overlay when collapsed and has overflow */} | |
| {!isExpanded && hasOverflow && ( | |
| <div className="absolute bottom-0 left-0 right-0 h-16 bg-gradient-to-t from-muted/80 to-transparent pointer-events-none" /> | |
| )} | |
| </div> | |
| {/* Expand/Collapse button */} | |
| {hasOverflow && ( | |
| <div className="flex justify-center mt-2"> | |
| <Button | |
| variant="ghost" | |
| size="sm" | |
| onClick={() => setIsExpanded(!isExpanded)} | |
| className="text-muted-foreground hover:text-foreground" | |
| > | |
| {isExpanded ? ( | |
| <> | |
| <ChevronUp className="h-4 w-4 mr-1" /> | |
| {t('tasks:metadata.showLess')} | |
| </> | |
| ) : ( | |
| <> | |
| <ChevronDown className="h-4 w-4 mr-1" /> | |
| {t('tasks:metadata.showMore')} | |
| </> | |
| )} | |
| </Button> | |
| </div> | |
| )} | |
| {/* Content container with conditional max-height */} | |
| <div className="relative"> | |
| <div | |
| ref={contentRef} | |
| id={`task-description-${task.id}`} | |
| className={cn( | |
| 'prose prose-sm dark:prose-invert max-w-none overflow-hidden prose-p:text-foreground/90 prose-p:leading-relaxed prose-headings:text-foreground prose-strong:text-foreground prose-li:text-foreground/90 prose-ul:my-2 prose-li:my-0.5 prose-a:break-all prose-pre:overflow-x-auto prose-img:max-w-full [&_img]:!max-w-full [&_img]:h-auto [&_code]:break-all [&_code]:whitespace-pre-wrap [&_*]:max-w-full', | |
| !isExpanded && hasOverflow && 'max-h-[200px]' | |
| )} | |
| style={{ wordBreak: 'break-word', overflowWrap: 'anywhere' }} | |
| > | |
| <ReactMarkdown remarkPlugins={[remarkGfm]}> | |
| {displayDescription} | |
| </ReactMarkdown> | |
| </div> | |
| {/* Gradient overlay when collapsed and has overflow */} | |
| {!isExpanded && hasOverflow && ( | |
| <div className="absolute bottom-0 left-0 right-0 h-16 bg-gradient-to-t from-muted/80 to-transparent pointer-events-none" /> | |
| )} | |
| </div> | |
| {/* Expand/Collapse button */} | |
| {hasOverflow && ( | |
| <div className="flex justify-center mt-2"> | |
| <Button | |
| type="button" | |
| aria-expanded={isExpanded} | |
| aria-controls={`task-description-${task.id}`} | |
| variant="ghost" | |
| size="sm" | |
| onClick={() => setIsExpanded(!isExpanded)} | |
| className="text-muted-foreground hover:text-foreground" | |
| > | |
| {isExpanded ? ( | |
| <> | |
| <ChevronUp className="h-4 w-4 mr-1" /> | |
| {t('tasks:metadata.showLess')} | |
| </> | |
| ) : ( | |
| <> | |
| <ChevronDown className="h-4 w-4 mr-1" /> | |
| {t('tasks:metadata.showMore')} | |
| </> | |
| )} | |
| </Button> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In `@apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx` around
lines 179 - 222, The toggle Button currently lacks accessibility attrs and type;
update the Button in the hasOverflow block to include type="button",
aria-expanded={isExpanded}, and aria-controls pointing to the content
container's id; add a stable id to the content container (either via React's
useId() or derive from contentRef) and set that id on the div referenced by
contentRef (the ReactMarkdown container that renders displayDescription) so
aria-controls targets it; keep existing onClick={() =>
setIsExpanded(!isExpanded)} and ensure the aria-expanded value reflects
isExpanded.
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🔴 BLOCKED
🔴 Blocked - Merge conflicts must be resolved before merge.
Blocked: PR has merge conflicts with base branch. Resolve conflicts before merge.
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | High | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
🚨 Blocking Issues (Must Fix)
- Merge Conflicts: PR has conflicts with base branch that must be resolved
Findings Summary
- Medium: 1 issue(s)
- Low: 1 issue(s)
Generated by Auto Claude PR Review
Findings (2 selected of 2 total)
🟡 [8317d62dc5bf] [MEDIUM] Expand state not reset when switching tasks
📁 apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx:63
The isExpanded state in TaskMetadata is initialized to false but is never reset when the task prop changes. When a user clicks 'Show more' to expand a description on Task A, then switches to Task B (with a long description), Task B will incorrectly appear in expanded mode because the state persists. The useLayoutEffect only updates hasOverflow when task.description changes, but does not reset isExpanded. This leads to inconsistent UI behavior where the expand/collapse state leaks between different tasks. | The isExpanded state in TaskMetadata is initialized to false but never resets when the task prop changes. If a user expands Task A's description (isExpanded=true), then views Task B which also has overflow, Task B will start in the expanded state. While hasOverflow is correctly recalculated via useLayoutEffect when task.description changes, the isExpanded state persists from the previous task.
This only affects UX - users may see new tasks pre-expanded if the previous task was expanded. Not a data integrity issue.
Suggested fix:
Add `setIsExpanded(false);` inside the useLayoutEffect callback, OR add a separate useEffect that resets state when task.id changes: `useEffect(() => { setIsExpanded(false); }, [task.id]);`
🔵 [c3e8b10a41fa] [LOW] Test mock signature doesn't match actual function signature
📁 tests/test_auth.py:468
The test mocks for get_token_from_keychain were changed from lambda config_dir=None: None to lambda: None. The actual function signature is get_token_from_keychain(config_dir: str | None = None). While this works in tests where CLAUDE_CODE_OAUTH_TOKEN is set (because the mock is never called), it creates fragile tests. If CLAUDE_CONFIG_DIR were set in the test environment, get_auth_token() would call get_token_from_keychain(effective_config_dir) with an argument, causing a TypeError. The original mock signature was more robust.
Suggested fix:
Keep the original mock signature: `lambda config_dir=None: None` to match the actual function signature and prevent potential test failures in environments where CLAUDE_CONFIG_DIR is set.
This review was generated by Auto Claude.
…task-descriptions Resolved conflicts in backend files by accepting develop version: - agents/coder.py - new concurrency error handling - agents/planner.py - updated run_agent_session return type - agents/session.py - new is_tool_concurrency_error function - core/client.py - updated authentication flow - core/simple_client.py - updated authentication flow Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Test User seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
github-actions
bot
added
area/fullstack
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/test_auth.py`:
- Line 468: The mocks for get_token_from_keychain in tests/test_auth.py use
parameterless lambdas and must match the real signature
get_token_from_keychain(config_dir: str | None = None); update every
monkeypatch.setattr that currently uses lambda: ... (e.g., the one shown and the
other similar mocks) to use lambda _config_dir=None: ... so the mock accepts the
optional config_dir argument and avoids TypeError when called with or without
that parameter.
- Fix test_integration_phase4.py: Register module in sys.modules before exec_module to allow dataclass decorator to find module by name - Fix ruff format issues in parallel_orchestrator_reviewer.py: Break long f-string lines for logger.error and RuntimeError calls - Fix ruff format issues in pydantic_models.py: Combine Field description on single line Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py (1)
1124-1196: 🧹 Nitpick | 🔵 TrivialConsider removing dead code block.
This entire block is guarded by
if False:(line 1124) and is explicitly marked as "DISABLED: Old orchestrator + Task tool approach". The comment at line 1125 indicates this code should be removed after testing.Since the parallel specialists approach is now the active implementation (via
_run_parallel_specialists), this dead code adds maintenance burden and cognitive overhead without providing value.♻️ Proposed removal of dead code
- # NOTE: The following block is kept but skipped via this marker - if False: # DISABLED: Old orchestrator + Task tool approach - # Old code for reference - to be removed after testing - prompt = self._build_orchestrator_prompt(context) - agent_defs = self._define_specialist_agents(project_root) - client = self._create_sdk_client(project_root, model, thinking_budget) - - MAX_RETRIES = 3 - RETRY_DELAY = 2.0 - - result_text = "" - structured_output = None - msg_count = 0 - last_error = None - - for attempt in range(MAX_RETRIES): - if attempt > 0: - logger.info( - f"[ParallelOrchestrator] Retry attempt {attempt}/{MAX_RETRIES - 1} " - f"after tool concurrency error" - ) - safe_print( - f"[ParallelOrchestrator] Retry {attempt}/{MAX_RETRIES - 1} " - f"(tool concurrency error detected)" - ) - await asyncio.sleep(RETRY_DELAY) - client = self._create_sdk_client( - project_root, model, thinking_budget - ) - - try: - async with client: - await client.query(prompt) - - safe_print( - f"[ParallelOrchestrator] Running orchestrator ({model})...", - flush=True, - ) - - stream_result = await process_sdk_stream( - client=client, - context_name="ParallelOrchestrator", - model=model, - system_prompt=prompt, - agent_definitions=agent_defs, - ) - - error = stream_result.get("error") - - if ( - error == "tool_use_concurrency_error" - and attempt < MAX_RETRIES - 1 - ): - last_error = error - continue - if error: - raise RuntimeError( - f"SDK stream processing failed: {error}" - ) - result_text = stream_result["result_text"] - structured_output = stream_result["structured_output"] - agents_invoked = stream_result["agents_invoked"] - break - except Exception as e: - if attempt < MAX_RETRIES - 1: - last_error = str(e) - continue - raise - else: - raise RuntimeError( - f"Orchestrator failed after {MAX_RETRIES} attempts" - ) - - # END DISABLED BLOCKtests/test_integration_phase4.py (1)
107-116:⚠️ Potential issue | 🟡 MinorModule registration not cleaned up after exec_module.
The fix to register the module before
exec_moduleis correct for resolving dataclass decorator issues. However,"parallel_orchestrator_reviewer"is added tosys.modulesat line 109 but is not included in the cleanup logic (lines 111-116).While this may not cause immediate issues since the module is intentionally loaded, it creates an inconsistency with the cleanup pattern used for the mocked modules. If test isolation is important, consider either:
- Adding it to
_modules_to_mockso it gets cleaned up, or- Documenting why this module should persist in
sys.modules🛡️ Proposed fix to ensure cleanup consistency
_modules_to_mock = [ "context_gatherer", "core.client", "gh_client", "phase_config", "services.pr_worktree_manager", "services.sdk_utils", "claude_agent_sdk", ] +# Track original state for the module we're about to register +_original_orchestrator = sys.modules.get("parallel_orchestrator_reviewer") _original_modules = {name: sys.modules.get(name) for name in _modules_to_mock} for name in _modules_to_mock: sys.modules[name] = MagicMock() # IMPORTANT: Register the module in sys.modules BEFORE exec_module # This is required for dataclass decorators to find the module by name sys.modules["parallel_orchestrator_reviewer"] = orchestrator_module orchestrator_spec.loader.exec_module(orchestrator_module) # Restore all mocked modules to avoid polluting other tests for name in _modules_to_mock: if _original_modules[name] is not None: sys.modules[name] = _original_modules[name] elif name in sys.modules: del sys.modules[name] +# Restore original state for orchestrator module +if _original_orchestrator is not None: + sys.modules["parallel_orchestrator_reviewer"] = _original_orchestrator +elif "parallel_orchestrator_reviewer" in sys.modules: + del sys.modules["parallel_orchestrator_reviewer"]
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🔴 Follow-up Review: Blocked
🔴 Blocked - 5 CI check(s) failing. Fix CI before merge.
Resolution Status
- ✅ Resolved: 0 previous findings addressed
- ❌ Unresolved: 2 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 0 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 2 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
🚨 Blocking Issues
- quality: [UNRESOLVED] Expand state not reset when switching tasks
- test: 5 CI checks failing - Python tests on all platforms
Verdict
BLOCKED due to multiple critical issues:\n\n1. CI FAILING (Critical): 5 CI checks are failing including test-python on macOS, Ubuntu, and Windows. The mock signature mismatches in test_auth.py (lines 468, 652, 675, 693) likely contribute to these failures.\n\n2. Unresolved MEDIUM Finding: The expand state issue (8317d62dc5bf) remains unresolved and was confirmed valid by finding-validator. isExpanded state is never reset when switching tasks, causing incorrect UX.\n\n3. Partially Resolved LOW Finding: Test mock signatures (c3e8b10a41fa) were only partially fixed - 4 instances still use incorrect parameterless lambdas.\n\n4. CLA Not Signed: Contributor license agreement is pending.\n\nTo unblock:\n1. Fix CI by updating remaining mock signatures in test_auth.py (lines 468, 652, 675, 693) to use lambda _config_dir=None: None\n2. Fix expand state bug by adding useEffect(() => { setIsExpanded(false); }, [task.id]) in TaskMetadata.tsx\n3. Sign the CLA
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, comment-analyzer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (3 selected of 3 total)
🟡 [8317d62dc5bf] [MEDIUM] [UNRESOLVED] Expand state not reset when switching tasks
📁 apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx:63
The isExpanded state in TaskMetadata is initialized to false but is never reset when the task prop changes. When a user clicks 'Show more' to expand a description on Task A, then switches to Task B (with a long description), Task B will incorrectly appear in expanded mode because the state persists. The useLayoutEffect only updates hasOverflow when task.description changes, but does not reset isExpanded. This leads to inconsistent UI behavior where the expand/collapse state leaks between different tasks. | The isExpanded state in TaskMetadata is initialized to false but never resets when the task prop changes. If a user expands Task A's description (isExpanded=true), then views Task B which also has overflow, Task B will start in the expanded state. While hasOverflow is correctly recalculated via useLayoutEffect when task.description changes, the isExpanded state persists from the previous task.
This only affects UX - users may see new tasks pre-expanded if the previous task was expanded. Not a data integrity issue.
Resolution note: const [isExpanded, setIsExpanded] = useState(false);
// No useEffect to reset isExpanded when task changes
// useLayoutEffect only updates hasOverflow, not isExpanded
// Parent renders without key prop
Suggested fix:
Add `setIsExpanded(false);` inside the useLayoutEffect callback, OR add a separate useEffect that resets state when task.id changes: `useEffect(() => { setIsExpanded(false); }, [task.id]);`
🔴 [CI-001] [CRITICAL] 5 CI checks failing - Python tests on all platforms
📁 N/A:0
CI tests are failing on test-python for macOS, Ubuntu, and Windows with Python 3.12 and 3.13. The 'CI Complete' check is also failing. This blocks merge regardless of code quality.
Suggested fix:
Review CI logs to identify failing tests. The mock signature mismatches in test_auth.py (lines 468, 652, 675, 693) may be causing failures.
🔵 [CMT-001] [LOW] [FROM COMMENTS] Dead code block should be removed
📁 apps/backend/runners/github/services/parallel_orchestrator_reviewer.py:1124
70+ line code block guarded by if False: marked as 'DISABLED: Old orchestrator + Task tool approach'. Comment explicitly says 'to be removed after testing'.
Suggested fix:
Remove the entire disabled code block (lines 1124-1196)
This review was generated by Auto Claude.
… remove dead code - Reset isExpanded when switching tasks to prevent stale expanded state leaking between tasks - Fix all remaining get_token_from_keychain mock signatures to accept _config_dir parameter - Remove disabled old orchestrator code block in parallel_orchestrator_reviewer.py Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- parallel_orchestrator_reviewer.py: keep dead code removal from our branch - task-store.ts: take develop's improved status+reviewReason check, keep our executionProgress logic for backlog/in_progress, include reviewReason in return Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| import { useProjectStore, loadProjects, addProject, initializeProject, removeProject } from './stores/project-store'; | ||
| import { useTaskStore, loadTasks } from './stores/task-store'; | ||
| import { useSettingsStore, loadSettings, loadProfiles, saveSettings } from './stores/settings-store'; | ||
| import { useClaudeProfileStore, loadClaudeProfiles } from './stores/claude-profile-store'; | ||
| import { useClaudeProfileStore } from './stores/claude-profile-store'; | ||
| import { useTerminalStore, restoreTerminalSessions } from './stores/terminal-store'; | ||
| import { initializeGitHubListeners } from './stores/github'; | ||
| import { initDownloadProgressListener } from './stores/download-store'; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🔴 BLOCKED
🔴 Blocked - 5 CI check(s) failing. Fix CI before merge.
Blocked: 5 CI check(s) failing. Fix CI before merge.
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Medium | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
🚨 Blocking Issues (Must Fix)
- Branch Out of Date: PR branch is behind the base branch and needs to be updated
- CI Failed: CI Complete
- CI Failed: test-python (3.13, ubuntu-latest)
- CI Failed: test-python (3.12, windows-latest)
- CI Failed: test-python (3.12, macos-latest)
- CI Failed: test-python (3.12, ubuntu-latest)
- Critical: Removed constants still imported by coder.py - will cause ImportError (apps/backend/agents/base.py:13)
- Critical: Test mock returns wrong tuple size - will cause ValueError at runtime (tests/test_issue_884_plan_schema.py:314)
- Critical: Test mocks return wrong tuple size - second occurrence (tests/test_issue_884_plan_schema.py:377)
Findings Summary
- Critical: 3 issue(s)
- Medium: 1 issue(s)
Generated by Auto Claude PR Review
Findings (4 selected of 4 total)
🔴 [d563522f1ca4] [CRITICAL] Removed constants still imported by coder.py - will cause ImportError
📁 apps/backend/agents/base.py:13
The constants INITIAL_RETRY_DELAY_SECONDS, MAX_CONCURRENCY_RETRIES, and MAX_RETRY_DELAY_SECONDS were removed from base.py, but they are still imported and actively used in coder.py lines 62-64, 225, 237, 433, 604, 642, 656, 678, 681-682. This will cause an ImportError when the backend attempts to load the coder module, completely breaking the autonomous agent functionality.
Suggested fix:
Either restore the removed constants to base.py, or move them to coder.py directly. The constants were: MAX_CONCURRENCY_RETRIES = 5, INITIAL_RETRY_DELAY_SECONDS = 2, MAX_RETRY_DELAY_SECONDS = 32
🔴 [a3aaeba1797a] [CRITICAL] Test mock returns wrong tuple size - will cause ValueError at runtime
📁 tests/test_issue_884_plan_schema.py:314
The fake_run_agent_session mock functions were changed to return tuple[str, str] (2 values), but the actual run_agent_session function in session.py:342 returns tuple[str, str, dict] (3 values). When coder.py calls status, response, error_info = await run_agent_session(...) (line 451), unpacking 2 values into 3 variables will raise ValueError: not enough values to unpack (expected 3, got 2). This affects both test functions at lines 314-316 and 377-409. | The tests in test_issue_884_plan_schema.py were changed to have fake_run_agent_session return a 2-tuple (str, str) instead of a 3-tuple (str, str, dict). However, the actual run_agent_session function in session.py returns tuple[str, str, dict] and callers in coder.py unpack 3 values: status, response, error_info = await run_agent_session(...). This will cause a ValueError: not enough values to unpack (expected 3, got 2) when tests run.
Suggested fix:
Change the return type annotation back to `tuple[str, str, dict]` and return 3 values: `return "error", "planner failed", {}` and `return "continue", "planned", {}` and `return "complete", "done", {}`
🔴 [30f97c8ea031] [CRITICAL] Test mocks return wrong tuple size - second occurrence
📁 tests/test_issue_884_plan_schema.py:377
Same issue in another test function. The worktree_planning_to_coding_sync test's fake_run_agent_session returns 2-tuple but callers unpack 3 values. Line 400 returns "continue", "planned" and line 409 returns "complete", "done" - both missing the required dict third element.
Suggested fix:
Change return type to `tuple[str, str, dict]` and add empty dict `{}` as third return value: `return "continue", "planned", {}` and `return "complete", "done", {}`
🟡 [1ffbc4a44567] [MEDIUM] Expand/collapse button missing accessibility attributes
📁 apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx:205
The new expand/collapse button in TaskMetadata.tsx does not follow the accessibility pattern established by CollapsibleSection.tsx. The existing pattern includes aria-expanded, aria-controls with useId(), and aria-hidden on icons. This ensures screen reader users can understand the expand/collapse state and the relationship between the button and content.
Suggested fix:
Add accessibility attributes to match CollapsibleSection pattern:
1. Add useId() to generate contentId
2. Add aria-expanded={isExpanded} to Button
3. Add aria-controls={contentId} to Button
4. Add id={contentId} to the content div (ref={contentRef})
5. Add aria-hidden="true" to ChevronUp and ChevronDown icons
This review was generated by Auto Claude.
…g-task-descriptions
- Restore missing constants in base.py that coder.py imports (MAX_CONCURRENCY_RETRIES, INITIAL_RETRY_DELAY_SECONDS, MAX_RETRY_DELAY_SECONDS) - Fix test_issue_884_plan_schema.py mock return types to match run_agent_session 3-tuple signature (str, str, dict) - Add accessibility attributes to expand/collapse button in TaskMetadata.tsx (aria-expanded, aria-controls, aria-hidden on icons, id on content) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 1 blocking issue(s) require fixes.
Resolution Status
- ✅ Resolved: 4 previous findings addressed
- ❌ Unresolved: 0 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 3 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 1 findings verified as genuine issues
- 👤 Needs Human Review: 1 findings require manual verification
🚨 Blocking Issues
- quality: console.warn changed to debugLog hides production diagnostic signal
Verdict
NEEDS_REVISION for two reasons:
-
CI pending — 1 CI check is still running. Cannot approve until all checks pass.
-
1 confirmed MEDIUM finding — NEW-004: The change from console.warn to debugLog for 'task not found' in task-store.ts silences a diagnostically important warning in production. This is a quick fix (revert one line to console.warn).
Positive progress: All 4 previous findings (3 critical, 1 medium) are verified as resolved. The fix commit properly addresses test mock tuple sizes, preserves backend constants, and adds full accessibility attributes to the expand/collapse button. 3 of 5 new findings were dismissed as false positives after validation.
Low-priority item: NEW-005 (OAuth profile display simplification) needs human confirmation that removing Claude profile info from the auth indicator is intentional — appears deliberate but should be verified.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (2 selected of 2 total)
🟡 [NEW-004] [MEDIUM] console.warn changed to debugLog hides production diagnostic signal
📁 apps/frontend/src/renderer/stores/task-store.ts:244
The updateTaskStatus function now uses debugLog instead of console.warn when a task is not found (line 244). debugLog only logs when process.env.DEBUG === 'true', which is not set in production. A 'task not found' condition during a status update indicates a real inconsistency (stale references, race conditions) that should be visible for diagnosing production issues. The original console.warn was appropriate for this anomalous condition.
Suggested fix:
Revert to console.warn('[updateTaskStatus] Task not found:', taskId) for this specific case. The other debugLog calls for normal transitions are fine.
🔵 [NEW-005] [LOW] OAuth profile display simplified - needs human confirmation of intent
📁 apps/frontend/src/renderer/components/AuthStatusIndicator.tsx:104
The Claude OAuth profile store references and usage data fallback logic were removed. OAuth users now see a generic 'OAuth / Anthropic' badge instead of their specific account name/email. The 'Account details for OAuth profiles' tooltip section was also removed. This appears intentional (loadClaudeProfiles removed from App.tsx, 'account' i18n key removed), but should be confirmed as not regressing multi-account OAuth users.
Suggested fix:
If intentional, no action needed. If OAuth users need to see which account is active, restore the Claude profile store lookup as a secondary source.
This review was generated by Auto Claude.
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
✅ Auto Claude Review - APPROVED
Status: Ready to Merge
Summary: Skipped review: Review already in progress (started 18m ago)
This automated review found no blocking issues. The PR can be safely merged.
Generated by Auto Claude
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🔴 BLOCKED
🔴 Blocked - 2 critical/high/medium issue(s) must be fixed.
Blocked by 1 critical issues
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Medium | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
🚨 Blocking Issues (Must Fix)
- Critical: Removing loadClaudeProfiles() breaks onboarding detection (apps/frontend/src/renderer/App.tsx:186)
Findings Summary
- Critical: 1 issue(s)
- Medium: 1 issue(s)
- Low: 1 issue(s)
Generated by Auto Claude PR Review
Findings (3 selected of 3 total)
🔴 [92b65c07a3cf] [CRITICAL] Removing loadClaudeProfiles() breaks onboarding detection
📁 apps/frontend/src/renderer/App.tsx:186
The loadClaudeProfiles() call was removed from the initial load useEffect (line 186), but claudeProfiles from useClaudeProfileStore is still actively used at line 263 to detect whether OAuth is configured (claudeProfiles.some(p => p.oauthToken || (p.isDefault && p.configDir))). Without the initial load, the claudeProfiles array will always be empty on startup (default store state is []), causing the onboarding wizard to incorrectly show for users who have OAuth configured but no API profiles. The only other places loadClaudeProfiles() is called are inside specific components (OAuthStep, AccountSettings, RateLimitModal) — none of which run at app startup before the onboarding check.
Suggested fix:
Restore the `loadClaudeProfiles()` call in the initial load useEffect, or refactor the onboarding detection to not depend on claude profile store state.
🔵 [61e0f9e59c02] [LOW] Missing newline at end of file
📁 .gitignore:176
The .gitignore file now lacks a trailing newline character. POSIX convention requires text files to end with a newline, and git will show 'No newline at end of file' warnings. The diff shows Agents.md without a trailing newline (\ No newline at end of file).
Suggested fix:
Add a trailing newline after the last line of .gitignore.
🟡 [c58ea71da8d9] [MEDIUM] Removing loadClaudeProfiles() breaks onboarding wizard check
📁 apps/frontend/src/renderer/App.tsx:187
The PR removes the loadClaudeProfiles() call from App.tsx's initial load effect (line 187), but App.tsx still reads from the claude profile store at line 135 (const claudeProfiles = useClaudeProfileStore(state => state.profiles)) and uses it at line 263 to determine if OAuth is configured: const hasOAuthConfigured = claudeProfiles.some(p => p.oauthToken || (p.isDefault && p.configDir)). Without loadClaudeProfiles() being called on app startup, the store will remain at its initial state (profiles: [], activeProfileId: 'default'), so hasOAuthConfigured will always be false. This means users who have ONLY OAuth configured (no API profiles) will see the onboarding wizard every time they open the app, because hasAnyAuth will be false even though they are authenticated. The RateLimitModal and SDKRateLimitModal components also read from useClaudeProfileStore and call loadClaudeProfiles() themselves, but those are lazy-loaded on rate limit events, not at startup.
Suggested fix:
Either restore the `loadClaudeProfiles()` call in the initial load effect, or remove the `claudeProfiles` dependency from the onboarding wizard check and find an alternative way to detect OAuth auth (e.g., via the usage snapshot or a dedicated IPC call).
This review was generated by Auto Claude.
- Restore loadClaudeProfiles() call in App.tsx initial load useEffect to fix onboarding detection for OAuth-only users - Add trailing newline to .gitignore per POSIX convention Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AndyMik90
deleted the
auto-claude/181-add-expand-button-for-long-task-descriptions
branch
2 participants
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Adds expand/collapse functionality for long task descriptions in the task detail modal. Task descriptions exceeding ~200px (8 lines) are now truncated by default with a "Show more" button at the bottom center. Users can click to reveal the full content, with a gradient overlay indicating additional content below.
Related Issue
Closes #181
Type of Change
Area
Commit Message Format
Follow conventional commits:
<type>: <subject>Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemAI Disclosure
Tool(s) used: Claude Code
Testing level:
Untested -- AI output not yet verified
Lightly tested -- ran the app / spot-checked key paths
Fully tested -- all tests pass, manually verified behavior
I understand what this PR does and how the underlying code works
Checklist
developbranchPlatform Testing Checklist
CRITICAL: This project supports Windows, macOS, and Linux. Platform-specific bugs are a common source of breakage.
platform/module instead of directprocess.platformchecksfindExecutable()or platform abstractions)If you only have access to one OS: CI now tests on all platforms. Ensure all checks pass before submitting.
CI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: No
Details:
Changes are backward compatible. The expand button appears only when descriptions exceed the truncation threshold, and short descriptions display normally without any changes to existing behavior.
Summary by CodeRabbit
Release Notes
New Features
Changes
Localization