Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 149 additions & 0 deletions pi/extensions/heartbeat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
* 2. Slack bridge — HTTP POST to localhost:7890/send returns 400
* 3. Stale worktrees — ~/workspace/worktrees/ has dirs with no matching in-progress todo
* 4. Stuck todos — in-progress for >2 hours with no matching dev-agent session
* 5. Unanswered Slack mentions — app_mention events in bridge log with no reply within 5 min
*
* Configuration (env vars):
* HEARTBEAT_INTERVAL_MS — interval between heartbeats (default: 600000 = 10 min)
* HEARTBEAT_ENABLED — set to "0" or "false" to disable (default: enabled)
* HEARTBEAT_EXPECTED_SESSIONS — comma-separated session aliases to check (default: "sentry-agent")
* HEARTBEAT_CHECK_UNANSWERED_MENTIONS — set to "1" or "true" to enable (default: enabled)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment is misleading - should say "enabled by default, set to 0/false/no to disable"

Suggested change
* HEARTBEAT_CHECK_UNANSWERED_MENTIONS set to "1" or "true" to enable (default: enabled)
* HEARTBEAT_CHECK_UNANSWERED_MENTIONS enabled by default, set to "0" or "false" to disable
Prompt To Fix With AI
This is a comment left during a code review.
Path: pi/extensions/heartbeat.ts
Line: 18

Comment:
Comment is misleading - should say "enabled by default, set to 0/false/no to disable"

```suggestion
 *   HEARTBEAT_CHECK_UNANSWERED_MENTIONS — enabled by default, set to "0" or "false" to disable
```

How can I resolve this? If you propose a fix, please make it concise.

*
* When all checks pass, zero LLM tokens are consumed. When something fails,
* a targeted prompt is injected describing only the failures so the control-agent
Expand All @@ -37,6 +39,9 @@ const SOCKET_DIR = join(homedir(), ".pi", "session-control");
const WORKTREES_DIR = join(homedir(), "workspace", "worktrees");
const TODOS_DIR = join(homedir(), ".pi", "todos");
const BRIDGE_URL = "http://127.0.0.1:7890/send";
const BRIDGE_LOG = join(homedir(), ".pi", "agent", "logs", "slack-bridge.log");
const SESSION_DIR = join(homedir(), ".pi", "agent", "sessions");
const UNANSWERED_MENTION_THRESHOLD_MS = 5 * 60 * 1000; // 5 minutes

