Bug plan and opencode update

[Gorkowski]

No description provided.

[Copilot]

Pull request overview

This pull request implements bug fixes and updates to OpenCode tooling, focusing on three main areas: environment file access security improvements, notebook validation enhancements, and new tool integrations (bun test runner, mkdocs builder, ast-grep refactoring).

Changes:

• Enhanced environment file access protection with template file exceptions (.env.example, .env.sample, .envrc.example)
• Added comprehensive bun test runner tool with Python backend and TypeScript wrapper
• Introduced mkdocs build validation tool for documentation integrity checks
• Integrated ast-grep refactoring tool for AST-aware code transformations
• Updated maintenance plan index with new charged coagulation kernel bugs entry
• Removed list tool permissions from multiple agents in favor of ripgrep for directory operations
• Added new issue batch review agents (description, scope, testing, technical, completeness)
• Fixed workflow builder and other tools to remove emoji/verbose output

Reviewed changes

Copilot reviewed 73 out of 73 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
adw-docs/dev-plans/maintenance/index.md	Added M5 plan entry and updated next available ID
.opencode/utils.ts	Enhanced env file access checks with template exceptions
.opencode/tool/workflow_builder.py	Removed emoji from output, cleaned up error messages
.opencode/tool/validate_notebook.py	Added inspect module, dynamic signature checking, large mtime tolerance
.opencode/tool/run_bun_test.py	New bun test runner implementation (392 lines)
.opencode/tool/run_bun_test.ts	TypeScript wrapper for bun test tool
.opencode/tool/build_mkdocs.py	New mkdocs build tool implementation (491 lines)
.opencode/tool/build_mkdocs.ts	TypeScript wrapper for mkdocs tool
.opencode/tool/refactor_astgrep.ts	New ast-grep refactoring tool (166 lines)
.opencode/tool/adw_spec.ts	Switched from shell API to process API with better error handling
.opencode/tool/adw_issues_spec.ts	New batch issue specification tool (374 lines)
.opencode/plugin/pre_tool_use.ts	Updated to use isEnvFileAccess utility function
.opencode/agent/*.md	Multiple agents updated to remove list tool, add new tools, fix paths
Test files	Comprehensive test coverage for new tools

Comments suppressed due to low confidence (5)

.opencode/utils.ts:144

• In the isEnvFileAccess function, tool names are normalized to lowercase, and safeEnvSuffixes includes ".env.sample", ".env.example", and ".envrc.example". However, the check also includes support for toolInput?.filePath as a fallback to toolInput?.file_path. This dual path handling suggests inconsistent naming conventions across tools. Consider standardizing on one parameter name or documenting why both are needed.
  .opencode/tool/adw_spec.ts:263
• The adw_spec.ts tool switches from using Bun.$ (shell API) to Bun.spawnSync (process API) for executing ADW spec commands. The error handling now checks result.exitCode !== 0 to determine failure. However, the success path at line 263 returns stdout without checking if it's empty. If a command succeeds but produces no output, the tool returns an empty string, which may be confusing. Consider adding a fallback message like "Command completed successfully (no output)" for empty stdout cases.
  .opencode/tool/validate_notebook.py:276
• The handling of convert_to_py and convert_to_ipynb attributes using getattr with a default of False is defensive, but the arguments should already be validated by the argument parser. Consider if this defensive check is necessary in both _validate_output_dir_usage and _ensure_validation_only_flags, or if it can be simplified since argparse guarantees these attributes exist.
  .opencode/tool/validate_notebook.py:31
• The import of inspect is added but should be placed with the other imports at the top of the file according to PEP 8. Currently it's inserted at line 31 after argparse but before json, which breaks alphabetical ordering of standard library imports.
  .opencode/tool/create_workspace.py:142
• The assertion assert result_adw_id is not None at line 141 is added with a comment "guaranteed when error is None". However, this assertion occurs after the check for error is None at line 126, meaning if error is not None, the code would have already returned. While the assertion is technically correct, it's redundant given the control flow. Consider removing it or moving the logic to make the guarantee more explicit in the type system.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The CHECK_SYNC_MTIME_TOLERANCE constant is set to 120_000_000_000 (120 billion nanoseconds, or 120 seconds). This tolerance is extremely large and may allow files that are significantly out of sync to pass validation. Consider whether 2 minutes is an appropriate tolerance for notebook sync checks. A more typical tolerance would be in the range of 1-5 seconds.

[Copilot]

The code uses introspection to check if sync_notebook_script supports a script argument by examining parameters and checking for VAR_POSITIONAL or VAR_KEYWORD. This is complex and fragile. If the signature changes, this detection logic may break silently. The try-except TypeError fallback is good, but consider documenting this signature requirement or using a more explicit check (e.g., versioning the function).