fix: parse Joy Trust API response correctly#18
fix: parse Joy Trust API response correctly#18MervinPraison merged 11 commits intoMervinPraison:mainfrom
Conversation
The /agents/discover endpoint returns {agents: [...]} not top-level fields.
This fix extracts the agent from the array before reading trust_score.
Fixes MervinPraison#17
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated JoyTrustTool.check_trust to query Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant JoyTrustTool
participant JoyTrustAPI
Caller->>JoyTrustTool: check_trust(agent_name, api_key?)
JoyTrustTool->>JoyTrustAPI: GET /agents/discover?query=<agent_name>\nHeader: x-api-key (optional)
JoyTrustAPI-->>JoyTrustTool: 200 OK { "agents": [ ... ] }
JoyTrustTool->>JoyTrustTool: locate agent by case-insensitive exact name
alt agent found
JoyTrustTool-->>Caller: success payload (agent_id, trust_score, verified, badges, tier, ...)
else not found
JoyTrustTool-->>Caller: error payload ("Agent '<name>' not found", meets_threshold: false, agent_id: null)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR fixes the core bug in the Joy Trust integration where The previous security concerns flagged in review (silent fallback to the wrong agent, Remaining items to address:
Confidence Score: 3/5Core bug and all previously flagged security issues are resolved; one P1 config-semantics issue and two P2 inconsistencies remain before merge. The primary parsing bug is correctly fixed and all prior security concerns (wrong-agent fallback, null score crash, agents:null bypass, misleading recommendation) are addressed. However, praisonai_tools/tools/joy_trust_tool.py — dead Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant JoyTrustTool
participant Cache
participant JoyAPI as JoyConnectAPI
Caller->>JoyTrustTool: check_trust(agent_name, min_score)
JoyTrustTool->>Cache: lookup cache_key
alt Cache hit (fresh)
Cache-->>JoyTrustTool: cached result
JoyTrustTool-->>Caller: cached result
else Cache miss or stale
JoyTrustTool->>JoyAPI: GET /agents/discover?query=agent_name with x-api-key header
alt HTTP success
JoyAPI-->>JoyTrustTool: agents array response
JoyTrustTool->>JoyTrustTool: Extract agents list (null-safe)
JoyTrustTool->>JoyTrustTool: Exact case-insensitive name match
alt Agent found and score valid
JoyTrustTool->>Cache: store result
JoyTrustTool-->>Caller: trust_score and meets_threshold
else Agent not found
JoyTrustTool-->>Caller: meets_threshold=False, error=not found
else Score invalid
JoyTrustTool-->>Caller: meets_threshold=False, fallback_used=False
end
else Network or HTTP error
JoyAPI-->>JoyTrustTool: error
JoyTrustTool-->>Caller: meets_threshold=False, fallback_used=False
end
end
Reviews (9): Last reviewed commit: "fix: Change default min_score from 3.0 t..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
praisonai_tools/tools/joy_trust_tool.py (1)
127-134:⚠️ Potential issue | 🔴 Critical
NameError:timeis used before it's imported.The cache check at line 127 calls
time.time(), buttimeis imported inside thetryblock at line 133. When a cached result exists, this will raiseNameError: name 'time' is not defined.Move
import timeto the top of the file with other standard library imports.🐛 Proposed fix
Add
timeto the imports at the top of the file:import os import logging +import time from typing import Any, Dict, Optional, Union, Callable, ListThen remove
timefrom the late import:try: import httpx - import time except ImportError:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@praisonai_tools/tools/joy_trust_tool.py` around lines 127 - 134, The code calls time.time() in the cache check (see cached_result and self.config.cache_duration) before time is imported, causing a NameError; move the standard-library import of time up with the other top-of-file imports and remove time from the late import inside the try/except that currently imports httpx so that the try block only imports third-party modules (e.g., httpx).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@praisonai_tools/tools/joy_trust_tool.py`:
- Around line 192-205: The response schema is inconsistent between the success
`result` dict and the error/"not found" branches; normalize all paths so they
return the same keys to avoid KeyError. Update the success `result` (the dict
created in the successful lookup) and every error/not-found response to include
the union of fields: agent_name, agent_id, trust_score, verified,
meets_threshold, threshold_used, vouch_count, capabilities, tier, badges,
reputation, recommendations, error, and _cached_at (use sensible defaults: 0 or
empty list/dict/False/None as appropriate and time.time() for _cached_at when
applicable). Make sure the same key names are used in the `result` dict and in
the error payloads returned by the functions that build those responses so
callers can rely on a stable schema.
---
Outside diff comments:
In `@praisonai_tools/tools/joy_trust_tool.py`:
- Around line 127-134: The code calls time.time() in the cache check (see
cached_result and self.config.cache_duration) before time is imported, causing a
NameError; move the standard-library import of time up with the other
top-of-file imports and remove time from the late import inside the try/except
that currently imports httpx so that the try block only imports third-party
modules (e.g., httpx).
🪄 Autofix (Beta)
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
Run ID: 08282f09-b432-4aaa-b752-f1d3b24b1d16
📒 Files selected for processing (1)
praisonai_tools/tools/joy_trust_tool.py
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the check_trust method in joy_trust_tool.py to accommodate changes in the Joy Trust Network API. Key updates include switching to header-based authentication, searching for agents within a returned list using case-insensitive matching, and updating the result dictionary with new fields such as agent_id and vouch_count. Review feedback recommends ensuring the error response structure matches the success response for consistency and suggests using a more idiomatic generator expression to locate the target agent.
| return { | ||
| "agent_name": agent_name, | ||
| "trust_score": 0.0, | ||
| "verified": False, | ||
| "meets_threshold": False, | ||
| "threshold_used": min_threshold, | ||
| "reputation": {}, | ||
| "recommendations": 0, | ||
| "error": f"Agent '{agent_name}' not found on Joy Trust Network" | ||
| } |
There was a problem hiding this comment.
The structure of the dictionary returned here when an agent is not found is inconsistent with the structure of the successful response. This response includes legacy fields reputation and recommendations, while the successful response (lines 192-205) includes new fields like agent_id, vouch_count, and capabilities. To ensure consumers of this function receive a consistent data structure, this error response should be updated to match the new format, using default values for the fields.
| return { | |
| "agent_name": agent_name, | |
| "trust_score": 0.0, | |
| "verified": False, | |
| "meets_threshold": False, | |
| "threshold_used": min_threshold, | |
| "reputation": {}, | |
| "recommendations": 0, | |
| "error": f"Agent '{agent_name}' not found on Joy Trust Network" | |
| } | |
| return { | |
| "agent_name": agent_name, | |
| "agent_id": None, | |
| "trust_score": 0.0, | |
| "verified": False, | |
| "meets_threshold": False, | |
| "threshold_used": min_threshold, | |
| "vouch_count": 0, | |
| "capabilities": [], | |
| "tier": "unknown", | |
| "badges": [], | |
| "error": f"Agent '{agent_name}' not found on Joy Trust Network" | |
| } |
| # Find matching agent by name (case-insensitive) | ||
| agent = None | ||
| for a in agents: | ||
| if a.get("name", "").lower() == agent_name.lower(): | ||
| agent = a | ||
| break |
There was a problem hiding this comment.
For better readability and conciseness, you can use a generator expression with next() to find the first matching agent. This is a more idiomatic Python approach.
| # Find matching agent by name (case-insensitive) | |
| agent = None | |
| for a in agents: | |
| if a.get("name", "").lower() == agent_name.lower(): | |
| agent = a | |
| break | |
| # Find matching agent by name (case-insensitive) | |
| agent = next((a for a in agents if a.get("name", "").lower() == agent_name.lower()), None) |
Greptile correctly identified that falling back to the first search result when no exact name match is found could return a different agent's trust score, allowing untrusted agents to pass verification. Now only exact name matches are accepted.
|
Addressed Greptile's security concern: Removed fallback-to-first-result logic - now only exact name matches are accepted. The previous code could return a different agent's trust score if no exact match was found, which is a security risk in a trust-gating context. |
|
Tip: Greploop — Automatically fix all review issues by running Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal. |
- Move 'import time' to top of file (fixes NameError on cache check) - Normalize response schema across success/error paths - All responses now include: agent_id, vouch_count, capabilities, tier, badges
- Use idiomatic next() with generator expression for agent lookup - Fix ImportError response to use consistent schema with new fields Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
time is already imported at module level (line 37) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@praisonai_tools/tools/joy_trust_tool.py`:
- Around line 249-264: The except block that catches broad exceptions should
fail closed: replace the current use of self.config.fallback_on_error for the
returned fields "meets_threshold" and "fallback_used" with hardcoded False, and
change logger.error(...) to logger.exception(...) to emit the full stack trace;
locate the handler in joy_trust_tool.py around the return that builds the dict
containing "agent_name", "trust_score", "meets_threshold", "fallback_used",
"threshold_used" and update those two fields to False while leaving other return
values (agent_id, trust_score, vouch_count, capabilities, tier, badges, error,
threshold_used) unchanged.
- Around line 195-203: The code reads agent.get("trust_score", 0.0) and uses it
directly in the comparison for "meets_threshold" which can fail for strings or
nulls; update the logic around trust_score (the local variable and the dict keys
"trust_score" and "meets_threshold") to coerce and validate the value: attempt
to convert agent.get("trust_score") to float in a try/except (falling back to
0.0 or a sentinel like None), set a boolean or include an "invalid_trust_score"
flag in the result if conversion fails or value is None, and then compute
"meets_threshold" using the validated float against min_threshold so malformed
payloads are handled explicitly rather than raising during comparison.
🪄 Autofix (Beta)
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
Run ID: d3d0635c-c918-479c-b4f1-57e1e1938f54
📒 Files selected for processing (1)
praisonai_tools/tools/joy_trust_tool.py
Use 'or' operator to handle both missing key AND null value, preventing TypeError on comparison and security bypass. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use 'or' operator to handle both missing key AND null value, preventing TypeError when iterating and security bypass. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
praisonai_tools/tools/joy_trust_tool.py (1)
217-264:⚠️ Potential issue | 🔴 CriticalFail closed on request/API/unexpected errors in trust-gating path.
Line 224, Line 240, and Line 256 currently allow
"meets_threshold"to becomeTruefrom config fallback; that value feedshandoff_safe(Line 280). Error paths should not mark unsafe handoffs as safe.Suggested fix
except httpx.RequestError as e: logger.error(f"Joy Trust request error: {e}") return { @@ - "meets_threshold": self.config.fallback_on_error, + "meets_threshold": False, @@ - "fallback_used": self.config.fallback_on_error + "fallback_used": False } except httpx.HTTPStatusError as e: logger.error(f"Joy Trust API error: {e.response.status_code}") return { @@ - "meets_threshold": self.config.fallback_on_error, + "meets_threshold": False, @@ - "fallback_used": self.config.fallback_on_error + "fallback_used": False } except Exception as e: - logger.error(f"Joy Trust unexpected error: {e}") + logger.exception("Joy Trust unexpected error") return { @@ - "meets_threshold": self.config.fallback_on_error, + "meets_threshold": False, @@ - "fallback_used": self.config.fallback_on_error + "fallback_used": False }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@praisonai_tools/tools/joy_trust_tool.py` around lines 217 - 264, The exception handlers in joy_trust_tool.py currently set "meets_threshold" to self.config.fallback_on_error which can incorrectly mark unsafe handoffs as safe on errors; update each except block (the httpx.RequestError, httpx.HTTPStatusError, and generic Exception handlers) to always set "meets_threshold": False and "fallback_used": True (or leave fallback_used as-is if already true), while preserving the other returned fields (agent_name, trust_score, error text, threshold_used, etc.), so that errors never result in a true meets_threshold value used by handoff_safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@praisonai_tools/tools/joy_trust_tool.py`:
- Around line 217-264: The exception handlers in joy_trust_tool.py currently set
"meets_threshold" to self.config.fallback_on_error which can incorrectly mark
unsafe handoffs as safe on errors; update each except block (the
httpx.RequestError, httpx.HTTPStatusError, and generic Exception handlers) to
always set "meets_threshold": False and "fallback_used": True (or leave
fallback_used as-is if already true), while preserving the other returned fields
(agent_name, trust_score, error text, threshold_used, etc.), so that errors
never result in a true meets_threshold value used by handoff_safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69b81220-b59f-4ea3-9acb-4dff7e80043a
📒 Files selected for processing (1)
praisonai_tools/tools/joy_trust_tool.py
Error handlers now always set meets_threshold=False instead of using fallback_on_error config. In trust-gating context, errors should deny handoffs, not silently allow them. Changes: - meets_threshold: False (was self.config.fallback_on_error) - fallback_used: True (indicates error occurred) - logger.exception() for unexpected errors (includes stack trace) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Thanks for the thorough reviews! The security catches were excellent: Null-safety fixes (from review feedback):
Fail-closed security (critical fix):
All feedback addressed and pushed. Ready for merge! |
|
Also thanks to @coderabbitai for catching the |
|
Tip For best results, initiate chat on the files or code changes.
|
|
Thanks @coderabbitai! Appreciate the quick reviews. Security-first approach for trust verification is critical - glad we caught those edge cases before merge. 🙏 |
|
Tip For best results, initiate chat on the files or code changes.
|
Set fallback_used=False so _get_handoff_recommendation returns "Handoff not recommended" instead of misleading "Proceed with caution" message when errors occur. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
praisonai_tools/tools/joy_trust_tool.py (1)
175-203:⚠️ Potential issue | 🟠 MajorNormalize payload fields before matching and thresholding.
Line 177 still blows up on
{"name": null}becausedict.get("name", "")returnsNonewhen the key exists, and Lines 196-203 still compare rawtrust_scoreagainst a float. A payload like{"trust_score": "2.3"}will raise here and fall into the generic error path instead of returning a structured fail-closed result.Proposed fix
- agent = next((a for a in agents if a.get("name", "").lower() == agent_name.lower()), None) + normalized_name = agent_name.lower() + agent = next( + ( + a + for a in agents + if (a.get("name") or "").lower() == normalized_name + ), + None, + ) @@ - trust_score = agent.get("trust_score") or 0.0 + raw_trust_score = agent.get("trust_score") + try: + trust_score = 0.0 if raw_trust_score in (None, "") else float(raw_trust_score) + except (TypeError, ValueError): + return { + "agent_name": agent.get("name", agent_name), + "agent_id": agent.get("id"), + "trust_score": 0.0, + "verified": False, + "meets_threshold": False, + "threshold_used": min_threshold, + "vouch_count": 0, + "capabilities": [], + "tier": None, + "badges": [], + "error": f"Invalid trust_score for agent '{agent_name}': {raw_trust_score!r}", + "fallback_used": False, + }#!/bin/bash set -euo pipefail echo "1) Show the current parsing logic in joy_trust_tool.py:" rg -n 'a\.get\("name", ""\)\.lower\(\)|trust_score = agent\.get\("trust_score"\) or 0\.0|"meets_threshold": trust_score >=' praisonai_tools/tools/joy_trust_tool.py -C2 echo echo "2) Demonstrate the two Python failure modes:" python - <<'PY' agent = {"name": None, "trust_score": "2.3"} checks = { "null_name_lower": lambda: agent.get("name", "").lower(), "string_score_threshold": lambda: (agent.get("trust_score") or 0.0) >= 3.0, } for label, fn in checks.items(): try: print(label, "=>", fn()) except Exception as exc: print(label, "=>", type(exc).__name__, str(exc)) PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@praisonai_tools/tools/joy_trust_tool.py` around lines 175 - 203, The code fails when agent["name"] is None and when trust_score is a string; update the matching and threshold logic to normalize fields before use: replace dict.get("name", "").lower() with (agent.get("name") or "").lower() for the case-insensitive exact match (affects the agent lookup using agent_name and agents), and coerce trust_score to a float safely before comparing to min_threshold (compute trust_score = safe_float(agent.get("trust_score")) where safe_float tries float(...) and falls back to 0.0 on TypeError/ValueError/None); then use that normalized trust_score in the result dict and in the meets_threshold comparison (also ensure agent.get("name", agent_name) remains a safe fallback).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@praisonai_tools/tools/joy_trust_tool.py`:
- Around line 175-203: The code fails when agent["name"] is None and when
trust_score is a string; update the matching and threshold logic to normalize
fields before use: replace dict.get("name", "").lower() with (agent.get("name")
or "").lower() for the case-insensitive exact match (affects the agent lookup
using agent_name and agents), and coerce trust_score to a float safely before
comparing to min_threshold (compute trust_score =
safe_float(agent.get("trust_score")) where safe_float tries float(...) and falls
back to 0.0 on TypeError/ValueError/None); then use that normalized trust_score
in the result dict and in the meets_threshold comparison (also ensure
agent.get("name", agent_name) remains a safe fallback).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f2d87e2-4041-4df3-8c9a-1a42f592ff10
📒 Files selected for processing (1)
praisonai_tools/tools/joy_trust_tool.py
- Use (a.get("name") or "").lower() to handle {"name": null}
- Safely convert trust_score to float with try/except
- Return fail-closed response for invalid trust_score format
- Use 'or' pattern consistently for null-safe field access
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Good catches @coderabbitai! Both edge cases fixed:
The |
|
Tip For best results, initiate chat on the files or code changes.
Quick summary of why these patterns are strictly better in this context:
The ( \ /) ✏️ Learnings added
|
|
Thanks @coderabbitai and @graborbot for the thorough reviews! The null-safety patterns (handling null values in API responses) and fail-closed security recommendations were good catches and have been implemented. Ready for merge when convenient. 🙏 |
|
Tip For best results, initiate chat on the files or code changes.
All the critical trust-gating invariants are now properly enforced. Best of luck with the merge! 🚀 (\ /) 🧠 Learnings used |
The Joy Trust Network has: - Network average: 1.11 - Network median: 1.10 - Network max: 2.40 - Agents >= 3.0: 0 A default of 3.0 blocks 100% of agents. Changed to 1.5 which is Joy's recommended 'standard' threshold. Joy now returns trust_context.recommended_thresholds: - permissive: 1.0 (low-risk tasks) - standard: 1.5 (general use) <- new default - moderate: 2.0 (established agents) - strict: 2.5 (high security) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2 participants
Summary
Fixes the Joy Trust integration to correctly parse the API response.
Problem
The
/agents/discoverendpoint returns:{"agents": [{"name": "...", "trust_score": 2.3, ...}], "count": 1}But the code was reading from the top level:
Solution
agentsarraytrust_scorefrom agent objectqueryparam instead ofnameparamTest
Fixes #17
Summary by CodeRabbit
Bug Fixes
New Features