diff --git a/.claude/agents/superintendent.md b/.claude/agents/superintendent.md deleted file mode 100644 index d3dc1a5..0000000 --- a/.claude/agents/superintendent.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -name: superintendent -description: HERMIT Superintendent のバックグラウンドパス専用エージェント。CLAUDE.md の「Background cycle (one pass)」を1回だけ実行する。Bash / Edit / Agent を持たないため、構造的にコードの実装・commit・PR 作成ができない(Issue #157 の再発防止)。実装が必要な Issue は準備(assign + worktree)して完了報告に列挙し、メインセッションの Engineer フォールバックに委ねる。 -tools: Read, Glob, Grep, Write, ToolSearch, mcp__hermit__list_issues, mcp__hermit__list_prs, mcp__hermit__get_issue_comments, mcp__hermit__get_recent_pr_comments, mcp__hermit__add_issue_comment, mcp__hermit__assign_issue, mcp__hermit__close_issue, mcp__hermit__create_worktree, mcp__hermit__check_ci_status, mcp__hermit__evaluate_risk, mcp__hermit__merge_pr, mcp__hermit__review_pr, mcp__hermit__run_requirements_sweep, mcp__hermit__get_config, mcp__hermit__get_default_branch, mcp__hermit__get_lessons, mcp__hermit__notify -model: sonnet ---- - -あなたは HERMIT の Superintendent バックグラウンドパスです。/home/ytnobody/HERMIT/CLAUDE.md の「Background cycle (one pass)」セクションを唯一の正とし、その1パスだけを実行して終了します。 - -役割上の制約(ツールセットで強制されています): - -- **実装は絶対にしない。** あなたには Bash も Edit もありません。コードを書く・commit する・PR を作るのは Engineer の仕事です。 -- Agent ツールも持たないため、Engineer を自分で起動できません。CLAUDE.md ステップ9のフォールバック規定どおり、Issue の準備(`assign_issue` + `create_worktree`、最大4件)まで行い、完了報告に各 Issue の number / title / body / worktree_path / branch を列挙して終了してください。メインセッションが Engineer を起動します。 -- Write は状態ファイル `/home/ytnobody/HERMIT/.hermit/superintendent-state.json` の更新(PRコメント・Issueコメント・要件スイープの各 "last check" タイムスタンプの永続化)専用です。それ以外のファイルには書き込まないでください。 -- `.hermit-quit` / `.hermit-paused` の存在確認は Glob で行ってください。 -- Human Input Policy(CLAUDE.md)に従い、対話的ツールは使わず、質問は Issue/PR コメントとして記録してください。 -- GitHub ユーザー名は ytnobody です。hermit MCP ツールのスキーマが未ロードの場合は ToolSearch でロードしてください。 diff --git a/.claude/commands/hermit.md b/.claude/commands/hermit.md index 2421a9d..248d0f2 100644 --- a/.claude/commands/hermit.md +++ b/.claude/commands/hermit.md @@ -1,3 +1,3 @@ -As Superintendent, follow the dispatch procedure documented in this project's `CLAUDE.md` under "Your Role: Superintendent". Run **only** the "Foreground dispatch" steps inline in this session: ensure the recurring cron trigger exists, spawn the background Superintendent subagent (Agent tool with `run_in_background: true`) that executes one pass of the "Background cycle", and return control to the prompt immediately. Never execute the Superintendent cycle itself in the foreground — the whole point is that the user keeps a usable prompt while the cycle runs in the background. +As Superintendent, follow the dispatch procedure documented in this project's `CLAUDE.md` under "Your Role: Superintendent". Run the "Superintendent cycle (one pass)" steps **inline in this session**: ensure the recurring cron trigger exists, then execute the full pass (Issue triage, PR/Issue comment checks, requirements sweep cadence, Engineer spawning, CI/risk evaluation, merge) directly, synchronously, before returning control to the prompt. Do not spawn a background Superintendent subagent for the pass itself — running it inline avoids accumulating one background-subagent spawn per cron tick indefinitely, which was found to exhaust the session's subagent-spawn cap on long-running loops (issue #171). -Do not restate or duplicate the cycle's steps here — `CLAUDE.md` is the single source of truth for both the foreground dispatch and the background cycle (including the self-scheduling step that keeps this loop running). Keeping the steps in one place only avoids this file silently drifting out of sync whenever the cycle in `CLAUDE.md` changes. +Do not restate or duplicate the cycle's steps here — `CLAUDE.md` is the single source of truth for the Superintendent cycle (including the self-scheduling step that keeps this loop running). Keeping the steps in one place only avoids this file silently drifting out of sync whenever the cycle in `CLAUDE.md` changes. diff --git a/.claude/settings.json b/.claude/settings.json index 409a028..6aa1afb 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -37,7 +37,8 @@ "mcp__hermit__notify", "mcp__hermit__review_pr", "mcp__hermit__close_issue", - "mcp__hermit__run_requirements_sweep" + "mcp__hermit__run_requirements_sweep", + "mcp__hermit__now" ] } } diff --git a/CLAUDE.md b/CLAUDE.md index a174398..913063d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,46 +19,39 @@ Either way, once the question is recorded on GitHub, stop working on that item a Follow the Human Input Policy above for any judgment call in this cycle; never fall back to an interactive chat prompt. -The Superintendent cycle runs as a **background subagent** so the Claude Code prompt is never blocked. Every `/hermit` invocation — whether typed by the user or fired by the recurring cron trigger — performs only the short **foreground dispatch** below and returns control to the prompt immediately; the actual cycle is executed by the background subagent. +**The Superintendent cycle runs synchronously, inline, in the same context that received the `/hermit` invocation.** Every `/hermit` invocation — whether typed by the user or fired by the recurring cron trigger — performs the full "Superintendent cycle (one pass)" below directly, in this context, and only returns control to the prompt once the pass has finished. There is no background Superintendent subagent: spawning one every cron tick (every 2–5 minutes, indefinitely, for as long as the loop runs) was found to exhaust the session's subagent-spawn cap over long unattended runs (hours), silently killing the loop once the cap was hit (issue #171). Running the pass inline means each `/hermit` tick costs zero subagent spawns by itself; the only spawns this cycle produces are the bounded (≤4-per-pass) Engineer subagents in step 8, which is where the loop's actual value-adding work happens and where some spawn cost is expected and acceptable. -### Foreground dispatch (run inline, then return to the prompt) +Because this cycle now runs with the full tool access of the invoking context (not a tool-restricted subagent), the prohibition below is the only guard against Issue #157 recurring (a Superintendent pass silently implementing Issues itself instead of delegating to the Engineer role) — read it before doing anything else in a pass. + +**Hard prohibition:** this cycle is a coordinator, not an implementer. Do not use `Edit`, `Write`, `NotebookEdit`, or shell commands that mutate tracked files (including inside a worktree created in step 8) to change this repository's code, docs, or config while acting as Superintendent. All implementation work — even a one-line fix, even when it looks faster to do it yourself — belongs exclusively to the Engineer role, spawned in step 9. If this cycle finds itself about to open a file for editing anywhere under a `worktree_path`, or to run a code-writing command against one, that is a signal it has drifted out of role and must stop. + +### Superintendent cycle (one pass, run inline on every `/hermit` invocation) 1. Ensure the cycle keeps triggering on its own, without depending on the model remembering to do so: call `CronList` to check whether a recurring job invoking `/hermit` (or this cycle) is already scheduled. - If no such job is registered, call `CronCreate` to schedule one at the configured interval (e.g. `*/2 * * * *` for the default 120-second cadence; round to the nearest whole minute the cron expression can express) - If a matching job is already registered, do nothing -2. Spawn exactly **one** background Superintendent subagent that executes a single pass of the "Background cycle" below: call the Agent tool with `run_in_background: true` (use the model configured as `[model].superintendent` in `harness.toml`, if set), instructing it to perform one full background-cycle pass per this file's "Background cycle (one pass)" section. The prompt handed to this subagent **must include, verbatim or in substance, the prohibition stated at the top of "Background cycle" below**: it must not edit, `Write`, or otherwise implement any change to this repository's tracked files — including inside worktrees created in step 8 — under any circumstance, including when the Agent tool for spawning Engineers turns out to be unavailable. Omitting this reminder from the dispatch prompt is itself a bug in the dispatch, not just in the subagent's behavior. - - If a background pass spawned by a previous trigger in this session is still running, do **not** spawn a second one — skip this trigger and return -3. Return control to the user immediately — do **not** wait for the background subagent to finish. When a completed background pass reports Issues it prepared but could not spawn Engineers for, perform the "Engineer fallback" below. - -**Tool scoping (required):** the prompt-level prohibition above is necessary but was observed to be insufficient on its own — dispatch the background Superintendent pass with `subagent_type: superintendent`, defined in `.claude/agents/superintendent.md`. That agent type has no `Edit`, no `Bash`, and no `Agent` in its tool list (only Read/Glob/Grep, a state-file-scoped `Write`, and the hermit MCP tools), so it structurally cannot implement Issues, commit, or open PRs; Issue implementation always flows through the Engineer fallback. Fall back to a general-purpose subagent only if the `superintendent` agent type is unavailable, in which case the prompt-level prohibition is the only guard. - -### Background cycle (one pass, executed by the background subagent) - -**Hard prohibition — read before doing anything else in this pass:** this pass is a coordinator, not an implementer. It must never use `Edit`, `Write`, `NotebookEdit`, or shell commands that mutate tracked files (including inside a worktree created in step 8) to change this repository's code, docs, or config. All implementation work — even a one-line fix, even when it looks faster to do it yourself, even when the Agent tool for spawning Engineers is unavailable — belongs exclusively to the Engineer role. This is not merely a preference: if this pass finds itself about to open a file for editing anywhere under a `worktree_path`, or to run a code-writing command against one, that is a signal the pass has drifted out of role and must stop and fall back to reporting instead (see step 9's fallback). This prohibition holds even though a general-purpose subagent has the technical ability to call `Edit`/`Write`/`Bash` — having the tool available is not permission to use it for implementation in this role. - -1. If a `.hermit-quit` file exists in the project root, stop entirely: end this pass immediately without doing any work, and do **not** schedule anything (quit). This is a terminal stop, unlike pause — it is not resumed by `hermit resume`; starting `/hermit` again is required to resume autonomous operation. -2. If a `.hermit-paused` file exists in the project root, end this pass immediately without doing any work (paused) — the recurring cron trigger re-checks on the next cycle -3. Retrieve open Issues with `list_issues` -4. Check open PRs for new review comments using `get_recent_pr_comments` with a `since` timestamp set to the last check time (store the current time before calling): +2. If a `.hermit-quit` file exists in the project root, stop entirely: end this pass immediately without doing any work, and do **not** schedule anything (quit). This is a terminal stop, unlike pause — it is not resumed by `hermit resume`; starting `/hermit` again is required to resume autonomous operation. +3. If a `.hermit-paused` file exists in the project root, end this pass immediately without doing any work (paused) — the recurring cron trigger re-checks on the next cycle +4. Retrieve open Issues with `list_issues` +5. Check open PRs for new review comments using `get_recent_pr_comments` with a `since` timestamp set to the last check time (store the current time before calling): - If new comments are found on any PR, post a summary comment on that PR acknowledging the feedback (use `add_issue_comment`) - Update the stored last-check timestamp to now -5. Check open Issues for new comments using `get_issue_comments` with a `since` timestamp set to the last check time (store the current time before calling; track this timestamp separately from step 4's PR-comment-check timestamp and from step 6's requirements-sweep timestamp): - - For each open Issue retrieved in step 3, call `get_issue_comments` with that `since` timestamp +6. Check open Issues for new comments using `get_issue_comments` with a `since` timestamp set to the last check time (store the current time before calling; track this timestamp separately from step 5's PR-comment-check timestamp and from step 7's requirements-sweep timestamp): + - For each open Issue retrieved in step 4, call `get_issue_comments` with that `since` timestamp - If new comments are found on an Issue, post a summary comment on that Issue acknowledging receipt (use `add_issue_comment`) - Update the stored last-check timestamp to now -6. Run the requirements reconcile sweep roughly once an hour using `run_requirements_sweep`, tracking a separate "last requirements-sweep time" across passes the same way step 4 tracks its own PR-comment-check "since" timestamp (store the current time before calling): +7. Run the requirements reconcile sweep roughly once an hour using `run_requirements_sweep`, tracking a separate "last requirements-sweep time" across passes the same way step 5 tracks its own PR-comment-check "since" timestamp (store the current time before calling): - Only call `run_requirements_sweep` when at least 3600 seconds have elapsed since the last recorded sweep time; otherwise skip this step for the current pass (do not call the tool early — it runs the configured `test_command` for every requirement and shouldn't be wasted on sub-hourly passes) - Update the stored last-sweep timestamp to now after calling -7. If there are no Issues, end this pass — the recurring cron trigger starts the next pass -8. For each Issue (up to 4 at a time): +8. If there are no Issues, end this pass — the recurring cron trigger starts the next pass +9. For each Issue (up to 4 at a time): a. Mark as in-progress with `assign_issue` (assignee: your own username) b. Create a worktree with `create_worktree` (base_branch: default branch) -9. **Spawn all Engineers for the Issues prepared in step 8 in parallel at once using the Agent tool** - - Information to pass to each Engineer: Issue number, title, body, `worktree_path` and `branch` returned by `create_worktree` - - If the parallel count exceeds 4, process the first 4 and defer the rest to the next pass - - **Fallback when subagent nesting is unavailable:** if the Agent tool is not available in your context (you are already a subagent and cannot spawn further subagents), do **not** implement the Issues yourself, no matter how small or obvious the fix looks, and even though you likely still have `Edit`/`Write`/`Bash` in your toolset — those tools remain reserved for coordination (e.g. inspecting the worktree to write the completion report), never for authoring the Issue's actual solution. Instead: end this pass and include in your completion report, for each prepared Issue: number, title, body, `worktree_path`, and `branch`, so the main session can spawn the Engineers (see "Engineer fallback") -10. Wait for all Engineers to complete -11. Run `check_ci_status` on the PR for each Issue (including PRs opened by Engineers spawned via the Engineer fallback on an earlier pass — use `list_prs` to find open HERMIT PRs still awaiting evaluation) +10. **Spawn all Engineers for the Issues prepared in step 9 in parallel at once using the Agent tool** (`run_in_background: true`, so this inline pass can wait for them without blocking on each one sequentially) + - Information to pass to each Engineer: Issue number, title, body, `worktree_path` and `branch` returned by `create_worktree` + - If the parallel count exceeds 4, process the first 4 and defer the rest to the next pass +11. Wait for all Engineers to complete +12. Run `check_ci_status` on the PR for each Issue (including PRs from Engineers spawned on an earlier pass that are still awaiting evaluation — use `list_prs` to find open HERMIT PRs) - If CI is failing: the tool automatically posts an investigation comment listing the failing checks; skip merging and wait for fixes - If CI is passing: run `evaluate_risk` - LOW / MEDIUM: run `merge_pr` with `worktree_path` and `branch` so the worktree is cleaned up automatically after a successful merge @@ -68,11 +61,7 @@ The Superintendent cycle runs as a **background subagent** so the Claude Code pr - Check whether the branch is stale relative to the base branch in a way that could hide semantic conflicts, not just textual `mergeable` conflicts - Post your findings as a separate PR comment via `add_issue_comment`: a short summary of what changed, anything concerning, and an explicit recommendation (e.g. "looks safe to merge pending approval" vs. "found X, should be fixed first") - Skip merging and wait for a human decision -12. End the pass with a short report of what was done — do **not** loop back to step 1 yourself; the recurring cron job fires the next pass - -### Engineer fallback (performed by the main session) - -If a completed background pass reports Issues it prepared (assigned + worktree created) but could not spawn Engineers for, the main session spawns them itself: for each reported Issue, call the Agent tool with `run_in_background: true` (use the model configured as `[model].engineer` in `harness.toml`, if set), passing the Issue number, title, body, `worktree_path`, and `branch`. The resulting PRs are picked up for CI/risk evaluation by a later background pass (background-cycle step 11). +13. End the pass with a short report of what was done, and return control to the prompt — do **not** loop back to step 1 yourself; the recurring cron job fires the next pass --- diff --git a/HERMIT.md b/HERMIT.md index af745cf..1b9cc3b 100644 --- a/HERMIT.md +++ b/HERMIT.md @@ -95,7 +95,7 @@ Returns a list of open GitHub Issues that have not been started. { "label": "string (optional)" } // Output -[{ "number": 42, "title": "...", "body": "...", "labels": [...] }] +[{ "Number": 42, "Title": "...", "Body": "...", "Labels": [...] }] ``` ### `assign_issue` @@ -180,14 +180,14 @@ Closes a resolved GitHub Issue. ### `list_prs` -Returns a list of open pull requests. +Returns a list of open pull requests. Optionally filter by Issue number. ```json // Input -{ "state": "open|closed|merged (optional, default: open)" } +{ "issue_number": "number (optional) — if given, only PRs referencing this Issue are returned" } // Output -[{ "number": 123, "title": "...", "branch": "...", "url": "..." }] +[{ "pr_number": 123, "title": "...", "head_branch": "...", "issue_number": 42 }] ``` ### `get_lessons` @@ -216,26 +216,26 @@ Returns current harness configuration values. ### `review_pr` -Performs static analysis on a PR and returns a review summary. +Performs static analysis on a PR and posts a structured review comment (summary, risk level, suggestions) directly on the PR. The tool call itself returns only an acknowledgement that the comment was posted. ```json // Input { "pr_number": 123 } // Output -{ "summary": "...", "risk_level": "LOW|MEDIUM|HIGH", "suggestions": ["..."] } +{ "pr_number": 123, "comment_posted": true } ``` ### `notify` -Sends a notification via configured webhook (Slack, Discord, or generic). +Sends a notification via configured webhook (Slack, Discord, or generic). Silently no-ops if no webhook is configured. ```json // Input -{ "message": "..." } +{ "event": "string — event name (e.g. issue_assigned, pr_merged, high_risk_detected)", "message": "string — human-readable notification message" } // Output -{ "success": true } +{ "sent": true, "event": "..." } ``` --- @@ -262,33 +262,26 @@ The CLAUDE.md generated by `hermit init` consists of the following 2 sections. ### Superintendent Section -The Superintendent cycle does not run in the Claude Code foreground. Each `/hermit` invocation (typed by the user or fired by the recurring cron trigger) performs a short **foreground dispatch** — ensure the cron trigger exists, spawn a background Superintendent subagent via the Agent tool with `run_in_background: true`, and return control to the prompt — while the subagent executes one **background-cycle pass** (simplified): +The Superintendent cycle runs **synchronously inline** in whatever context receives the `/hermit` invocation (typed by the user or fired by the recurring cron trigger) — there is no background Superintendent subagent. An earlier design (issue #147) spawned one background Superintendent subagent per `/hermit` tick so the foreground prompt stayed unblocked; over long unattended runs, that accumulated one subagent spawn per cron tick indefinitely and exhausted the session's subagent-spawn cap, silently stalling the loop (issue #171). Running the pass inline costs zero subagent spawns per tick by itself; only Engineer/Analyst spawns (bounded to `{{ max_engineers }}` in parallel) remain, since that's where the loop's actual value-adding work happens (simplified): ```markdown ## Your Role: Superintendent -### Foreground dispatch (run inline, then return to the prompt) +### Superintendent cycle (one pass, run inline on every `/hermit` invocation) 1. Ensure a recurring cron job invoking `/hermit` exists (CronList / CronCreate) -2. Spawn one background Superintendent subagent (Agent tool, `run_in_background: true`) - that executes a single pass of the background cycle -3. Return control to the user immediately - -### Background cycle (one pass, executed by the background subagent) - -1. If `.hermit-quit` or `.hermit-paused` exists, end the pass without doing any work -2. Retrieve open Issues with `list_issues` -3. If there are no Issues, end the pass (the cron trigger starts the next one) -4. Mark Issues as in-progress with `assign_issue` and create worktrees -5. For each Issue, spawn Engineers with the Agent tool (up to {{ max_engineers }} in parallel) +2. If `.hermit-quit` or `.hermit-paused` exists, end the pass without doing any work +3. Retrieve open Issues with `list_issues` +4. If there are no Issues, end the pass (the cron trigger starts the next one) +5. Mark Issues as in-progress with `assign_issue` and create worktrees +6. For each Issue, spawn Engineers with the Agent tool (`run_in_background: true`, + up to {{ max_engineers }} in parallel) - Pass Issue number, title, body, and worktree path to each Engineer - - If subagent nesting is unavailable, report the prepared Issues back so the - main session can spawn the Engineers instead (Engineer fallback) -6. Wait for all Engineers to complete -7. If a PR has been created, run `evaluate_risk` for risk evaluation +7. Wait for all Engineers to complete +8. If a PR has been created, run `evaluate_risk` for risk evaluation - LOW/MEDIUM: run `merge_pr` with `worktree_path`/`branch` so the worktree is cleaned up automatically - HIGH: post a comment on the PR and skip -8. End the pass — the recurring cron job fires the next one +9. End the pass and return control to the prompt — the recurring cron job fires the next one ``` ### Engineer Section diff --git a/REQUIREMENTS.md b/REQUIREMENTS.md index 1609909..dbf2f59 100644 --- a/REQUIREMENTS.md +++ b/REQUIREMENTS.md @@ -62,7 +62,7 @@ GitHub Issue を入力として、Superintendent (監督) と Engineer (実装 - 受け入れ条件: MCP サーバのツール登録一覧に上記全ツールが含まれ、各ツールが HERMIT.md 記載の入出力スキーマに従うこと - verify: test -- 実装状況: 実装済み — `internal/mcp/tools.go` で上記 12 ツールすべてに加え、設計後に追加された `get_default_branch` / `get_issue_comments` / `check_ci_status` / `get_recent_pr_comments` / `run_requirements_sweep` の計 17 ツールを登録 (HERMIT.md 側が未追従。`get_config` は `owner`/`repo` を返さない点のみ設計と乖離 — REQ-011 参照) +- 実装状況: 実装済み — `internal/mcp/tools.go` で上記 12 ツールすべてに加え、設計後に追加された `get_default_branch` / `get_issue_comments` / `check_ci_status` / `get_recent_pr_comments` / `run_requirements_sweep` の計 17 ツールを登録。`TestREQ002_RequiredMCPToolsRegistered` はツール登録一覧のみを検証しており「各ツールが HERMIT.md 記載の入出力スキーマに従うこと」は未検証だったため、`TestREQ002_ToolSchemasMatchHERMITDoc` を追加し入出力スキーマの整合性も検証するようにした (Issue #163)。あわせて棚卸しの過程で `list_prs` (`state` ではなく `issue_number` を受け取る)・`notify` (`event` が必須入力、出力は `success` ではなく `sent`/`event`)・`review_pr` (出力は `summary`/`risk_level`/`suggestions` ではなく `pr_number`/`comment_posted`)・`list_issues` (出力キーが小文字ではなく `Number`/`Title`/`Body`/`Labels`) の 4 ツールで HERMIT.md の記述が実装と乖離していたことが判明したため HERMIT.md 側を実装に合わせて修正した。`get_config` の `owner`/`repo` 相違は既知のとおり REQ-011 側の扱いのまま据え置き ## REQ-003: list_issues は未着手のオープン Issue を返す @@ -120,17 +120,17 @@ GitHub Issue を入力として、Superintendent (監督) と Engineer (実装 ## REQ-009: Superintendent ロール — Issue 駆動の自律サイクル -Superintendent サイクルはフォアグラウンドを塞がない。`/hermit` の起動 (ユーザー入力または cron トリガー) はフォアグラウンドで「ディスパッチ」(cron ジョブの存在確認と、Agent tool `run_in_background: true` によるバックグラウンド Superintendent サブエージェントの生成) のみを行い、直ちにプロンプトを返す。サイクル本体はバックグラウンドサブエージェントが 1 パスずつ実行する (Issue #147)。 +Superintendent サイクルは `/hermit` の起動 (ユーザー入力または cron トリガー) を受けたコンテキストで、同期的に・インラインで 1 パス丸ごと実行する。以前の設計 (Issue #147) は cron ティックごとにバックグラウンド Superintendent サブエージェントを 1 体生成してフォアグラウンドの占有を避けていたが、長時間ループを継続すると cron ティックの回数だけサブエージェント生成が際限なく積み上がり、セッションの subagent spawn 上限を消費してループがサイレントに停止する不具合が判明した (Issue #171)。インライン実行に戻すことで、パスそのものによる spawn コストはゼロになり、残る spawn は 1 パスあたり上限付き (最大 {{ max_engineers }} 並列) の Engineer/Analyst 生成のみになる。 1. `list_issues` でオープン Issue を取得する (なければパスを終了し、次の cron トリガーを待つ) 2. Issue を `assign_issue` で着手中にし、`create_worktree` で作業環境を用意する -3. Agent tool で Engineer を並列に生成し、Issue 番号・タイトル・本文・worktree_path・branch を渡す (サブエージェントのネストが不可能な場合は準備済み Issue を報告し、メインセッションが Engineer を生成する) +3. Agent tool (`run_in_background: true`) で Engineer を並列に生成し、Issue 番号・タイトル・本文・worktree_path・branch を渡す 4. 全 Engineer の完了を待ち、PR に対して CI 確認とリスク評価を行う 5. LOW / MEDIUM は `merge_pr` でマージ、HIGH はレビューコメントを残して人間の判断を待つ -- 受け入れ条件: `hermit init` が生成する CLAUDE.md テンプレートに、フォアグラウンドディスパッチ (`run_in_background: true` によるバックグラウンドサブエージェント生成) と上記のバックグラウンドサイクルが含まれること +- 受け入れ条件: `hermit init` が生成する CLAUDE.md テンプレートに、cron トリガーの存在確認と、上記のサイクルをインラインで実行する記述が含まれ、バックグラウンド Superintendent サブエージェントの生成を指示する記述が含まれないこと - verify: manual -- 実装状況: 実装済み — `cmd/hermit/templates/CLAUDE.md.tmpl` の Superintendent セクション (Foreground dispatch / Background cycle / Engineer fallback)。設計の 7 ステップから大幅に拡張され、`.hermit-quit` / `.hermit-paused` の検出 (バックグラウンド側で実施)、要件ヒアリング Issue の分岐、Issue 粒度チェック、PR/Issue コメント検出、HIGH リスク時の実質レビュー実施などが追加されている +- 実装状況: 実装済み — `cmd/hermit/templates/CLAUDE.md.tmpl` の Superintendent セクション (「Superintendent cycle (one pass)」に統合済み。旧来の Foreground dispatch / Background cycle / Engineer fallback の分離構成は Issue #171 で廃止)。`.hermit-quit` / `.hermit-paused` の検出、要件ヒアリング Issue の分岐、Issue 粒度チェック、PR/Issue コメント検出、HIGH リスク時の実質レビュー実施は維持されている ## REQ-010: Engineer ロール — worktree 内での独立した実装フロー @@ -151,7 +151,7 @@ Superintendent が同時に生成する Engineer の数は `harness.toml` の `[ - 受け入れ条件: `get_config` が harness.toml の max_engineers 値を返し、CLAUDE.md テンプレートが並列上限としてこの値を参照していること - verify: test -- 実装状況: 実装済み — テンプレート側 (`cmd/hermit/templates/CLAUDE.md.tmpl` が `{{ .MaxEngineers }}` を展開し、超過分の繰り越しも記述) に加え、`get_config` (`internal/mcp/tools.go`) が `[agent].max_engineers` (既定値未設定または 0 以下のときは既定 4、`cmd/hermit/main.go` の `loadConfig`) を `max_engineers` として返すようになった (`internal/mcp/req_test.go` の `TestREQ011_GetConfig_ReturnsMaxEngineers`)。`owner`/`repo` を返さない点は HERMIT.md との差分として残るが、この要件の受け入れ条件 (`get_config` が max_engineers を返し、テンプレートがこの値を参照すること) には含まれない +- 実装状況: 実装済み — テンプレート側 (`cmd/hermit/templates/CLAUDE.md.tmpl` が `{{ .MaxEngineers }}` を展開し、超過分の繰り越しも記述) に加え、`get_config` (`internal/mcp/tools.go`) が `[agent].max_engineers` (既定値未設定または 0 以下のときは既定 4、`cmd/hermit/main.go` の `loadConfig`) を `max_engineers` として返すようになった。受け入れ条件の両半分がそれぞれテストで検証されている: `get_config` 側は `internal/mcp/req_test.go` の `TestREQ011_GetConfig_ReturnsMaxEngineers`、CLAUDE.md テンプレート側は `cmd/hermit/inprocess_test.go` の `TestREQ011_ClaudeMdReferencesConfiguredMaxEngineersAsCap` (`hermit init` を任意の max_engineers 値で実行し、生成された CLAUDE.md の並列上限ステップにその値が実際に反映されることを検証。従来はコメントで「テンプレート側は実装済み」と主張するのみでテストが存在しなかった)。`owner`/`repo` を返さない点は HERMIT.md との差分として残るが、この要件の受け入れ条件には含まれない。Issue #174 での再点検: PR #172 で Superintendent サイクルが単一の「Superintendent cycle」ステップ列に統合された後も、上記 2 テストが検証する文字列 (`up to N at a time` / `exceeds N` / `max_engineers = N`) はテンプレート内に維持されており、両テストは変更なしで現行の受け入れ条件を引き続き正しく検証していることを確認した ## REQ-012: harness.toml による設定と GITHUB_TOKEN の非保存 diff --git a/cmd/hermit/inprocess_test.go b/cmd/hermit/inprocess_test.go index 54dd5c5..c5e0608 100644 --- a/cmd/hermit/inprocess_test.go +++ b/cmd/hermit/inprocess_test.go @@ -438,6 +438,79 @@ func TestCmdInit_ClaudeMdIncludesGhSnapGuideline(t *testing.T) { } } +// TestREQ011_ClaudeMdReferencesConfiguredMaxEngineersAsCap verifies the +// second half of REQ-011's acceptance criteria: the CLAUDE.md template +// generated by `hermit init` actually references the harness.toml-configured +// max_engineers value as the parallel-Engineer cap in the Superintendent +// spawn step, not just a hardcoded default. +// +// This complements internal/mcp.TestREQ011_GetConfig_ReturnsMaxEngineers, +// which covers only the get_config half of the acceptance criteria; that +// test's own doc comment asserted the template half was "already" correct +// without any test actually exercising the rendered template, which is the +// gap this test closes. +// +// Re-verified under Issue #174 after PR #172 merged the previously separate +// "Foreground dispatch" / "Background cycle" sections into a single +// "Superintendent cycle": the assertions below still target substrings that +// remain present verbatim in the rendered template, so no changes were +// needed here. +func TestREQ011_ClaudeMdReferencesConfiguredMaxEngineersAsCap(t *testing.T) { + dir := t.TempDir() + prev, _ := os.Getwd() + os.Chdir(dir) + defer os.Chdir(prev) + + r, w, _ := os.Pipe() + origStdin := os.Stdin + os.Stdin = r + defer func() { os.Stdin = origStdin }() + + // Use a distinctive, non-default max_engineers value so this test can't + // pass merely by matching the hardcoded default (4). + const configuredMaxEngineers = "9" + + go func() { + sc := bufio.NewWriter(w) + fmt.Fprintln(sc, "test-owner") + fmt.Fprintln(sc, "test-repo") + fmt.Fprintln(sc, "en") + fmt.Fprintln(sc, configuredMaxEngineers) + sc.Flush() + w.Close() + }() + + pr2, pw2, _ := os.Pipe() + origOut := os.Stdout + os.Stdout = pw2 + cmdInit() + pw2.Close() + os.Stdout = origOut + var buf bytes.Buffer + buf.ReadFrom(pr2) + + claudeMd, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md")) + if err != nil { + t.Fatalf("CLAUDE.md not created: %v", err) + } + content := string(claudeMd) + + if !strings.Contains(content, "up to "+configuredMaxEngineers+" at a time") { + t.Errorf("CLAUDE.md parallel-cap step missing configured max_engineers value (%s) in \"up to N at a time\":\n%s", configuredMaxEngineers, content) + } + if !strings.Contains(content, "exceeds "+configuredMaxEngineers) { + t.Errorf("CLAUDE.md spawn step missing configured max_engineers value (%s) in the \"exceeds N\" overflow check:\n%s", configuredMaxEngineers, content) + } + + harnessToml, err := os.ReadFile(filepath.Join(dir, "harness.toml")) + if err != nil { + t.Fatalf("harness.toml not created: %v", err) + } + if !strings.Contains(string(harnessToml), "max_engineers = "+configuredMaxEngineers) { + t.Errorf("harness.toml missing configured max_engineers value (%s):\n%s", configuredMaxEngineers, string(harnessToml)) + } +} + // TestCmdInit_DefaultsApplied verifies that empty answers for language and // max_engineers use the defaults ("ja" and 4). func TestCmdInit_DefaultsApplied(t *testing.T) { @@ -617,12 +690,13 @@ func TestCmdInit_ClaudeMdWiresEngineerModelAndEffort(t *testing.T) { } } -// TestCmdInit_ClaudeMdDispatchesBackgroundSuperintendent verifies that the -// generated CLAUDE.md instructs the foreground /hermit invocation to delegate -// the Superintendent cycle to a background subagent (Agent tool with -// run_in_background: true) wired to the configured Superintendent -// model/effort, instead of running the cycle inline (issue #147). -func TestCmdInit_ClaudeMdDispatchesBackgroundSuperintendent(t *testing.T) { +// TestCmdInit_ClaudeMdRunsSuperintendentInline verifies that the generated +// CLAUDE.md instructs /hermit to run the Superintendent cycle synchronously +// inline (no background Superintendent subagent per cron tick, which was +// found to exhaust the session's subagent-spawn cap on long-running loops — +// issue #171), while still wiring the configured Engineer/Analyst +// model/effort onto the (bounded, per-pass) Engineer/Analyst spawns. +func TestCmdInit_ClaudeMdRunsSuperintendentInline(t *testing.T) { dir := t.TempDir() prev, _ := os.Getwd() os.Chdir(dir) @@ -661,37 +735,48 @@ func TestCmdInit_ClaudeMdDispatchesBackgroundSuperintendent(t *testing.T) { t.Fatalf("CLAUDE.md not created: %v", err) } content := string(claudeMd) - if !strings.Contains(content, "### Foreground dispatch") { - t.Errorf("CLAUDE.md missing foreground dispatch section:\n%s", content) + if !strings.Contains(content, "### Superintendent cycle (one pass, run inline") { + t.Errorf("CLAUDE.md missing inline Superintendent cycle section:\n%s", content) } - if !strings.Contains(content, "### Background cycle") { - t.Errorf("CLAUDE.md missing background cycle section:\n%s", content) + if strings.Contains(content, "### Foreground dispatch") { + t.Errorf("CLAUDE.md should no longer have a separate Foreground dispatch section:\n%s", content) } - // Superintendent model/effort wiring on the background dispatch step - // (claude-cheap preset superintendent model, effort answered as "high"). - want := "`run_in_background: true` and `model: \"claude-sonnet-5\"` and `effort: \"high\"`" - if !strings.Contains(content, want) { - t.Errorf("CLAUDE.md dispatch step missing superintendent model/effort wiring (want %s):\n%s", want, content) + if strings.Contains(content, "### Background cycle") { + t.Errorf("CLAUDE.md should no longer have a separate Background cycle section:\n%s", content) + } + if strings.Contains(content, "### Engineer fallback") { + t.Errorf("CLAUDE.md should no longer have an Engineer fallback section (issue #171):\n%s", content) + } + // The Superintendent pass itself is no longer spawned via the Agent tool at + // all, so its own model/effort no longer appears in an Agent-tool wiring + // clause. Engineer spawns remain bounded per-pass background subagents + // (`run_in_background: true`), wired to the configured Engineer + // model/effort (claude-cheap preset Engineer model, effort answered as + // "medium"). + if !strings.Contains(content, "run_in_background: true") { + t.Errorf("CLAUDE.md Engineer-spawn step missing run_in_background: true:\n%s", content) } - if !strings.Contains(content, "### Engineer fallback") { - t.Errorf("CLAUDE.md missing Engineer fallback section:\n%s", content) + want := "`model: \"claude-haiku-4-5-20251001\"` and `effort: \"medium\"`" + if !strings.Contains(content, want) { + t.Errorf("CLAUDE.md Engineer-spawn step missing engineer model/effort wiring (want %s):\n%s", want, content) } } -// TestEmbeddedHermitCommandDispatchesBackground verifies that the embedded -// /hermit slash-command template tells the model to run only the foreground -// dispatch and delegate the cycle to a background subagent (issue #147). -func TestEmbeddedHermitCommandDispatchesBackground(t *testing.T) { +// TestEmbeddedHermitCommandRunsSuperintendentInline verifies that the +// embedded /hermit slash-command template tells the model to run the +// Superintendent cycle synchronously inline, instead of dispatching a +// background Superintendent subagent per cron tick (issue #171). +func TestEmbeddedHermitCommandRunsSuperintendentInline(t *testing.T) { data, err := templateFS.ReadFile("templates/commands/hermit.md") if err != nil { t.Fatalf("embedded templates/commands/hermit.md not readable: %v", err) } content := string(data) - if !strings.Contains(content, "run_in_background: true") { - t.Errorf("hermit.md command missing run_in_background dispatch instruction:\n%s", content) + if !strings.Contains(content, "inline") { + t.Errorf("hermit.md command missing instruction to run the cycle inline:\n%s", content) } - if !strings.Contains(content, "Foreground dispatch") { - t.Errorf("hermit.md command missing reference to the foreground dispatch steps:\n%s", content) + if strings.Contains(content, "Foreground dispatch") { + t.Errorf("hermit.md command should no longer reference a separate foreground dispatch step:\n%s", content) } } diff --git a/cmd/hermit/serve_test.go b/cmd/hermit/serve_test.go index 9318242..2c5055e 100644 --- a/cmd/hermit/serve_test.go +++ b/cmd/hermit/serve_test.go @@ -103,8 +103,8 @@ language = "ja" continue } tools, _ := msg.Result["tools"].([]any) - if len(tools) != 17 { - t.Errorf("expected 17 tools, got %d", len(tools)) + if len(tools) != 18 { + t.Errorf("expected 18 tools, got %d", len(tools)) } return case <-deadline: @@ -207,8 +207,8 @@ language = "ja" continue } tools, _ := msg.Result["tools"].([]any) - if len(tools) != 17 { - t.Errorf("expected 17 tools, got %d", len(tools)) + if len(tools) != 18 { + t.Errorf("expected 18 tools, got %d", len(tools)) } return case <-deadline: @@ -314,8 +314,8 @@ language = "ja" continue } tools, _ := msg.Result["tools"].([]any) - if len(tools) != 17 { - t.Errorf("expected 17 tools, got %d", len(tools)) + if len(tools) != 18 { + t.Errorf("expected 18 tools, got %d", len(tools)) } return case <-deadline: diff --git a/cmd/hermit/templates/CLAUDE.md.tmpl b/cmd/hermit/templates/CLAUDE.md.tmpl index e4c998e..2431c77 100644 --- a/cmd/hermit/templates/CLAUDE.md.tmpl +++ b/cmd/hermit/templates/CLAUDE.md.tmpl @@ -4,44 +4,39 @@ ## Your Role: Superintendent -The Superintendent cycle runs as a **background subagent** so the Claude Code prompt is never blocked. Every `/hermit` invocation — whether typed by the user or fired by the recurring cron trigger — performs only the short **foreground dispatch** below and returns control to the prompt immediately; the actual cycle is executed by the background subagent. +**The Superintendent cycle runs synchronously, inline, in the same context that received the `/hermit` invocation.** Every `/hermit` invocation — whether typed by the user or fired by the recurring cron trigger — performs the full "Superintendent cycle (one pass)" below directly, in this context, and only returns control to the prompt once the pass has finished. There is no background Superintendent subagent: spawning one every cron tick, indefinitely, for as long as the loop runs, was found to exhaust the session's subagent-spawn cap over long unattended runs, silently killing the loop once the cap was hit. Running the pass inline means each `/hermit` tick costs zero subagent spawns by itself; the only spawns this cycle produces are the bounded (≤{{ .MaxEngineers }}-per-pass) Engineer/Analyst subagents below, which is where the loop's actual value-adding work happens. -### Foreground dispatch (run inline, then return to the prompt) +**Hard prohibition:** this cycle is a coordinator, not an implementer. Do not use `Edit`, `Write`, `NotebookEdit`, or shell commands that mutate tracked files (including inside a worktree created below) to change this repository's code, docs, or config while acting as Superintendent. All implementation work — even a one-line fix, even when it looks faster to do it yourself — belongs exclusively to the Engineer role. If this cycle finds itself about to open a file for editing anywhere under a `worktree_path`, or to run a code-writing command against one, that is a signal it has drifted out of role and must stop. + +### Superintendent cycle (one pass, run inline on every `/hermit` invocation) 1. Ensure the cycle keeps triggering on its own, without depending on the model remembering to do so: call `CronList` to check whether a recurring job invoking `/hermit` (or this cycle) is already scheduled. - If no such job is registered, call `CronCreate` to schedule one at the configured interval (e.g. `*/2 * * * *` for the default 120-second cadence; round to the nearest whole minute the cron expression can express) - If a matching job is already registered, do nothing -2. Spawn exactly **one** background Superintendent subagent that executes a single pass of the "Background cycle" below: call the Agent tool with `run_in_background: true` and `model: "{{ .SuperintendentModel }}"`{{ if .SuperintendentEffort }} and `effort: "{{ .SuperintendentEffort }}"`{{ end }}, instructing it to perform one full background-cycle pass per this file's "Background cycle (one pass)" section. - - If a background pass spawned by a previous trigger in this session is still running, do **not** spawn a second one — skip this trigger and return -3. Return control to the user immediately — do **not** wait for the background subagent to finish. When a completed background pass reports Issues it prepared but could not spawn Engineers for, perform the "Engineer fallback" below. - -### Background cycle (one pass, executed by the background subagent) - -1. If a `.hermit-quit` file exists in the project root, stop entirely: end this pass immediately without doing any work, and do **not** schedule anything (quit). This is a terminal stop, unlike pause — it is not resumed by `hermit resume`; starting `/hermit` again is required to resume autonomous operation. -2. If a `.hermit-paused` file exists in the project root, end this pass immediately without doing any work (paused) — the recurring cron trigger re-checks on the next cycle -3. Retrieve open Issues with `list_issues` -4. Treat any Issue labeled `requirements` (or whose title starts with `[Requirements]`) as a **standing requirements-hearing channel**, not a work item — this is where non-engineers converse with HERMIT to create/update the requirements document. Exclude these Issues from the Engineer-spawning steps below (steps 6–7), and instead: +2. If a `.hermit-quit` file exists in the project root, stop entirely: end this pass immediately without doing any work, and do **not** schedule anything (quit). This is a terminal stop, unlike pause — it is not resumed by `hermit resume`; starting `/hermit` again is required to resume autonomous operation. +3. If a `.hermit-paused` file exists in the project root, end this pass immediately without doing any work (paused) — the recurring cron trigger re-checks on the next cycle +4. Retrieve open Issues with `list_issues` +5. Treat any Issue labeled `requirements` (or whose title starts with `[Requirements]`) as a **standing requirements-hearing channel**, not a work item — this is where non-engineers converse with HERMIT to create/update the requirements document. Exclude these Issues from the Engineer-spawning steps below (steps 7–8), and instead: - Call `get_issue_comments` with a `since` timestamp set to the last check time for that Issue (store the current time before calling) - - If there are new comments authored by a human (not HERMIT's own account), spawn an Analyst to respond — call the Agent tool with `model: "{{ .AnalystModel }}"`{{ if .AnalystEffort }} and `effort: "{{ .AnalystEffort }}"`{{ end }}, passing the Issue number, the new comment bodies, and their comment URLs. See "Your Role: Analyst" below for what the Analyst must do. + - If there are new comments authored by a human (not HERMIT's own account), spawn an Analyst to respond — call the Agent tool with `run_in_background: true` and `model: "{{ .AnalystModel }}"`{{ if .AnalystEffort }} and `effort: "{{ .AnalystEffort }}"`{{ end }}, passing the Issue number, the new comment bodies, and their comment URLs. See "Your Role: Analyst" below for what the Analyst must do. - Update the stored last-check timestamp for that Issue to now -5. Run the requirements reconcile sweep roughly once an hour using `run_requirements_sweep`, tracking a "last requirements-sweep time" across passes the same way step 4 tracks each hearing Issue's own last-checked comment timestamp (store the current time before calling): +6. Run the requirements reconcile sweep roughly once an hour using `run_requirements_sweep`, tracking a "last requirements-sweep time" across passes the same way step 5 tracks each hearing Issue's own last-checked comment timestamp (store the current time before calling): - Only call `run_requirements_sweep` when at least 3600 seconds have elapsed since the last recorded sweep time; otherwise skip this step for the current pass - Update the stored last-sweep timestamp to now after calling - - Note this is distinct from step 4: step 4 is about a human dictating new/changed requirements via conversation; this step is about detecting when already-declared requirements have silently regressed or gone unimplemented in the code, by re-running the reconcile sweep against the current requirements document -6. If there are no non-requirements Issues, end this pass — the recurring cron trigger starts the next pass -7. For each remaining Issue (up to {{ .MaxEngineers }} at a time), perform a granularity check: + - Note this is distinct from step 5: step 5 is about a human dictating new/changed requirements via conversation; this step is about detecting when already-declared requirements have silently regressed or gone unimplemented in the code, by re-running the reconcile sweep against the current requirements document +7. If there are no non-requirements Issues, end this pass — the recurring cron trigger starts the next pass +8. For each remaining Issue (up to {{ .MaxEngineers }} at a time), perform a granularity check: - If the Issue body is empty, extremely short, or covers multiple independent features, judge it as **ambiguous or too large** - Ambiguous/oversized Issues: post a `[Clarification Needed]` comment via `add_issue_comment` and skip - For Issues of appropriate scope only: a. Mark as in-progress with `assign_issue` (assignee: your own username) b. Create a worktree with `create_worktree` (base_branch: default branch) -8. **Spawn all Engineers for the Issues prepared in step 7 in parallel at once using the Agent tool** +9. **Spawn all Engineers for the Issues prepared in step 8 in parallel at once using the Agent tool** (`run_in_background: true`, so this inline pass can wait for them without blocking on each one sequentially) - Information to pass to each Engineer: Issue number, title, body, `worktree_path` and `branch` returned by `create_worktree` - Call the Agent tool with `model: "{{ .EngineerModel }}"`{{ if .EngineerEffort }} and `effort: "{{ .EngineerEffort }}"`{{ end }} so each Engineer runs with the model{{ if .EngineerEffort }}/effort{{ end }} configured in `harness.toml`'s `[model]` section - If the parallel count exceeds {{ .MaxEngineers }}, process the first {{ .MaxEngineers }} and defer the rest to the next pass - - **Fallback when subagent nesting is unavailable:** if the Agent tool is not available in your context (you are already a subagent and cannot spawn further subagents), do **not** implement the Issues yourself — end this pass and include in your completion report, for each prepared Issue: number, title, body, `worktree_path`, and `branch`, so the main session can spawn the Engineers (see "Engineer fallback") -9. Wait for all Engineers to complete -10. Run `evaluate_risk` on the PR for each Issue (including PRs opened by Engineers spawned via the Engineer fallback on an earlier pass — use `list_prs` to find open HERMIT PRs still awaiting evaluation) +10. Wait for all Engineers to complete +11. Run `evaluate_risk` on the PR for each Issue (including PRs opened by Engineers spawned on an earlier pass that are still awaiting evaluation — use `list_prs` to find open HERMIT PRs) - LOW / MEDIUM: run `merge_pr` with `worktree_path`/`branch` so the worktree is cleaned up automatically - HIGH: `evaluate_risk` auto-posts a generic risk comment (`⚠️ HERMIT: HIGH risk detected.\nReasons: [...]`) restating the `risk_reasons`. That comment is not a review — before skipping, perform a substantive review of the PR yourself: - Read the actual diff (not just the file paths / line counts in `risk_reasons`) @@ -49,11 +44,7 @@ The Superintendent cycle runs as a **background subagent** so the Claude Code pr - Check whether the branch is stale relative to the base branch in a way that could hide semantic conflicts, not just textual `mergeable` conflicts - Post your findings as a separate PR comment via `add_issue_comment`: a short summary of what changed, anything concerning, and an explicit recommendation (e.g. "looks safe to merge pending approval" vs. "found X, should be fixed first") - Skip merging and wait for a human decision -11. End the pass with a short report of what was done — do **not** loop back to step 1 yourself; the recurring cron job fires the next pass - -### Engineer fallback (performed by the main session) - -If a completed background pass reports Issues it prepared (assigned + worktree created) but could not spawn Engineers for, the main session spawns them itself: for each reported Issue, call the Agent tool with `run_in_background: true` and `model: "{{ .EngineerModel }}"`{{ if .EngineerEffort }} and `effort: "{{ .EngineerEffort }}"`{{ end }}, passing the Issue number, title, body, `worktree_path`, and `branch`. The resulting PRs are picked up for risk evaluation by a later background pass (background-cycle step 10). +12. End the pass with a short report of what was done, and return control to the prompt — do **not** loop back to step 1 yourself; the recurring cron job fires the next pass --- diff --git a/cmd/hermit/templates/commands/hermit.md b/cmd/hermit/templates/commands/hermit.md index 2421a9d..248d0f2 100644 --- a/cmd/hermit/templates/commands/hermit.md +++ b/cmd/hermit/templates/commands/hermit.md @@ -1,3 +1,3 @@ -As Superintendent, follow the dispatch procedure documented in this project's `CLAUDE.md` under "Your Role: Superintendent". Run **only** the "Foreground dispatch" steps inline in this session: ensure the recurring cron trigger exists, spawn the background Superintendent subagent (Agent tool with `run_in_background: true`) that executes one pass of the "Background cycle", and return control to the prompt immediately. Never execute the Superintendent cycle itself in the foreground — the whole point is that the user keeps a usable prompt while the cycle runs in the background. +As Superintendent, follow the dispatch procedure documented in this project's `CLAUDE.md` under "Your Role: Superintendent". Run the "Superintendent cycle (one pass)" steps **inline in this session**: ensure the recurring cron trigger exists, then execute the full pass (Issue triage, PR/Issue comment checks, requirements sweep cadence, Engineer spawning, CI/risk evaluation, merge) directly, synchronously, before returning control to the prompt. Do not spawn a background Superintendent subagent for the pass itself — running it inline avoids accumulating one background-subagent spawn per cron tick indefinitely, which was found to exhaust the session's subagent-spawn cap on long-running loops (issue #171). -Do not restate or duplicate the cycle's steps here — `CLAUDE.md` is the single source of truth for both the foreground dispatch and the background cycle (including the self-scheduling step that keeps this loop running). Keeping the steps in one place only avoids this file silently drifting out of sync whenever the cycle in `CLAUDE.md` changes. +Do not restate or duplicate the cycle's steps here — `CLAUDE.md` is the single source of truth for the Superintendent cycle (including the self-scheduling step that keeps this loop running). Keeping the steps in one place only avoids this file silently drifting out of sync whenever the cycle in `CLAUDE.md` changes. diff --git a/internal/mcp/req_test.go b/internal/mcp/req_test.go index 8119c98..88ba5dc 100644 --- a/internal/mcp/req_test.go +++ b/internal/mcp/req_test.go @@ -17,7 +17,10 @@ import ( "testing" "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" gh "github.com/ytnobody/hermit/internal/github" + "github.com/ytnobody/hermit/internal/readiness" + "github.com/ytnobody/hermit/internal/risk" ) // TestREQ002_RequiredMCPToolsRegistered verifies REQ-002: the MCP server @@ -46,6 +49,247 @@ func TestREQ002_RequiredMCPToolsRegistered(t *testing.T) { } } +// toolIOSpec describes the documented input/output shape of one MCP tool as +// recorded in HERMIT.md's "4. MCP Tool Specifications" section. +type toolIOSpec struct { + // inputRequired/inputOptional list the documented input field names by + // required-ness. Implementations may register additional fields beyond + // these (e.g. multi-repo owner/repo, or fields added after the original + // 12-tool design) — that is treated as a compatible extension, not a + // schema violation, consistent with how REQUIREMENTS.md already + // describes HERMIT.md as trailing (not gating) the implementation. + inputRequired []string + inputOptional []string + // outputKeys lists top-level keys that must appear in a successful + // response, when checkOutput is non-nil. + outputKeys []string + // checkOutput, if set, invokes the tool (via callTool with the given + // args) and returns the decoded top-level JSON object to check + // outputKeys against. Left nil for tools whose output is already + // exercised by a dedicated test elsewhere in this package (e.g. merge_pr + // by TestREQ007/TestREQ008) or that require infra unrelated to schema + // shape (see create_worktree below). + checkOutput func(t *testing.T, s *server.MCPServer) map[string]any +} + +// TestREQ002_ToolSchemasMatchHERMITDoc verifies the second half of REQ-002's +// acceptance criteria, which TestREQ002_RequiredMCPToolsRegistered above does +// not cover: "each tool follows the input/output schema documented in +// HERMIT.md". For every required tool it checks that the registered MCP +// input schema contains (at least) the documented fields with matching +// required/optional-ness, and, where practical, that a successful call +// returns the documented top-level output keys. +// +// Issue #163: REQUIREMENTS.md's REQ-002 block was re-hashed after an +// unrelated edit, and reviewing this test against the current HERMIT.md +// surfaced four tools (list_prs, notify, review_pr, list_issues) whose +// documented schema had silently drifted from the implementation; HERMIT.md +// was corrected to match the implementation for those four. get_config's +// long-known owner/repo gap remains out of scope here (tracked by REQ-011). +// +// Issue #173: the requirements-sweep flagged REQ-002's hash as changed again. +// The REQ-002 block's acceptance-criteria text itself is unchanged since +// Issue #163 (git history confirms the only edit to the block after #163 was +// this comment documenting the #163 fix, which is itself hashed as part of +// the block and so re-triggers the sweep's change detection even though the +// requirement it verifies did not move). Reviewed both required-tools +// registration and the schema/output checks above against the current +// REQUIREMENTS.md and HERMIT.md: all 12 required tools are covered, and the +// documented input/output shapes for list_prs, notify, review_pr, and +// list_issues (the four corrected under #163) still match the +// implementation. No test changes were needed for #173 beyond this note. +func TestREQ002_ToolSchemasMatchHERMITDoc(t *testing.T) { + specs := map[string]toolIOSpec{ + "list_issues": { + inputOptional: []string{"label"}, + }, + "assign_issue": { + inputRequired: []string{"issue_number", "assignee"}, + outputKeys: []string{"success"}, + checkOutput: func(t *testing.T, s *server.MCPServer) map[string]any { + return mustToolJSON(t, callTool(t, s, "assign_issue", map[string]any{ + "issue_number": float64(1), + "assignee": "someone", + })) + }, + }, + "create_worktree": { + inputRequired: []string{"issue_number", "base_branch"}, + outputKeys: []string{"worktree_path", "branch"}, + checkOutput: checkCreateWorktreeOutput, + }, + "evaluate_risk": { + inputRequired: []string{"pr_number"}, + outputKeys: []string{"level", "reasons"}, + checkOutput: func(t *testing.T, _ *server.MCPServer) map[string]any { + mock := &mockGithubClient{prStatus: &gh.PRStatus{Number: 1}} + s2 := newTestServer(t, mock) + return mustToolJSON(t, callTool(t, s2, "evaluate_risk", map[string]any{"pr_number": float64(1)})) + }, + }, + "merge_pr": { + inputRequired: []string{"pr_number"}, + inputOptional: []string{"worktree_path", "branch"}, + // Output (merged/reason) is already exercised by + // TestREQ007_MergePR_CIGatingAndHighRiskRejection and + // TestREQ008_MergePR_WorktreeCleanup. + }, + "add_issue_comment": { + inputRequired: []string{"issue_number", "body"}, + outputKeys: []string{"success"}, + checkOutput: func(t *testing.T, s *server.MCPServer) map[string]any { + return mustToolJSON(t, callTool(t, s, "add_issue_comment", map[string]any{ + "issue_number": float64(1), + "body": "hi", + })) + }, + }, + "close_issue": { + inputRequired: []string{"issue_number"}, + outputKeys: []string{"success"}, + checkOutput: func(t *testing.T, s *server.MCPServer) map[string]any { + return mustToolJSON(t, callTool(t, s, "close_issue", map[string]any{ + "issue_number": float64(1), + })) + }, + }, + "list_prs": { + inputOptional: []string{"issue_number"}, + }, + "get_lessons": { + outputKeys: []string{"lessons"}, + checkOutput: func(t *testing.T, s *server.MCPServer) map[string]any { + return mustToolJSON(t, callTool(t, s, "get_lessons", map[string]any{})) + }, + }, + "get_config": { + // owner/repo are documented but intentionally not returned; see + // REQ-011 and the 現状把握サマリ gap table in REQUIREMENTS.md. + outputKeys: []string{"max_engineers", "loop_interval"}, + checkOutput: func(t *testing.T, s *server.MCPServer) map[string]any { + return mustToolJSON(t, callTool(t, s, "get_config", map[string]any{})) + }, + }, + "review_pr": { + inputRequired: []string{"pr_number"}, + outputKeys: []string{"pr_number", "comment_posted"}, + checkOutput: func(t *testing.T, s *server.MCPServer) map[string]any { + return mustToolJSON(t, callTool(t, s, "review_pr", map[string]any{"pr_number": float64(1)})) + }, + }, + "notify": { + inputRequired: []string{"event", "message"}, + outputKeys: []string{"sent", "event"}, + checkOutput: func(t *testing.T, s *server.MCPServer) map[string]any { + return mustToolJSON(t, callTool(t, s, "notify", map[string]any{ + "event": "issue_assigned", + "message": "hello", + })) + }, + }, + } + + s := newTestServer(t, &mockGithubClient{}) + for name, spec := range specs { + t.Run(name, func(t *testing.T) { + st := s.GetTool(name) + if st == nil { + t.Fatalf("tool %q is not registered", name) + } + props := st.Tool.InputSchema.Properties + required := map[string]bool{} + for _, r := range st.Tool.InputSchema.Required { + required[r] = true + } + for _, field := range spec.inputRequired { + if _, ok := props[field]; !ok { + t.Errorf("documented required input %q is not registered", field) + } else if !required[field] { + t.Errorf("documented required input %q is registered but not marked required", field) + } + } + for _, field := range spec.inputOptional { + if _, ok := props[field]; !ok { + t.Errorf("documented optional input %q is not registered", field) + } else if required[field] { + t.Errorf("documented optional input %q is registered as required", field) + } + } + + if spec.checkOutput == nil { + return + } + got := spec.checkOutput(t, s) + for _, key := range spec.outputKeys { + if _, ok := got[key]; !ok { + t.Errorf("documented output key %q missing from response %v", key, got) + } + } + }) + } +} + +// mustToolJSON decodes a successful tool result's text content into a +// top-level JSON object for output-key assertions. +func mustToolJSON(t *testing.T, result *mcp.CallToolResult) map[string]any { + t.Helper() + if result.IsError { + t.Fatalf("expected success, got error: %v", result.Content) + } + tc, ok := result.Content[0].(mcp.TextContent) + if !ok { + t.Fatalf("expected TextContent, got %T", result.Content[0]) + } + var got map[string]any + if err := json.Unmarshal([]byte(tc.Text), &got); err != nil { + t.Fatalf("unmarshal error: %v (text: %s)", err, tc.Text) + } + return got +} + +// checkCreateWorktreeOutput exercises create_worktree end-to-end against a +// throwaway git repository (git.CreateWorktree shells out to git against the +// process's current working directory), verifying the response contains the +// documented worktree_path/branch keys. +func checkCreateWorktreeOutput(t *testing.T, _ *server.MCPServer) map[string]any { + t.Helper() + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + repo := t.TempDir() + gitIn := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = repo + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + gitIn("init", "-b", "main") + gitIn("config", "user.email", "hermit-test@example.com") + gitIn("config", "user.name", "hermit-test") + gitIn("commit", "--allow-empty", "-m", "init") + + // CreateWorktree runs git against the process working directory. + t.Chdir(repo) + + srv := server.NewMCPServer("hermit-test", "0.0.0") + registerTools(srv, &mockGithubClient{}, 0, t.TempDir(), "req002schema/gh-test", 120, "", "", nil, "", readiness.DefaultConfig(), risk.DefaultConfig(), nil, ModelConfig{}, RequirementsConfig{}, 4) + + got := mustToolJSON(t, callTool(t, srv, "create_worktree", map[string]any{ + "issue_number": float64(163), + "base_branch": "main", + })) + + if wt, _ := got["worktree_path"].(string); wt != "" { + t.Cleanup(func() { + _ = exec.Command("git", "-C", repo, "worktree", "remove", "--force", wt).Run() + }) + } + return got +} + // TestREQ003_ListIssues_ExcludesNonQueueIssues verifies the exclusion half of // REQ-003: Issues flagged as not workable (needs-clarification, hermit-hearing) // are excluded from list_issues, while ready open Issues are returned. The @@ -104,12 +348,16 @@ func TestREQ004_AssignIssue_ReturnsSuccess(t *testing.T) { } } -// TestREQ011_GetConfig_ReturnsMaxEngineers verifies REQ-011: get_config -// reports the [agent].max_engineers value from harness.toml so the -// Superintendent can look up the configured parallel-Engineer cap via MCP -// instead of relying on a hardcoded number (the CLAUDE.md template already -// references {{ .MaxEngineers }} at render time; this covers the runtime -// half of the acceptance criteria). +// TestREQ011_GetConfig_ReturnsMaxEngineers verifies the get_config half of +// REQ-011's acceptance criteria: get_config reports the [agent].max_engineers +// value from harness.toml so the Superintendent can look up the configured +// parallel-Engineer cap via MCP instead of relying on a hardcoded number. +// The other half of the acceptance criteria — the CLAUDE.md template +// referencing this value as the parallel cap — is verified separately by +// cmd/hermit.TestREQ011_ClaudeMdReferencesConfiguredMaxEngineersAsCap, which +// renders the template with a distinctive max_engineers value and asserts it +// appears in the generated CLAUDE.md's cap-related steps (a claim this +// package's own doc comment previously made without any test backing it). func TestREQ011_GetConfig_ReturnsMaxEngineers(t *testing.T) { s := newTestServerWithMaxEngineers(t, &mockGithubClient{}, 7) diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 92b80c1..8749e3b 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "strings" + "time" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" @@ -540,6 +541,16 @@ func registerTools(s *server.MCPServer, client githubClient, rateLimitThreshold }, ) + s.AddTool( + mcp.NewTool("now", + mcp.WithDescription("Returns the current wall-clock time as an RFC3339 string. Use this as an authoritative 'now' when computing elapsed time for cadence tracking (e.g. the PR-comment check, Issue-comment check, and requirements-sweep 'since'/last-run timestamps in the background cycle), instead of estimating the current time from context."), + ), + func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { + b, _ := json.Marshal(map[string]any{"now": time.Now().UTC().Format(time.RFC3339)}) + return mcp.NewToolResultText(string(b)), nil + }, + ) + s.AddTool( mcp.NewTool("review_pr", mcp.WithDescription("Posts a structured automated review comment on a PR based on static analysis of the diff"), diff --git a/internal/mcp/tools_test.go b/internal/mcp/tools_test.go index 713a5f8..28358e6 100644 --- a/internal/mcp/tools_test.go +++ b/internal/mcp/tools_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" @@ -1005,6 +1006,42 @@ func TestCheckCIStatus_Passing_DoesNotRecordFailure(t *testing.T) { } } +// --- now (authoritative wall-clock timestamp) --- + +func TestNow_ReturnsParseableRFC3339CloseToActualTime(t *testing.T) { + s := newTestServer(t, &mockGithubClient{}) + + before := time.Now().UTC() + result := callTool(t, s, "now", map[string]any{}) + after := time.Now().UTC() + + if result.IsError { + t.Fatalf("expected success, got error: %v", result.Content) + } + tc, ok := result.Content[0].(mcp.TextContent) + if !ok { + t.Fatalf("expected TextContent") + } + var got map[string]any + if err := json.Unmarshal([]byte(tc.Text), &got); err != nil { + t.Fatalf("unmarshal error: %v", err) + } + + nowStr, ok := got["now"].(string) + if !ok || nowStr == "" { + t.Fatalf("expected non-empty 'now' string, got %v", got["now"]) + } + + parsed, err := time.Parse(time.RFC3339, nowStr) + if err != nil { + t.Fatalf("expected 'now' to be a parseable RFC3339 timestamp, got %q: %v", nowStr, err) + } + + if parsed.Before(before.Add(-5*time.Second)) || parsed.After(after.Add(5*time.Second)) { + t.Errorf("expected 'now' (%v) to be close to actual current time (between %v and %v)", parsed, before, after) + } +} + // --- get_config / risk policy exposure --- func TestGetConfig_IncludesDefaultRiskConfig(t *testing.T) {