Please address the comments from this code review:

## Overall Comments
- The global `toolStartData` map in `pi-embedded-subscribe.handlers.tools.ts` is only cleaned up in `handleToolExecutionEnd`, so if a tool call never emits an `*_end` event (e.g., aborted runs) entries will accumulate; consider adding a timeout/cleanup or a defensive delete on `handleToolExecutionStart` to avoid leaks across long-lived processes.
- Hook failures are silently swallowed in several places (`message_sending` / `message_sent` in `deliverOutboundPayloads`, and session hooks in `initSessionState`) while similar hooks elsewhere log warnings; for easier debugging and consistent behavior you may want to at least log a warning in these `catch` blocks.

## Individual Comments

### Comment 1
<location> `src/infra/outbound/deliver.ts:379-367` </location>
<code_context>
       params.onPayload?.(payloadSummary);
-      if (handler.sendPayload && payload.channelData) {
-        results.push(await handler.sendPayload(payload));
+      if (handler.sendPayload && effectivePayload.channelData) {
+        results.push(await handler.sendPayload(effectivePayload));
         continue;
       }
       if (payloadSummary.mediaUrls.length === 0) {
</code_context>

<issue_to_address>
**issue (bug_risk):** message_sent hook is never called for handlers using sendPayload due to early continue

In the `handler.sendPayload && effectivePayload.channelData` branch, the function `continue`s before invoking the `message_sent` hook, so plugins relying on that hook won’t see messages sent via `sendPayload`, unlike the media/text paths below. To keep hook behavior consistent across all successful send paths, move the `message_sent` invocation before the `continue` or refactor so all branches go through a shared post-send section that triggers it.
</issue_to_address>

### Comment 2
<location> `src/infra/heartbeat-runner.ts:119` </location>
<code_context>
+
+const HEARTBEAT_OK_PREFIX = HEARTBEAT_TOKEN.toLowerCase();
+
+// Detect heartbeat-specific noise so cron reminders don't trigger on non-reminder events.
+function isHeartbeatAckEvent(evt: string): boolean {
+  const trimmed = evt.trim();
</code_context>

<issue_to_address>
**issue (complexity):** Consider replacing the multiple small predicate helpers with a single event classifier plus an analyzer so the heartbeat flow reads as classify → analyze → choose prompt.

You can reduce the mental overhead by collapsing the tiny predicates into a single classifier + analyzer, and drive `runHeartbeatOnce` from that. This keeps behavior while making the flow “read” like the domain: classify events → analyze → choose prompt.

### 1. Collapse predicates into a classifier

Instead of `isHeartbeatAckEvent`, `isHeartbeatNoiseEvent`, `isExecCompletionEvent`, and `isCronSystemEvent` all composing each other, fold them into one `classifySystemEvent` and a small analyzer. You can keep `HEARTBEAT_OK_PREFIX` and the regex logic, just move it inside.

```ts
type SystemEventKind = "cron" | "exec-completion" | "heartbeat-noise" | "other";

function classifySystemEvent(evt: string): SystemEventKind {
  const trimmed = evt.trim();
  if (!trimmed) return "other";

  const lower = trimmed.toLowerCase();

  // exec completion detection (previous isExecCompletionEvent)
  if (lower.includes("exec finished")) {
    return "exec-completion";
  }

  // heartbeat noise detection (previous isHeartbeatAckEvent + isHeartbeatNoiseEvent)
  const isHeartbeatAck = (() => {
    if (!lower.startsWith(HEARTBEAT_OK_PREFIX)) return false;
    const suffix = lower.slice(HEARTBEAT_OK_PREFIX.length);
    if (!suffix.length) return true;
    return !/[a-z0-9_]/.test(suffix[0]);
  })();

  if (
    isHeartbeatAck ||
    lower.includes("heartbeat poll") ||
    lower.includes("heartbeat wake")
  ) {
    return "heartbeat-noise";
  }

  // Anything else is treated as cron content
  return "cron";
}

// Preserve the existing export but simplify it to use the classifier.
export function isCronSystemEvent(evt: string) {
  return classifySystemEvent(evt) === "cron";
}
```

This removes layered negations and consolidates “what kind of system event is this?” into one place.

### 2. Centralize analysis of pending system events

You can keep `buildCronEventPrompt` as‑is, but co‑locate it with a small analyzer so `runHeartbeatOnce` doesn’t need to know all the details:

```ts
function analyzeSystemEvents(
  reason: string | undefined,
  pendingEvents: string[]
): { cronEvents: string[]; hasExecCompletion: boolean; hasCronEvents: boolean } {
  const isExecEvent = reason === "exec-event";
  const isCronEvent = Boolean(reason?.startsWith("cron:"));

  let cronEvents: string[] = [];
  let hasExecCompletion = false;

  for (const evt of pendingEvents) {
    const kind = classifySystemEvent(evt);
    if (kind === "exec-completion") {
      hasExecCompletion = true;
    } else if (kind === "cron") {
      cronEvents.push(evt);
    }
  }

  const hasCronEvents = isCronEvent && cronEvents.length > 0;
  return { cronEvents, hasExecCompletion, hasCronEvents };
}
```

### 3. Simplify `runHeartbeatOnce` call site

Now `runHeartbeatOnce` can be simpler and more self‑descriptive, without multiple low‑level helpers:

```ts
const isExecEvent = opts.reason === "exec-event";
const isCronEvent = Boolean(opts.reason?.startsWith("cron:"));
const pendingEvents = isExecEvent || isCronEvent ? peekSystemEvents(sessionKey) : [];

const { cronEvents, hasExecCompletion, hasCronEvents } = analyzeSystemEvents(
  opts.reason,
  pendingEvents
);

const prompt = hasExecCompletion
  ? EXEC_EVENT_PROMPT
  : hasCronEvents
    ? buildCronEventPrompt(cronEvents)
    : resolveHeartbeatPrompt(cfg, heartbeat);

const ctx = {
  Body: appendCronStyleCurrentTimeLine(prompt, cfg, startedAt),
  From: sender,
  To: sender,
  Provider: hasExecCompletion ? "exec-event" : hasCronEvents ? "cron-event" : "heartbeat",
  SessionKey: sessionKey,
};
```

Behavior is preserved:

- Exec completion detection still uses the same `"exec finished"` logic.
- Heartbeat noise (ack, poll, wake) is still filtered out with the same normalization and regex.
- Cron events are now the explicit `"cron"` classification, not “not noise and not exec”.

But the complexity at the top level is reduced to “classify → analyze → prompt”, and the string‑level nuance is contained in one place.
</issue_to_address>

### Comment 3
<location> `src/infra/outbound/deliver.ts:342` </location>
<code_context>
     return normalized ? [normalized] : [];
   });
+  const hookRunner = getGlobalHookRunner();
   for (const payload of normalizedPayloads) {
     const payloadSummary: NormalizedOutboundPayload = {
       text: payload.text ?? "",
</code_context>

<issue_to_address>
**issue (complexity):** Consider extracting the hook-handling logic into small helper functions so the delivery loop focuses on core behavior and is easier to read.

You can keep the new behavior but reduce cognitive load in the loop by pushing the hook logic into small helpers and consolidating the duplicated `message_sent` calls.

### 1. Extract `message_sending` hook into a helper

Encapsulate the try/catch, cancel semantics, and mutation of payload into a dedicated function:

```ts
type MessageSendingInput = {
  to: string;
  channel: string;
  accountId?: string | null;
  payload: NormalizedOutboundPayloadInputType; // whatever `payload` is
  summary: NormalizedOutboundPayload;
};

async function runMessageSendingHook(
  hookRunner: ReturnType<typeof getGlobalHookRunner> | undefined,
  { to, channel, accountId, payload, summary }: MessageSendingInput,
): Promise<{ canceled: boolean; effectivePayload: typeof payload }> {
  if (!hookRunner?.hasHooks("message_sending")) {
    return { canceled: false, effectivePayload: payload };
  }

  try {
    const sendingResult = await hookRunner.runMessageSending(
      {
        to,
        content: summary.text,
        metadata: { channel, accountId, mediaUrls: summary.mediaUrls },
      },
      { channelId: channel, accountId: accountId ?? undefined },
    );

    if (sendingResult?.cancel) {
      return { canceled: true, effectivePayload: payload };
    }

    if (sendingResult?.content != null) {
      summary.text = sendingResult.content;
      return { canceled: false, effectivePayload: { ...payload, text: sendingResult.content } };
    }

    return { canceled: false, effectivePayload: payload };
  } catch {
    // Don't block delivery on hook failure
    return { canceled: false, effectivePayload: payload };
  }
}
```

Then the loop becomes easier to scan:

```ts
for (const payload of normalizedPayloads) {
  const payloadSummary: NormalizedOutboundPayload = { /* ... */ };

  try {
    throwIfAborted(abortSignal);

    const { canceled, effectivePayload } = await runMessageSendingHook(hookRunner, {
      to,
      channel,
      accountId,
      payload,
      summary: payloadSummary,
    });
    if (canceled) continue;

    params.onPayload?.(payloadSummary);

    if (handler.sendPayload && effectivePayload.channelData) {
      results.push(await handler.sendPayload(effectivePayload));
      continue;
    }
    // rest of delivery logic unchanged...
```

### 2. Consolidate `message_sent` success/failure into a helper

The success/failure paths are nearly identical; a helper makes the intent explicit and removes duplication:

```ts
function fireMessageSentHook(
  hookRunner: ReturnType<typeof getGlobalHookRunner> | undefined,
  {
    to,
    channel,
    accountId,
    content,
    success,
    error,
  }: {
    to: string;
    channel: string;
    accountId?: string | null;
    content: string;
    success: boolean;
    error?: unknown;
  },
): void {
  if (!hookRunner?.hasHooks("message_sent")) return;

  const errorMessage =
    !success || error != null
      ? error instanceof Error
        ? error.message
        : String(error)
      : undefined;

  void hookRunner
    .runMessageSent(
      { to, content, success, error: errorMessage },
      { channelId: channel, accountId: accountId ?? undefined },
    )
    .catch(() => {});
}
```

Then in the loop:

```ts
// on success
fireMessageSentHook(hookRunner, {
  to,
  channel,
  accountId,
  content: payloadSummary.text,
  success: true,
});

// in catch
} catch (err) {
  fireMessageSentHook(hookRunner, {
    to,
    channel,
    accountId,
    content: payloadSummary.text,
    success: false,
    error: err,
  });

  if (!params.bestEffort) {
    throw err;
  }
  params.onError?.(err, payloadSummary);
}
```

This keeps all existing behavior (including fire‑and‑forget semantics and error swallowing for hooks) but makes the core loop mostly about delivery, with hook behavior clearly named and localized.
</issue_to_address>