fix: Hades smart cleanup — dead code, duplication, suppressions#132
fix: Hades smart cleanup — dead code, duplication, suppressions#132
Conversation
…ressions 32 files changed (-354 lines net). Audited by 17 Opus agents across 3 phases with adversarial verification. Smart ID: SMART-2026-02-15-1771184309. Key eliminations: - hookify: extract shared hook_runner.py from 4 near-identical handlers - exodia: extract lib.sh shared functions, add permit.sh active subcommand - Delete .gemini/, docs/AGENTS.md, hookify empty packages, template cruft - Narrow bare except clauses, fix Pyright type annotations - Sync AGENTS.md decision tree with CLAUDE.md, fix stale doc references - Standardize plugin.json fields across exodia, feature-dev, otelwiki Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughRemoves Gemini config/styleguide, centralizes hook execution via a new hook_runner and updates hook modules to delegate to it, shifts skills/agents to plugin-scoped locations in docs/ and ADRs, updates plugin manifests and templates, and adds/adjusts exodia helper scripts and permit handling. Changes
Sequence Diagram(s)sequenceDiagram
participant HookScript as Hook script (e.g., pretooluse)
participant Runner as hook_runner.run_hook
participant Loader as Rule loader (load_rules)
participant Engine as RuleEngine
participant Permit as permit.sh / external helpers
HookScript->>Runner: invoke run_hook(hook_event_name, fixed_event?)
Runner->>Runner: read stdin -> input_data
Runner->>Loader: load_rules(event)
Loader-->>Runner: rules
Runner->>Engine: RuleEngine(rules).evaluate(input_data)
Engine->>Permit: (if _hades_permit_active) call permit.sh via bash
Permit-->>Engine: permit result
Engine-->>Runner: evaluation result
Runner-->>HookScript: print JSON result / systemMessage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! 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. Comment |
Summary of ChangesHello @ANcpLua, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a comprehensive cleanup and refactoring initiative, primarily targeting the Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent pull request that performs a significant and valuable cleanup across the repository. The refactoring to extract shared logic into hook_runner.py for Python hooks and lib.sh for shell scripts is a great improvement for maintainability and follows the DRY principle. The removal of dead code, standardization of plugin.json files, and modernization of the plugin template are all welcome changes. The improved exception handling and type hints also increase code quality. I've found a couple of minor areas for improvement, but overall this is a very strong contribution.
| local now expires_epoch | ||
| now="$(date -u +%s)" | ||
| expires_epoch="$(grep -o '"expires_epoch":[0-9]*' "$PERMIT_FILE" | grep -o '[0-9]*')" |
There was a problem hiding this comment.
The active function uses grep to parse expires_epoch from JSON. This can be brittle and will fail with an error if the field is not found because set -u is enabled. For consistency with other scripts in this directory and for improved robustness, consider using jq with a grep fallback. This also allows you to safely handle cases where the field might be missing.
| local now expires_epoch | |
| now="$(date -u +%s)" | |
| expires_epoch="$(grep -o '"expires_epoch":[0-9]*' "$PERMIT_FILE" | grep -o '[0-9]*')" | |
| local now expires_epoch | |
| now="$(date -u +%s)" | |
| if has_jq; then | |
| expires_epoch="$(jq -r '.expires_epoch // 0' "$PERMIT_FILE")" | |
| else | |
| expires_epoch="$(grep -o '"expires_epoch":[0-9]*' "$PERMIT_FILE" | grep -o '[0-9]*' || echo 0)" | |
| fi |
References
- Repository style guide requires proper error handling in shell scripts (line 115). The current implementation can fail with an error if the JSON field is missing. (link)
| ### Added | ||
|
|
||
| - **docs/designs/ directory**: Referenced in CLAUDE.md but didn't exist, now created with .gitkeep | ||
|
|
||
| ### Added | ||
|
|
||
| - **ecosystem architecture section**: New Section 8 in `docs/ARCHITECTURE.md` documenting the full developer setup beyond this marketplace (LSP plugins, IDE MCP, Service MCP, Browser MCP), how layers compose, and why separation matters |
There was a problem hiding this comment.
There appears to be a duplicated ### Added heading. You can merge the content under a single heading to improve readability.
| ### Added | |
| - **docs/designs/ directory**: Referenced in CLAUDE.md but didn't exist, now created with .gitkeep | |
| ### Added | |
| - **ecosystem architecture section**: New Section 8 in `docs/ARCHITECTURE.md` documenting the full developer setup beyond this marketplace (LSP plugins, IDE MCP, Service MCP, Browser MCP), how layers compose, and why separation matters | |
| ### Added | |
| - **docs/designs/ directory**: Referenced in CLAUDE.md but didn't exist, now created with .gitkeep | |
| - **ecosystem architecture section**: New Section 8 in `docs/ARCHITECTURE.md` documenting the full developer setup beyond this marketplace (LSP plugins, IDE MCP, Service MCP, Browser MCP), how layers compose, and why separation matters |
There was a problem hiding this comment.
Pull request overview
Refactors and de-duplicates hook and “Smart/Hades” infrastructure across the plugin marketplace, while removing dead code and syncing documentation/templates to the repo’s current layout.
Changes:
- Extracts shared hookify hook execution logic into
hookify/core/hook_runner.py, leaving thin per-event wrappers. - DRYs exodia smart scripts via a shared
lib.shand addspermit.sh activefor a standardized “permit enabled” check. - Removes dead/duplicated docs/config (.gemini, docs/AGENTS.md), syncs specs/ADRs, and modernizes the plugin template + plugin manifests.
Reviewed changes
Copilot reviewed 31 out of 34 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tooling/templates/plugin-template/hooks/hooks.json | Updates template hook config format and adds an example PreToolUse hook |
| tooling/templates/plugin-template/agents/.gitkeep | Removes placeholder file from template |
| tooling/templates/plugin-template/README.md | Simplifies/modernizes plugin template README and structure example |
| tooling/templates/plugin-template/CLAUDE.md | Adds template plugin CLAUDE context file |
| tooling/templates/plugin-template/.claude-plugin/plugin.json | Updates template manifest placeholders and removes template-only comment |
| plugins/otelwiki/.claude-plugin/plugin.json | Adds keywords metadata |
| plugins/metacognitive-guard/hooks/scripts/ralph-loop.sh | Emits warning when jq is missing (instead of silent disable) |
| plugins/metacognitive-guard/hooks/scripts/epistemic-guard.sh | Delegates permit bypass check to exodia permit.sh active |
| plugins/hookify/hooks/userpromptsubmit.py | Converts to thin wrapper calling shared hook runner |
| plugins/hookify/hooks/stop.py | Converts to thin wrapper calling shared hook runner |
| plugins/hookify/hooks/pretooluse.py | Converts to thin wrapper calling shared hook runner |
| plugins/hookify/hooks/posttooluse.py | Converts to thin wrapper calling shared hook runner |
| plugins/hookify/hooks/init.py | Removed empty package marker (per PR description) |
| plugins/hookify/hookify/utils/init.py | Removed empty utils package marker (per PR description) |
| plugins/hookify/hookify/core/rule_engine.py | Narrows permit-check exception handling; fixes Optional typing |
| plugins/hookify/hookify/core/hook_runner.py | New shared hook execution implementation |
| plugins/feature-dev/.claude-plugin/plugin.json | Adds repository/license/keywords metadata |
| plugins/exodia/scripts/smart/session-state.sh | Uses shared has_jq helper |
| plugins/exodia/scripts/smart/permit.sh | Uses shared atomic_write; adds active subcommand |
| plugins/exodia/scripts/smart/lib.sh | Introduces shared helpers (has_jq, has_flock, atomic_write) |
| plugins/exodia/scripts/smart/ledger.sh | Uses shared atomic_write for append |
| plugins/exodia/scripts/smart/checkpoint.sh | Uses shared has_jq helper |
| plugins/exodia/.claude-plugin/plugin.json | Adds keywords metadata |
| plugins/dotnet-architecture-lint/scripts/precheck-dotnet.py | Delegates permit bypass to permit.sh active; narrows exception handling |
| plugins/dotnet-architecture-lint/CLAUDE.md | Removes stale reference to non-existent skill file |
| docs/specs/spec-0001-framework-architecture.md | Updates repo layout references (removes repo-level skills/) |
| docs/decisions/ADR-0001-marketplace-architecture.md | Updates repo layout references (skills/agents live inside plugins) |
| docs/ARCHITECTURE.md | Removes .gemini/ from tree; updates docs entry |
| docs/AGENTS.md | Deletes duplicated subset doc (root AGENTS.md becomes SSOT) |
| docs/designs/.gitkeep | Adds missing directory placeholder referenced elsewhere |
| CHANGELOG.md | Documents refactors, removals, and doc sync updates |
| AGENTS.md | Updates routing tree + command counts and adds new decision branches |
| .gemini/styleguide.md | Removed Gemini config/doc (dead code purge) |
| .gemini/config.yaml | Removed Gemini config (dead code purge) |
| "PreToolUse": [ | ||
| { | ||
| "matcher": "Bash|Edit|Write", | ||
| "hooks": [ |
There was a problem hiding this comment.
The template PreToolUse matcher omits the MultiEdit tool, so any hook logic won’t run for MultiEdit operations by default. Other repo hooks (e.g. plugins/hookify/hooks/hooks.json) include MultiEdit in these matchers; consider updating the template matcher to include MultiEdit for parity and to avoid surprising gaps for new plugin authors.
| expires_epoch="$(grep -o '"expires_epoch":[0-9]*' "$PERMIT_FILE" | grep -o '[0-9]*')" | ||
|
|
There was a problem hiding this comment.
active() runs under set -euo pipefail and assigns expires_epoch via a grep | grep pipeline. If the permit file is malformed (missing expires_epoch) that pipeline will fail and can cause the script to exit early due to -e/pipefail rather than simply returning inactive. Consider making expires_epoch extraction fail-closed (default 0 / return 1) so callers using permit.sh active don’t get unexpected hard failures.
| expires_epoch="$(grep -o '"expires_epoch":[0-9]*' "$PERMIT_FILE" | grep -o '[0-9]*')" | |
| expires_epoch="$(sed -n 's/.*"expires_epoch":\([0-9][0-9]*\).*/\1/p' "$PERMIT_FILE")" | |
| if [ -z "$expires_epoch" ]; then | |
| return 1 | |
| fi |
CHANGELOG.md
Outdated
|
|
||
| ### Added | ||
|
|
There was a problem hiding this comment.
There are two consecutive ### Added sections here, which breaks the Keep a Changelog structure and makes the Unreleased section harder to scan. These entries should be merged under a single ### Added heading.
| ### Added |
CHANGELOG.md
Outdated
|
|
||
| - **hookify: extract shared hook_runner.py**: 4 near-identical handlers (pretooluse/posttooluse/stop/userpromptsubmit) reduced from 60-78 lines each to 18-line thin wrappers. Shared logic in `hookify/core/hook_runner.py` (~197 lines eliminated) | ||
| - **exodia smart scripts: extract lib.sh shared functions**: `has_jq()`, `has_flock()`, `atomic_write()` extracted from inline duplications across permit.sh, ledger.sh, checkpoint.sh, session-state.sh | ||
| - **exodia permit.sh: add `active` subcommand**: Canonical permit-active check replaces 3 inline reimplementations in epistemic-guard.sh, precheck-dotnet.py, and rule_engine.py |
There was a problem hiding this comment.
This entry says the new permit.sh active check replaces inline implementations in epistemic-guard.sh, precheck-dotnet.py, and rule_engine.py, but hookify’s rule_engine.py still reads .smart/delete-permit.json directly (no delegation to permit.sh). Either update the changelog text (and PR summary if applicable) or switch rule_engine to use the canonical permit.sh active check to match the documented behavior.
| - **exodia permit.sh: add `active` subcommand**: Canonical permit-active check replaces 3 inline reimplementations in epistemic-guard.sh, precheck-dotnet.py, and rule_engine.py | |
| - **exodia permit.sh: add `active` subcommand**: Adds canonical permit-active check for use by epistemic-guard.sh, precheck-dotnet.py, and rule_engine.py in place of their inline implementations (migration in progress) |
- CLAUDE.md: replace deleted docs/AGENTS.md with actual docs/ contents (QUICK-REFERENCE.md, ENGINEERING-PRINCIPLES.md, designs/) - CHANGELOG: merge duplicate Added sections, fix plugin.json wording - AGENTS.md: add missing deep-analysis.md to compressed docs index Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- permit.sh: use jq with grep fallback for expires_epoch parsing (fail-safe under set -euo pipefail) - lib.sh: add has_jq(), has_flock(), atomic_write() shared helpers - CHANGELOG.md: merge duplicate ### Added headings, correct permit.sh scope claim - plugin template hooks.json: add MultiEdit to matcher for parity Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| # Check expiration | ||
| local now expires_epoch | ||
| now="$(date -u +%s)" | ||
| expires_epoch="$(grep -o '"expires_epoch":[0-9]*' "$PERMIT_FILE" | grep -o '[0-9]*')" | ||
| if has_jq; then | ||
| expires_epoch="$(jq -r '.expires_epoch // 0' "$PERMIT_FILE")" | ||
| else | ||
| expires_epoch="$(grep -o '"expires_epoch":[0-9]*' "$PERMIT_FILE" | grep -o '[0-9]*' || echo 0)" | ||
| fi |
There was a problem hiding this comment.
The PR description claims to add a permit.sh active subcommand, but this implementation only updates the validate() function to use has_jq() from lib.sh. The active subcommand mentioned in the PR description and referenced by precheck-dotnet.py (line 129: ['bash', matches[0], 'active']) appears to be missing from the implementation.
|
|
||
| print(json.dumps(result), file=sys.stdout) | ||
|
|
||
| except Exception as e: |
There was a problem hiding this comment.
The PR description claims to narrow bare exceptions in hook_runner.py, but line 53 still uses except Exception as e which is a broad catch-all. For consistency with the narrowed exceptions in rule_engine.py (line 50) and precheck-dotnet.py (line 133), this should be narrowed to specific exception types like (json.JSONDecodeError, KeyError, AttributeError) or similar relevant exceptions that could occur during rule evaluation.
| except Exception as e: | |
| except (json.JSONDecodeError, KeyError, AttributeError, ValueError) as e: |
Summary
hook_runner.pyfrom 4 near-identical handlers (215→128 lines, ~197 lines eliminated)lib.shshared functions (has_jq,has_flock,atomic_write), addpermit.sh activesubcommand replacing 3 inline reimplementations.gemini/directory,docs/AGENTS.md(subset of root), hookify empty packages (matchers/,utils/,hooks/__init__.py), templateagents/.gitkeepexcept Exceptionin rule_engine.py and precheck-dotnet.py, fix Pyright type annotations (Optional[str],Optional[Dict]), warn on missing jq in ralph-loop.shskills/refs fixed in spec-0001/ADR-0001,.gemini/removed from ARCHITECTURE.md tree32 files changed, 310 insertions, 589 deletions (-279 lines net)
Hades Audit Trail
SMART-2026-02-15-1771184309xTt3dMIk91F2OlEDui6ZTest plan
permit.sh activereturns correct exit codes (0=active, 1=inactive/expired)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores