Repository navigation
Accept Absent, Empty, or Wildcard SessionEnd Matchers - #1834
Conversation
registration_problems rejected any SessionEnd group carrying a matcher key at all, so an empty matcher or `*` was reported as running on one exit reason when it runs on every one, the opposite of what happens. Add matcher_covers_every_exit_reason, the SessionEnd counterpart of matcher_sees_bash, and use it in place of the bare `"matcher" in group` check, so only a matcher naming specific reasons is reported. Fixes #1638 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
registration_problems now accepts an absent, empty, or `*` SessionEnd matcher as covering every exit reason, but the installer's own group selection still looked only for a matcherless group when deciding which one to reuse. Reinstalling onto a `*` or `""` group therefore left it emptied and added a second, matcherless one, disagreeing with what the report now calls sound. Route the selection through the same matcher_covers_every_exit_reason check so both sides agree. Found by a local strict review pass on this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 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. Comment |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Behavior and tests align with the stated intent, with only minor in-file comment wording needing follow-up for clarity.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR fixes false-positive reporting and reinstall behavior for the Claude Code host-safety kit by treating SessionEnd matchers that mean “all exit reasons” (absent, empty string, or *) as valid, and by reusing those groups during reinstall to avoid duplicates.
Changes:
- Add
matcher_covers_every_exit_reason()to classify SessionEnd matchers that cover all exit reasons. - Use that helper in
registration_problems()and in the reinstall reuse logic to avoid incorrect “under a matcher” reports and duplicate groups. - Expand unit tests to cover absent/empty/
*/reason-specific matcher shapes and reinstall reuse for""and*.
| File | Description |
|---|---|
| host-setup/agent-safety/claude/install.py | Adds and applies SessionEnd “covers all reasons” matcher classification for reporting and reinstall group reuse. |
| host-setup/agent-safety/claude/test_install.py | Adds tests for reinstall reuse with ""/* and for excluding those shapes from “under a matcher” defect reporting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ptr727
added
the
comments
Both comments described a SessionEnd matcher's effect in terms that predated matcher_covers_every_exit_reason: one said a matcher present at all narrows the sweep to one exit reason, the other said the reused or created group always carries no matcher. Neither holds once an absent, empty, or `*` matcher counts as covering every reason. Reword both to state what actually gates the sweep now. Found by GitHub Copilot review on PR #1834. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The change is narrowly scoped, corrects the matcher semantics consistently across report and reinstall paths, and is covered by passing unit tests for the relevant cases.
Review effort: Lite
Findings: None
Resolved since last review (1)
… Fixes to Main (#1839) Promotes `develop` to `main`. ## Carried - #1830: stops the prose gate reading a `#` after `$` or `${` as a comment marker, per #1646. - #1832: corrects the comment on the `pr_review.py` quota-refusal remedy, per #1647. - #1834: accepts an absent, empty, or wildcard SessionEnd matcher as covering every exit reason, per #1638. - #1838: routes every question and every offer of more work through the interface's prompt mechanism, blocking or not. - #1841: describes a SessionEnd matcher by its shape in the installer report, following #1834. Closes #1646 Closes #1647 Closes #1638 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Summary
registration_problemsrejected any SessionEnd group carrying amatcherkey at all, so an empty matcher or*was reported as running on one exit reason when it actually runs on every one. Addmatcher_covers_every_exit_reason, the SessionEnd counterpart ofmatcher_sees_bash, and use it in place of the bare"matcher" in groupcheck, so only a matcher naming specific reasons is reported.main's reinstall path) still looked only for a matcherless group when deciding which SessionEnd group to reuse, disagreeing with what the report now calls sound. Route that selection through the samematcher_covers_every_exit_reasoncheck so reinstalling onto an already-sound*/""group reuses it instead of leaving it emptied and adding a duplicate.test_install.pycover all four matcher shapes (absent, empty,*, reason-specific) on the report side, and reinstall-reuse on the*/""shapes.Closes on promotion: #1638
Test plan
python3 -m unittest host-setup.agent-safety.claude.test_install(65 tests, all pass)🤖 Generated with Claude Code