feat(registry): auto-mark stale agents inactive via dynamic TTL windows - #19
feat(registry): auto-mark stale agents inactive via dynamic TTL windows#19mijinummi wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughAdds TTL-based stale agent detection to the registry store: ChangesAgent Heartbeat and Registry TTL Staleness
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.env.example:
- Around line 143-144: Remove the duplicate environment variable definitions
from the .env.example file. The `PORT` and `REGISTRY_URL` variables are defined
in multiple sections, creating ambiguity about which is the authoritative
source. Consolidate these variables into a single location in the file, keeping
only one definition of each environment variable to maintain a clear and
unambiguous source of truth that cannot drift over time.
In `@packages/agent_shared_directory/src/server.ts`:
- Line 4: The AGENT_ID constant currently has a default fallback value of
'agent-core-01' which can cause multiple daemon instances to share the same
identity if the environment variable is not properly configured. Remove the
default fallback value from the AGENT_ID assignment on that line and add error
handling to ensure the AGENT_ID environment variable is explicitly set, throwing
an error or exiting the process if it is missing or undefined.
- Around line 13-19: The heartbeat loop in the setInterval callback lacks
timeout protection and overlap prevention. Add a timeout option to the
axios.post call (e.g., using the timeout configuration) and introduce a boolean
flag outside the setInterval to track whether a heartbeat request is currently
in-flight. Before making the axios.post request, check this flag and skip the
request if one is already pending, then set the flag to true before the request
and false in both the try and catch blocks to prevent overlapping requests.
Additionally, URL-encode the AGENT_ID using encodeURIComponent when constructing
the URL to handle special characters safely.
In `@packages/registry/src/store.ts`:
- Around line 40-47: The TTL and timestamp validation logic lacks input
validation which allows incorrect staleness evaluation. First, validate the
parsed ttlSeconds value to ensure it is a positive number greater than zero
before using it to calculate ttlThresholdMs, rejecting zero or negative values.
Second, when parsing the agent.last_seen timestamp in the agents.map function,
validate that the resulting lastSeenMs is a valid number (not NaN) before
calculating isStale, and skip or handle agents with invalid timestamps
appropriately to prevent them from being incorrectly classified as active. This
ensures only valid TTL and timestamp values are used in the stale-state
evaluation logic.
🪄 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: 682aad20-345a-4bef-bea1-2e68f5123be1
📒 Files selected for processing (3)
.env.examplepackages/agent_shared_directory/src/server.tspackages/registry/src/store.ts
| PORT=3000 | ||
| REGISTRY_URL=http://localhost:3000 |
There was a problem hiding this comment.
Remove duplicated env keys to avoid ambiguous source-of-truth.
REGISTRY_URL (and likely PORT) now appears in multiple sections. Even with identical values today, duplication is fragile and can drift.
Suggested fix
-# Cluster Runtime Environments
-PORT=3000
-REGISTRY_URL=http://localhost:3000
-
# Agent Cluster Core Policies
# Time-To-Live (TTL) ceiling boundary value in seconds to mark uncommunicative workers inactive.
AGENT_TTL_SECONDS=120🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 144-144: [DuplicatedKey] The REGISTRY_URL key is duplicated
(DuplicatedKey)
🤖 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 @.env.example around lines 143 - 144, Remove the duplicate environment
variable definitions from the .env.example file. The `PORT` and `REGISTRY_URL`
variables are defined in multiple sections, creating ambiguity about which is
the authoritative source. Consolidate these variables into a single location in
the file, keeping only one definition of each environment variable to maintain a
clear and unambiguous source of truth that cannot drift over time.
Source: Linters/SAST tools
| import axios from 'axios'; | ||
|
|
||
| const REGISTRY_URL = process.env.REGISTRY_URL || 'http://localhost:3000'; | ||
| const AGENT_ID = process.env.AGENT_ID || 'agent-core-01'; |
There was a problem hiding this comment.
Avoid a shared default AGENT_ID; require explicit identity.
Defaulting to agent-core-01 risks multiple daemons heartbeating under the same registry record if env wiring is missed.
Suggested fix
-const AGENT_ID = process.env.AGENT_ID || 'agent-core-01';
+const AGENT_ID = process.env.AGENT_ID;
+if (!AGENT_ID) {
+ throw new Error('AGENT_ID is required for heartbeat initialization');
+}📝 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.
| const AGENT_ID = process.env.AGENT_ID || 'agent-core-01'; | |
| const AGENT_ID = process.env.AGENT_ID; | |
| if (!AGENT_ID) { | |
| throw new Error('AGENT_ID is required for heartbeat initialization'); | |
| } |
🤖 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 `@packages/agent_shared_directory/src/server.ts` at line 4, The AGENT_ID
constant currently has a default fallback value of 'agent-core-01' which can
cause multiple daemon instances to share the same identity if the environment
variable is not properly configured. Remove the default fallback value from the
AGENT_ID assignment on that line and add error handling to ensure the AGENT_ID
environment variable is explicitly set, throwing an error or exiting the process
if it is missing or undefined.
| const heartbeatInterval = setInterval(async () => { | ||
| try { | ||
| await axios.post(`${REGISTRY_URL}/agents/${AGENT_ID}/heartbeat`); | ||
| } catch (error: any) { | ||
| console.warn(`[Lifecycle Warning] Pulse telemetry transmission dropped: ${error.message}`); | ||
| } | ||
| }, 60000); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "setInterval|axios\\.post|timeout|inFlight|encodeURIComponent" packages/agent_shared_directory/src/server.tsRepository: clevercon-protocol/clevercon
Length of output: 204
🏁 Script executed:
cat -n packages/agent_shared_directory/src/server.ts | head -30Repository: clevercon-protocol/clevercon
Length of output: 1265
🏁 Script executed:
rg -n "AGENT_ID" packages/agent_shared_directory/src/server.ts | head -5Repository: clevercon-protocol/clevercon
Length of output: 303
Add timeout and overlap protection to the heartbeat loop.
The heartbeat mechanism has no protection against hung network calls or request overlap. If an axios.post call takes longer than the 60-second interval, subsequent intervals will fire while the previous request is still in-flight, causing overlapping requests and potential resource exhaustion. Additionally, the AGENT_ID is not URL-encoded, which could cause issues if it contains special characters.
Suggested fix
- const heartbeatInterval = setInterval(async () => {
+ let inFlight = false;
+ const heartbeatInterval = setInterval(async () => {
+ if (inFlight) return;
+ inFlight = true;
try {
- await axios.post(`${REGISTRY_URL}/agents/${AGENT_ID}/heartbeat`);
+ await axios.post(
+ `${REGISTRY_URL}/agents/${encodeURIComponent(AGENT_ID)}/heartbeat`,
+ undefined,
+ { timeout: 5000 }
+ );
} catch (error: any) {
console.warn(`[Lifecycle Warning] Pulse telemetry transmission dropped: ${error.message}`);
+ } finally {
+ inFlight = false;
}
}, 60000);📝 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.
| const heartbeatInterval = setInterval(async () => { | |
| try { | |
| await axios.post(`${REGISTRY_URL}/agents/${AGENT_ID}/heartbeat`); | |
| } catch (error: any) { | |
| console.warn(`[Lifecycle Warning] Pulse telemetry transmission dropped: ${error.message}`); | |
| } | |
| }, 60000); | |
| let inFlight = false; | |
| const heartbeatInterval = setInterval(async () => { | |
| if (inFlight) return; | |
| inFlight = true; | |
| try { | |
| await axios.post( | |
| `${REGISTRY_URL}/agents/${encodeURIComponent(AGENT_ID)}/heartbeat`, | |
| undefined, | |
| { timeout: 5000 } | |
| ); | |
| } catch (error: any) { | |
| console.warn(`[Lifecycle Warning] Pulse telemetry transmission dropped: ${error.message}`); | |
| } finally { | |
| inFlight = false; | |
| } | |
| }, 60000); |
🤖 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 `@packages/agent_shared_directory/src/server.ts` around lines 13 - 19, The
heartbeat loop in the setInterval callback lacks timeout protection and overlap
prevention. Add a timeout option to the axios.post call (e.g., using the timeout
configuration) and introduce a boolean flag outside the setInterval to track
whether a heartbeat request is currently in-flight. Before making the axios.post
request, check this flag and skip the request if one is already pending, then
set the flag to true before the request and false in both the try and catch
blocks to prevent overlapping requests. Additionally, URL-encode the AGENT_ID
using encodeURIComponent when constructing the URL to handle special characters
safely.
| const ttlSeconds = Number(process.env.AGENT_TTL_SECONDS) || 120; | ||
| const ttlThresholdMs = ttlSeconds * 1000; | ||
| let mutated = false; | ||
|
|
||
| // Process live lifecycle updates based on absolute timestamp drift | ||
| const updatedAgents = agents.map((agent) => { | ||
| const lastSeenMs = new Date(agent.last_seen).getTime(); | ||
| const isStale = nowMs - lastSeenMs > ttlThresholdMs; |
There was a problem hiding this comment.
Validate TTL and timestamp inputs before stale-state evaluation.
Current parsing allows invalid behavior: negative TTL values are accepted, 0 is silently replaced by 120, and malformed last_seen parses to NaN (so records never become stale). This can misclassify agent activity.
Suggested fix
- const ttlSeconds = Number(process.env.AGENT_TTL_SECONDS) || 120;
+ const parsedTtl = Number(process.env.AGENT_TTL_SECONDS);
+ const ttlSeconds = Number.isFinite(parsedTtl) && parsedTtl > 0 ? parsedTtl : 120;
const ttlThresholdMs = ttlSeconds * 1000;
@@
- const lastSeenMs = new Date(agent.last_seen).getTime();
- const isStale = nowMs - lastSeenMs > ttlThresholdMs;
+ const lastSeenMs = Date.parse(agent.last_seen);
+ const isStale = Number.isNaN(lastSeenMs) || nowMs - lastSeenMs > ttlThresholdMs;📝 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.
| const ttlSeconds = Number(process.env.AGENT_TTL_SECONDS) || 120; | |
| const ttlThresholdMs = ttlSeconds * 1000; | |
| let mutated = false; | |
| // Process live lifecycle updates based on absolute timestamp drift | |
| const updatedAgents = agents.map((agent) => { | |
| const lastSeenMs = new Date(agent.last_seen).getTime(); | |
| const isStale = nowMs - lastSeenMs > ttlThresholdMs; | |
| const parsedTtl = Number(process.env.AGENT_TTL_SECONDS); | |
| const ttlSeconds = Number.isFinite(parsedTtl) && parsedTtl > 0 ? parsedTtl : 120; | |
| const ttlThresholdMs = ttlSeconds * 1000; | |
| let mutated = false; | |
| // Process live lifecycle updates based on absolute timestamp drift | |
| const updatedAgents = agents.map((agent) => { | |
| const lastSeenMs = Date.parse(agent.last_seen); | |
| const isStale = Number.isNaN(lastSeenMs) || nowMs - lastSeenMs > ttlThresholdMs; |
🤖 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 `@packages/registry/src/store.ts` around lines 40 - 47, The TTL and timestamp
validation logic lacks input validation which allows incorrect staleness
evaluation. First, validate the parsed ttlSeconds value to ensure it is a
positive number greater than zero before using it to calculate ttlThresholdMs,
rejecting zero or negative values. Second, when parsing the agent.last_seen
timestamp in the agents.map function, validate that the resulting lastSeenMs is
a valid number (not NaN) before calculating isStale, and skip or handle agents
with invalid timestamps appropriately to prevent them from being incorrectly
classified as active. This ensures only valid TTL and timestamp values are used
in the stale-state evaluation logic.
|
@mijinummi pls fix failed CI and address major comments from coderabbit. Nice work |
2 participants
Summary
Summary by CodeRabbit