Orchestrator 終了時に orchestrator.pid が削除されない問題を修正#516
Merged
clonable-eden merged 3 commits intomainfrom Mar 23, 2026
Merged
Conversation
… (RED) Tests verify that orchctl gc detects and removes stale orchestrator.pid (dead process), preserves live orchestrator.pid, and also cleans orchestrator.spawned and repo metadata files. Includes dry-run support and empty session directory cleanup after orchestrator metadata removal. closes #513 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When the orchestrator process is dead, orchctl gc now removes orchestrator.pid, orchestrator.spawned, and repo metadata files from the session directory. Supports --dry-run. After cleanup, empty session directories are removed by the existing rmdir step. closes #513 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…REFACTOR) Rewrite the Completion section with MANDATORY callout, checklist format, and explicit warning about stale not-running entries. Mention orchctl gc as a safety net fallback but emphasize the Orchestrator's responsibility. closes #513 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
clonable-eden
commented
Mar 23, 2026
Owner
Author
clonable-eden
left a comment
clonable-eden
left a comment
There was a problem hiding this comment.
Review: APPROVE ✅
Issue #513 の2つの対策が正しく実装されています。
Correctness
orchctl gcのセーフティネット: 死んだ Orchestrator のorchestrator.pid、orchestrator.spawned、repoファイルを正しく検出・削除。kill -0によるプロセス生存チェックは適切。orchestrator.mdの改善: Completion セクションが MANDATORY チェックリスト形式になり、PID cleanup の見落としを防止。「safety net に頼るな」の注記も良い。- step 番号の renumbering (3→4→5) も正しい。
Conventions
cleaned=$((cleaned + 1))パターン使用 (bash 3.2 安全) ✓tr -d '[:space:]'で PID のホワイトスペース対策 ✓--dry-run対応済み ✓
Tests
6 テスト追加、全46件パス (CI green):
- 死んだ PID 削除 / 生きた PID 保持
- orchestrator.spawned, repo の連動削除
- dry-run での非削除確認
- 空セッションディレクトリの自動削除
エッジケースも十分にカバーされています。
Design
- Rule of Repair: stale state を検出してノイジーに除去
- Rule of Robustness: orchestrator 側の cleanup + gc のセーフティネットの二重防御
- Rule of Modularity: gc の既存ステップ構造に自然に統合
LGTM 🚀
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #513
Summary
orchctl gcにセーフティネットとして、死んだ Orchestrator のorchestrator.pid、orchestrator.spawned、repoファイルを自動削除する機能を追加orchestrator.mdの Completion セクションを MANDATORY チェックリスト形式に改善し、PID ファイル削除の見落としを防止--dry-run対応済み。空になったセッションディレクトリも自動削除Test Plan
🤖 Generated with Claude Code