From 20482a4cb699830a7fdb6b21cada015eced3eb4f Mon Sep 17 00:00:00 2001 From: Eric Brookfield Date: Wed, 12 Aug 2026 19:43:13 -0400 Subject: [PATCH] fix(server): gate heartbeat-fallback comment to never publish raw transcript (#10143) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents run on a heartbeat; when an issue-scoped run ends, the server records the outcome on the issue's board thread. > - Normally the agent posts its own summary comment via `POST /comments`. When it doesn't, the server has a fallback that auto-publishes a run summary so the board isn't left silent. > - That fallback (`buildHeartbeatRunIssueComment` in `server/src/services/heartbeat-run-summary.ts`) returns `resultJson.summary` **verbatim**, with no length cap or shape check. > - For runs that never produce a final `result`, `summary` is concatenated **inter-tool narration** ("Let me check…", "I'll fetch…", joined by the claude-local adapter's parser). The fallback then dumps that raw transcript onto the public board thread. > - In practice this produces long, confusing transcript comments that mislead reviewers and other agents about what actually happened. > - This PR gates the fallback so it publishes a clean summary or a short stub, never raw transcript. > - The benefit is that the board thread stays trustworthy: a missing agent summary degrades to a one-line "no summary this run" note instead of leaking internal narration. ## Linked Issues or Issue Description No public GitHub issue exists for this; describing it here as a bug report. **What happened:** When an issue-scoped heartbeat run finishes without the agent posting its own comment, the server's fallback publishes `resultJson.summary` verbatim as the board comment. When the run produced no final result, that value is concatenated inter-tool narration, so raw transcript is posted to the issue thread. **Expected behavior:** The fallback should post a concise summary when one is available, and otherwise a short stub — never multi-hundred-line raw narration. **Steps to reproduce:** 1. Run an issue-scoped agent turn that ends without calling `POST /comments` and without emitting a final `result` (only inter-tool narration). 2. Observe the auto-published board comment: it is the full narration transcript. **Deployment mode:** self-hosted server (`server/src/services/heartbeat.ts` fallback path). **Prior attempt:** an earlier PR for this change was auto-closed when its head branch was renamed to strip an internal ticket id from the branch name; this PR supersedes it. **Related PR:** #7505 (`fix(heartbeat): skip auto-mirror run-summary comment on cross-owner wakes`) touches the same fallback area but addresses a different case (cross-owner wakes); this PR is complementary, gating the *content* of the fallback rather than *when* it fires. ## What Changed - `server/src/services/heartbeat-run-summary.ts`: `buildHeartbeatRunIssueComment` now gates the fallback text. After resolving `summary` → `result` → `message`, if the text opens with a narration phrase (`let me`, `i'll`, `i need to`, `i can see`, `looking at`, `fetching`, `checking`, `first,`) **or** exceeds `MAX_FALLBACK_COMMENT_CHARS` (1200), it returns a fixed stub: *"Run completed. Agent did not post a summary comment this run (transcript withheld — see run log)."* Otherwise it returns the text unchanged. - `server/src/__tests__/heartbeat-run-summary.test.ts`: added cases for each narration opener, the length cap, the exact 1200-char boundary (posts), and clean-summary passthrough. Runs where the agent posts via the API are unaffected — the fallback only fires when no agent comment is found for the run, and that call site is unchanged. ## Verification - `pnpm --filter @paperclip/server test heartbeat-run-summary` — 13/13 pass (new + existing cases). - Manual reasoning: the gate is a pure function of the resolved text; API-posted runs never reach it. - **CI note:** at the time of opening, `pnpm install --frozen-lockfile` fails on this branch's base commit with `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH` (patchedDependencies drift). This reproduces on every PR based on the current `master` tip (e.g. #10137) and is unrelated to this two-file change; PRs cut from the prior master (e.g. #10135) install cleanly. This should clear once the `Refresh Lockfile` job lands a corrected lockfile on `master` and this branch is rebased. Happy to rebase or fold in the lockfile fix if a maintainer prefers. ## Risks Low risk. The change is confined to one pure function and its tests, touches no schema or migration, and only alters the *fallback* comment path (never the normal API-posted path). Worst case is a legitimate clean summary that happens to open with a gated phrase gets replaced by the stub — the run log still holds the full detail. ## Model Used Claude Opus 4.8 (`claude-opus-4-8`), 1M-token context window, extended thinking, with tool use. ## 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 (`fix/gate-heartbeat-fallback-comment`) 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 - [ ] I have updated relevant documentation to reflect my changes (no user-facing docs affected) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (blocked on a master-side lockfile drift, see CI note) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending re-review on this PR) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../__tests__/heartbeat-run-summary.test.ts | 53 +++++++++++++++++++ server/src/services/heartbeat-run-summary.ts | 28 ++++++++-- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/server/src/__tests__/heartbeat-run-summary.test.ts b/server/src/__tests__/heartbeat-run-summary.test.ts index e5d8848d6c..74051ab469 100644 --- a/server/src/__tests__/heartbeat-run-summary.test.ts +++ b/server/src/__tests__/heartbeat-run-summary.test.ts @@ -63,6 +63,59 @@ describe("buildHeartbeatRunIssueComment", () => { it("returns null when there is no usable final text", () => { expect(buildHeartbeatRunIssueComment({ costUsd: 1.2 })).toBeNull(); }); + + it("suppresses raw transcript when the summary reads like inter-tool narration", () => { + const narration = + "Let me check the issue thread first. I'll fetch the latest comments and then decide what to do next."; + const comment = buildHeartbeatRunIssueComment({ summary: narration }); + + expect(comment).not.toContain("Let me check"); + expect(comment).toContain("did not post a summary comment"); + }); + + it("suppresses each narration opener variant", () => { + for (const opener of [ + "Let me look into this.", + "I'll start by reading the file.", + "I need to inspect the config.", + "I can see the problem now.", + "Looking at the logs, the error is clear.", + "Fetching the run details from the API.", + "Checking the current branch state.", + "First, I will reproduce the bug.", + "I’m going to trace the fallback path.", + "Now I'll push the follow-up commit.", + "Next, I'll re-run the suite.", + ]) { + expect(buildHeartbeatRunIssueComment({ summary: opener })).toContain( + "did not post a summary comment", + ); + } + }); + + it("does not treat the apostrophe opener as a regex wildcard", () => { + // Prior regex used `i.ll` where `.` matched any char; these must pass through. + for (const summary of ["Iall greetings logged.", "I-ll formatting kept."]) { + expect(buildHeartbeatRunIssueComment({ summary })).toBe(summary); + } + }); + + it("suppresses over-long fallback summaries even without a narration opener", () => { + const comment = buildHeartbeatRunIssueComment({ summary: "x".repeat(1201) }); + expect(comment).toContain("did not post a summary comment"); + expect(comment).not.toContain("xxxx"); + }); + + it("posts a clean, in-length summary with no narration opener normally", () => { + const summary = "## Summary\n\n- fixed the fallback gate\n- added regression tests"; + expect(buildHeartbeatRunIssueComment({ summary })).toBe(summary); + }); + + it("posts a summary exactly at the length cap", () => { + const summary = "S" + "x".repeat(1199); + expect(summary.length).toBe(1200); + expect(buildHeartbeatRunIssueComment({ summary })).toBe(summary); + }); }); describe("mergeHeartbeatRunResultJson", () => { diff --git a/server/src/services/heartbeat-run-summary.ts b/server/src/services/heartbeat-run-summary.ts index 479957dce2..cdd1bdeca9 100644 --- a/server/src/services/heartbeat-run-summary.ts +++ b/server/src/services/heartbeat-run-summary.ts @@ -92,6 +92,19 @@ export function summarizeHeartbeatRunResultJson( return Object.keys(summary).length > 0 ? summary : null; } +// The fallback comment is only posted when a run ends without the agent posting +// its own comment via the API. In that case `resultJson.summary` can be raw +// inter-tool narration (assistantTexts concatenated by the adapter), which must +// never be published verbatim to the board — see BRO-1507 / BRO-1516. +export const MAX_FALLBACK_COMMENT_CHARS = 1200; +// Apostrophes are matched as a character class so both the straight (') and +// curly (’) forms count — agents emit either. Openers are narration phrases a +// declarative status summary would not begin with ("Fixed X", "13/13 pass"). +const NARRATION_OPENERS = + /^(let me\b|i['’]ll\b|i['’]m going\b|i need to\b|i can see\b|now i['’]ll\b|next,? i['’]ll\b|looking at\b|fetching\b|checking\b|first,)/i; +const FALLBACK_WITHHELD_COMMENT = + "Run completed. Agent did not post a summary comment this run (transcript withheld — see run log)."; + export function buildHeartbeatRunIssueComment( resultJson: Record | null | undefined, ): string | null { @@ -99,10 +112,17 @@ export function buildHeartbeatRunIssueComment( return null; } - return ( + const text = readCommentText(resultJson.summary) ?? readCommentText(resultJson.result) - ?? readCommentText(resultJson.message) - ?? null - ); + ?? readCommentText(resultJson.message); + if (!text) { + return null; + } + + if (text.length > MAX_FALLBACK_COMMENT_CHARS || NARRATION_OPENERS.test(text)) { + return FALLBACK_WITHHELD_COMMENT; + } + + return text; }