-
Notifications
You must be signed in to change notification settings - Fork 40
fix(sparc-service): strip agent-injected keys before SPARC evaluates tool calls #738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,9 @@ | |
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| import logging | ||
| import os | ||
|
|
||
| from fastapi import FastAPI, HTTPException | ||
| from fastapi.concurrency import run_in_threadpool | ||
|
|
@@ -19,6 +21,37 @@ | |
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
| # When SPARC_LOG_REQUESTS=true, log the full incoming ReflectRequest JSON so | ||
| # you can inspect exactly what the caller sends (useful for diagnosing | ||
| # unexpected tool argument keys). Disabled by default — payloads can be large. | ||
| _LOG_REQUESTS = os.getenv("SPARC_LOG_REQUESTS", "").strip().lower() in {"1", "true", "yes"} | ||
|
|
||
| # When SPARC_STRIP_TOOL_ARG_KEYS is set (comma-separated key names), those keys | ||
| # are removed from every tool_calls[].function.arguments JSON object before the | ||
| # request reaches SPARC. Use to drop agent-injected keys that are not in the | ||
| # tool spec and would cause SPARC to reject the call. | ||
| # Example: SPARC_STRIP_TOOL_ARG_KEYS=session_id,request_id | ||
| _STRIP_KEYS: frozenset[str] = frozenset( | ||
| k.strip() for k in os.getenv("SPARC_STRIP_TOOL_ARG_KEYS", "").split(",") if k.strip() | ||
| ) | ||
|
|
||
|
|
||
| def _strip_tool_arg_keys(tool_calls: list[dict], keys: frozenset[str]) -> list[dict]: | ||
| """Return a copy of tool_calls with the named argument keys removed.""" | ||
| result = [] | ||
| for tc in tool_calls: | ||
| fn = tc.get("function", {}) | ||
| raw_args = fn.get("arguments", "") | ||
|
Comment on lines
+43
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 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.
🤖 Prompt for AI Agents |
||
| try: | ||
| args = json.loads(raw_args) if isinstance(raw_args, str) else raw_args | ||
| if isinstance(args, dict): | ||
| args = {k: v for k, v in args.items() if k not in keys} | ||
| new_args = json.dumps(args) if isinstance(args, dict) else raw_args | ||
| except (json.JSONDecodeError, TypeError): | ||
| new_args = raw_args | ||
| result.append({**tc, "function": {**fn, "arguments": new_args}}) | ||
| return result | ||
|
|
||
|
|
||
| def create_app(engine: ReflectionEngine | None = None) -> FastAPI: | ||
| """Build the FastAPI app. Inject ``engine`` in tests; defaults to env config.""" | ||
|
|
@@ -51,6 +84,16 @@ def readyz() -> dict[str, object]: | |
|
|
||
| @app.post("/reflect", response_model=ReflectResponse) | ||
| async def reflect(request: ReflectRequest) -> ReflectResponse: | ||
| if _LOG_REQUESTS: | ||
| log.info("incoming reflect request: %s", request.model_dump_json()) | ||
|
|
||
| if _STRIP_KEYS and request.tool_calls: | ||
| request = request.model_copy( | ||
| update={"tool_calls": _strip_tool_arg_keys(request.tool_calls, _STRIP_KEYS)} | ||
| ) | ||
| if _LOG_REQUESTS: | ||
| log.info("after strip (%s): tool_calls=%s", sorted(_STRIP_KEYS), request.tool_calls) | ||
|
|
||
| # SPARCReflectionComponent.process is synchronous (and CPU/IO bound on the | ||
| # LLM call); run it off the event loop so the service stays responsive. | ||
| try: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: 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:
Route application logs to stdout.
logging.basicConfig()defaults tosys.stderr, so thesparc_service.loggercall in this module does not meet the stdout contract. Addstream=sys.stdout, or route this logger through Uvicorn’s logging configuration. Verify the deployed entry point usesloguruorsparc_service.loggerdirectly.🤖 Prompt for AI Agents