mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-07 07:23:08 +02:00
Adam TealeandClaude Sonnet 5
43ab441f0f
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Plugins can extend Paperclip with their own multi-step workflow/graph engines that own an issue's lifecycle across many agent handoffs > - When such a plugin-managed issue legitimately stays `in_progress` for a while (e.g. an anchor issue parked at a fan-out step, waiting on child issues it spawned), Paperclip's generic recovery mechanisms have no way to know that's intentional > - The first commit on this branch fixed one such mechanism (`decideSuccessfulRunHandoff`) to skip plugin-owned issues, and was deployed to a real instance to verify the fix > - Watching that same instance afterward, the identical symptom (repeated "give a disposition" nags, the agent repeating "completed", the plugin's own enforcement correctly reverting the status) recurred on the same class of issue — meaning a second, independent code path had the exact same gap > - Traced it to `reconcileStrandedAssignedIssues` in `service.ts`: it detects a stale successful-run-handoff corrective run via `isExhaustedSuccessfulRunHandoff` and, once "exhausted" (default max attempts is 1, so effectively immediate), escalates via `escalateStrandedAssignedIssue` — with no check on who owns the issue's lifecycle at all > - Rather than duplicate the `originKind` check inline a second time (which is exactly how it got missed the first time), extracted it into a shared, exported, unit-tested helper (`isPluginManagedIssueLifecycle`) that both recovery paths now call > - The benefit is the same as the first commit, but closing the second loop this pull request's earlier version left open: generic plugin-owned issues (any workflow/graph-engine plugin, not just one specific plugin) stop burning real agent-run cost in a loop that can never actually resolve, across both recovery mechanisms that can trigger it ## Linked Issues or Issue Description No existing public issue covers this — describing it directly, following the bug report fields: **What happened?** An issue owned by a workflow-engine-style plugin (`originKind` starting with `"plugin:"`) was correctly held at `in_progress` by the plugin while it waited on spawned child issues to finish. The assigned agent's heartbeat succeeded and posted a well-formed completion comment, but `issue.status` stayed `in_progress` (the plugin's own enforcement reverted it, correctly, since the underlying work wasn't done). - **Path 1 (fixed in the first commit):** `decideSuccessfulRunHandoff()` saw `status === "in_progress"` after a successful run and enqueued a "missing disposition" corrective wake. The agent responded again, the plugin reverted the status again, and the recovery re-triggered again. - **Path 2 (fixed in the second commit, found after deploying and verifying the first fix on a live instance):** separately, `reconcileStrandedAssignedIssues` periodically re-scans `in_progress` issues, sees the corrective run from Path 1 (or any prior successful-run-handoff wake) as "exhausted" evidence, and escalates the issue via `escalateStrandedAssignedIssue` regardless of plugin ownership — producing the same nag-revert-nag cycle through a completely different call path that the first commit's fix did not touch. **Expected behavior** Neither recovery mechanism should nag an agent for a disposition, or escalate for one, on an issue whose lifecycle is already owned and managed by a plugin — that plugin's own enforcement/recovery path is the correct owner of "what happens next," not these generic core mechanisms. **Steps to reproduce** 1. Install a plugin that creates/owns issues via the plugin host bridge (`ctx.issues.create`/`ctx.issues.update`) with an `originKind` of `"plugin:<pluginKey>"`. 2. Have the plugin's own graph/workflow logic hold an issue at `in_progress` while some multi-step process it owns is still pending (e.g. spawned child issues not yet complete). 3. Let an agent run a successful heartbeat on that issue that produces visible progress (a comment) but does not change `issue.status` away from `in_progress` in a way that sticks (the plugin's own logic reverts any change back to `in_progress` on the next event). 4. Observe `decideSuccessfulRunHandoff` enqueue a corrective handoff wake (Path 1), and/or `reconcileStrandedAssignedIssues` treat that wake's run as exhausted and escalate (Path 2). Either one repeats indefinitely on its own. **Paperclip version or commit** `eb2cb916be3271e3e7ab5f643ad3ca3eb7c34d01` (current `master` at time of the first commit; rebased onto `ad961227f` for the second) **Deployment mode** Self-hosted server ## What Changed - **First commit:** Added `originKind: issues.originKind` to the issue query in `heartbeat.ts` that feeds `decideSuccessfulRunHandoff`, and a skip condition there for `originKind` starting with `"plugin:"`. - **Second commit:** - Extracted the plugin-ownership check out of `decideSuccessfulRunHandoff` into a new exported helper, `isPluginManagedIssueLifecycle(issue)`, in `successful-run-handoff.ts`. - Added the same check to `reconcileStrandedAssignedIssues` in `service.ts`, immediately before it would otherwise escalate an issue based on `isExhaustedSuccessfulRunHandoff` evidence — skipping plugin-managed issues there too. - Added unit tests for the new helper directly (plugin-prefixed origin kinds → `true`; non-plugin/missing origin kinds → `false`), alongside the existing `decideSuccessfulRunHandoff` tests (updated to import and rely on the shared helper, behavior unchanged). ## Verification - `npx vitest run server/src/services/recovery/successful-run-handoff.test.ts` — 20/20 passed (18 pre-existing/from the first commit + 2 new for the extracted helper). - `npx vitest run server/src/__tests__/heartbeat-comment-wake-batching.test.ts server/src/__tests__/heartbeat-process-recovery.test.ts server/src/services/recovery/successful-run-handoff.test.ts` — full suite still passes with the refactor. - `pnpm --filter @paperclipai/server exec tsc --noEmit` — no new errors introduced by either commit (confirmed against a pre-existing baseline of unrelated `@paperclipai/plugin-sdk` module-resolution errors from the workspace, present identically with the changes stashed out). - Manually verified Path 1 against a real plugin-managed issue stuck in that loop: after deploying the first commit and restarting the server, the same agent posted the same completion comment again, and the corrective-handoff recovery did not re-trigger. - Path 2 was found live on the same instance after that first deploy (the loop recurred through the second, independent mechanism) — root-caused via direct inspection of `heartbeat_runs`/`agent_wakeup_requests`/issue comment history, then fixed in the second commit. Not yet re-verified live on the instance (pending redeploy of this updated branch). ## Risks - Low risk. Both changes are additive skip conditions — they only cause a recovery decision to return early for a specific, narrow case (`originKind` starting with `"plugin:"`) that previously fell through to escalation/enqueue. No existing skip conditions are changed or reordered in a way that affects non-plugin issues. - Behavioral shift: plugin-managed issues that are genuinely stuck (not just correctly mid-flight) will no longer get either of these corrective nags. This is intentional — the plugin owning the issue is expected to have its own recovery path — but it does mean these mechanisms are no longer a safety net for buggy plugins that leave issues stranded. Plugin authors should ensure their own enforcement handles stranded states. - The refactor (extracting `isPluginManagedIssueLifecycle`) is a pure code-motion change for the first commit's check — no behavior change there, only a new call site added in `service.ts`. - No migration required (query-shape and control-flow changes only, no schema change). ## Model Used Claude (Anthropic), model `claude-sonnet-5`, used within a coding-agent harness (Claude Code) with tool use (file edit, test execution, git operations, live production-instance debugging via SSH/SQL) and extended reasoning across two sessions: the first implemented and deployed the initial fix, the second discovered the second recovery path was still looping on a live instance, root-caused it, and implemented/tested this follow-up commit. ## 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 - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] 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: Claude Sonnet 5 <noreply@anthropic.com>