Add pytest suite for critical plugin scripts (241 tests)

[AnExiledDev]

Summary

• Adds 241 pytest tests covering 6 plugin scripts that previously had zero test coverage
• All tests pass; existing npm test unaffected
• Focuses on the highest-risk untested code: security guards, scope enforcement, and agent safety

Coverage

Plugin Script	Tests	What's Covered
block-dangerous.py	46	All 22 dangerous command regex patterns, safe command false-positive checks
guard-workspace-scope.py	40	Blacklist/scope/allowlist logic, 2-layer bash enforcement, primary command extraction
guard-protected.py	55	All protected file patterns (secrets, locks, keys, certs, auth dirs, SSH keys)
guard-protected-bash.py	24	Write target extraction from bash commands, protected path integration
guard-readonly-bash.py	63	General-readonly & git-readonly modes, bypass prevention (path prefix, backslash, chaining)
redirect-builtin-agents.py	13	All redirect mappings, passthrough, output structure, error handling

Bugs Discovered During Testing

The test suite documents several existing bugs in the source code (tests are written to match current behavior):

1. --force-with-lease false positive in block-dangerous.py — the \bgit\s+push\s+--force\b regex matches --force-with-lease because \b treats the - as a word boundary
2. rm -rf /etc not caught — only bare rm -rf / and rm -rf ~ are blocked; rm -rf /etc passes through
3. Append redirect >> parsing in guard-protected-bash.py — regex (?:>|>>) matches > before >> due to alternation order
4. Duplicate cat > targets in guard-protected-bash.py — both generic redirect and cat-specific patterns match

Test Infrastructure

• tests/conftest.py — importlib.util loader for plugin scripts (no package structure)
• npm run test:plugins — runs pytest suite
• npm run test:all — runs both JS and Python tests

Test plan

• pytest tests/ -v — 241 passed
• npm test — existing JS tests unaffected
• Verify CI picks up the new test scripts (if pytest is in CI)

Summary by CodeRabbit

• Tests

  • Added a comprehensive plugin test suite (241 pytest tests) covering command blocking, path protections, bash behaviors, workspace scope checks, and agent redirection across six plugins.

• Chores

  • Added npm scripts test:plugins and test:all to run the plugin tests.

• Documentation

  • Updated changelog to document the new plugin testing effort and tidy formatting.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e0b9583 and e5b18f6.

📒 Files selected for processing (3)

• .devcontainer/CHANGELOG.md
• tests/plugins/test_block_dangerous.py
• tests/plugins/test_redirect_builtin_agents.py

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/plugins/test_redirect_builtin_agents.py

---

📝 Walkthrough

Walkthrough

Adds a comprehensive pytest plugin test suite (241 tests) plus a test loader in tests/conftest.py, and two npm scripts to run the tests. Documentation changelog updated to document the new test suite.

Changes

Cohort / File(s)	Summary
Changelog & npm scripts \.devcontainer/CHANGELOG.md, package.json	Changelog updated to document the Unreleased plugin test suite. Added npm scripts: test:plugins (pytest tests/ -v) and test:all (npm test && pytest tests/ -v).
Test infrastructure tests/conftest.py	New conftest with PLUGINS_ROOT and _load_script() helper that loads plugin script files as modules; preloads six plugin modules for use by tests.
Plugin test suites tests/plugins/test_block_dangerous.py, tests/plugins/test_guard_protected.py, tests/plugins/test_guard_protected_bash.py, tests/plugins/test_guard_readonly_bash.py, tests/plugins/test_guard_workspace_scope.py, tests/plugins/test_redirect_builtin_agents.py	Six new pytest modules providing extensive coverage for dangerous-command-blocker, protected-files-guard (and bash variant), readonly-bash guard, workspace-scope guard, and redirect-built-in-agents. Include helpers, many parametrized cases, edge-case coverage, and assertions for blocked vs allowed behaviors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 In a burrow of tests I hide,
I load each plugin far and wide.
Two-forty-one hops, assertions clear,
Blocking danger, keeping things near.
A rabbit cheers: the suite is tried!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding a pytest suite with 241 tests for plugin scripts, which is the primary purpose of this PR.
Description check	✅ Passed	The PR description covers all required sections: a comprehensive summary with test coverage breakdown, related bugs discovered, test infrastructure details, and test plan status. All template sections are addressed.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch test-gaps

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/conftest.py (1)

36-39: Add null check for spec.loader.

spec_from_file_location can return None for the spec, and even when it succeeds, spec.loader could theoretically be None. Adding a guard prevents cryptic AttributeError if the path or module type is unexpected.

🛡️ Proposed defensive check

     spec = importlib.util.spec_from_file_location(module_name, script_path)
+    if spec is None or spec.loader is None:
+        raise ImportError(f"Cannot load module from {script_path}")
     module = importlib.util.module_from_spec(spec)
 
     spec.loader.exec_module(module)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/conftest.py` around lines 36 - 39, The current code calls
spec.loader.exec_module(module) without guarding against spec or spec.loader
being None; update the import block that uses
importlib.util.spec_from_file_location and the local variable spec to check that
spec is not None and spec.loader is not None before calling
spec.loader.exec_module(module), and raise or log a clear error (or skip) if
either is missing so you avoid an AttributeError when loading unexpected paths
or module types.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/plugins/test_redirect_builtin_agents.py`:
- Around line 21-37: The run_main helper must always return an int exit code;
update the SystemExit handling in run_main so you normalize exc.code to an int
(e.g., treat None as 0, and coerce non-int codes with a safe fallback like
int(...) in a try/except and default to 1 on failure) and also return a
deterministic int when main() returns normally (replace the current return of
None with 0). Modify the logic around redirect_builtin_agents.main() and the
final return so the function signature tuple[int, str] is always honored.

---

Nitpick comments:
In `@tests/conftest.py`:
- Around line 36-39: The current code calls spec.loader.exec_module(module)
without guarding against spec or spec.loader being None; update the import block
that uses importlib.util.spec_from_file_location and the local variable spec to
check that spec is not None and spec.loader is not None before calling
spec.loader.exec_module(module), and raise or log a clear error (or skip) if
either is missing so you avoid an AttributeError when loading unexpected paths
or module types.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5cf554c and e0b9583.

📒 Files selected for processing (11)

• .devcontainer/CHANGELOG.md
• package.json
• tests/__init__.py
• tests/conftest.py
• tests/plugins/__init__.py
• tests/plugins/test_block_dangerous.py
• tests/plugins/test_guard_protected.py
• tests/plugins/test_guard_protected_bash.py
• tests/plugins/test_guard_readonly_bash.py
• tests/plugins/test_guard_workspace_scope.py
• tests/plugins/test_redirect_builtin_agents.py