From a0f7d3dabaf5308ade45cae0c64ebd133948dca2 Mon Sep 17 00:00:00 2001 From: Reasonofmoon <124511116+Reasonofmoon@users.noreply.github.com> Date: Wed, 10 Jun 2026 06:16:46 +0900 Subject: [PATCH] Reset task session on timer-driven wakes (PF-4) (#4838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Each agent is woken via the heartbeat scheduler — `heartbeat_timer` for periodic interval wakes, `issue_assigned` / `execution_*` / `issue_commented` for event-driven wakes > - The heartbeat reuses the prior task session by default; only specific wake reasons trigger a fresh session via `shouldResetTaskSessionForWake` (assignment, review, approval, changes-requested) or explicit `forceFreshSession` > - In CEO run `292a5fd1`, repeated context compaction warnings appeared near the 64k threshold for the long-lived manager session — symptomatic of repeated `heartbeat_timer` wakes accumulating low-value "checked, nothing new" inbox-scan traces inside one ever-growing session > - PF-4 in the 2026-04-16 hangeul-school operational issue set asks for a compaction-aware session freshness policy: "manager sessions can rotate before low-value compaction pressure accumulates" and "repeated timer wakes do not indefinitely bloat the same session" > - This pull request adds `wakeReason === "heartbeat_timer"` to both `shouldResetTaskSessionForWake` and `describeSessionResetReason`, so each interval wake starts fresh and the run log explicitly records why. Event-driven wakes (`issue_commented`, `transient_failure_retry`, etc.) keep their existing reuse behavior. > - The benefit is that timer wakes — which are exploratory and carry no continuation state — stop bloating long-lived manager sessions. Compaction pressure that previously accumulated across N timer wakes is now bounded to a single interval's worth of context. ## Linked Issues or Issue Description No external GitHub issue is linked. Describing the problem inline following the bug-report template: **What happened:** Long-lived manager/CEO agent sessions hit the 64k context-compaction threshold after many `heartbeat_timer` wakes accumulated low-value inbox-scan traces inside one ever-growing task session. Reproduced in CEO run `292a5fd1`. **Expected behavior:** Periodic timer wakes — which carry no continuation state — should not indefinitely bloat the same session. The heartbeat should rotate sessions on timer wakes the way it already does on assignment/review/approval/changes-requested wakes. **Actual behavior:** `shouldResetTaskSessionForWake` only reset on `issue_assigned`, `execution_review_requested`, `execution_approval_requested`, `execution_changes_requested`, or explicit `forceFreshSession`. `heartbeat_timer` reused the prior session indefinitely, causing compaction pressure. **Scope of fix:** Add `heartbeat_timer` to the reset list and to `describeSessionResetReason` so the run log records why. Event-driven wakes keep their existing reuse behavior. ## What Changed - `shouldResetTaskSessionForWake` (`server/src/services/heartbeat.ts`) now also returns `true` when `wakeReason === "heartbeat_timer"`. The existing reset reasons (`issue_assigned`, `execution_review_requested`, `execution_approval_requested`, `execution_changes_requested`, `forceFreshSession`) are unchanged. - `describeSessionResetReason` returns a paired explanation `"wake reason is heartbeat_timer (timer-driven wake starts fresh)"` so run logs make session reset behavior legible. - `describeSessionResetReason` was promoted from internal to `export` so the paired contract can be unit-tested directly alongside `shouldResetTaskSessionForWake`. This is the only API surface change in this PR. Wake reasons whose reuse behavior is intentionally **unchanged**: - `issue_commented` — the comment is the reason to engage; continuation context matters - `issue_comment_mentioned` — same rationale - `transient_failure_retry` — resuming a previously-failed run; want continuity - `process_lost_retry` — resuming after process loss; want continuity - `missing_issue_comment`, recovery reasons — out of scope; can be revisited as follow-ups if observed bloat shows up ## Verification ```bash cd server pnpm vitest run src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts # 12/12 pass pnpm vitest run \ src/__tests__/heartbeat-stale-queue-invalidation.test.ts \ src/__tests__/heartbeat-process-recovery.test.ts \ src/__tests__/heartbeat-comment-wake-batching.test.ts # 48/48 adjacent heartbeat tests pass ``` The 12 new tests assert: 1. `shouldResetTaskSessionForWake` resets on `heartbeat_timer` 2. `shouldResetTaskSessionForWake` still resets on the four existing reasons 3. `forceFreshSession === true` still triggers reset 4. `issue_commented`, `transient_failure_retry`, unknown reasons, and null/undefined context do **not** trigger reset 5. `describeSessionResetReason` describes `heartbeat_timer` explicitly so logs are legible 6. `describeSessionResetReason` keeps the exact wording for the four existing reasons 7. `describeSessionResetReason` returns the `forceFreshSession` message 8. `describeSessionResetReason` returns `null` for non-resetting reasons 9. **Parity invariant**: the two functions agree on every input — `describeSessionResetReason(ctx)` is non-null iff `shouldResetTaskSessionForWake(ctx)` returns true. This locks the pair so future changes to one must update the other. ## Risks - **Low–medium.** This changes behavior for every `heartbeat_timer` wake on every agent: the prior task session is no longer reused. - For **manager / CEO agents** (the documented case): this is the intended improvement. Timer wakes carry no continuation state for these roles. - For **worker agents** that may have used timer wakes to resume in-flight work: any genuine continuation should already be triggered by issue/execution wake reasons (which still reuse) or by an active checkout being resumed via `process_lost_retry` / `transient_failure_retry`. Timer wakes themselves do not create checkouts. - If a deployment relied on timer wakes to preserve mid-task context — which is fragile by design — the right path is to switch to a non-timer wake reason or accept the reset. The PR doesn't add a new opt-out flag because the goal is to bound session size; introducing an opt-out would re-open the bloat path this PR is closing. - No schema or API surface change beyond exporting `describeSessionResetReason`. No migration. No client-visible API change. ## Model Used Claude Opus 4.7 (1M context), model ID `claude-opus-4-7[1m]`. Used in interactive Claude Code session with extended reasoning, tool use (Read/Edit/Write/Bash), and verification gates between exploration → fix → tests → push. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched the open PR list for similar/duplicate work — distinct from #4080 (force-fresh follow-up wake — codex/general) and #4195 (codex session reset on model change); this PR specifically targets the `heartbeat_timer` reuse path - [x] I have run tests locally and they pass (12 new + 48 adjacent = 60 tests, no regressions) - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots — N/A, server-only change - [x] I have updated relevant documentation to reflect my changes — none needed; the new export carries clear semantics and the run log message is self-explanatory - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Irene Co-authored-by: Devin Foley Co-authored-by: Paperclip Co-authored-by: Devin Foley --- ...tbeat-timer-wake-session-reset-pf4.test.ts | 117 ++++++++++++++++++ server/src/services/heartbeat.ts | 22 +++- 2 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 server/src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts diff --git a/server/src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts b/server/src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts new file mode 100644 index 00000000..45d8f855 --- /dev/null +++ b/server/src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, it } from "vitest"; +import { + describeSessionResetReason, + shouldResetTaskSessionForWake, +} from "../services/heartbeat.ts"; + +// PF-4: timer-driven wakes ("heartbeat_timer") are exploratory and do not +// carry continuation state. Reusing the prior task session for repeated +// timer wakes accumulates low-value context and pushes the session toward +// the 64k compaction threshold (observed in CEO run 292a5fd1). The +// shouldResetTaskSessionForWake / describeSessionResetReason pair must +// agree that timer wakes start a fresh session, while preserving the +// existing reset rules for assignment / review / approval / changes wakes +// and the existing reuse policy for issue_commented and other reasons. + +describe("PF-4 shouldResetTaskSessionForWake", () => { + it("resets the session when wakeReason is heartbeat_timer", () => { + expect( + shouldResetTaskSessionForWake({ + source: "scheduler", + reason: "interval_elapsed", + wakeReason: "heartbeat_timer", + }), + ).toBe(true); + }); + + it("still resets for the existing reset reasons", () => { + for (const wakeReason of [ + "issue_assigned", + "execution_review_requested", + "execution_approval_requested", + "execution_changes_requested", + ] as const) { + expect(shouldResetTaskSessionForWake({ wakeReason })).toBe(true); + } + }); + + it("still respects forceFreshSession === true", () => { + expect(shouldResetTaskSessionForWake({ forceFreshSession: true })).toBe(true); + }); + + it("does not reset for issue_commented (preserve continuation context)", () => { + expect(shouldResetTaskSessionForWake({ wakeReason: "issue_commented" })).toBe(false); + }); + + it("does not reset for transient_failure_retry (resume in-flight work)", () => { + expect(shouldResetTaskSessionForWake({ wakeReason: "transient_failure_retry" })).toBe(false); + }); + + it("does not reset for unknown wake reasons", () => { + expect(shouldResetTaskSessionForWake({ wakeReason: "unknown_reason" })).toBe(false); + }); + + it("does not reset when context is null/undefined", () => { + expect(shouldResetTaskSessionForWake(null)).toBe(false); + expect(shouldResetTaskSessionForWake(undefined)).toBe(false); + }); +}); + +describe("PF-4 describeSessionResetReason", () => { + it("describes heartbeat_timer wakes explicitly so run logs explain the reset", () => { + const reason = describeSessionResetReason({ + wakeReason: "heartbeat_timer", + }); + expect(reason).toBe("wake reason is heartbeat_timer (timer-driven wake starts fresh)"); + }); + + it("returns the existing reasons for the existing reset triggers", () => { + expect(describeSessionResetReason({ wakeReason: "issue_assigned" })).toBe( + "wake reason is issue_assigned", + ); + expect(describeSessionResetReason({ wakeReason: "execution_review_requested" })).toBe( + "wake reason is execution_review_requested", + ); + expect(describeSessionResetReason({ wakeReason: "execution_approval_requested" })).toBe( + "wake reason is execution_approval_requested", + ); + expect(describeSessionResetReason({ wakeReason: "execution_changes_requested" })).toBe( + "wake reason is execution_changes_requested", + ); + }); + + it("returns the forceFreshSession message when explicitly requested", () => { + expect(describeSessionResetReason({ forceFreshSession: true })).toBe( + "forceFreshSession was requested", + ); + }); + + it("returns null for non-resetting wake reasons", () => { + expect(describeSessionResetReason({ wakeReason: "issue_commented" })).toBeNull(); + expect(describeSessionResetReason({ wakeReason: "transient_failure_retry" })).toBeNull(); + expect(describeSessionResetReason({ wakeReason: "unknown_reason" })).toBeNull(); + expect(describeSessionResetReason(null)).toBeNull(); + expect(describeSessionResetReason(undefined)).toBeNull(); + }); + + it("agrees with shouldResetTaskSessionForWake on every input — non-null reason iff should reset", () => { + const cases: Array | null | undefined> = [ + { wakeReason: "heartbeat_timer" }, + { wakeReason: "issue_assigned" }, + { wakeReason: "execution_review_requested" }, + { wakeReason: "execution_approval_requested" }, + { wakeReason: "execution_changes_requested" }, + { forceFreshSession: true }, + { wakeReason: "issue_commented" }, + { wakeReason: "transient_failure_retry" }, + { wakeReason: "unknown_reason" }, + null, + undefined, + ]; + for (const ctx of cases) { + const shouldReset = shouldResetTaskSessionForWake(ctx); + const reason = describeSessionResetReason(ctx); + expect(Boolean(reason)).toBe(shouldReset); + } + }); +}); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index e54bfdb4..4b0a0c00 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -1995,8 +1995,12 @@ function deriveTaskKey( /** * Extended task key derivation that falls back to a stable synthetic key - * for timer/heartbeat wakes. This ensures timer wakes can resume their - * previous session via `agentTaskSessions` instead of starting fresh. + * for timer/heartbeat wakes. The synthetic key keeps the + * `agentTaskSessions` row addressable across heartbeats so the row can be + * cleared and re-keyed deterministically; it does NOT mean the prior + * session is resumed. Since PF-4 (#4838), `heartbeat_timer` wakes always + * go through `shouldResetTaskSessionForWake` and start a fresh session — + * see `describeSessionResetReason` for the paired log message. * * The synthetic key is only used when: * - No explicit task/issue key exists in the context @@ -2025,7 +2029,14 @@ export function shouldResetTaskSessionForWake( wakeReason === "issue_assigned" || wakeReason === "execution_review_requested" || wakeReason === "execution_approval_requested" || - wakeReason === "execution_changes_requested" + wakeReason === "execution_changes_requested" || + // PF-4: timer-driven wakes are exploratory ("any new work?"). They do not + // carry meaningful continuation state, so reusing the prior task session + // for repeated timer wakes accumulates low-value context and pushes the + // session toward the 64k compaction threshold (observed in CEO run + // 292a5fd1, where timer wakes repeatedly bloated a long-lived manager + // session). Reset on every timer wake so each interval starts fresh. + wakeReason === "heartbeat_timer" ) { return true; } @@ -2090,7 +2101,7 @@ export function formatRuntimeWorkspaceWarningLog(warning: string) { }; } -function describeSessionResetReason( +export function describeSessionResetReason( contextSnapshot: Record | null | undefined, ) { if (contextSnapshot?.forceFreshSession === true) return "forceFreshSession was requested"; @@ -2100,6 +2111,9 @@ function describeSessionResetReason( if (wakeReason === "execution_review_requested") return "wake reason is execution_review_requested"; if (wakeReason === "execution_approval_requested") return "wake reason is execution_approval_requested"; if (wakeReason === "execution_changes_requested") return "wake reason is execution_changes_requested"; + // PF-4: paired with shouldResetTaskSessionForWake — keep the reason wording + // explicit so run logs make session reuse/reset behavior legible. + if (wakeReason === "heartbeat_timer") return "wake reason is heartbeat_timer (timer-driven wake starts fresh)"; return null; }