Skip to content

fix(subagent-dev): add post-commit verification for untracked files#661

Open
mvanhorn wants to merge 4 commits intoobra:mainfrom
mvanhorn:osc/633-fix-subagent-verify-committed-state
Open

fix(subagent-dev): add post-commit verification for untracked files#661
mvanhorn wants to merge 4 commits intoobra:mainfrom
mvanhorn:osc/633-fix-subagent-verify-committed-state

Conversation

@mvanhorn
Copy link
Contributor

@mvanhorn mvanhorn commented Mar 9, 2026

Summary

  • Adds a post-commit verification step to the subagent-driven-development skill
  • After each commit, the implementer checks git status for untracked files that should have been committed
  • Adds a controller-side safety net to flag untracked files before moving to the next task
  • Prevents silent failures where the committed state diverges from the working tree

Fixes #633

This contribution was developed with AI assistance (Claude Code).

mvanhorn and others added 3 commits March 9, 2026 07:58
The skill now prompts the agent to check for related GitHub issues
and include closing keywords (Closes #NNN) in the PR body. This
enables automatic issue closure on merge instead of requiring
manual cleanup.

Fixes obra#600

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… for Windows compatibility

On Windows, cmd.exe doesn't recognize single quotes as path
delimiters, causing the SessionStart hook to fail with "'C:' is not
recognized". Double quotes are used instead to preserve space handling
in paths like C:\Program Files\...

Fixes obra#644

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Subagents sometimes commit without adding all new files, leaving
the committed state broken while the working tree appears green.
This adds a verification step after each commit to check for
untracked files and a controller-side safety net.

Fixes obra#633

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 772fd68c-3615-44c4-a935-e3fcf39d0f18

📥 Commits

Reviewing files that changed from the base of the PR and between 296af90 and f7485ab.

📒 Files selected for processing (1)
  • skills/subagent-driven-development/implementer-prompt.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • skills/subagent-driven-development/implementer-prompt.md

📝 Walkthrough

Walkthrough

Adds post-commit verification and a controller-level git status check to the subagent workflow, updates implementer prompts, and fixes quoting of the SessionStart hook command in hooks/hooks.json; also adds conditional inclusion of GitHub-closing keywords in PR guidance.

Changes

Cohort / File(s) Summary
Hook Configuration
hooks/hooks.json
Changed SessionStart hook command quoting from '${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd' to an escaped double-quoted "\"${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd\"" within the JSON command string.
Development Branch Guidance
skills/finishing-a-development-branch/SKILL.md
Added conditional logic and PR body guidance to include a "Closes #NNN" line only when a related issue is detected (via branch, commits, or plan).
Subagent Workflow
skills/subagent-driven-development/SKILL.md
Introduced a controller-level post-review git status check that gates task completion; if untracked files are present, controller prompts remediation or dispatches an implementer subagent. Added controller-visible statuses and notes.
Implementer Prompt Template
skills/subagent-driven-development/implementer-prompt.md
Inserted an "After Committing" verification step (run git status, check for untracked files, amend commits if needed) and reordered lifecycle steps to verify committed state before self-review/reporting back.

Sequence Diagram(s)

sequenceDiagram
    participant Controller
    participant Implementer
    participant Repo as Git/Repo
    participant Reviewer as CodeQualityReviewer

    Controller->>Implementer: Assign task
    Implementer->>Repo: Commit changes
    Implementer->>Reviewer: Request code-quality review
    Reviewer-->>Controller: Approve?
    Controller->>Repo: Run git status (post-review)
    alt no untracked files
        Controller->>Controller: Mark task complete
    else untracked files found
        Controller->>Implementer: Request stage/amend and re-commit
        Implementer->>Repo: Stage/amend commit
        Implementer->>Controller: Notify completion
        Controller->>Repo: Re-run git status
        Controller->>Controller: Mark task complete
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • yoavsion

Poem

🐰
I hopped through branches, checks in paw,
Ensured each commit had all it saw.
Controllers watch and implementers mend,
No stray files left at journey's end.
Happy repo — hop, hop, hurrah! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning One unrelated change detected: the hooks.json modification to the SessionStart hook command quoting is outside the scope of issue #633 regarding subagent-driven development verification. Remove or separate the hooks.json changes into a distinct PR focused on the SessionStart hook command quoting modification.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and accurately summarizes the main change: adding post-commit verification for untracked files in the subagent-driven development skill.
Description check ✅ Passed The description is clearly related to the changeset, explaining the post-commit verification step and safety net for untracked files that is implemented in the PR.
Linked Issues check ✅ Passed The PR fully implements the requirements from issue #633: adds post-commit verification via implementer prompt updates and controller-side safety net for detecting untracked files before task completion.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
skills/finishing-a-development-branch/SKILL.md (1)

109-110: Consider clarifying the placeholder syntax.

The placeholder <Closes #NNN if a related issue was found, otherwise omit this line> is an instruction to the agent, not literal text to include. While this is clear in context, you might consider using a more explicit instruction format to avoid any ambiguity:

-<Closes `#NNN` if a related issue was found, otherwise omit this line>
+<!-- If related issue found: Closes `#NNN` -->

Or simply move the conditional note outside the heredoc as a comment.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/finishing-a-development-branch/SKILL.md` around lines 109 - 110, The
placeholder line "<Closes `#NNN` if a related issue was found, otherwise omit this
line>" in SKILL.md is ambiguous; update the heredoc so the agent sees only the
intended commit message template and move the conditional guidance out of the
literal block (or rephrase it as an explicit directive). Specifically, replace
the literal placeholder inside the heredoc with either a clear example like
"<Closes `#123`>" or nothing, and relocate the sentence "If a related issue
exists, include 'Closes `#NNN`' — otherwise omit" as a comment or explanatory line
outside the heredoc so the template text and instructional note are not
conflated.
skills/subagent-driven-development/SKILL.md (1)

160-162: Consider adding re-verification after implementer fixes untracked files.

The example shows dispatching the implementer to stage and amend, but doesn't show a subsequent git status re-check before marking complete. This could leave a gap if the implementer's fix introduces new issues.

 [Controller: git status check - found untracked test fixture file]
 [Dispatch implementer to stage and amend]
+[Controller: git status re-check - clean]

 [Mark Task 2 complete]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/subagent-driven-development/SKILL.md` around lines 160 - 162, Add a
re-verification step after dispatching the implementer to stage/amend: once the
implementer reports completion, have the Controller re-run the git status check
(the same "Controller: git status check - found untracked test fixture file"
logic) and assert the working tree is clean (no untracked or unstaged files)
before marking the task complete; if the re-check still shows issues, repeat the
dispatch or surface an error so completion is not recorded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@skills/subagent-driven-development/implementer-prompt.md`:
- Line 49: Update the sentence about staging so it accurately explains git
behavior: replace the claim "git add only stages tracked files by default" with
a clarification that "git add <file> stages both tracked and untracked files,
whereas git commit -a only commits changes to already-tracked files," and
explicitly advise implementers to run git add for any newly created files before
committing so subagents don't forget to include them; reference the terms "git
add", "git commit -a", and "newly created files" in the revised line.

---

Nitpick comments:
In `@skills/finishing-a-development-branch/SKILL.md`:
- Around line 109-110: The placeholder line "<Closes `#NNN` if a related issue was
found, otherwise omit this line>" in SKILL.md is ambiguous; update the heredoc
so the agent sees only the intended commit message template and move the
conditional guidance out of the literal block (or rephrase it as an explicit
directive). Specifically, replace the literal placeholder inside the heredoc
with either a clear example like "<Closes `#123`>" or nothing, and relocate the
sentence "If a related issue exists, include 'Closes `#NNN`' — otherwise omit" as
a comment or explanatory line outside the heredoc so the template text and
instructional note are not conflated.

In `@skills/subagent-driven-development/SKILL.md`:
- Around line 160-162: Add a re-verification step after dispatching the
implementer to stage/amend: once the implementer reports completion, have the
Controller re-run the git status check (the same "Controller: git status check -
found untracked test fixture file" logic) and assert the working tree is clean
(no untracked or unstaged files) before marking the task complete; if the
re-check still shows issues, repeat the dispatch or surface an error so
completion is not recorded.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 2b2f4b51-e153-4ba4-89a1-e88742704202

📥 Commits

Reviewing files that changed from the base of the PR and between e4a2375 and 296af90.

📒 Files selected for processing (4)
  • hooks/hooks.json
  • skills/finishing-a-development-branch/SKILL.md
  • skills/subagent-driven-development/SKILL.md
  • skills/subagent-driven-development/implementer-prompt.md

git add <file> stages both tracked and untracked files. The real issue is
git commit -a only commits tracked files, so new files need explicit git add.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

Subagent-driven development: subagents should verify committed state, not just working tree

1 participant