Skip to content

Add Claude Code GitHub Workflow#1

Merged
terisuke merged 2 commits into001-proedit-mvp-browserfrom
add-claude-github-actions-1760421745921
Oct 14, 2025
Merged

Add Claude Code GitHub Workflow#1
terisuke merged 2 commits into001-proedit-mvp-browserfrom
add-claude-github-actions-1760421745921

Conversation

@terisuke
Copy link
Copy Markdown
Contributor

@terisuke terisuke commented Oct 14, 2025

🤖 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!

@terisuke terisuke merged commit f21a551 into 001-proedit-mvp-browser Oct 14, 2025
1 check passed
terisuke added a commit that referenced this pull request Oct 14, 2025
@terisuke @claude
fix: Apply 2 critical fixes before MVP deployment (Code Review)
2db15ac 
This commit addresses 2 Must Fix (Before Merge) issues identified in the
comprehensive code review to ensure production stability and security.

## Critical Fixes Applied:

### 1. Memory Leak Prevention (Compositor.ts) 🔴
**Issue**: PIXI.js resources were not properly cleaned up, causing memory
leaks during long editing sessions.

**Fix** (features/compositor/utils/Compositor.ts:338-384):
- Enhanced destroy() method with explicit resource disposal
- Remove all effects from stage before manager cleanup
- Call removeFromStage() for each effect type (video, image, text)
- Clear currentlyPlayedEffects map before destroying managers
- Destroy managers in correct order (media first, then text)
- Ensure all textures and sprites are released before PIXI cleanup

**Impact**:
- Prevents browser crashes during extended editing sessions
- Reduces memory footprint by properly releasing PIXI resources
- Improves stability for production users

### 2. Properties Validation (projects.ts) 🔴
**Issue**: Effect properties were stored in database without validation,
creating a security risk for malicious data injection.

**Fix** (app/actions/projects.ts:4, 255-260, 272):
- Import validateEffectProperties from effect-schemas
- Validate properties based on effect kind (video/audio/image/text)
- Use comprehensive Zod schemas for type-safe validation
- Prevent malicious data from being stored in database

**Code**:
```typescript
// CR-FIX: Validate properties based on effect kind
const validatedProperties = validateEffectProperties(
  validated.kind,
  effectData.properties || {}
);

return {
  // ...
  properties: validatedProperties as Record<string, unknown>,
};
```

**Impact**:
- Closes security vulnerability for data injection
- Ensures all stored properties conform to expected schema
- Validates fonts, colors, dimensions, and all effect parameters
- Protects database integrity

## Testing:
- ✅ TypeScript compilation passes (0 errors)
- ✅ Supabase integration verified (local + remote)
- ✅ All CRUD operations tested successfully
- ✅ Memory leak prevention verified through code review
- ✅ Properties validation tested with Zod schemas

## Code Review Status:
- 🔴 Must Fix #1: Memory Leaks → ✅ FIXED
- 🔴 Must Fix #2: Properties Validation → ✅ FIXED
- 🟡 Should Fix: To be addressed in follow-up PRs

## Ready for MVP Deployment:
All critical blockers resolved. Application is now stable and secure
for production deployment.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude claude bot mentioned this pull request Oct 14, 2025
Merged
5 tasks
terisuke added a commit that referenced this pull request Oct 14, 2025
@terisuke
fix: コードレビュー#2対応 - timecode同期とパフォーマンス改善
d74dcc1 
🔴 Critical: timecode同期の問題を完全解決

**問題**:
1. 2つの異なるtimecode値を使用（compositorから取得 vs state）
2. playback loop中、毎フレームuseEffectが実行される無駄

**修正**:
- timecodeRefで最新のstate timecodeを追跡
- effects変更時のみcomposeEffectsを実行
- timecodeを依存配列から除外
- playback loopは内部でtimecodeを更新（callback経由）

**効果**:
✅ timecode同期の一貫性確保
✅ 無限ループ完全防止
✅ パフォーマンス改善（不要なcomposeEffects呼び出しなし）

## コードレビュー対応
- ✅ Critical #1: timecode同期の問題
- ✅ Performance #6: 不要なcomposeEffects呼び出し削減
terisuke added a commit that referenced this pull request Oct 14, 2025
@terisuke
docs: PIXI.js v8をv7.4.2に統一（実装に合わせて修正）
1f6cc2b 
🔴 BLOCKER: PIXI.js Version Mismatch解消

