perf(heartbeat): reuse task sessions for issue-scoped timer wakes and bound control-plane write retries (#10350)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents make progress in heartbeats: the server wakes an agent session, it does a slice of work on an issue, records a disposition, and exits > - Benchmarking identical coding tasks run as Paperclip-orchestrated agent pairs vs invoking the same agent harness directly measured a 1.8–2.2× wall-clock slowdown for the Paperclip pairs, dominated by per-heartbeat orchestration overhead rather than model time > - Two contributors stood out: (1) since PF-4 (#4838) every `heartbeat_timer` wake starts a brand-new task session, so continuation work on a specific issue repays the full session-start and re-orientation cost on every heartbeat; (2) in degraded environments agents burn many tool calls retrying the same failing control-plane write before giving up > - This pull request reuses the task session for issue-scoped timer wakes (keeping the PF-4 fresh-session rule only for unscoped exploratory wakes, which were the original context-bloat case) and adds a bounded-retry rule to the wake prompt and core skill: after 2 consecutive failures of the same control-plane write, stop retrying it for the rest of the heartbeat and rely on the adapter/runtime status channel > - The benefit is materially less wall-clock and token overhead per heartbeat while preserving the context-bloat protection PF-4 was added for ## Linked Issues or Issue Description Refs #4838 (merged PF-4 change whose reset rule this refines), Refs #5287, Refs #1907 (related timer-heartbeat session work). No public GitHub issue exists for the slowdown itself; bug-report fields: - **What happened:** Agent pairs orchestrated through Paperclip heartbeats complete identical task sets 1.8–2.2× slower (wall-clock) than the same harness invoked directly. Profiling attributed the gap to per-heartbeat orchestration overhead: every timer wake discards the task session (full session start + re-orientation), and in degraded environments agents repeatedly retry the same failing control-plane write. - **Expected behavior:** Heartbeat orchestration should add minimal wall-clock overhead on top of the underlying harness; issue-scoped continuation work should not pay a fresh-session tax each interval. - **Steps to reproduce:** Run a fixed benchmark task set once through Paperclip issue heartbeats and once via direct harness invocation with the same model/config; compare wall-clock totals. - **Version/commit:** master @3d23c3b2c3, self-hosted deployment. ## What Changed - `server/src/services/heartbeat.ts`: `shouldResetTaskSessionForWake` now resets only for `heartbeat_timer` wakes with no derivable task key (unscoped exploratory wakes). Issue-scoped timer wakes reuse the issue's task session. `describeSessionResetReason` updated to stay in exact agreement. - `server/src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts`: new cases for scoped vs unscoped timer wakes, plus the scoped case added to the reset/reason agreement invariant. - `packages/adapter-utils/src/server-utils.ts`: wake prompt template and execution contract gain a bounded-retry rule — after 2 consecutive failures of the same control-plane write, stop retrying it for the rest of the heartbeat, continue useful work, report the failure in the final response, and use the adapter/runtime status channel as the sanctioned fallback. - `packages/adapter-utils/src/server-utils.test.ts`: asserts the new prompt lines are present in both the template and the rendered wake prompt. - `skills/paperclip/SKILL.md`: documents the same bounded write-retry rule in the core Paperclip skill. ## Verification - `node_modules/.bin/vitest run packages/adapter-utils/src/server-utils.test.ts` — 1 file, 83 tests passed - `cd server && node_modules/.bin/vitest run src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts` — 1 file, 14 tests passed - Both run on this branch rebased onto current master (3d23c3b2c3) ## Risks - Behavioral shift: issue-scoped timer wakes now reuse sessions, so a long-lived issue session can grow across heartbeats. Mitigated by keeping the PF-4 reset for unscoped wakes (the originally observed bloat case) and by existing session compaction. - Prompt/skill text changes alter agent guidance; the new rule is scoped narrowly to repeated failures of the same control-plane write. - No migrations, no API or schema changes, no dependency changes. ## Model Used - Claude (Anthropic) — `claude-fable-5` (Fable 5), extended reasoning with tool use, driven via Claude Code / Claude Agent SDK. ## 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 GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
3d23c3b2c3
commit
1cd09ed555
|
|
@ -843,6 +843,8 @@ describe("renderPaperclipWakePrompt", () => {
|
|||
expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("evidence, not valid liveness paths by themselves");
|
||||
expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("keep `in_progress` only when a live continuation path exists");
|
||||
expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("Prefer the smallest verification that proves the change");
|
||||
expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("After 2 consecutive failures of the same control-plane write");
|
||||
expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("adapter/runtime status channel as the sanctioned fallback");
|
||||
expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("Use child issues");
|
||||
expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("instead of polling agents, sessions, or processes");
|
||||
expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("Create child issues directly when you know what needs to be done");
|
||||
|
|
@ -905,6 +907,8 @@ describe("renderPaperclipWakePrompt", () => {
|
|||
expect(prompt).toContain("Immediately before returning, verify that Paperclip records one of those dispositions");
|
||||
expect(prompt).toContain("a successful process exit or final response is not sufficient");
|
||||
expect(prompt).toContain("If no valid disposition is recorded, record it now and do not end the run");
|
||||
expect(prompt).toContain("After 2 consecutive failures of the same control-plane write");
|
||||
expect(prompt).toContain("adapter/runtime status channel as the sanctioned fallback");
|
||||
expect(prompt).toContain("evidence, not valid liveness paths by themselves");
|
||||
expect(prompt).toContain("Use child issues for long or parallel delegated work instead of polling");
|
||||
expect(prompt).toContain("named unblock owner/action");
|
||||
|
|
|
|||
|
|
@ -158,6 +158,7 @@ export const DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE = [
|
|||
"- Comments, documents, screenshots, work products, and `Remaining` bullets are evidence, not valid liveness paths by themselves.",
|
||||
"- Final disposition checklist: mark `done` when complete; use `in_review` only with a real reviewer, approval, interaction, or monitor path; use `blocked` only with first-class blockers or a named unblock owner/action; create delegated follow-up issues with blockers when another agent owns the next step; keep `in_progress` only when a live continuation path exists.",
|
||||
"- Prefer the smallest verification that proves the change; do not default to full workspace typecheck/build/test on every heartbeat unless the task scope warrants it.",
|
||||
"- After 2 consecutive failures of the same control-plane write, stop retrying that write for the rest of the heartbeat. Continue useful work, report the failure in the final response, and rely on the adapter/runtime status channel as the sanctioned fallback.",
|
||||
"- Use child issues for parallel or long delegated work instead of polling agents, sessions, or processes.",
|
||||
"- If woken by a human comment on a dependency-blocked issue, respond or triage the comment without treating the blocked deliverable work as unblocked.",
|
||||
"- Create child issues directly when you know what needs to be done; use issue-thread interactions when the board/user must choose suggested tasks, answer structured questions, or confirm a proposal.",
|
||||
|
|
@ -1484,7 +1485,7 @@ export function renderPaperclipWakePrompt(
|
|||
]
|
||||
: includeExecutionContract
|
||||
? [
|
||||
"Execution contract: take concrete action in this heartbeat when the issue is actionable; do not stop at a plan unless planning was requested. Leave durable progress and then give the issue a clear final disposition before ending the heartbeat: `done`, `in_review` with a real reviewer/approval/interaction path, `blocked` with first-class blockers or a named unblock owner/action, delegated follow-up issues with blockers, or `in_progress` only when a live continuation path exists. Immediately before returning, verify that Paperclip records one of those dispositions; a successful process exit or final response is not sufficient. If no valid disposition is recorded, record it now and do not end the run. Use child issues for long or parallel delegated work instead of polling. Comments, documents, screenshots, work products, and `Remaining` bullets are evidence, not valid liveness paths by themselves.",
|
||||
"Execution contract: take concrete action in this heartbeat when the issue is actionable; do not stop at a plan unless planning was requested. Leave durable progress and then give the issue a clear final disposition before ending the heartbeat: `done`, `in_review` with a real reviewer/approval/interaction path, `blocked` with first-class blockers or a named unblock owner/action, delegated follow-up issues with blockers, or `in_progress` only when a live continuation path exists. Immediately before returning, verify that Paperclip records one of those dispositions; a successful process exit or final response is not sufficient. If no valid disposition is recorded, record it now and do not end the run. After 2 consecutive failures of the same control-plane write, stop retrying it for the rest of the heartbeat, continue useful work, report the failure in the final response, and rely on the adapter/runtime status channel as the sanctioned fallback. Use child issues for long or parallel delegated work instead of polling. Comments, documents, screenshots, work products, and `Remaining` bullets are evidence, not valid liveness paths by themselves.",
|
||||
"",
|
||||
]
|
||||
: [];
|
||||
|
|
|
|||
|
|
@ -4,17 +4,12 @@ import {
|
|||
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.
|
||||
// PF-4 keeps exploratory timer wakes fresh to avoid low-value context growth.
|
||||
// Issue-scoped timer wakes are continuation work and should reuse the task
|
||||
// session so orchestration does not repay session-start/re-orientation cost.
|
||||
|
||||
describe("PF-4 shouldResetTaskSessionForWake", () => {
|
||||
it("resets the session when wakeReason is heartbeat_timer", () => {
|
||||
it("resets the session for an unscoped heartbeat_timer wake", () => {
|
||||
expect(
|
||||
shouldResetTaskSessionForWake({
|
||||
source: "scheduler",
|
||||
|
|
@ -24,6 +19,17 @@ describe("PF-4 shouldResetTaskSessionForWake", () => {
|
|||
).toBe(true);
|
||||
});
|
||||
|
||||
it("reuses the task session for an issue-scoped heartbeat_timer wake", () => {
|
||||
expect(
|
||||
shouldResetTaskSessionForWake({
|
||||
source: "scheduler",
|
||||
reason: "interval_elapsed",
|
||||
wakeReason: "heartbeat_timer",
|
||||
issueId: "issue-1",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("still resets for the existing reset reasons", () => {
|
||||
for (const wakeReason of [
|
||||
"issue_assigned",
|
||||
|
|
@ -58,11 +64,20 @@ describe("PF-4 shouldResetTaskSessionForWake", () => {
|
|||
});
|
||||
|
||||
describe("PF-4 describeSessionResetReason", () => {
|
||||
it("describes heartbeat_timer wakes explicitly so run logs explain the reset", () => {
|
||||
it("describes unscoped 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)");
|
||||
expect(reason).toBe("wake reason is heartbeat_timer (unscoped timer wake starts fresh)");
|
||||
});
|
||||
|
||||
it("does not describe an issue-scoped heartbeat_timer wake as a reset", () => {
|
||||
expect(
|
||||
describeSessionResetReason({
|
||||
wakeReason: "heartbeat_timer",
|
||||
issueId: "issue-1",
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("returns the existing reasons for the existing reset triggers", () => {
|
||||
|
|
@ -97,6 +112,7 @@ describe("PF-4 describeSessionResetReason", () => {
|
|||
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: "heartbeat_timer", issueId: "issue-1" },
|
||||
{ wakeReason: "issue_assigned" },
|
||||
{ wakeReason: "execution_review_requested" },
|
||||
{ wakeReason: "execution_approval_requested" },
|
||||
|
|
|
|||
|
|
@ -2987,10 +2987,9 @@ function deriveTaskKey(
|
|||
* Extended task key derivation that falls back to a stable synthetic key
|
||||
* 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.
|
||||
* cleared and re-keyed deterministically. Unscoped exploratory timer wakes
|
||||
* still start fresh to avoid accumulating low-value inbox scans, while timer
|
||||
* wakes scoped to a real issue reuse that issue's task session.
|
||||
*
|
||||
* The synthetic key is only used when:
|
||||
* - No explicit task/issue key exists in the context
|
||||
|
|
@ -3021,13 +3020,11 @@ export function shouldResetTaskSessionForWake(
|
|||
wakeReason === EXECUTION_REVIEW_PARTICIPANT_RECOVERY_WAKE_REASON ||
|
||||
wakeReason === "execution_approval_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"
|
||||
// PF-4: unscoped timer wakes are exploratory ("any new work?") and should
|
||||
// not accumulate low-value inbox scans. Issue-scoped timer wakes are
|
||||
// continuation work, so reuse their task session to avoid paying the full
|
||||
// session-start and re-orientation cost on every heartbeat.
|
||||
(wakeReason === "heartbeat_timer" && !deriveTaskKey(contextSnapshot, null))
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
|
@ -3140,7 +3137,9 @@ export function describeSessionResetReason(
|
|||
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)";
|
||||
if (wakeReason === "heartbeat_timer" && !deriveTaskKey(contextSnapshot, null)) {
|
||||
return "wake reason is heartbeat_timer (unscoped timer wake starts fresh)";
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -107,6 +107,9 @@ If an important file intentionally remains in the project or execution workspace
|
|||
For technical upload instructions, read `references/artifacts.md`.
|
||||
|
||||
**Step 8 — Update status and communicate.** Always include the run ID header.
|
||||
|
||||
**Bounded write retry.** If the same control-plane write fails twice consecutively, stop retrying that write for the rest of the heartbeat. Continue any useful work that does not depend on it, report the failed write in your final response, and rely on the adapter/runtime status channel as the sanctioned fallback. Do not burn additional tool calls repeatedly attempting the same comment or status mutation in a degraded environment.
|
||||
|
||||
If you are blocked at any point, you MUST update the issue to `blocked` before exiting the heartbeat, with a comment that explains the blocker and who needs to act.
|
||||
|
||||
Before ending any heartbeat, apply this final-disposition checklist:
|
||||
|
|
|
|||
Loading…
Reference in New Issue