refactor(guardrails): split guardrail.ts into 6 modules (Issue #150)#154
refactor(guardrails): split guardrail.ts into 6 modules (Issue #150)#154
Conversation
Split monolithic guardrail.ts (1944 lines) into modular architecture:
- guardrail-patterns.ts (225L): constants, pure utilities
- guardrail-context.ts (343L): GuardrailContext type, createContext()
- guardrail-review.ts (198L): auto-review pipeline, dual review gate
- guardrail-git.ts (432L): branch hygiene, merge gates, CI checks
- guardrail-access.ts (296L): file access control, delegation, tracking
- guardrail.ts (568L): entry point, hook registration
Architecture: Context Object + Handler Factories pattern.
Dependency DAG: patterns <- context <- {review, git, access} <- entry.
All files under 800-line limit. 34/34 scenario tests pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex CLI <noreply@openai.com>
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
New PR opened -- automated review will run on the next push. To trigger a manual review, comment |
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Pull request overview
Refactors the guardrail plugin by splitting the previously monolithic guardrail.ts into smaller, responsibility-focused modules while preserving existing behavior.
Changes:
- Extracted shared constants/pure helpers into
guardrail-patterns.ts. - Introduced
GuardrailContext+createContext()to centralize state paths, gating helpers, and utilities. - Split hook logic into dedicated handler factories for access control, auto-review, and git/merge gates.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/guardrails/profile/plugins/guardrail.ts | New entrypoint wiring: builds context and composes access/review/git handlers into the plugin hooks. |
| packages/guardrails/profile/plugins/guardrail-context.ts | Adds GuardrailContext and createContext() to own core state, gating helpers, and shared utilities. |
| packages/guardrails/profile/plugins/guardrail-patterns.ts | Centralizes regex patterns and pure helper utilities previously embedded in guardrail.ts. |
| packages/guardrails/profile/plugins/guardrail-access.ts | Moves file/tool access policy enforcement + mutation tracking into a dedicated handler module. |
| packages/guardrails/profile/plugins/guardrail-review.ts | Moves auto-review pipeline, dual-review gate logic, and Codex-review detection into a dedicated module. |
| packages/guardrails/profile/plugins/guardrail-git.ts | Moves branch hygiene, merge gates, CI checks, and related advisories into a dedicated module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| free, | ||
| has, | ||
| line, | ||
| list, |
There was a problem hiding this comment.
list is imported from ./guardrail-patterns but not used in this module. Removing the unused import will keep the dependency surface minimal and avoid confusion during future refactors.
| list, |
| if (/\bgit\s+merge(\s|$)/i.test(cmd) || /\bgh\s+pr\s+merge(\s|$)/i.test(cmd)) { | ||
| const checks = review.checklist(data) | ||
| if (checks.score < 3) { | ||
| out.output = (out.output || "") + `\n\nCompletion checklist (${checks.score}/${checks.total}): ${checks.summary}\nBlocking: ${checks.blocking.join(", ")}` | ||
| } | ||
| } |
There was a problem hiding this comment.
The merge/PR-merge detection regex is repeated here even though the same condition was already evaluated earlier in bashBeforeGit (see the first if block). Consider computing a single isMerge boolean once and reusing it to reduce duplication and keep the merge-gating logic easier to maintain.
Codex CLI review P2: helper modules had `export default { id, server }`
which matches the plugin shape. While opencode.json explicitly lists
plugins (no auto-discovery), removing these prevents confusion and
keeps helper modules as pure library code.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…licate merge regex - Remove unused `list` import from guardrail-context.ts - Compute isMerge/isPrMerge booleans once in bashBeforeGit and reuse - Same pattern applied to afterBashGit handler (afterIsMerge, afterIsPrMerge, etc.) Addresses review comments on PR #154. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix(guardrails): address PR #154 review — remove unused import, deduplicate merge regex
Summary
guardrail.ts(1944 lines) into 6 modules, all under 800 linespatterns <- context <- {review, git, access} <- guardrail.tsModule breakdown
guardrail-patterns.tsguardrail-context.tsguardrail-review.tsguardrail-git.tsguardrail-access.tsguardrail.tsCloses #150
Test plan
🤖 Generated with Claude Code + Codex CLI