Skip to content

Add Claude Code GitHub Workflow - #1

Merged
frankbria merged 2 commits into
mainfrom
add-claude-github-actions-1760588728529
Oct 16, 2025
Merged

Add Claude Code GitHub Workflow#1
frankbria merged 2 commits into
mainfrom
add-claude-github-actions-1760588728529

Conversation

@frankbria

@frankbria frankbria commented Oct 16, 2025

Copy link
Copy Markdown
Owner

🤖 Installing Claude Code GitHub App

This PR adds a GitHub Actions workflow that enables Claude Code integration in our repository.

What is Claude Code?

Claude Code is an AI coding agent that can help with:

  • Bug fixes and improvements
  • Documentation updates
  • Implementing new features
  • Code reviews and suggestions
  • Writing tests
  • And more!

How it works

Once this PR is merged, we'll be able to interact with Claude by mentioning @claude in a pull request or issue comment.
Once the workflow is triggered, Claude will analyze the comment and surrounding context, and execute on the request in a GitHub action.

Important Notes

  • This workflow won't take effect until this PR is merged
  • @claude mentions won't work until after the merge is complete
  • The workflow runs automatically whenever Claude is mentioned in PR or issue comments
  • Claude gets access to the entire PR or issue context including files, diffs, and previous comments

Security

  • Our Anthropic API key is securely stored as a GitHub Actions secret
  • Only users with write access to the repository can trigger the workflow
  • All Claude runs are stored in the GitHub Actions run history
  • Claude's default tools are limited to reading/writing files and interacting with our repo by creating comments, branches, and commits.
  • We can add more allowed tools by adding them to the workflow file like:
allowed_tools: Bash(npm install),Bash(npm run build),Bash(npm run lint),Bash(npm run test)

There's more information in the Claude Code action repo.

After merging this PR, let's try mentioning @claude in a comment on any PR to get started!

Summary by CodeRabbit

  • Chores
    • Introduced automated AI-powered code reviews on pull requests.
    • Enabled on-demand AI assistance triggered by mentions in issues, PR comments, and reviews.
    • Configured workflows to post review feedback directly on PRs and issues.

@coderabbitai

coderabbitai Bot commented Oct 16, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds two GitHub Actions workflows to integrate Claude-driven automation: one runs code reviews on pull_request events; the other responds to mentions and comments. Both check out the repo and invoke anthropics/claude-code-action@v1 with OAuth token and scoped permissions.

Changes

Cohort / File(s) Summary
GitHub Actions: Claude automation
\.github/workflows/claude\-code\-review\.yml, \.github/workflows/claude\.yml
Introduces two workflows: (1) pull_request-triggered automated code review with configured prompt, tools, and permissions; (2) event-driven Claude invocation on issue/PR comments, reviews, and issues mentioning @claude, with repository checkout and action execution using secrets-based OAuth token.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant GH as GitHub (PR Events)
  participant Runner as Actions Runner
  participant Repo as Repo Checkout
  participant ClaudeAct as anthropics/claude-code-action@v1
  participant GHAPI as GitHub API

  Dev->>GH: Open/Synchronize Pull Request
  GH->>Runner: Trigger workflow (claude-code-review.yml)
  Runner->>Repo: actions/checkout@v4 (fetch-depth: 1)
  Runner->>ClaudeAct: Run with OAuth token + prompt
  ClaudeAct->>GHAPI: Read PR diff/metadata
  ClaudeAct->>GHAPI: Post review comment via gh pr comment
Loading
sequenceDiagram
  autonumber
  actor User as User
  participant GH as GitHub (Comment/Review Events)
  participant Runner as Actions Runner
  participant Repo as Repo Checkout
  participant ClaudeAct as anthropics/claude-code-action@v1
  participant GHAPI as GitHub API

  User->>GH: Comment/Review/@claude mention
  GH->>Runner: Trigger workflow (claude.yml)
  Runner->>Repo: actions/checkout@v4 (fetch-depth: 1)
  Runner->>ClaudeAct: Run with OAuth token
  ClaudeAct->>GHAPI: Read context (issue/PR)
  ClaudeAct->>GHAPI: Respond in thread (comment)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paws at CI’s new tune,
