fix(core): route safety approval prompts through logging or renderer#613
fix(core): route safety approval prompts through logging or renderer#613pavsoss wants to merge 2 commits into
Conversation
|
@pavsoss is attempting to deploy a commit to the sreerevanth's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe safety approval handler now supports optional prompt rendering and logs its default output. The watch CLI supplies a structured safety-check renderer through a wrapped approval callback. ChangesSafety approval rendering
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧪 PR Test Results
Python 3.12 · commit d3ae162 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
agentwatch/cli/main.py (1)
364-380: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the safety-check display format into a shared helper to avoid duplication.
cli_rendererreproduces the exact same line-by-line format as thelogger.infofallback insafety.py(lines 898–912). If the format changes in one location, the other can silently drift. Consider extracting a content generator insafety.pythat returns the display lines, then have each path handle only the output mechanism.♻️ Proposed refactor: shared content generator
In
agentwatch/core/safety.py:def _safety_check_lines(event: AgentEvent, safety: SafetyCheckData) -> list[str]: """Return the formatted safety-check display lines.""" tool = event.tool_call lines = [ "\n" + "=" * 60, "⚠️ AGENTWATCH SAFETY CHECK", "=" * 60, f"Agent: {event.agent_name or event.agent_id}", f"Action: {tool.tool_name if tool else 'unknown'}", ] if tool and tool.raw_command: lines.append(f"Command: {tool.raw_command}") if tool and tool.affected_resources: lines.append(f"Resources: {', '.join(tool.affected_resources)}") lines.append(f"Risk Level: {safety.risk_level.value.upper()}") lines.append(f"Risk Score: {safety.risk_score:.2f}") lines.append("Reasons:") lines.extend(f" • {r}" for r in safety.reasons) lines.append("=" * 60) return linesThen the logger fallback in
cli_approval_handlerbecomes:else: - logger.info("\n" + "=" * 60) - logger.info("⚠️ AGENTWATCH SAFETY CHECK") - logger.info("=" * 60) - logger.info(f"Agent: {event.agent_name or event.agent_id}") - logger.info(f"Action: {tool.tool_name if tool else 'unknown'}") - if tool and tool.raw_command: - logger.info(f"Command: {tool.raw_command}") - if tool and tool.affected_resources: - logger.info(f"Resources: {', '.join(tool.affected_resources)}") - logger.info(f"Risk Level: {safety.risk_level.value.upper()}") - logger.info(f"Risk Score: {safety.risk_score:.2f}") - logger.info("Reasons:") - for r in safety.reasons: - logger.info(f" • {r}") - logger.info("=" * 60) + for line in _safety_check_lines(event, safety): + logger.info(line)And
cli_rendererinmain.pybecomes:- def cli_renderer(event, safety): - tool = event.tool_call - print("\n" + "=" * 60) - print("⚠️ AGENTWATCH SAFETY CHECK") - print("=" * 60) - print(f"Agent: {event.agent_name or event.agent_id}") - print(f"Action: {tool.tool_name if tool else 'unknown'}") - if tool and tool.raw_command: - print(f"Command: {tool.raw_command}") - if tool and tool.affected_resources: - print(f"Resources: {', '.join(tool.affected_resources)}") - print(f"Risk Level: {safety.risk_level.value.upper()}") - print(f"Risk Score: {safety.risk_score:.2f}") - print("Reasons:") - for r in safety.reasons: - print(f" • {r}") - print("=" * 60) + def cli_renderer(event, safety): + for line in _safety_check_lines(event, safety): + print(line)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agentwatch/cli/main.py` around lines 364 - 380, Extract the duplicated safety-check formatting from cli_renderer and the logger fallback in cli_approval_handler into a shared _safety_check_lines helper in safety.py. Have the helper return the complete ordered display lines, preserving the existing conditional tool fields, risk details, reasons, and separators; update both callers to only handle their respective output mechanisms.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@agentwatch/cli/main.py`:
- Around line 364-380: Extract the duplicated safety-check formatting from
cli_renderer and the logger fallback in cli_approval_handler into a shared
_safety_check_lines helper in safety.py. Have the helper return the complete
ordered display lines, preserving the existing conditional tool fields, risk
details, reasons, and separators; update both callers to only handle their
respective output mechanisms.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d80c6637-2248-459b-a3be-9b488bb5cc3a
📒 Files selected for processing (2)
agentwatch/cli/main.pyagentwatch/core/safety.py
1 participant
Closes #595
This PR addresses the issue of
core/safety.pyunconditionally writing approval prompts to stdout from a library path.cli_approval_handlerto accept an optionalrenderercallback.logger.info(), preventing stdout corruption for structured consumers.agentwatch/cli/main.pyto pass in acli_rendererusingprint(), ensuring the CLI keeps its exact same "pretty box" styling without any behavioral regressions.Summary by CodeRabbit
New Features
Bug Fixes