Skip to content

fix(caido): normalize string null and sentinels for scopeId and parentId (#1140) - #1151

Open
vardhans07 wants to merge 1 commit into
usestrix:mainfrom
vardhans07:fix/caido-null-scope-id
Open

fix(caido): normalize string null and sentinels for scopeId and parentId (#1140)#1151
vardhans07 wants to merge 1 commit into
usestrix:mainfrom
vardhans07:fix/caido-null-scope-id

Conversation

@vardhans07

Copy link
Copy Markdown
Contributor

Closes #1140

Context & Fix

Some LLMs serialize omitted or null arguments as literal strings like "null", "none", or "undefined" instead of actual null/None. In list_requests and list_sitemap, we were checking truthiness directly (if scope_id:), which allowed these strings to pass through to the Caido GraphQL query, throwing an Invalid ID format, should be an i32 error.

To fix this, I added a _normalize_optional_id helper in caido_api.py that:

  • Treats common string null sentinels and empty whitespace as None.
  • Ensures any provided ID parses cleanly into a signed 32-bit integer.
  • Normalizes scope_id and parent_id before building the requests.

Also added test cases in tests/test_caido_api.py to cover these sentinel strings, valid numeric IDs, invalid non-integers, and integer overflow.
image

@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR normalizes optional Caido scope and sitemap-parent identifiers, treating common string null sentinels as absent and validating supplied IDs as signed 32-bit integers.

  • Applies normalization before request scope filtering and sitemap query selection.
  • Adds direct unit coverage for sentinels, numeric normalization, invalid values, and overflow.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking test-coverage gap around the production query-building paths.

The normalization and call-site wiring are internally consistent, but the tests exercise only the helper and therefore do not protect the agent-facing filter and sitemap-query behavior.

Files Needing Attention: tests/test_caido_api.py

Important Files Changed

Filename Overview
strix/tools/proxy/caido_api.py Adds consistent optional-ID normalization to request filtering and sitemap traversal; no concrete runtime defect was established.
tests/test_caido_api.py Thoroughly covers the helper itself but omits production call-site behavior such as scope-filter omission and sitemap branch selection.
Prompt To Fix All With AI
### Issue 1
tests/test_caido_api.py:8-9
**Call-site normalization remains untested**

These tests invoke only `_normalize_optional_id`, so they do not verify that sentinel scope IDs omit the request filter or that sentinel parent IDs select the sitemap roots query. Add call-site tests to protect the agent-facing behavior this change is intended to fix.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(caido): normalize string null and se..." | Re-trigger Greptile

Comment thread tests/test_caido_api.py
@vardhans07
vardhans07 force-pushed the fix/caido-null-scope-id branch from 0c165f4 to 36f2df6 Compare August 23, 2026 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Caido GraphQL tools pass string "null" as scopeId, causing "Invalid ID format, should be an i32"

1 participant