feat: Job式からsession式に変更#7
Conversation
There was a problem hiding this comment.
Pull request overview
Job 1回ごとに完結していた実行モデルを、Session → Iteration → Proposal の対話モデルへ置き換え、成果物管理を hostPath/stdout 埋め込みから S3/MinIO に移行するPRです。フロントもセッション単位の管理・タイムライン表示に更新されています。
Changes:
- ドメイン/DB/APIを Job 中心から Session/Iteration 中心に刷新（jobs API削除、sessions API追加）
- ワーカー成果物（before/after/patch/plan/pr_url）を S3/MinIO に集約し、累積パッチ（base からの単一 format-patch）方式へ変更
- フロントを Session UI（詳細ページ、サイドバー、Dashboard導線、Polling）に更新
Reviewed changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/setup-kind.sh | kind セットアップスクリプト削除（hostPath 前提のローカル運用撤去） |
| k8s/kind-config.yaml | kind クラスタ設定削除（hostPath/artifacts マウント前提撤去） |
| frontend/src/services/api.ts | sessions API（create/list/get/iterate/create-pr/diff）に更新 |
| frontend/src/pages/SessionDetail.tsx | セッション詳細 + イテレーションタイムライン + 継続指示/PR作成 UI 追加 |
| frontend/src/pages/JobDetail.tsx | Job 詳細 UI 削除 |
| frontend/src/pages/Dashboard.tsx | Job 作成導線を Session 開始へ変更 |
| frontend/src/main.tsx | ルーティングを jobs/:jobId → sessions/:sessionId に変更 |
| frontend/src/hooks/useSessionPolling.ts | Session のポーリング hook 新規追加 |
| frontend/src/hooks/useLayoutContext.ts | Layout context を refreshSessions に変更 |
| frontend/src/hooks/useJobPolling.ts | Job のポーリング hook 削除 |
| frontend/src/components/Sidebar.tsx | サイドバーを jobs → sessions 表示に更新 |
| frontend/src/components/Layout.tsx | listJobs → listSessions に更新、Outlet context 更新 |
| docker/worker.Dockerfile | boto3 追加（S3アクセス） |
| docker/worker-implement.py | 実装ワーカーを S3成果物/累積パッチ方式に移行（push廃止） |
| docker/worker-entrypoint.sh | ログ表示を Session/Iteration に更新 |
| docker/worker-createpr.py | PR作成ワーカーを S3パッチ取得方式に移行（このワーカーのみ push） |
| docker/worker-analyze.py | 解析ワーカーを S3成果物方式に移行、提案JSON抽出を堅牢化 |
| backend/uv.lock | boto3/botocore/jmespath/s3transfer 追加 |
| backend/tests/test_repository.py | Job系テストを Session/Iteration/Proposal リポジトリテストへ更新 |
| backend/tests/test_api.py | jobs API テストを sessions API テストへ更新、artifact service 前提削除 |
| backend/tests/conftest.py | テスト用モデル登録を session モデルへ更新 |
| backend/pyproject.toml | boto3 依存追加、mypy ignore 追加 |
| backend/migration/versions/c3d4e5f6g7h8_add_session_iteration_tables.py | sessions/iterations/proposals テーブル作成、旧 jobs/proposals 削除 |
| backend/migration/versions/b2c3d4e5f6g7_add_job_chaining_columns.py | 旧 job chaining 追加マイグレーション（履歴として追加） |
| backend/migration/env.py | Alembic のモデル参照を session モデルへ更新 |
| backend/docker-compose.yml | MinIO サービス追加（ローカルS3互換） |
| backend/app/workflow/state.py | workflow state を session ベースに改名/拡張（S3 keys 前提） |
| backend/app/workflow/session_implementation_graph.py | session 実装グラフ新規追加（S3 読み書き） |
| backend/app/workflow/session_create_pr_graph.py | PR作成グラフを S3 取得方式に更新 |
| backend/app/workflow/session_analyzer_graph.py | 解析グラフを S3 取得方式に更新 |
| backend/app/workflow/implementation_graph.py | 旧 job 実装グラフ削除 |
| backend/app/workflow/init.py | session グラフの export に更新 |
| backend/app/usecase/session_usecase.py | session 作成/継続/PR作成 + バックグラウンド実行/復旧処理を新規実装 |
| backend/app/usecase/job_usecase.py | 旧 job usecase 削除 |
| backend/app/service/s3_service.py | S3/MinIO 成果物サービス新規追加 |
| backend/app/service/k8s_service.py | session ワーカー Job 作成API追加（S3 env vars 付与） |
| backend/app/schema/setting_schema.py | settings schema を分離して新規追加 |
| backend/app/schema/session_schema.py | session/iteration/proposal の request/response schema 追加 |
| backend/app/schema/init.py | schema export を sessions/settings に更新 |
| backend/app/router/settings.py | settings schema import を新ファイルへ更新 |
| backend/app/router/sessions.py | sessions API 追加（iterate/create-pr/screenshot/diff含む） |
| backend/app/router/jobs.py | 旧 jobs API 削除 |
| backend/app/repository/setting_repository.py | Setting import を session モデルへ更新 |
| backend/app/repository/session_repository.py | SessionRepository 新規追加 |
| backend/app/repository/proposal_repository.py | Proposal を iteration 紐付け + 楽観ロック update へ変更 |
| backend/app/repository/job_repository.py | 旧 JobRepository 削除 |
| backend/app/repository/iteration_repository.py | IterationRepository 新規追加（楽観ロック update 含む） |
| backend/app/repository/init.py | repository exports を sessions/iterations ベースに更新 |
| backend/app/model/session.py | Session/Iteration/Proposal/Setting モデル新規追加 |
| backend/app/model/job.py | 旧 Job/Proposal/Setting モデル削除 |
| backend/app/model/init.py | model exports を session ベースに更新 |
| backend/app/main.py | sessions router 追加 + 起動時 stuck proposal 復旧を lifespan に追加 |
| backend/app/di/dependencies.py | artifact service を廃止し S3 service DI に更新 |
| backend/app/di/container.py | job/proposal repo DI 削除、setting のみ維持 |
| backend/app/core/config.py | S3/MinIO 設定追加（endpoint/credentials/bucket 等） |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except Exception: | ||
| logger.exception("Session %s iter %d analysis failed", session_id, iteration_index) | ||
| try: | ||
| iteration = iter_repo.get_by_id(UUID(iteration_id)) | ||
| if iteration: | ||
| iter_repo.update_status_optimistic( | ||
| UUID(iteration_id), | ||
| iteration.version, | ||
| IterationStatus.FAILED, |
There was a problem hiding this comment.
In _run_session_analysis, the except Exception block references iter_repo even though it’s defined inside the try. If an exception is raised before iter_repo = IterationRepository(db) executes (or before it’s assigned), this will raise UnboundLocalError and skip the intended status update. Initializing iter_repo to None before the try (and guarding its use) or moving repository creation outside the try would make the error handling reliable.
| except Exception: | ||
| logger.exception( | ||
| "Session %s iter %d proposal %d implementation failed", | ||
| session_id, | ||
| iteration_index, | ||
| proposal_index, | ||
| ) | ||
| try: | ||
| proposal = proposal_repo.get_by_id(UUID(proposal_id)) | ||
| if proposal: | ||
| proposal_repo.update_status_optimistic( | ||
| UUID(proposal_id), | ||
| proposal.version, | ||
| ProposalStatus.FAILED, | ||
| error_message="Internal error during implementation", | ||
| ) | ||
| _check_iteration_completion(db, UUID(iteration_id)) |
There was a problem hiding this comment.
In _run_session_implementation, the except Exception handler uses proposal_repo (and proposal_repo.get_by_id(...)) but proposal_repo is created inside the try. If an exception occurs before proposal_repo is assigned, the handler will raise UnboundLocalError and the proposal/iteration won’t be marked failed. Consider defining proposal_repo outside the try (or setting it to None and guarding) so the fallback status update always works.
| except Exception: | ||
| logger.exception("PR creation failed for session %s", session_id) | ||
| try: | ||
| proposal = proposal_repo.get_by_id(UUID(proposal_id)) | ||
| if proposal: | ||
| proposal_repo.update_status_optimistic( | ||
| UUID(proposal_id), | ||
| proposal.version, | ||
| ProposalStatus.COMPLETED, | ||
| pr_status="failed", | ||
| error_message="Internal error during PR creation", | ||
| ) |
There was a problem hiding this comment.
In _run_session_create_pr, the exception handler references proposal_repo even though it’s defined inside the try. If an exception happens before proposal_repo = ProposalRepository(db) runs, the handler will raise UnboundLocalError and the proposal’s pr_status won’t be updated to failed. Initializing proposal_repo before the try (and guarding usage) would make this robust.
| # Generate format-patch (cumulative: base_branch → HEAD) | ||
| patch_result = subprocess.run( | ||
| ["git", "format-patch", "-1", "HEAD", "--stdout"], | ||
| capture_output=True, text=True, cwd=repo_dir, | ||
| ) | ||
| with open(f"{artifact_dir}/changes.diff", "w") as f: | ||
| local_diff = f"{tmp_dir}/changes.diff" | ||
| with open(local_diff, "w") as f: | ||
| f.write(patch_result.stdout) |
There was a problem hiding this comment.
patch_result = subprocess.run([... 'git', 'format-patch' ...]) isn’t checked for success; if git format-patch fails, the code will still upload an empty/partial changes.diff to S3 and mark the proposal as completed upstream. Consider using check=True (or explicitly validating returncode/stdout) and failing the worker if patch generation fails.
| # Drop legacy tables (proposals has FK to jobs, drop first) | ||
| op.drop_table("proposals") | ||
| op.drop_table("jobs") | ||
| op.execute("DROP TYPE IF EXISTS proposalstatus") | ||
| op.execute("DROP TYPE IF EXISTS jobstatus") |
There was a problem hiding this comment.
This migration drops the legacy proposals/jobs tables (and enum types) without any data migration/backfill. If this project has existing production data, upgrading will irreversibly delete it. If data retention matters, consider migrating existing rows into the new sessions/iterations schema (or at least renaming old tables and providing a one-off export path) rather than unconditional drop_table.
| healthcheck: | ||
| test: ["CMD", "mc", "ready", "local"] | ||
| interval: 10s | ||
| timeout: 5s | ||
| retries: 5 | ||
| start_period: 10s |
There was a problem hiding this comment.
MinIO healthcheck uses mc ready local, but the minio/minio image typically doesn’t ship with the mc client and there’s no local alias configured, so this healthcheck is likely to fail and block app startup (depends_on: condition=service_healthy). Consider switching to MinIO’s built-in readiness endpoint (e.g. an HTTP GET to /minio/health/ready) or adding an explicit minio/mc sidecar/initialization step if you want to use mc.
| except ClientError: | ||
| return False |
There was a problem hiding this comment.
S3Service.exists() currently returns False for any ClientError from head_object, which can silently mask non-404 errors (e.g. AccessDenied, invalid credentials, expired token) and lead to incorrect control flow (e.g. treating real outages/permission issues as “file not found”). It would be safer to only return False for “not found” error codes and re-raise (or at least log) other errors.
| except ClientError: | |
| return False | |
| except ClientError as e: | |
| error_code = e.response.get("Error", {}).get("Code") | |
| # Only treat explicit "not found" errors as absence; surface all others. | |
| if error_code in ("NoSuchKey", "NotFound", "404"): | |
| return False | |
| logger.exception( | |
| "Error checking existence of S3 object %s in bucket %s (code=%s)", | |
| key, | |
| self.bucket, | |
| error_code, | |
| ) | |
| raise |
| def list_all(self) -> list[Session]: | ||
| return self.db.query(Session).order_by(Session.created_at.desc()).all() |
There was a problem hiding this comment.
list_all() returns Session rows without eager-loading iterations/proposals, but the API layer serializes session.iterations and iteration.proposals, which will trigger additional queries per session/iteration (N+1). Consider using selectinload/joinedload in this query so GET /api/sessions/ stays efficient as data grows.
| # Install Claude Agent SDK | ||
| RUN pip install --no-cache-dir claude-agent-sdk | ||
| # Install Claude Agent SDK and boto3 (S3 access) | ||
| RUN pip install --no-cache-dir claude-agent-sdk boto3 |
There was a problem hiding this comment.
The RUN pip install --no-cache-dir claude-agent-sdk boto3 line installs third-party Python packages from PyPI without pinning them to immutable versions or verifying integrity, which exposes the worker image build to supply-chain attacks if those packages or the PyPI index are compromised. Because this container will run with access to repository contents and likely secrets (API tokens, credentials), a malicious update to claude-agent-sdk or boto3 would lead to remote code execution in the worker environment. To mitigate this, pin these dependencies to specific versions (or use a locked requirements file with hashes) and keep them updated via a controlled process.
毎回新規Jobを投げていたが、連続的な修正を行いたい場合に親Job->子Job->孫Jobとユーザーから見たドメインとシステムにズレがあった。したがって以下の修正を行った。
changes.diffはbase_branchからの全変更を1パッチに