type HeartbeatState = {
enabled: boolean;
Expand Down Expand Up @@ -72,6 +77,12 @@ function getExpectedSessions(): string[] {
return ["sentry-agent"];
}

function isUnansweredMentionsCheckEnabled(): boolean {
const val = process.env.HEARTBEAT_CHECK_UNANSWERED_MENTIONS?.trim().toLowerCase();
// Default to enabled unless explicitly disabled
return val !== "0" && val !== "false" && val !== "no";
}

// ── Health Check Functions ──────────────────────────────────────────────────

function checkSessions(): CheckResult[] {
Expand Down Expand Up @@ -300,6 +311,131 @@ function checkStuckTodos(): CheckResult[] {
return results;
}

function checkUnansweredMentions(): CheckResult[] {
const results: CheckResult[] = [];
const now = Date.now();

if (!existsSync(BRIDGE_LOG)) return results;

try {
// Read the last 500 lines of the bridge log to find recent app_mention events
const { execSync } = require("node:child_process");
const logTail = execSync(`tail -500 "${BRIDGE_LOG}"`, { encoding: "utf-8" });

// Parse log lines looking for app_mention events
const mentionPattern = /\[([^\]]+)\].*app_mention.*ts: (\d+\.\d+)/g;
const mentions: Array<{ timestamp: string; ts: string }> = [];

let match;
while ((match = mentionPattern.exec(logTail)) !== null) {
mentions.push({
timestamp: match[1],
ts: match[2],
});
}

// Filter to recent mentions (within last hour)
const oneHourAgo = now - 60 * 60 * 1000;
const recentMentions = mentions.filter(m => {
try {
const mentionTime = new Date(m.timestamp).getTime();
return mentionTime > oneHourAgo;
} catch {
return false;
}
});

// For each recent mention, check if we replied to it
for (const mention of recentMentions) {
const mentionTime = new Date(mention.timestamp).getTime();
const age = now - mentionTime;

// Skip very recent mentions (< 5 min) - agent might still be processing
if (age < UNANSWERED_MENTION_THRESHOLD_MS) continue;

// Check if we sent a reply to this thread_ts
const replied = hasRepliedToThread(mention.ts);

if (!replied) {
const minutesAgo = Math.round(age / (60 * 1000));
results.push({
name: `unanswered:${mention.ts}`,
ok: false,
detail: `Slack mention at ts ${mention.ts} (${minutesAgo} min ago) has no reply — may have been lost during restart`,
});
}
}
} catch (err: unknown) {
// Log read failure or exec error - non-fatal, but log it
const msg = err instanceof Error ? err.message : String(err);
// Don't report this as a failure unless we have a specific problem to report
}

return results;
}

function hasRepliedToThread(threadTs: string): boolean {
// Check multiple sources for evidence of a reply to this thread_ts.

// 1. Check the reply tracking log (most reliable — written by the agent).
// File: ~/.pi/agent/slack-reply-log.jsonl
// Each line: {"thread_ts":"...","replied_at":"..."}
const replyLogPath = join(homedir(), ".pi", "agent", "slack-reply-log.jsonl");
if (existsSync(replyLogPath)) {
try {
const content = readFileSync(replyLogPath, "utf-8");
if (content.includes(threadTs)) return true;
} catch {
// File read error — fall through to other checks
}
}

// 2. Check control-agent session logs for the thread_ts.
// Session files are in ~/.pi/agent/sessions/--home-baudbot_agent--/
// and named <timestamp>_<uuid>.jsonl.
// If the agent processed a thread_ts (via curl /send with thread_ts),
// the JSONL will contain it in the tool call arguments.
const controlAgentSessionDir = join(SESSION_DIR, "--home-baudbot_agent--");
if (existsSync(controlAgentSessionDir)) {
try {
const sessionFiles = readdirSync(controlAgentSessionDir)
.filter(f => f.endsWith(".jsonl"))
.sort()
.reverse()
.slice(0, 3); // Check last 3 sessions

for (const file of sessionFiles) {
try {
const content = readFileSync(join(controlAgentSessionDir, file), "utf-8");
// Look for evidence of a reply: the thread_ts appearing in a curl /send command
// or in a send_to_session message. The thread_ts in an outbound context
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple string search could have false positives if threadTs appears in error messages or logs without being an actual reply. Consider checking for more specific patterns like the thread being mentioned in a tool call or message send context.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: pi/extensions/heartbeat.ts
Line: 411

Comment:
Simple string search could have false positives if `threadTs` appears in error messages or logs without being an actual reply. Consider checking for more specific patterns like the thread being mentioned in a tool call or message send context.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

// (not just the inbound mention) indicates we replied.
if (content.includes(`"thread_ts":"${threadTs}"`) ||
content.includes(`"thread_ts": "${threadTs}"`) ||
content.includes(`\\"thread_ts\\":\\"${threadTs}\\"`) ||
content.includes(`\\"thread_ts\\":\\"${threadTs}\\"`)) {
return true;
}
} catch {
// File read error - skip
}
}
} catch {
// Dir read error
}
}

// 3. Check the bridge log for the ✅ check reaction resolving this thread.
// The bridge logs failures like "✅ check reaction failed" but successful
// ack reactions are silent. Still worth checking for the thread_ts in
// any outbound context (e.g., reaction calls).
// Also check for the 👀 eyes reaction — if we reacted with eyes AND
// the thread_ts appears in an outbound /send context, we likely replied.
// (This is a weak signal but better than nothing.)

return false;
}

// ── Helper Functions ────────────────────────────────────────────────────────

function hasMatchingTodo(devAgentName: string): boolean {
Expand Down Expand Up @@ -412,12 +548,16 @@ export default function heartbeatExtension(pi: ExtensionAPI): void {
const bridgeResult = await checkBridge();
const worktreeResults = checkWorktrees();
const stuckTodoResults = checkStuckTodos();
const unansweredMentionResults = isUnansweredMentionsCheckEnabled()
? checkUnansweredMentions()
: [];

const allResults: CheckResult[] = [
Comment on lines 413 to 416
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The hasRepliedToThread() function incorrectly identifies inbound messages as replies by using a broad string search for thread_ts in session logs, leading to false negatives.
Severity: HIGH

Suggested Fix

Refine the search logic in hasRepliedToThread() to differentiate between inbound and outbound messages. Instead of a simple string search, parse the session log entries and only check for thread_ts within the context of an outbound action, such as a send_to_session tool call or a curl command to the Slack API. This ensures that only actual replies are counted.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: pi/extensions/heartbeat.ts#L413-L416

Potential issue: The `hasRepliedToThread()` function checks for replies by searching for
the string `"thread_ts":"<threadTs>"` anywhere in the session log file. However, inbound
Slack messages are logged with their metadata, which includes the `thread_ts`. If the
agent processes an inbound mention and logs it but then crashes before sending a reply,
the function will find the `thread_ts` in the logged inbound message and incorrectly
conclude that a reply was sent. This defeats the purpose of the unanswered mention
detection feature, as lost messages will go undetected.

Did we get this right? 👍 / 👎 to inform future reviews.

...sessionResults,
bridgeResult,
...worktreeResults,
...stuckTodoResults,
...unansweredMentionResults,
];

const failures = allResults.filter((r) => !r.ok);
Expand Down Expand Up @@ -550,12 +690,16 @@ export default function heartbeatExtension(pi: ExtensionAPI): void {
const bridgeResult = await checkBridge();
const worktreeResults = checkWorktrees();
const stuckTodoResults = checkStuckTodos();
const unansweredMentionResults = isUnansweredMentionsCheckEnabled()
? checkUnansweredMentions()
: [];

const allResults: CheckResult[] = [
...sessionResults,
bridgeResult,
...worktreeResults,
...stuckTodoResults,
...unansweredMentionResults,
];

const failures = allResults.filter((r) => !r.ok);
Expand Down Expand Up @@ -592,6 +736,7 @@ export default function heartbeatExtension(pi: ExtensionAPI): void {

case "config": {
const expected = getExpectedSessions();
const checkUnanswered = isUnansweredMentionsCheckEnabled();
return {
content: [
{
Expand All @@ -604,11 +749,15 @@ export default function heartbeatExtension(pi: ExtensionAPI): void {
` Backoff multiplier: ${BACKOFF_MULTIPLIER}x per error`,
` Max backoff: ${MAX_BACKOFF_MS / 1000}s`,
` Expected sessions: ${expected.join(", ")} (env: HEARTBEAT_EXPECTED_SESSIONS)`,
` Check unanswered mentions: ${checkUnanswered ? "enabled" : "disabled"} (env: HEARTBEAT_CHECK_UNANSWERED_MENTIONS)`,
` Unanswered mention threshold: ${UNANSWERED_MENTION_THRESHOLD_MS / (60 * 1000)} min`,
` Stuck todo threshold: ${STUCK_TODO_THRESHOLD_MS / (60 * 60 * 1000)}h`,
` Bridge URL: ${BRIDGE_URL}`,
` Bridge log: ${BRIDGE_LOG}`,
` Socket dir: ${SOCKET_DIR}`,
` Worktrees dir: ${WORKTREES_DIR}`,
` Todos dir: ${TODOS_DIR}`,
` Session dir: ${SESSION_DIR}`,
].join("\n"),
},
],
Expand Down