**問題**:
- package.jsonではv7.4.2を使用
- ドキュメントではv8と記載
- コードレビューで何度も指摘を受ける

**修正**:
- docs/CLAUDE.md: v8 → v7.4.2に修正
- VideoManager.ts: コメントをv7に修正
- TextManager.ts: コメントをv7に修正

**理由**:
実装はv7.4.2で安定動作しており、v7特有の問題
（cancelResizeエラー等）に対応済み。
将来v8にアップグレードする際は、別PRで対応。

## コードレビュー対応
- ✅ Critical #1: PIXI.js Version Mismatch解消
- ✅ Documentation整合性確保
terisuke added a commit that referenced this pull request Oct 14, 2025
@terisuke
fix: recomposeIfNeeded()にエラーハンドリング追加
1f41ff9 
🔴 P0: 非同期エラーの無視を防止

**問題**: void this.recomposeIfNeeded()により、
async例外が無視されてplayback loopがクラッシュする可能性

**修正**:
- try-catchでcomposeEffects()の例外をキャッチ
- エラーをログに記録
- playback loopは継続（クラッシュしない）

**効果**:
✅ 例外が発生してもplayback継続
✅ デバッグ情報をログに記録
✅ ユーザー体験が保護される

## PRレビュー対応
- ✅ Critical #1: Race condition対策
- ✅ P0: 非同期エラーハンドリング
This was referenced Oct 15, 2025
Merged
Merged
terisuke added a commit that referenced this pull request Oct 18, 2025
@terisuke
feat(export): implement async job queue and proxy improvements
4a65da7 
Address PR review feedback with production-ready enhancements:

## Core Improvements

### 1. Async Job Processing (Critical Fix #1)
- Add background job queue with concurrency controls (lib/export/queue.ts)
- Implement global (default: 2) and per-user (default: 1) export limits
- Queue state persists across requests using globalThis
- API returns jobId immediately, client polls for status

### 2. Proxy Generation Race Condition Fix (Critical Fix #2)
- Implement atomic DB lock with conditional update (lib/media/proxy.ts:169-192)
- Use .neq('proxy_status', 'processing') to prevent duplicate jobs
- Add retry logic with configurable max attempts (default: 3)
- Support force regeneration via options parameter

### 3. Resource Management
- Stream-based file uploads to prevent OOM (lib/export/server.ts:335-361)
- Proper cleanup of concat files in finally block (lib/export/server.ts:113-126)
- Service role client for background operations (lib/supabase/admin.ts)

### 4. Data Validation & Security
- Sanitize export filenames with length limit (lib/export/server.ts:157-167)
- Extend signed URL expiration to 24h (lib/export/server.ts:15)
- Add user_id to export_jobs with RLS policies (007_export_job_enhancements.sql)
- Rate limiting: return 429 when limits exceeded

### 5. API Enhancements
- New polling endpoint: GET /api/render/[jobId] (app/api/render/[jobId]/route.ts)
- POST /api/render now enqueues jobs instead of blocking
- Fix Next.js 15 async params compatibility
- Export progress tracking with real-time updates

### 6. Configuration
- Externalize magic numbers as env variables:
  - PROXY_TARGET_WIDTH/HEIGHT (default: 1280x720)
  - PROXY_MAX_ATTEMPTS (default: 3)
  - EXPORT_MAX_CONCURRENT (default: 2)
  - EXPORT_MAX_PER_USER (default: 1)
  - EXPORT_SIGNED_URL_TTL (default: 86400)
  - EXPORT_AUDIO_BITRATE (default: 192000)
  - EXPORT_MAX_FILENAME_LENGTH (default: 80)

Tests:
- npm run type-check ✓
- Supabase migration 007 applied ✓

Migration Status:
- 006_proxy_workflow.sql: Applied ✓
- 007_export_job_enhancements.sql: Applied ✓

Next Steps:
1. Set SUPABASE_SERVICE_ROLE_KEY in Vercel environment
2. Test export queue under concurrent load
3. Monitor FFmpeg resource usage in production
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@terisuke