Conversation
… post-deploy verify hooks - CI hard block: runs `gh pr checks` before allowing `gh pr merge`, blocks if any check is failing/pending - Architecture layer advisory: warns when UI layer imports DB layer or API layer imports UI layer - Post-deploy verification advisory: reminds to verify deployment status after `gh pr merge` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds additional high-priority Guardrails hooks to improve merge safety (CI gating), enforce architectural boundaries (advisory), and prompt operational follow-ups after merges.
Changes:
- Add a pre-merge CI gate for
gh pr mergeby runninggh pr checksand blocking merges when checks are failing/pending. - Add an architecture-layer advisory on edits/writes to warn about UI↔DB and API↔UI import direction violations.
- Add a post-merge advisory to remind users to verify deployments after
gh pr merge.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const proc = Bun.spawn(["gh", "pr", "checks", ...(prArg ? [prArg] : [])], { | ||
| cwd: input.worktree, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }) | ||
| const [ciOut] = await Promise.all([ | ||
| new Response(proc.stdout).text(), | ||
| new Response(proc.stderr).text(), | ||
| proc.exited, | ||
| ]) | ||
| if (proc.exitCode !== 0 || /fail|pending/i.test(ciOut)) { | ||
| await mark({ last_block: "bash", last_command: cmd, last_reason: "CI checks not all green" }) | ||
| throw new Error(text("merge blocked: CI checks not all green — run `gh pr checks` to verify")) | ||
| } | ||
| } catch (e) { | ||
| if (String(e).includes("blocked")) throw e | ||
| // gh unavailable or network failure — warn so user knows CI was not verified |
There was a problem hiding this comment.
gh pr checks is spawned without any timeout/abort handling. If gh hangs (network/auth prompt), this hook can block gh pr merge indefinitely. Consider adding a reasonable timeout and killing the process (or treating timeout as a non-verifiable-CI warning) so merges don’t stall the session.
| const proc = Bun.spawn(["gh", "pr", "checks", ...(prArg ? [prArg] : [])], { | |
| cwd: input.worktree, | |
| stdout: "pipe", | |
| stderr: "pipe", | |
| }) | |
| const [ciOut] = await Promise.all([ | |
| new Response(proc.stdout).text(), | |
| new Response(proc.stderr).text(), | |
| proc.exited, | |
| ]) | |
| if (proc.exitCode !== 0 || /fail|pending/i.test(ciOut)) { | |
| await mark({ last_block: "bash", last_command: cmd, last_reason: "CI checks not all green" }) | |
| throw new Error(text("merge blocked: CI checks not all green — run `gh pr checks` to verify")) | |
| } | |
| } catch (e) { | |
| if (String(e).includes("blocked")) throw e | |
| // gh unavailable or network failure — warn so user knows CI was not verified | |
| const ciCheckTimeoutMs = 15000 | |
| const proc = Bun.spawn(["gh", "pr", "checks", ...(prArg ? [prArg] : [])], { | |
| cwd: input.worktree, | |
| stdout: "pipe", | |
| stderr: "pipe", | |
| }) | |
| let timeoutId: ReturnType<typeof setTimeout> | undefined | |
| try { | |
| const timeout = new Promise<never>((_, reject) => { | |
| timeoutId = setTimeout(() => { | |
| try { proc.kill() } catch { } | |
| reject(new Error("gh pr checks timed out")) | |
| }, ciCheckTimeoutMs) | |
| }) | |
| const [ciOut] = await Promise.race([ | |
| Promise.all([ | |
| new Response(proc.stdout).text(), | |
| new Response(proc.stderr).text(), | |
| proc.exited, | |
| ]), | |
| timeout, | |
| ]) | |
| if (proc.exitCode !== 0 || /fail|pending/i.test(ciOut)) { | |
| await mark({ last_block: "bash", last_command: cmd, last_reason: "CI checks not all green" }) | |
| throw new Error(text("merge blocked: CI checks not all green — run `gh pr checks` to verify")) | |
| } | |
| } finally { | |
| if (timeoutId) clearTimeout(timeoutId) | |
| } | |
| } catch (e) { | |
| if (String(e).includes("blocked")) throw e | |
| // gh unavailable, network failure, or timeout — warn so user knows CI was not verified |
| } catch (e) { | ||
| if (String(e).includes("blocked")) throw e | ||
| // gh unavailable or network failure — warn so user knows CI was not verified | ||
| await mark({ last_block: "bash:ci-warn", last_command: cmd, last_reason: "CI check verification failed" }) |
There was a problem hiding this comment.
In the fallback path (when gh pr checks can’t be executed), the code only calls mark(...) but doesn’t emit any user-visible warning. That contradicts the inline comment (“warn so user knows CI was not verified”) and the PR description’s fallback-to-warning behavior. Consider appending a warning to the bash tool output for gh pr merge (e.g., via tool.execute.after when the last state indicates bash:ci-warn) so the user actually sees it.
| await mark({ last_block: "bash:ci-warn", last_command: cmd, last_reason: "CI check verification failed" }) | |
| await mark({ last_block: "bash:ci-warn", last_command: cmd, last_reason: "CI check verification failed" }) | |
| process.stderr.write(`${text("warning: unable to verify CI checks before merge; proceeding without CI verification")}\n`) |
| } | ||
|
|
||
| // Post-merge deployment verification advisory | ||
| if (item.tool === "bash" && /\bgh\s+pr\s+merge\b/i.test(str(item.args?.command))) { |
There was a problem hiding this comment.
The post-merge verification message is emitted for any gh pr merge invocation, even when the merge command fails. Since the bash tool provides out.metadata.exit, consider gating this advisory on a successful exit code (e.g., exit === 0) so it only triggers after a successful merge.
| if (item.tool === "bash" && /\bgh\s+pr\s+merge\b/i.test(str(item.args?.command))) { | |
| if ( | |
| item.tool === "bash" && | |
| /\bgh\s+pr\s+merge\b/i.test(str(item.args?.command)) && | |
| out.metadata?.exit === 0 | |
| ) { |
de46803 to
4e3c710
Compare
Issue for this PR
Closes #115
Type of change
What does this PR do?
Adds 3 high-priority guardrail hooks to
guardrail.ts(708 → 757 lines):CI Hard Block (tool.execute.before): Runs
gh pr checksbefore allowinggh pr merge. Blocks if any check is failing/pending. Falls back to warning ifghis unavailable (with state tracking viamark()).Architecture Layer Advisory (tool.execute.after): Warns when source file edits introduce cross-layer imports:
Post-deploy Verification Advisory (tool.execute.after): Reminds to verify deployment status after
gh pr mergesucceeds.All existing functionality is preserved. The file remains under 800 lines (757).
How did you verify your code works?
bun turbo typecheck— 13/13 passmark())Checklist