Greptile Summary

This PR fixes a long-standing async context routing bug by propagating sessionKey through the exec/hooks heartbeat wake paths, ensuring exec completion responses reliably route back to the originating session and channel without requiring any user configuration changes.

Key changes:

• resolveEventSessionKey — new helper that remaps cron session keys to the agent's main session key before event enqueue, so the heartbeat runner can locate events stored under the correct key.
• scopedHeartbeatWakeOptions — now also strips sessionKey for cron sessions (in addition to non-agent keys), maintaining the legacy unscoped wake behavior for those paths.
• forceLastTargetWhenNone — new flag on resolveHeartbeatDeliveryTarget that forces exec completion events to the session's last delivery channel even when heartbeat.target = none, bypassing the silent-discard path that caused the original regression.
• isExecCompletionEvent — expanded with a start-anchored regex (/^exec (?:completed|failed|killed) \(/) to cover the maybeNotifyOnExit backgrounded command path; addresses the false-positive concern raised in earlier review by anchoring patterns instead of doing substring matches.
• resolveHeartbeatReasonKind — maps exec:<id>:exit reasons to the "exec-event" kind, feeding correctly into the isExecEventReason preflight flag.
• The JSDoc on resolveEventSessionKey clearly documents the known limitation: custom session.mainKey setups are not supported for cron event remapping due to lack of config access in that call path. No action required, but worth noting for future config-aware refactors.

Confidence Score: 5/5

• Safe to merge — targeted bug fix with comprehensive test coverage and no regressions identified.
• All four change pillars (session key propagation, wake scoping, delivery target override, event classification) are implemented correctly and independently tested. Previous review concerns about false-positive substring matching have been fully addressed with start-anchored patterns. The one theoretical edge case (invalid rawTarget string with a configured heartbeat.to) is benign in practice. Test suite covers both the happy path and the false-positive guard cases added based on prior feedback.
• No files require special attention.

_{Reviews (2): Last reviewed commit: "fix: anchor exec completion regex to sta..." | Re-trigger Greptile}

Thanks for the review feedback. Pushed a commit addressing both Greptile suggestions:

• isExecCompletionEvent: Anchored to exec (?:completed|failed|killed) \( regex to avoid false positives in free-form cron text
• targets.ts: Collapsed redundant double-check into single if/else

Regarding @chatgpt-codex-connector's concern about forceLastTargetWhenNone firing for non-completion exec events: in practice, requestHeartbeatNow({ reason: "exec-event" }) is only called from completion paths (server-node-events.ts for exec.finished, and maybeNotifyOnExit for backgrounded commands). The exec.denied/exec.running events use enqueueSystemEvent without triggering a heartbeat wake, so isExecEventReason won't be true for those. The current gating is safe, but open to tightening it if maintainers prefer belt-and-suspenders.

To use Codex here, create an environment for this repo.

Good catch on the stale heartbeat.to/heartbeat.accountId edge case. Pushed a fix:

• Added a forcedToLast flag that's true when forceLastTargetWhenNone activates on a target: "none" config
• When active, explicitTo passes undefined to resolveSessionDeliveryTarget instead of heartbeat.to
• heartbeatAccountId is also set to undefined under the same condition
• sessionChatTypeHint now correctly normalizes chat type when forced (treats it as if heartbeat.to is absent)

No config mutation — the original heartbeat object is untouched. The flag only affects downstream resolution for that call.

Heads up — scopedHeartbeatWakeOptions in src/routing/session-key.ts passes through sessionKey for any valid agent session key, including cron run sessions. This means exec completions inside cron jobs (e.g. a gws CLI call in an isolated cron task) fire a heartbeat wake scoped to the cron session instead of falling back to main.

In practice this injects the heartbeat prompt into an in-flight cron session, confusing the model mid-task. We've hit this multiple times — an isolated overnight-thinking cron job running sequential gws gmail exec calls gets heartbeat wakes injected after each completion, causing the model to re-run the task or narrate its internal steps into the delivery channel.

The fix would be a one-line guard in scopedHeartbeatWakeOptions:

export function scopedHeartbeatWakeOptions<T extends object>(
  sessionKey: string,
  wakeOptions: T,
): T | (T & { sessionKey: string }) {
  if (!parseAgentSessionKey(sessionKey)) return wakeOptions;
  if (isCronSessionKey(sessionKey)) return wakeOptions;
  return { ...wakeOptions, sessionKey };
}

isCronSessionKey is already imported in that file.

Good catch @briannewman — applied in a3d2cfc. Added the isCronSessionKey guard to scopedHeartbeatWakeOptions so cron sessions skip wake scoping entirely.

Squashed the incremental review-driven commits into a single clean commit (347ca05) for easier review. Here's what's in it:

1. sessionKey propagation — the original fix. requestHeartbeatNow calls in hooks and node-events now carry the originating session key via scopedHeartbeatWakeOptions.

2. Exec completion detection — isExecCompletionEvent anchored to the parenthesised format (Exec completed/failed/killed () to avoid false positives in free-form cron text. Regex pre-compiled.

3. Target routing for exec wakes — when heartbeat.target is "none" or omitted, exec completion wakes force fallback to the session's last delivery target. Stale heartbeat.to and heartbeat.accountId are cleared when forced (forcedToLast covers both explicit "none" and undefined).

4. Cron session isolation (from @briannewman's feedback) — scopedHeartbeatWakeOptions strips sessionKey for cron sessions to prevent heartbeat prompt injection. To avoid orphaning exec completion events, resolveEventSessionKey remaps system events from cron sessions to the agent's main session key. Applied to all 4 callers: bash-tools.exec-runtime.ts, server-node-events.ts, acp-spawn-parent-stream.ts, cli-runner.ts.

5. Test coverage — new tests for scopedHeartbeatWakeOptions (cron/agent/non-agent keys), resolveEventSessionKey (remapping + passthrough), and exec completion false-positive resistance on free-form cron text. 136 tests passing.

Known limitation: resolveEventSessionKey uses the default main key ("main"). Setups with a custom session.mainKey are not supported for cron event remapping — callers in the exec path don't have config access. Documented in code.

Branch rebased onto latest main, resolving merge conflicts and stale CI.

Re: @greptile-apps suggestion to collapse the target === "none" guards into a single if/else — the current early-return pattern is intentional and reads more clearly than the nested alternative. The two guards handle distinct cases (no-force → early return with no-delivery target; force → fall through to "last") and the early return makes the control flow explicit. No functional change needed here.

Requesting re-review. cc @greptile-apps @chatgpt-codex-connector

Fixed the P1 issue raised by @chatgpt-codex-connector:

Unscoped cron wakes — Cron session keys now resolve to the originating agent's main session key (e.g., agent:main:cron:backup:run:abc → agent:main:main). This prevents wake requests from fanning out to all agents while avoiding reference to the ephemeral cron session key.

This is consistent with how resolveEventSessionKey already handles cron remapping (same file, line 58-65).

Test updated to verify the new scoping behavior. Commit: 3d854506d3.

Rebase onto upstream/main (2026-05-09)

Resolved conflict in src/routing/session-key.ts: upstream cleaned up unused top-level imports (type ParsedAgentSessionKey, DEFAULT_ACCOUNT_ID); kept isCronRunSessionKey which the PR's new function bodies require.

Updated this PR for current upstream/main and pushed c766369ff0.

Changes since the earlier revision:

• Rebased onto latest main.
• No functional code changes were needed after the rebase.
• Applied the current formatter to src/routing/session-key.ts.

Validation:

• pnpm test src/routing/session-key.test.ts src/infra/heartbeat-events-filter.test.ts passed.
• node scripts/check-deprecated-api-usage.mjs passed.
• node scripts/check-no-runtime-action-load-config.mjs passed.
• pnpm exec oxfmt --check --threads=1 ... on the changed files passed.
• git diff --check passed.

Note: standalone oxlint over this PR's changed files still reports the existing __testing export naming issue in src/agents/pi-tools.ts and src/agents/bash-tools.exec.ts; those exports are already present on current upstream/main, so I left them untouched.

Pushed follow-up commit 735f2c3c7c (fix(heartbeat): strengthen cron-run routing proof).

Local verification:

• pnpm test src/routing/session-key.test.ts src/infra/heartbeat-events-filter.test.ts src/agents/bash-tools.exec-runtime.test.ts src/cron/session-reaper.test.ts src/agents/acp-spawn-parent-stream.test.ts passed.
• node scripts/check-deprecated-api-usage.mjs passed.
• node scripts/check-no-runtime-action-load-config.mjs passed.
• pnpm exec oxfmt --check --threads=1 ... over the changed files passed.
• git diff --check passed.
• Required local Claude review completed after the final follow-up; it found no regressions and marked the cron-run descendant pruning and ACP relay routing asks addressed.

Notes:

• Standalone oxlint over the touched files still reports only the existing upstream export const __testing no-underscore-dangle errors in src/agents/pi-tools.ts and src/agents/bash-tools.exec.ts.
• I did not do live gateway/channel heartbeat proof from this environment, so that remains a runtime validation gap.

Closing in favor of a clean replacement PR from current main: #80214.\n\nThe replacement keeps the current narrowed fix for #52305, drops the stale review history, and presents the change as one clean commit with current validation.

Superseded by #80214.