From c6727e7b203ef341a5f58a15c3062b078e4fd9d9 Mon Sep 17 00:00:00 2001 From: Eric Brookfield Date: Wed, 12 Aug 2026 19:43:56 -0400 Subject: [PATCH] fix(server): don't implicitly reopen a blocked issue when the same PATCH wires blockers (#10269) 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 > - Issues coordinate that work, and first-class blockers (`blockedByIssueIds`) are how dependent work auto-resumes when its prerequisites finish > - A human commenting on a blocked issue implicitly reopens it to `todo` — a deliberate heuristic so "please continue" comments revive parked work > - But that heuristic evaluates the issue's *pre-update* blocker set, ignoring blockers being wired in by the very same PATCH > - So the natural repair action for a bare-blocked issue — one PATCH adding `blockedByIssueIds` plus an explanatory comment — silently flips the issue to `todo`, contradicting the dependency edit it just made > - This pull request suppresses the implicit reopen when the request itself declares a non-empty blocker list > - The benefit is that structured dependency edits always win over the conversational-comment heuristic, so blocked issues keep their intended waiting posture and auto-resume via `issue_blockers_resolved` as designed ## Linked Issues or Issue Description No existing issue describes this exact behavior; per the bug-report template: - **What happened:** On a `blocked` issue with an empty blocker set, a board user sent one `PATCH /api/issues/:id` containing `blockedByIssueIds: [""]` and a `comment`. The response showed `status: "todo"` — the implicit comment-reopen fired even though the same request wired an unresolved blocker. A follow-up `PATCH { status: "blocked" }` was then needed to restore the waiting posture (and because the blocker array replaces on every update, the two fields had to be re-sent together). - **Expected behavior:** A request that explicitly declares dependencies is stating that the issue is waiting on other work. The implicit reopen exists for plain conversational comments; it should not override a structured dependency edit made in the same request. - **Steps to reproduce:** (1) Create issue A with `status: "blocked"` and no blockers; (2) as a board user, `PATCH /api/issues/A` with `{ "blockedByIssueIds": [""], "comment": "wiring the dependency" }`; (3) observe the response/issue status is `todo` instead of remaining `blocked`. - **Version/commit:** reproduced on `master` @ `d1b9448b5`. - **Deployment mode:** `authenticated`, single-host (macOS launchd), embedded Postgres. Related (not fixed here): the family of "blocked with empty `blockedByIssueIds` zombie" reports — Refs #9201, Refs #6523 — this bug is one way an issue's status and blocker list end up contradicting each other; and Refs #8062, which proposes a different auto-transition at the status/blocker boundary. ## What Changed - `shouldImplicitlyMoveCommentedIssueToTodo` (server/src/routes/issues.ts) accepts an optional `requestAddsExplicitBlockers` input and returns `false` when set, alongside the existing suppression guards, with a comment documenting the rationale. - The `PATCH /api/issues/:id` call site passes `requestAddsExplicitBlockers: Array.isArray(req.body.blockedByIssueIds) && req.body.blockedByIssueIds.length > 0`. - Two route tests in `issue-comment-reopen-routes.test.ts`: a regression test (comment + non-empty blocker list on a blocked issue must not flip status) and a boundary test (comment + `blockedByIssueIds: []` still implicitly reopens, preserving the existing clear-blockers behavior). Deliberately unchanged: explicit `reopen`/`resume` flags still behave as before, and the `POST /comments` route is untouched (its body cannot carry `blockedByIssueIds`). ## Verification - `cd server && pnpm vitest run src/__tests__/issue-comment-reopen-routes.test.ts` → 74/74 pass. - Reverting the `issues.ts` change makes the new regression test fail with `expected 'todo' to be undefined` — it bites. - `cd server && pnpm tsc --noEmit` → clean. ## Risks - Low. The change is a single additional suppression guard on the *implicit* reopen path, scoped to requests that carry a non-empty `blockedByIssueIds` array; all other reopen behavior is untouched. - Edge case considered: a request wiring only already-resolved blockers plus a comment now stays `blocked` instead of implicitly reopening. This is the conservative reading of caller intent (an explicit dependency edit), and an explicit `status`/`reopen` in the same request still wins. ## Model Used - Anthropic Claude — Fable 5 (`claude-fable-5`), extended thinking enabled, agentic tool use via Claude Code (CLI). Production repro, diagnosis, fix, and tests all model-authored under human direction. ## 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 (none applicable — behavior comment added inline) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending first CI run on this PR) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending first review pass) - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Claude Fable 5 --- .../issue-comment-reopen-routes.test.ts | 57 +++++++++++++++++++ server/src/routes/issues.ts | 9 +++ 2 files changed, 66 insertions(+) diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts index 96dcde474f..eb450d13b1 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -13,6 +13,7 @@ const mockIssueService = vi.hoisted(() => ({ findMentionedAgents: vi.fn(), listWakeableBlockedDependents: vi.fn(), getWakeableParentAfterChildCompletion: vi.fn(), + getRelationSummaries: vi.fn(), })); const mockAccessService = vi.hoisted(() => ({ @@ -1240,6 +1241,62 @@ describe.sequential("issue comment reopen routes", () => { )); }); + it("does not implicitly reopen a blocked issue via PATCH when the same request wires blockers", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue("blocked")); + mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] }); + mockIssueService.getDependencyReadiness.mockResolvedValue({ + issueId: "11111111-1111-4111-8111-111111111111", + blockerIssueIds: [], + unresolvedBlockerIssueIds: [], + unresolvedBlockerCount: 0, + allBlockersDone: true, + isDependencyReady: true, + }); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...makeIssue("blocked"), + ...patch, + })); + + const res = await request(await installActor(createApp())) + .patch("/api/issues/11111111-1111-4111-8111-111111111111") + .send({ + blockedByIssueIds: ["33333333-3333-4333-8333-333333333333"], + comment: "wired the dependency this issue is waiting on", + }); + + expect(res.status).toBe(200); + expect(mockIssueService.update).toHaveBeenCalled(); + const patch = mockIssueService.update.mock.calls[0][1] as Record; + expect(patch.status).toBeUndefined(); + expect(patch.blockedByIssueIds).toEqual(["33333333-3333-4333-8333-333333333333"]); + }); + + it("still implicitly reopens a blocked issue via PATCH when the same request clears blockers", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue("blocked")); + mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] }); + mockIssueService.getDependencyReadiness.mockResolvedValue({ + issueId: "11111111-1111-4111-8111-111111111111", + blockerIssueIds: [], + unresolvedBlockerIssueIds: [], + unresolvedBlockerCount: 0, + allBlockersDone: true, + isDependencyReady: true, + }); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...makeIssue("blocked"), + ...patch, + })); + + const res = await request(await installActor(createApp())) + .patch("/api/issues/11111111-1111-4111-8111-111111111111") + .send({ blockedByIssueIds: [], comment: "nothing left to wait on, please continue" }); + + expect(res.status).toBe(200); + expect(mockIssueService.update).toHaveBeenCalled(); + const patch = mockIssueService.update.mock.calls[0][1] as Record; + expect(patch.status).toBe("todo"); + }); + it("does not implicitly reopen closed issues via POST comments when no agent is assigned", async () => { mockIssueService.getById.mockResolvedValue({ ...makeIssue("done"), diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 43ac00e6e4..dbdfe47064 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -1836,7 +1836,14 @@ function shouldImplicitlyMoveCommentedIssueToTodo(input: { actorRunId: string | null | undefined; checkoutRunId: string | null | undefined; executionRunId: string | null | undefined; + requestAddsExplicitBlockers?: boolean; }) { + // A request that wires a non-empty blockedByIssueIds list is declaring that + // the issue is waiting on other work. The implicit reopen exists for plain + // conversational comments ("please continue"), not structured dependency + // edits — flipping to todo here would contradict the caller's stated intent + // in the same request. + if (input.requestAddsExplicitBlockers) return false; // Local-CLI agents post comments under user auth, so the actor.type is "user" // even though the comment originates from the same heartbeat run that owns // the issue lock. Without this guard, an agent that closes its own issue and @@ -8709,6 +8716,8 @@ export function issueRoutes( actorRunId: actor.runId, checkoutRunId: existing.checkoutRunId, executionRunId: existing.executionRunId, + requestAddsExplicitBlockers: + Array.isArray(req.body.blockedByIssueIds) && req.body.blockedByIssueIds.length > 0, })) || shouldResumeInProgressScheduledRetry); const updateReferenceSummaryBefore = titleOrDescriptionChanged