Reset task session on timer-driven wakes (PF-4) (#4838)
## 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 <irene@users.noreply.github.com> Co-authored-by: Devin Foley <devin@devinfoley.com> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Devin Foley <devin@paperclip.ing>
This commit is contained in:
Reasonofmoon
@@ -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<Record<string, unknown> | 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);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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<string, unknown> | 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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user