Fix commit-reminder blocking and test-runner tmp file mismatch#29
Merged
AnExiledDev merged 2 commits intomainfrom Feb 26, 2026
Merged
Fix commit-reminder blocking and test-runner tmp file mismatch#29AnExiledDev merged 2 commits intomainfrom
AnExiledDev merged 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds edit-tracking via a new PostToolUse hook and collect-session-edits.py, refactors the commit reminder to emit an advisory systemMessage only for meaningful edits, and fixes the auto-code-quality temp-file prefix to a unified Changes
Sequence DiagramssequenceDiagram
participant User as Editor/Session
participant Hook as PostToolUse Hook
participant Collector as collect-session-edits.py
participant TempFile as /tmp/claude-session-edits-{id}
participant StopHook as Stop Hook (commit-reminder.py)
participant Git as Git
User->>Hook: finish edit
Hook->>Collector: invoke (stdin JSON with session_id, file_path)
Collector->>Collector: parse & validate
Collector->>TempFile: append edited path
Collector-->>Hook: exit 0
Note over StopHook: On Session Stop
StopHook->>TempFile: read collected edits
StopHook->>StopHook: classify files (source/test) & evaluate meaningfulness
alt meaningful edits
StopHook->>Git: check uncommitted changes
Git-->>StopHook: status
StopHook->>User: emit advisory wrapped in <system-reminder>
else small/no edits
StopHook-->>User: silent (no output)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.devcontainer/CHANGELOG.md (1)
5-5: Consider consolidating duplicate### Fixedsections under oneUnreleasedheading.There are multiple top-level
### Fixedblocks in## [Unreleased]. Merging them into a single### Fixedsection with####sub-headings will keep the changelog structure cleaner and consistent.As per coding guidelines: "Group changelog entries under the appropriate
### Added,### Changed,### Fixed, or### Removedheading" and "Use sub-headings (####) in CHANGELOG to organize by area".Also applies to: 63-63, 128-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/CHANGELOG.md at line 5, Consolidate the duplicate "### Fixed" headings in the "## [Unreleased]" section into a single "### Fixed" block: move all individual fixed entries under that one heading and group related items with "#### <area>" sub-headings (e.g., backend, docs, devcontainer) so each former "### Fixed" becomes a "####" subsection; ensure no other top-level duplicate "### Fixed" headings remain in the file and preserve all existing bullet entries under the new organized structure..devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/commit-reminder.py (1)
21-28: Consider whether common extensions are missing from SOURCE_EXTS.The extension list looks comprehensive for most use cases. You might consider adding
.cs(C#),.swift, or.phpif those are common in your user base, but this can be deferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/commit-reminder.py around lines 21 - 28, SOURCE_EXTS currently omits some common language extensions; update the frozenset named SOURCE_EXTS to include additional extensions such as ".cs", ".swift", and ".php" (and any others relevant to your codebase) so those files are treated as source code, and optionally add a short comment above SOURCE_EXTS explaining which languages are included for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/CHANGELOG.md:
- Around line 8-10: The changelog bullets expose implementation details; rewrite
them to describe user-facing behavior for "Commit reminder": replace mentions of
internal flags and mechanisms (e.g., decision: "block", systemMessage,
<system-reminder>, PostToolUse) with clear user outcomes — e.g., state that the
Commit reminder no longer prevents stopping Claude and is now an advisory
suggestion, that it only suggests commits for substantive edits (explain
criteria in plain terms like "multiple files or source/test changes"), and that
it only appears when the session actually modified files; keep the phrasing
user-focused and concise.
---
Nitpick comments:
In @.devcontainer/CHANGELOG.md:
- Line 5: Consolidate the duplicate "### Fixed" headings in the "##
[Unreleased]" section into a single "### Fixed" block: move all individual fixed
entries under that one heading and group related items with "#### <area>"
sub-headings (e.g., backend, docs, devcontainer) so each former "### Fixed"
becomes a "####" subsection; ensure no other top-level duplicate "### Fixed"
headings remain in the file and preserve all existing bullet entries under the
new organized structure.
In
@.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/commit-reminder.py:
- Around line 21-28: SOURCE_EXTS currently omits some common language
extensions; update the frozenset named SOURCE_EXTS to include additional
extensions such as ".cs", ".swift", and ".php" (and any others relevant to your
codebase) so those files are treated as source code, and optionally add a short
comment above SOURCE_EXTS explaining which languages are included for clarity.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.devcontainer/CHANGELOG.md.devcontainer/plugins/devs-marketplace/plugins/auto-code-quality/README.md.devcontainer/plugins/devs-marketplace/plugins/auto-code-quality/scripts/advisory-test-runner.py.devcontainer/plugins/devs-marketplace/plugins/session-context/README.md.devcontainer/plugins/devs-marketplace/plugins/session-context/hooks/hooks.json.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/collect-session-edits.py.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/commit-reminder.py
Commit reminder (session-context plugin): - Switch from decision:"block" to advisory systemMessage - Add tiered logic: meaningful changes suggest committing, small changes are silent - Only fire when session actually edited files (new PostToolUse collector) - Wrap output in <system-reminder> tags with explicit no-auto-commit instruction Test runner (auto-code-quality plugin): - Fix tmp file prefix from claude-edited-files to claude-cq-edited to match collector
Matches sibling scripts in the same directory. While hooks invoke via python3 (not direct execution), consistency prevents confusion.
AnExiledDev
force-pushed
the
fix/stop-hook-bugs
branch
from
226ec94 to
607f19b
Compare
Owner
Author
Review SummaryReviewed 7 files (191+/28-) covering commit-reminder refactor, new edit-tracking hook, and test-runner tmp file fix. Verified changes:
Fix applied: Set executable permission on Issues found: None (file mode was functional since hooks invoke via |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.devcontainer/CHANGELOG.md (1)
13-13: Consider simplifying to focus on user outcome rather than implementation details.While this entry does explain the impact, the tmp file prefix details are implementation specifics. A more user-focused version might be: "Advisory test runner now correctly detects edited files, fixing an issue that prevented it from running tests".
As per coding guidelines: "Write CHANGELOG entries from the user's perspective — what changed, not how it was implemented".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/CHANGELOG.md at line 13, Update the CHANGELOG entry for "Advisory test runner" to be user-focused: replace the implementation-detail sentence that mentions tmp file prefixes (`claude-cq-edited`, `claude-edited-files`) with a concise user-outcome statement such as "Advisory test runner now correctly detects edited files, fixing an issue that prevented it from running tests", ensuring the entry emphasizes the observed behavior change rather than internal filenames..devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/collect-session-edits.py (1)
33-38: Consider using a less predictable temp path (optional).The static analyzer flags
/tmp/claude-session-edits-{session_id}as potentially insecure (S108). While the risk is low here—data written is just file paths, not secrets, and this runs locally—a malicious local user could create a symlink at this path before the session starts, causing writes to go elsewhere.If you want to harden this, you could use
tempfile.gettempdir()combined with a random suffix, or create a session-specific subdirectory with restricted permissions. Given the advisory-only nature of this feature, this is optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/collect-session-edits.py around lines 33 - 38, The tmp_path construction using "/tmp/claude-session-edits-{session_id}" is predictable and can be hijacked; change the logic that builds tmp_path (where session_id and file_path are used) to create a secure, unique temp location via tempfile (e.g., tempfile.mkdtemp or tempfile.gettempdir() + a random suffix) or use a securely created file (mkstemp) and ensure you set restricted permissions on the created file/dir; update the block that opens/writes tmp_path to use that secure path and avoid race conditions (use exclusive creation semantics where possible).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.devcontainer/CHANGELOG.md:
- Line 13: Update the CHANGELOG entry for "Advisory test runner" to be
user-focused: replace the implementation-detail sentence that mentions tmp file
prefixes (`claude-cq-edited`, `claude-edited-files`) with a concise user-outcome
statement such as "Advisory test runner now correctly detects edited files,
fixing an issue that prevented it from running tests", ensuring the entry
emphasizes the observed behavior change rather than internal filenames.
In
@.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/collect-session-edits.py:
- Around line 33-38: The tmp_path construction using
"/tmp/claude-session-edits-{session_id}" is predictable and can be hijacked;
change the logic that builds tmp_path (where session_id and file_path are used)
to create a secure, unique temp location via tempfile (e.g., tempfile.mkdtemp or
tempfile.gettempdir() + a random suffix) or use a securely created file
(mkstemp) and ensure you set restricted permissions on the created file/dir;
update the block that opens/writes tmp_path to use that secure path and avoid
race conditions (use exclusive creation semantics where possible).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.devcontainer/CHANGELOG.md.devcontainer/plugins/devs-marketplace/plugins/auto-code-quality/README.md.devcontainer/plugins/devs-marketplace/plugins/auto-code-quality/scripts/advisory-test-runner.py.devcontainer/plugins/devs-marketplace/plugins/session-context/README.md.devcontainer/plugins/devs-marketplace/plugins/session-context/hooks/hooks.json.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/collect-session-edits.py.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/commit-reminder.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .devcontainer/plugins/devs-marketplace/plugins/auto-code-quality/README.md
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.
Summary
decision: "block"on every stop, forcing Claude to continue and hound the user about committing — sometimes auto-committing. Now uses advisorysystemMessagewith tiered logic: meaningful changes (3+ files, 2+ source files, or test files) get a suggestion; small changes are silent. Only fires when the session actually edited files via a new PostToolUse edit tracker./tmp/claude-edited-files-{session_id}but the collector writes to/tmp/claude-cq-edited-{session_id}— prefix mismatch meant it never found edited files. Fixed to read from the correct prefix.Test plan
systemMessage(notdecision: block)claude-cq-editedprefixSummary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation