Skip to content

feat(skills): add opt-in AST diagnostics#30918

Closed
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:feat/skills-audit-ast-deep
Closed

feat(skills): add opt-in AST diagnostics#30918
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:feat/skills-audit-ast-deep

Conversation

@Tranquil-Flow

Copy link
Copy Markdown
Contributor

Summary

This follows up on the maintainer guidance from closed PR #7436 by moving the AST-based skill checks out of Skills Guard and into an explicitly opt-in diagnostic path.

  • Add hermes skills inspect <source> --ast-deep for pre-install bundle review
  • Add hermes skills audit [name] --deep for already-installed hub skills
  • Keep AST diagnostics isolated in tools/skills_ast_audit.py
  • Keep tools/skills_guard.py untouched; this does not change install/scan verdicts
  • Frame output as diagnostic hints, not security verdicts or a trust boundary

Rationale

PR #7436 was closed because the AST logic was wired into Skills Guard as an automatic security gate. This version preserves the useful review signal while matching the suggested opt-in/diagnostic shape: hermes skills inspect --ast-deep or similar.

Test plan

  • python -m pytest tests/tools/test_skills_ast_audit.py tests/tools/test_skills_guard.py -q --timeout=30
  • python -m ruff check tools/skills_ast_audit.py tests/tools/test_skills_ast_audit.py hermes_cli/skills_hub.py hermes_cli/main.py hermes_cli/commands.py
  • python -m hermes_cli.main skills inspect --help
  • python -m hermes_cli.main skills audit --help

Notes

  • tools/skills_guard.py has no diff in this PR.
  • AST findings are labeled as diagnostic review hints only.

Add opt-in AST diagnostics for skill review without making Skills Guard stricter by default.

- Add hermes skills inspect --ast-deep to scan fetched skill bundles before installation
- Add hermes skills audit --deep to scan already-installed hub skills
- Keep AST analysis in tools/skills_ast_audit.py, separate from tools/skills_guard.py
- Label output as diagnostic hints, not security verdicts
- Cover dynamic import/access patterns: importlib, __import__(computed), getattr(computed), and __dict__[computed]

This follows the maintainer guidance from closed PR NousResearch#7436: useful AST-level analysis belongs in an opt-in diagnostic path, not in Skills Guard's default heuristic scan.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard tool/skills Skills system (list, view, manage) labels May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants