Add persistent agent state and reputation-aware service selection - #464
Add persistent agent state and reputation-aware service selection#464vaishnavidesai09 wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughThe agent now persists score history, provider outcomes, and cumulative spend in ChangesPersistent agent state
Registry TTL annotation
Nullable service display data
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
|
@vaishnavidesai09 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Hey @ritik4ever Kindly review pr |
![coderabbitai[bot]](https://avatars.githubusercontent.com/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 `@agent/agent.js`:
- Around line 408-420: Update the settled endpoint-error branch near
submitReputation to add the paid amount to cumulativeSpend before retrying
another provider. Reuse the existing cumulative-spend update mechanism and
selected.price_usdc value, while preserving the current failure tracking and
state persistence behavior.
In `@agent/state.js`:
- Around line 15-20: Update the state-loading logic around the JSON.parse flow
to validate that the parsed value is a non-null object, then normalize each
agent entry so required nested fields such as providerHistory are initialized to
the shapes expected by agent/agent.js, including an array for scoreHistory.
Treat invalid or schema-incompatible state like corrupt input by returning a
safe normalized empty/default state, while preserving valid entries and the
existing warning behavior.
🪄 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 Plus
Run ID: 52140ede-94bf-4697-a1bb-5b47dd312ee9
📒 Files selected for processing (4)
README.mdagent/agent.jsagent/agent.test.jsagent/state.js
| if (scoringEnabled) await recordOutcome(selected.price_usdc, false, selected.id); | ||
| // Payment settled but service returned bad data — penalise service reputation. | ||
| await submitReputation(selected.id, false); | ||
| if (agentState?.providerHistory) { | ||
| const history = agentState.providerHistory[selected.id] ?? { | ||
| successes: 0, | ||
| failures: 0, | ||
| }; | ||
|
|
||
| history.failures++; | ||
| agentState.providerHistory[selected.id] = history; | ||
| saveAgentState(AGENT_ADDRESS, agentState); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Count settled endpoint-error payments in cumulative spend.
This branch says the payment has settled, but only increments failures. Retrying another provider then leaves cumulativeSpend lower than the amount actually paid.
Proposed fix
history.failures++;
agentState.providerHistory[selected.id] = history;
+ agentState.cumulativeSpend += parseFloat(selected.price_usdc);
saveAgentState(AGENT_ADDRESS, agentState);📝 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.
| if (scoringEnabled) await recordOutcome(selected.price_usdc, false, selected.id); | |
| // Payment settled but service returned bad data — penalise service reputation. | |
| await submitReputation(selected.id, false); | |
| if (agentState?.providerHistory) { | |
| const history = agentState.providerHistory[selected.id] ?? { | |
| successes: 0, | |
| failures: 0, | |
| }; | |
| history.failures++; | |
| agentState.providerHistory[selected.id] = history; | |
| saveAgentState(AGENT_ADDRESS, agentState); | |
| } | |
| if (scoringEnabled) await recordOutcome(selected.price_usdc, false, selected.id); | |
| // Payment settled but service returned bad data — penalise service reputation. | |
| await submitReputation(selected.id, false); | |
| if (agentState?.providerHistory) { | |
| const history = agentState.providerHistory[selected.id] ?? { | |
| successes: 0, | |
| failures: 0, | |
| }; | |
| history.failures++; | |
| agentState.providerHistory[selected.id] = history; | |
| agentState.cumulativeSpend += parseFloat(selected.price_usdc); | |
| saveAgentState(AGENT_ADDRESS, agentState); | |
| } |
🤖 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 `@agent/agent.js` around lines 408 - 420, Update the settled endpoint-error
branch near submitReputation to add the paid amount to cumulativeSpend before
retrying another provider. Reuse the existing cumulative-spend update mechanism
and selected.price_usdc value, while preserving the current failure tracking and
state persistence behavior.
| try { | ||
| const raw = fs.readFileSync(STATE_FILE, "utf8"); | ||
| return JSON.parse(raw); | ||
| } catch (err) { | ||
| console.warn("Ignoring corrupt state file:", err.message); | ||
| return {}; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Recover from schema-invalid state as well as invalid JSON.
null parses successfully but makes state[agentAddress] throw. Partial entries such as { "address": { "providerHistory": {} } } later make agent/agent.js attempt scoreHistory.push(...) on undefined. Validate the top-level object and normalize every agent entry before returning it.
Proposed fix
+function isRecord(value) {
+ return value !== null && typeof value === "object" && !Array.isArray(value);
+}
+
function readState() {
...
- return JSON.parse(raw);
+ const state = JSON.parse(raw);
+ if (!isRecord(state)) throw new Error("State must be an object");
+ return state;
...
}
export function getAgentState(agentAddress) {
const state = readState();
-
- if (!state[agentAddress]) {
+ const saved = state[agentAddress];
+ if (
+ !isRecord(saved) ||
+ !Array.isArray(saved.scoreHistory) ||
+ !isRecord(saved.providerHistory) ||
+ !Number.isFinite(saved.cumulativeSpend)
+ ) {
state[agentAddress] = {
- scoreHistory: [],
- providerHistory: {},
- cumulativeSpend: 0,
+ scoreHistory: Array.isArray(saved?.scoreHistory) ? saved.scoreHistory : [],
+ providerHistory: isRecord(saved?.providerHistory) ? saved.providerHistory : {},
+ cumulativeSpend: Number.isFinite(saved?.cumulativeSpend) ? saved.cumulativeSpend : 0,
};
writeState(state);
}Also applies to: 35-42
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 15-15: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(STATE_FILE, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🤖 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 `@agent/state.js` around lines 15 - 20, Update the state-loading logic around
the JSON.parse flow to validate that the parsed value is a non-null object, then
normalize each agent entry so required nested fields such as providerHistory are
initialized to the shapes expected by agent/agent.js, including an array for
scoreHistory. Treat invalid or schema-incompatible state like corrupt input by
returning a safe normalized empty/default state, while preserving valid entries
and the existing warning behavior.
|
@vaishnavidesai09 please fix ci |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/src/routes/registry.test.js (1)
88-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove temporary console logging from tests.
These
console.logcalls print response details during normal test runs, polluting CI output. Remove them after debugging.Also applies to: 932-933
🤖 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 `@backend/src/routes/registry.test.js` around lines 88 - 89, Remove the temporary console.log calls for response status and body from the affected test cases in the registry tests, including both referenced locations, while leaving the test assertions and behavior unchanged.
🤖 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 `@backend/src/routes/registry.js`:
- Around line 64-78: Update the comment above annotateTtlWarning to state that
ttl_warning:true is appended when the estimated remaining TTL is at or below
SERVICE_TTL_WARNING_LEDGERS, reflecting the inclusive >= predicate; keep the
unavailable-currentLedger behavior unchanged.
In `@frontend/lib/sort.test.ts`:
- Around line 20-29: Update the makeAgent fixture to match the AgentEntry type
exactly: remove the unsupported endpoint property and add required description,
owner, payment-total, last_active, active, flagged, and flag_reason fields with
appropriate test defaults, while preserving the existing overrides behavior.
---
Nitpick comments:
In `@backend/src/routes/registry.test.js`:
- Around line 88-89: Remove the temporary console.log calls for response status
and body from the affected test cases in the registry tests, including both
referenced locations, while leaving the test assertions and behavior unchanged.
🪄 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 Plus
Run ID: 44317052-ffe7-4d3e-bde3-61804bb2f64b
⛔ Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
backend/src/routes/registry.jsbackend/src/routes/registry.test.jsfrontend/components/ServiceCard.tsxfrontend/lib/sort.test.ts
| // Appends ttl_warning:true when the entry's estimated remaining TTL falls | ||
| // below SERVICE_TTL_WARNING_LEDGERS. Omits the field entirely when currentLedger | ||
| // is unavailable so callers can always treat absence as "no warning data". | ||
| function annotateTtlWarning(service, currentLedger) { | ||
| if (currentLedger == null) return service; | ||
|
|
||
| return { | ||
| ...service, | ||
| ttl_warning: | ||
| currentLedger >= | ||
| service.registered_at + | ||
| SERVICE_MAX_TTL - | ||
| SERVICE_TTL_WARNING_LEDGERS, | ||
| }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the inclusive TTL threshold.
The predicate uses >=, so ttl_warning is emitted when the remaining TTL is at or below SERVICE_TTL_WARNING_LEDGERS, not only when it is below. Update the comment to match the tested behavior.
🤖 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 `@backend/src/routes/registry.js` around lines 64 - 78, Update the comment
above annotateTtlWarning to state that ttl_warning:true is appended when the
estimated remaining TTL is at or below SERVICE_TTL_WARNING_LEDGERS, reflecting
the inclusive >= predicate; keep the unavailable-currentLedger behavior
unchanged.
| function makeAgent(overrides: Partial<AgentEntry> = {}): AgentEntry { | ||
| return { | ||
| address: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWFL', | ||
| name: 'Test Agent', | ||
| endpoint: 'https://example.com', | ||
| score: 0, | ||
| total_payments: '0', | ||
| registered_at: '0', | ||
| ...overrides, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A20 'interface AgentEntry' frontend/lib/types.ts
rg -n -A20 'function makeAgent' frontend/lib/sort.test.tsRepository: Stellar-Ecosystem/lodestar
Length of output: 1398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- frontend/tsconfig files ---\n'
fd 'tsconfig.*json$' frontend -x sh -c 'echo "### $1"; sed -n "1,220p" "$1"\'' sh {}
printf '\n--- package scripts and deps ---\n'
for f in package.json frontend/package.json; do
[ -f "$f" ] && { echo "### $f"; cat "$f"; }
done
printf '\n--- sort.test.ts imports and relevant slice ---\n'
sed -n '1,220p' frontend/lib/sort.test.ts
printf '\n--- read-only shape verifier ---\n'
python3 - <<'PY'
import json, pathlib, re
types = pathlib.Path('frontend/lib/types.ts').read_text()
test = pathlib.Path('frontend/lib/sort.test.ts').read_text()
agent_iface = re.search(r'export interface\s+(\w+)\s*\{(?P<body>.*?)\n\}', types, re.S).groupdict()
assert agent_iface['name'] == 'AgentEntry'
fields = {m.group(1) for m in re.finditer(r'^\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*:', agent_iface['body'], re.M)}
print('AgentEntry fields:', fields)
return_stmt = re.search(r'function\s+makeAgent\([^)]*\)\s*:\s*AgentEntry\s*\{[^}]*return\s*\{(?P<body>.*?)\n\s*\};'? , test, re.S)
if not return_stmt:
return_stmt = re.search(r'function\s+makeAgent\([^)]*\)\s*:\s*AgentEntry\s*\{[^}]*return\s*\{(?P<body>.*?)\n\s*\}\s*\};'? , test, re.S)
fixture = {}
for m in re.finditer(r'^\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*:', return_stmt.group('body'), re.M):
fixture[m.group(1)] = True
print('fixture fields:', fixture)
print('missing required fields:', sorted(fields - set(fixture)))
print('extra fixture fields:', sorted(set(fixture) - fields))
print('has endpoint:', 'endpoint' in fixture)
PYRepository: Stellar-Ecosystem/lodestar
Length of output: 255
Fix the AgentEntry fixture shape before merging.
makeAgent is declared to return AgentEntry, but it omits required fields (description, owner, payment totals, last_active, active, flagged, and flag_reason) and adds unsupported endpoint. This is a TypeScript compile-time failure.
Proposed fixture fix
function makeAgent(overrides: Partial<AgentEntry> = {}): AgentEntry {
return {
address: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWFL',
name: 'Test Agent',
- endpoint: 'https://example.com',
+ description: 'Test agent',
+ owner: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWFL',
score: 0,
total_payments: '0',
+ successful_payments: '0',
+ failed_payments: '0',
+ total_volume_stroops: '0',
registered_at: '0',
+ last_active: '0',
+ active: true,
+ flagged: false,
+ flag_reason: '',
...overrides,
};
}📝 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.
| function makeAgent(overrides: Partial<AgentEntry> = {}): AgentEntry { | |
| return { | |
| address: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWFL', | |
| name: 'Test Agent', | |
| endpoint: 'https://example.com', | |
| score: 0, | |
| total_payments: '0', | |
| registered_at: '0', | |
| ...overrides, | |
| }; | |
| function makeAgent(overrides: Partial<AgentEntry> = {}): AgentEntry { | |
| return { | |
| address: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWFL', | |
| name: 'Test Agent', | |
| description: 'Test agent', | |
| owner: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWFL', | |
| score: 0, | |
| total_payments: '0', | |
| successful_payments: '0', | |
| failed_payments: '0', | |
| total_volume_stroops: '0', | |
| registered_at: '0', | |
| last_active: '0', | |
| active: true, | |
| flagged: false, | |
| flag_reason: '', | |
| ...overrides, | |
| }; |
🤖 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 `@frontend/lib/sort.test.ts` around lines 20 - 29, Update the makeAgent fixture
to match the AgentEntry type exactly: remove the unsupported endpoint property
and add required description, owner, payment-total, last_active, active,
flagged, and flag_reason fields with appropriate test defaults, while preserving
the existing overrides behavior.
|
@ritik4ever please run tests to if ci passes |
|
This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
|
@ritik4ever done |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/routes/registry.js (1)
51-62: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winRemove the duplicate
annotateTtlWarningdeclaration.
backend/src/routes/registry.jsis an ES module and declaresannotateTtlWarningtwice at the module top level, so the module fails to load. Keep one implementation.🤖 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 `@backend/src/routes/registry.js` around lines 51 - 62, Remove the duplicate top-level annotateTtlWarning declaration in backend/src/routes/registry.js, keeping a single implementation with the existing currentLedger and ttl_warning behavior so the ES module loads successfully.
🧹 Nitpick comments (1)
agent/agent.js (1)
444-453: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract repeated provider-history update into a helper.
Three blocks (network failure, endpoint error, success) each read-or-initialize
agentState.providerHistory[selected.id], mutate a counter, and callsaveAgentState. Consolidate this into one helper, e.g.recordProviderOutcome(id, { success, spend }). This reduces duplication and removes the risk of one branch silently diverging from the others, as happened with the missingcumulativeSpendupdate in the endpoint-error branch.Proposed refactor
+function recordProviderOutcome(id, { success, spendUsdc } = {}) { + if (!agentState?.providerHistory) return; + const history = agentState.providerHistory[id] ?? { successes: 0, failures: 0 }; + if (success) { + history.successes++; + if (spendUsdc !== undefined) agentState.cumulativeSpend += parseFloat(spendUsdc); + } else { + history.failures++; + } + agentState.providerHistory[id] = history; + saveAgentState(AGENT_ADDRESS, agentState); +}Then call
recordProviderOutcome(selected.id, { success: false })andrecordProviderOutcome(selected.id, { success: true, spendUsdc: selected.price_usdc })at the respective sites.Also applies to: 474-483, 508-521
🤖 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 `@agent/agent.js` around lines 444 - 453, Extract the repeated provider-history read, counter update, cumulative-spend update, and save logic from the network-failure, endpoint-error, and success branches into a shared recordProviderOutcome helper. Have it accept the provider id and outcome data such as success and spendUsdc, then replace each duplicated block with the appropriate helper call while preserving existing failure, success, and spend behavior.
🤖 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.
Outside diff comments:
In `@backend/src/routes/registry.js`:
- Around line 51-62: Remove the duplicate top-level annotateTtlWarning
declaration in backend/src/routes/registry.js, keeping a single implementation
with the existing currentLedger and ttl_warning behavior so the ES module loads
successfully.
---
Nitpick comments:
In `@agent/agent.js`:
- Around line 444-453: Extract the repeated provider-history read, counter
update, cumulative-spend update, and save logic from the network-failure,
endpoint-error, and success branches into a shared recordProviderOutcome helper.
Have it accept the provider id and outcome data such as success and spendUsdc,
then replace each duplicated block with the appropriate helper call while
preserving existing failure, success, and spend behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b99a6c6-683b-4c49-b822-ba1983e131f1
📒 Files selected for processing (6)
README.mdagent/agent.jsagent/agent.test.jsbackend/src/routes/registry.jsbackend/src/routes/registry.test.jsfrontend/components/ServiceCard.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/src/routes/registry.test.js
- agent/agent.test.js
- frontend/components/ServiceCard.tsx
2 participants
Summary
Testing
All 28 tests pass.
Closes #364
Summary by CodeRabbit