Two workflows hop beneath the moon—
Mentions, PRs, I swiftly view,
Leave tidy notes, then bid adieu.
With tokens snug and steps in queue,
I nibble diffs—review, review! 🐇✨

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-claude-github-actions-1760588728529

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e6f5e15 and 9d9e699.

📒 Files selected for processing (2)
  • .github/workflows/claude-code-review.yml (1 hunks)
  • .github/workflows/claude.yml (1 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@frankbria
frankbria merged commit b171d4c into main Oct 16, 2025
2 of 3 checks passed
@frankbria
frankbria deleted the add-claude-github-actions-1760588728529 branch October 16, 2025 04:26
frankbria added a commit that referenced this pull request Oct 28, 2025
…urity

This commit addresses the major issues raised in the PR review:

Issue #1: Convert always-on table drop to one-time migration
- Created migration_002_refactor_projects_schema.py
- Removed DROP TABLE from _create_schema() - now only runs once via migration
- Migration properly checks if already applied before running
- Added rollback capability

Issue #2: Add comprehensive error handling to WorkspaceManager
- Added try-except blocks with cleanup on failure in create_workspace()
- Enhanced _init_empty() with timeout and detailed error messages
- Enhanced _init_from_git() with:
  * Input validation (git_url required)
  * 5-minute timeout for large repos
  * Specific error messages for network, repo not found, branch not found, auth failures
  * Shallow clone (--depth 1) for faster operations
- Enhanced _init_from_local() with permission and existence checks
- All subprocess calls now have timeouts and proper exception handling

Issue #3: Add path validation and security for LOCAL_PATH
- Added _is_safe_path() method to validate file system access
- Only allows paths under user's $HOME directory
- Prevents access to /etc/passwd, system files, other users' files
- Checks: path existence, is directory, readable, path traversal protection
- Added symlinks=False to shutil.copytree for security

All tests pass (24/24):
- test_workspace_manager.py: 3/3 ✓
- test_agent_factory.py: 21/21 ✓

Changes prioritize major issues per reviewer guidance, not nitpicks.
frankbria added a commit that referenced this pull request Nov 20, 2025
Fixed critical runtime issues in server.py (lines 586-718):

1. WebSocket Manager Reference (Issue #1)
   - BEFORE: Used app.state.websocket_manager (not initialized)
   - AFTER: Use module-level global 'manager' (line 155)
   - Fixed lines: 680, 691, 698

2. API Key Validation (Issue #2)
   - BEFORE: No validation before LeadAgent creation
   - AFTER: Validate ANTHROPIC_API_KEY and raise HTTPException(500) if missing
   - Added lines: 629-635
   - Error: "ANTHROPIC_API_KEY environment variable is not set"

3. Status Key Access Adaptation (Issue #3)
   - BEFORE: Direct key access (status["is_complete"], etc.)
   - AFTER: Computed from LeadAgent.get_discovery_status() format
   - Computed values (lines 667-674):
     * is_complete = status.get("state") == "completed"
     * total_questions = status.get("total_required", 0)
     * current_question_index = answered_count
     * current_question_id = status.get("current_question", {}).get("id", "")
     * current_question_text = status.get("current_question", {}).get("question", "")

4. Import Organization (Issue #4)
   - BEFORE: In-function import (line 608)
   - AFTER: Module-scope import (lines 21-22)
   - Moved DiscoveryAnswer, DiscoveryAnswerResponse to top

Error Handling:
- Added HTTPException re-raise to preserve 400 errors
- API key validation returns 500 with clear message
- All WebSocket broadcast errors are non-fatal (logged warnings)

Impact:
- Prevents runtime AttributeError on app.state.websocket_manager
- Prevents runtime TypeError on missing API key
- Prevents runtime KeyError on status dict access
- Cleaner import organization

Related: PR #25
frankbria added a commit that referenced this pull request Nov 22, 2025
frankbria added a commit that referenced this pull request Nov 22, 2025
…urity

This commit addresses the major issues raised in the PR review:

Issue #1: Convert always-on table drop to one-time migration
- Created migration_002_refactor_projects_schema.py
- Removed DROP TABLE from _create_schema() - now only runs once via migration
- Migration properly checks if already applied before running
- Added rollback capability

Issue #2: Add comprehensive error handling to WorkspaceManager
- Added try-except blocks with cleanup on failure in create_workspace()
- Enhanced _init_empty() with timeout and detailed error messages
- Enhanced _init_from_git() with:
  * Input validation (git_url required)
  * 5-minute timeout for large repos
  * Specific error messages for network, repo not found, branch not found, auth failures
  * Shallow clone (--depth 1) for faster operations
- Enhanced _init_from_local() with permission and existence checks
- All subprocess calls now have timeouts and proper exception handling

Issue #3: Add path validation and security for LOCAL_PATH
- Added _is_safe_path() method to validate file system access
- Only allows paths under user's $HOME directory
- Prevents access to /etc/passwd, system files, other users' files
- Checks: path existence, is directory, readable, path traversal protection
- Added symlinks=False to shutil.copytree for security

All tests pass (24/24):
- test_workspace_manager.py: 3/3 ✓
- test_agent_factory.py: 21/21 ✓

Changes prioritize major issues per reviewer guidance, not nitpicks.
frankbria added a commit that referenced this pull request Nov 22, 2025
Fixed critical runtime issues in server.py (lines 586-718):

1. WebSocket Manager Reference (Issue #1)
   - BEFORE: Used app.state.websocket_manager (not initialized)
   - AFTER: Use module-level global 'manager' (line 155)
   - Fixed lines: 680, 691, 698

2. API Key Validation (Issue #2)
   - BEFORE: No validation before LeadAgent creation
   - AFTER: Validate ANTHROPIC_API_KEY and raise HTTPException(500) if missing
   - Added lines: 629-635
   - Error: "ANTHROPIC_API_KEY environment variable is not set"

3. Status Key Access Adaptation (Issue #3)
   - BEFORE: Direct key access (status["is_complete"], etc.)
   - AFTER: Computed from LeadAgent.get_discovery_status() format
   - Computed values (lines 667-674):
     * is_complete = status.get("state") == "completed"
     * total_questions = status.get("total_required", 0)
     * current_question_index = answered_count
     * current_question_id = status.get("current_question", {}).get("id", "")
     * current_question_text = status.get("current_question", {}).get("question", "")

4. Import Organization (Issue #4)
   - BEFORE: In-function import (line 608)
   - AFTER: Module-scope import (lines 21-22)
   - Moved DiscoveryAnswer, DiscoveryAnswerResponse to top

Error Handling:
- Added HTTPException re-raise to preserve 400 errors
- API key validation returns 500 with clear message
- All WebSocket broadcast errors are non-fatal (logged warnings)

Impact:
- Prevents runtime AttributeError on app.state.websocket_manager
- Prevents runtime TypeError on missing API key
- Prevents runtime KeyError on status dict access
- Cleaner import organization

Related: PR #25
frankbria added a commit that referenced this pull request Dec 5, 2025
…ype safety

Addresses 5 issues from code review:

## Issue #1: Performance - Re-render Optimization (High Priority)
- ✅ Extracted FindingCard into separate memoized component
- ✅ Prevents unnecessary re-renders when toggling individual findings
- ✅ Only affected finding card re-renders on state change

## Issue #2: Accessibility Improvements (High Priority)
- ✅ Added semantic button role to clickable divs
- ✅ Implemented keyboard navigation (Enter/Space keys)
- ✅ Added ARIA attributes (aria-expanded, aria-label, aria-hidden)
- ✅ Added focus indicators (focus:ring-2 focus:ring-blue-500)
- ✅ Screen readers announce expansion state and finding details

## Issue #3: Type Safety - ID Collision Prevention (Medium Priority)
- ✅ Changed from `finding.id || 0` to `finding.id ?? index`
- ✅ Uses array index as fallback to prevent ID collisions
- ✅ Ensures unique keys for each finding card

## Issue #5: Error Handling - Defensive Checks (Medium Priority)
- ✅ Added defensive checks for SEVERITY_COLORS lookup
- ✅ Added defensive checks for CATEGORY_ICONS lookup
- ✅ Fallback values prevent crashes from malformed data
- ✅ Default severity: gray, default icon: 📄

## Issue #6: Enhanced Test Coverage (Low Priority)
- ✅ Verify lightbulb icon (💡) presence in recommendations
- ✅ Verify blue background styling (bg-blue-50) applied correctly
- ✅ Improved test assertions for recommendation display

## Issue #4: Not Applicable
- TaskTreeView.tsx was NOT modified in this PR
- Only ReviewSummary.tsx and test_review_ui.spec.ts changed

## Test Results
All 6 Chromium tests passing (17.4s):
- ✅ should display review findings panel
- ✅ should display severity badges correctly
- ✅ should display review score chart
- ✅ should expand/collapse review finding details
- ✅ should filter findings by severity
- ✅ should display actionable recommendations (enhanced)

## Accessibility Features Added
- role="button" on finding cards
- tabIndex={0} for keyboard focus
- aria-expanded state tracking
- aria-label with finding context
- aria-hidden on decorative icons
- onKeyDown handler for Enter/Space
- focus:ring visual indicator

## Performance Improvements
- React.memo on FindingCard component
- Prevents cascade re-renders on toggle
- Optimized for lists with 100+ findings

## Files Modified
- web-ui/src/components/reviews/ReviewSummary.tsx (+80 lines, refactored)
- tests/e2e/test_review_ui.spec.ts (+9 lines, enhanced assertions)
frankbria added a commit that referenced this pull request Dec 5, 2025
…endations (#52)

* feat: Add inline dependency rendering to TaskTreeView

- Replace hover tooltip with inline "Depends on: task-1, task-3" text
- Remove .skip from two dependency tests (lines 248, 382)
- Update dependency count test to match new format
- Simplify implementation from 40 lines to 6 lines

Fixes #42

Test Results:
- All 38 TaskTreeView tests pass
- Full suite: 1096 tests pass
- No regressions introduced

Visual Change:
Before: "↳ 1 dependency" (hover for details)
After: "Depends on: task-1, task-3" (inline, immediately visible)

* feat: Implement detailed Review Findings UI with filtering and recommendations

Closes #45

## Changes

### ReviewSummary Component Enhancement
- Added individual findings list with expand/collapse functionality
- Implemented severity filter dropdown (All, Critical, High, Medium, Low, Info)
- Display actionable recommendations with 💡 icon and blue background styling
- Added all required test IDs for E2E testing
- Ensured component always renders findings list container for test consistency

### E2E Test Updates
- Removed .skip decorators from 3 previously failing tests:
  - should expand/collapse review finding details (line 59)
  - should filter findings by severity (line 82)
  - should display actionable recommendations (line 111)

## Features Implemented

1. **Individual Findings List**
   - Each finding displayed as clickable card
   - File path, line number, severity badge, category icon
   - testid: review-findings-list, review-finding-{id}

2. **Expand/Collapse Details**
   - Click to toggle finding details visibility
   - Shows full message, code snippet, file details
   - testid: finding-details

3. **Severity Filtering**
   - Dropdown to filter findings by severity
   - Dynamically filters visible findings
   - testid: severity-filter

4. **Actionable Recommendations**
   - Display recommendation for each finding when available
   - Distinct styling with lightbulb icon
   - testid: finding-recommendation

5. **Severity Badges**
   - Color-coded badges (red/orange/yellow/blue/gray)
   - testid: severity-badge

## Test Results

All 30 E2E tests passing (25.5s):
- Chromium: 6/6 ✅
- Firefox: 6/6 ✅
- WebKit: 6/6 ✅
- Mobile Chrome: 6/6 ✅
- Mobile Safari: 6/6 ✅

## Edge Cases Handled

- Empty review data (null reviewResult)
- No findings after filtering
- Missing recommendations
- File-level findings (no line number)
- Missing code snippets

## Files Modified

- web-ui/src/components/reviews/ReviewSummary.tsx
- tests/e2e/test_review_ui.spec.ts

* fix: Address code review feedback - performance, accessibility, and type safety

Addresses 5 issues from code review:

## Issue #1: Performance - Re-render Optimization (High Priority)
- ✅ Extracted FindingCard into separate memoized component
- ✅ Prevents unnecessary re-renders when toggling individual findings
- ✅ Only affected finding card re-renders on state change

## Issue #2: Accessibility Improvements (High Priority)
- ✅ Added semantic button role to clickable divs
- ✅ Implemented keyboard navigation (Enter/Space keys)
- ✅ Added ARIA attributes (aria-expanded, aria-label, aria-hidden)
- ✅ Added focus indicators (focus:ring-2 focus:ring-blue-500)
- ✅ Screen readers announce expansion state and finding details

## Issue #3: Type Safety - ID Collision Prevention (Medium Priority)
- ✅ Changed from `finding.id || 0` to `finding.id ?? index`
- ✅ Uses array index as fallback to prevent ID collisions
- ✅ Ensures unique keys for each finding card

## Issue #5: Error Handling - Defensive Checks (Medium Priority)
- ✅ Added defensive checks for SEVERITY_COLORS lookup
- ✅ Added defensive checks for CATEGORY_ICONS lookup
- ✅ Fallback values prevent crashes from malformed data
- ✅ Default severity: gray, default icon: 📄

## Issue #6: Enhanced Test Coverage (Low Priority)
- ✅ Verify lightbulb icon (💡) presence in recommendations
- ✅ Verify blue background styling (bg-blue-50) applied correctly
- ✅ Improved test assertions for recommendation display

## Issue #4: Not Applicable
- TaskTreeView.tsx was NOT modified in this PR
- Only ReviewSummary.tsx and test_review_ui.spec.ts changed

## Test Results
All 6 Chromium tests passing (17.4s):
- ✅ should display review findings panel
- ✅ should display severity badges correctly
- ✅ should display review score chart
- ✅ should expand/collapse review finding details
- ✅ should filter findings by severity
- ✅ should display actionable recommendations (enhanced)

## Accessibility Features Added
- role="button" on finding cards
- tabIndex={0} for keyboard focus
- aria-expanded state tracking
- aria-label with finding context
- aria-hidden on decorative icons
- onKeyDown handler for Enter/Space
- focus:ring visual indicator

## Performance Improvements
- React.memo on FindingCard component
- Prevents cascade re-renders on toggle
- Optimized for lists with 100+ findings

## Files Modified
- web-ui/src/components/reviews/ReviewSummary.tsx (+80 lines, refactored)
- tests/e2e/test_review_ui.spec.ts (+9 lines, enhanced assertions)
frankbria added a commit that referenced this pull request Dec 5, 2025
ISSUE #2 - POTENTIAL LOGIC ISSUE (Investigated):
- Backend does not support gates_evaluated field
- Current conservative logic is acceptable:
  * Only marks gate as passed if overall status is passed AND no failures exist
  * Prevents false positives without additional backend support

ISSUE #3 - API ERROR HANDLING (Fixed):
- Add specific error messages based on error type
- Differentiate between 404, network errors, and server errors
- Improves user experience with actionable error messages

ISSUE #4 - MAGIC NUMBERS IN GRID LAYOUT (Fixed):
- Add comment explaining hardcoded grid column count (5)
- Grid layout: 2 cols mobile, 3 cols tablet, 5 cols desktop
- Matches fixed gate count (tests, coverage, type-check, lint, review)

ISSUE #5 - INCONSISTENT NULL HANDLING (Fixed):
- Replace logical OR (||) with nullish coalescing (??)
- Explicitly handles null/undefined vs falsy values
- More semantically correct for optional status field

CHANGES:
- web-ui/src/components/quality-gates/QualityGatesPanel.tsx:
  * Improve error handling with specific messages for 404 and network errors
  * Add comment explaining grid layout column count
- web-ui/src/components/quality-gates/GateStatusIndicator.tsx:
  * Use nullish coalescing (??) instead of logical OR (||) for statusText

TESTING:
- Build passes with no errors
- TypeScript compilation successful
- ESLint passing

NOTES:
- Issue #1 (Missing Unit Tests) tracked in Issue #56
frankbria added a commit that referenced this pull request Dec 5, 2025
ISSUE #1 - LOGIC LIMITATION (Documented):
- Added detailed comment explaining getGateStatus() limitation
- Documents potential false positives when only some gates have run
- Suggests backend enhancement: add gates_evaluated field
- Current workaround assumes if overall status is passed, all gates passed

ISSUE #2 - USEEFFECT CLEANUP (Fixed):
- Add isMounted flag to prevent state updates on unmounted component
- Prevents "Can't perform React state update on unmounted component" warnings
- Cleanup function sets isMounted=false on unmount

ISSUE #4 - INTERFACE DOCUMENTATION (Fixed):
- Add JSDoc comments to QualityGatesPanelProps interface
- Document projectId for API scoping
- Document tasks array filtering behavior

ISSUE #5 - HARDCODED GATE TYPES (Fixed):
- Created ALL_GATE_TYPES_E2E constant in qualityGates.ts
- Export as readonly array with 'as const' for type safety
- Import and use constant in QualityGatesPanel
- Ensures gate types stay in sync across components

CHANGES:
- web-ui/src/components/quality-gates/QualityGatesPanel.tsx:
  * Add TODO comment for gates_evaluated backend enhancement
  * Add isMounted cleanup flag in useEffect
  * Add JSDoc to interface
  * Use ALL_GATE_TYPES_E2E constant
- web-ui/src/types/qualityGates.ts:
  * Export ALL_GATE_TYPES_E2E constant

TESTING:
- Build passes with no errors
- TypeScript compilation successful
- ESLint passing

NOTES:
- Issue #3 (Performance - double rendering) deferred as minor optimization
frankbria pushed a commit that referenced this pull request May 30, 2026
- Replace assert in upsert with RuntimeError so the dict-return contract holds
  under python -O (claude review #1).
- Surface failed DELETE in WorkspaceSelector via console.warn instead of a fully
  silent catch (claude review #3).
- Add NOT NULL to workspaces_registry created_at/last_opened_at (always written;
  brand-new table, no migration impact) (claude review #8).
- Comment the per-entry path_exists stat() tradeoff in the async list handler (#2).
- Clean up confusing makeItem test id default (#7).

Skipped: UUID-in-upsert (#4, required in single-statement INSERT...ON CONFLICT
VALUES), shared column constant (#5, polish), and removing 'void localVersion'
(#6/CodeRabbit nitpick — removal reintroduces the eslint exhaustive-deps warning).
frankbria added a commit that referenced this pull request Jun 16, 2026
#687)

Replace 84 bare print() calls in core/conductor.py and 2 in core/tasks.py with
logger.{info,warning,error,debug} mapped by intent — these leaked to the FastAPI
server's stdout, violating CLAUDE.md headless-core rules (#1, #3). Decorative
separators/leading whitespace stripped. runtime.py:1020/1091 left as-is (print()
inside docstring Example blocks, never executed); events.py left as-is (Rich
console, CLI-intended). CLI UX preserved via events.py milestones + the CLI's own
Rich summary. Adds an AST guard plus a caplog behavioral test; migrates 3
conductor tests from capsys to caplog.

Closes #649.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant