diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 6695377594..9259598469 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -20,6 +20,8 @@ export { type DecisionSpecInput, } from "./validators/decision.js"; +export { decisionEffectTargetIssueIds } from "./types/decision.js"; + export type { DecisionEffectStaleness, DecisionOptionStyle, diff --git a/packages/shared/src/types/decision.ts b/packages/shared/src/types/decision.ts index 79d4605a39..b0d40077d4 100644 --- a/packages/shared/src/types/decision.ts +++ b/packages/shared/src/types/decision.ts @@ -67,6 +67,18 @@ export type DecisionEffect = | CancelIssueTreeDecisionEffect | ResolveBlockerDecisionEffect; +export function decisionEffectTargetIssueIds(effect: DecisionEffect): string[] { + const ids = new Set([effect.targetIssueId]); + if (effect.type === "create_issue") { + if (effect.draft.parentId) ids.add(effect.draft.parentId); + for (const id of effect.draft.blockedByIssueIds ?? []) ids.add(id); + } + if (effect.type === "resolve_blocker") { + for (const id of effect.removeBlockedByIssueIds) ids.add(id); + } + return [...ids]; +} + export interface DecisionOption { id: string; label: string; diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 33f71c4db5..7e0c2fcb97 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -1,3 +1,4 @@ +export { decisionEffectTargetIssueIds } from "./decision.js"; export type { Company, InteractionResolverGovernance, diff --git a/server/src/__tests__/decisions-service.test.ts b/server/src/__tests__/decisions-service.test.ts index 033dcac332..594ffb41e1 100644 --- a/server/src/__tests__/decisions-service.test.ts +++ b/server/src/__tests__/decisions-service.test.ts @@ -542,6 +542,61 @@ describePg("decisionService", () => { expect(wakes).toHaveLength(2); }); + it("expires strict decisions whose targets completed after they were proposed", async () => { + const completed = await createCommentDecision("strict", { idempotencyKey: "target-completed" }); + const lenient = await createCommentDecision("lenient", { idempotencyKey: "lenient-survives" }); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, targetIssueId)); + + expect((await service().sweepExpired()).expired).toBe(1); + + expect((await service().get(completed.id))?.metadata).toMatchObject({ expiredReason: "target_completed" }); + expect((await service().get(lenient.id))?.status).toBe("open"); + expect(wakes).toEqual([{ companyId, agentId, issueId: originIssueId, decisionId: completed.id, outcome: "expired" }]); + }); + + it("keeps strict decisions that intentionally target an already-done issue", async () => { + await db.update(issues).set({ status: "done" }).where(eq(issues.id, targetIssueId)); + const reopen = await createCommentDecision("strict", { idempotencyKey: "already-done" }); + + expect((await service().sweepExpired()).expired).toBe(0); + expect((await service().get(reopen.id))?.status).toBe("open"); + }); + + it("expires when a strict secondary target completes after proposal", async () => { + const blockerId = randomUUID(); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, targetIssueId)); + await db.insert(issues).values({ + id: blockerId, + companyId, + title: "Secondary blocker", + status: "todo", + priority: "medium", + responsibleUserId: decidedByUserId, + }); + const created = await service().create({ + companyId, + actor: agentActor(), + agentId, + runId, + title: "Create follow-up?", + body: "Body", + options: [{ + id: "yes", + label: "Yes", + effects: [{ + type: "create_issue", + targetIssueId, + staleness: "strict", + draft: { title: "Follow-up", blockedByIssueIds: [blockerId] }, + }], + }], + }); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, blockerId)); + + expect((await service().sweepExpired()).expired).toBe(1); + expect((await service().get(created.id))?.metadata).toMatchObject({ expiredReason: "target_completed" }); + }); + it("groups rule-key stats and separates explicit dismissals from expiry", async () => { const accepted = await service().create({ companyId, actor: agentActor(), agentId, runId, ruleKey: "routing.assign", title: "Assign?", body: "Body", diff --git a/server/src/services/decisions.ts b/server/src/services/decisions.ts index 06a56f7c64..704a713ae4 100644 --- a/server/src/services/decisions.ts +++ b/server/src/services/decisions.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import { and, asc, count, desc, eq, gt, gte, inArray, lte, or, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { companyMemberships, decisionBundles, decisionEffectExecutions, decisionRetention, decisions, decisionTargetIssues, heartbeatRuns, issueRelations, issues } from "@paperclipai/db"; -import { ATTENTION_SOURCE_KINDS } from "@paperclipai/shared"; +import { ATTENTION_SOURCE_KINDS, decisionEffectTargetIssueIds } from "@paperclipai/shared"; import type { AttentionArchiveManifestEntry, DecisionEffect, DecisionInput, DecisionOption, DecisionStatsCounts, DecisionStatsResponse } from "@paperclipai/shared"; import { conflict, forbidden, notFound, tooManyRequests, unprocessable } from "../errors.js"; import { authorizationService, type AuthorizationActor } from "./authorization.js"; @@ -17,19 +17,9 @@ type Wake = (input: { companyId: string; agentId: string; issueId: string; decis export type DecisionServiceOptions = { wakeOriginAgent: Wake }; const DAY = 86_400_000; -function effectTargetIds(effect: DecisionEffect) { - const result = new Set([effect.targetIssueId]); - if (effect.type === "create_issue") { - if (effect.draft.parentId) result.add(effect.draft.parentId); - for (const id of effect.draft.blockedByIssueIds ?? []) result.add(id); - } - if (effect.type === "resolve_blocker") for (const id of effect.removeBlockedByIssueIds) result.add(id); - return [...result]; -} - function targetIds(options: DecisionOption[]) { const result = new Set(); - for (const option of options) for (const effect of option.effects) for (const id of effectTargetIds(effect)) result.add(id); + for (const option of options) for (const effect of option.effects) for (const id of decisionEffectTargetIssueIds(effect)) result.add(id); return [...result]; } @@ -37,7 +27,7 @@ function targetActions(options: DecisionOption[]) { const result = new Map>(); for (const option of options) for (const effect of option.effects) { const action = effect.type === "comment_on_issue" ? "issue:comment" as const : "issue:mutate" as const; - for (const id of effectTargetIds(effect)) { + for (const id of decisionEffectTargetIssueIds(effect)) { const actions = result.get(id) ?? new Set(); actions.add(action); result.set(id, actions); @@ -431,7 +421,7 @@ export function decisionService(db: Db, options: DecisionServiceOptions) { const [row] = await tx.update(decisionEffectExecutions).set({ status, error: reason, result: details, activityLogId: activity?.id ?? null, executedAt: new Date() }).where(eq(decisionEffectExecutions.id, execution!.id)).returning(); return row; }; - const directReferencedIds = new Set(effectTargetIds(effect)); + const directReferencedIds = new Set(decisionEffectTargetIssueIds(effect)); const snapshots = decision.targetSnapshots as Record; const cancellationDescendantIds = effect.type === "cancel_issue_tree" ? snapshots[effect.targetIssueId]?.descendantIds @@ -757,13 +747,21 @@ export function decisionService(db: Db, options: DecisionServiceOptions) { : []; targetSweepCursor = targetRows.length === remaining && targetRows.length > 0 ? targetRows[targetRows.length - 1]!.id : null; const rows = [...new Map([...ttlRows, ...targetRows].map((row) => [row.id, row])).values()]; let expired = 0; - for (const decision of rows) { const strictTargetIds = new Set(decision.options.flatMap((option) => option.effects.filter((effect) => effect.staleness === "strict").map((effect) => effect.targetIssueId))); + for (const decision of rows) { const strictTargetIds = new Set(decision.options.flatMap((option) => option.effects.filter((effect) => effect.staleness === "strict").flatMap(decisionEffectTargetIssueIds))); const targets = strictTargetIds.size > 0 ? await db.select({ id: issues.id, status: issues.status }).from(issues).where(and(eq(issues.companyId, decision.companyId), inArray(issues.id, [...strictTargetIds]))) : []; const targetGone = targets.length !== strictTargetIds.size || targets.some((target) => target.status === "cancelled"); - if (!targetGone && decision.expiresAt >= now) continue; - const reason = targetGone ? "target_gone" : "ttl"; + // A decision whose strict targets all completed after it was proposed is moot: + // the strict guard would skip its effects anyway, so retire it instead of + // leaving it pending until TTL. Targets that were already done at proposal + // time don't count — those decisions intentionally act on a done issue. + const snapshots = decision.targetSnapshots as Record; + const targetsCompleted = !targetGone && strictTargetIds.size > 0 && + targets.every((target) => target.status === "done") && + targets.some((target) => snapshots[target.id]?.status !== "done"); + if (!targetGone && !targetsCompleted && decision.expiresAt >= now) continue; + const reason = targetGone ? "target_gone" : targetsCompleted ? "target_completed" : "ttl"; const [updated] = await db.update(decisions).set({ status: "expired", updatedAt: now, metadata: { ...decision.metadata, expiredReason: reason, ...(decision.continuationPolicy === "wake_origin_agent" ? { continuationPending: true } : {}) } }).where(and(eq(decisions.id, decision.id), eq(decisions.status, "open"))).returning(); if (!updated) continue; expired += 1; diff --git a/ui/src/components/DecisionCard.test.tsx b/ui/src/components/DecisionCard.test.tsx index e64a8e729f..1ca851a4f4 100644 --- a/ui/src/components/DecisionCard.test.tsx +++ b/ui/src/components/DecisionCard.test.tsx @@ -38,6 +38,8 @@ vi.mock("@/lib/router", () => ({ const ISSUES: Record = { "issue-origin": { id: "issue-origin", identifier: "PAP-123", title: "Gardener sweep", href: "/PAP/issues/PAP-123", status: "in_progress" }, "issue-target": { id: "issue-target", identifier: "PAP-456", title: "Stale epic", href: "/PAP/issues/PAP-456", status: "backlog" }, + "issue-parent": { id: "issue-parent", identifier: "PAP-789", title: "Parent task", href: "/PAP/issues/PAP-789", status: "todo" }, + "issue-blocker": { id: "issue-blocker", identifier: "PAP-790", title: "Blocking task", href: "/PAP/issues/PAP-790", status: "todo" }, "issue-new": { id: "issue-new", identifier: "PAP-999", title: "Follow-up", href: "/PAP/issues/PAP-999", status: "todo" }, }; const resolveIssue = (id: string): DecisionIssueRef | null => ISSUES[id] ?? null; @@ -135,6 +137,54 @@ describe("DecisionCard", () => { expect([...el.querySelectorAll("button")].some((b) => b.textContent?.includes("Dismiss"))).toBe(true); }); + it("links the target issue the decision applies to, not just the origin", () => { + const el = render({}); + expect(el.textContent).toContain("applies to"); + const provenance = el.querySelector("p"); + expect(provenance?.textContent).toContain("PAP-456"); + expect( + [...(provenance?.querySelectorAll("a") ?? [])].some((a) => a.getAttribute("href") === "/PAP/issues/PAP-456"), + ).toBe(true); + }); + + it("omits the applies-to link when the decision only targets its origin issue", () => { + const el = render({ + decision: mkDecision({ + options: [ + { id: "comment", label: "Comment", effects: [{ type: "comment_on_issue", targetIssueId: "issue-origin", staleness: "lenient", bodyMarkdown: "nudge" }] }, + ], + }), + }); + expect(el.textContent).not.toContain("applies to"); + }); + + it("links secondary target issues referenced by an effect", () => { + const el = render({ + decision: mkDecision({ + options: [{ + id: "create", + label: "Create follow-up", + effects: [{ + type: "create_issue", + targetIssueId: "issue-target", + staleness: "strict", + draft: { + title: "Follow-up", + parentId: "issue-parent", + blockedByIssueIds: ["issue-blocker"], + }, + }], + }], + }), + }); + const targetHrefs = [...(el.querySelector("p")?.querySelectorAll("a") ?? [])].map((a) => a.getAttribute("href")); + expect(targetHrefs).toEqual(expect.arrayContaining([ + "/PAP/issues/PAP-456", + "/PAP/issues/PAP-789", + "/PAP/issues/PAP-790", + ])); + }); + it("fires onDecide with the chosen option id", () => { const onDecide = vi.fn(); const el = render({ onDecide }); @@ -262,4 +312,12 @@ describe("DecisionCard", () => { expect(dismissed.textContent).toContain("Dismissed"); expect(dismissed.textContent).toContain("no effects were run"); }); + + it("explains when a decision expires because its targets completed", () => { + const expired = render({ + decision: mkDecision({ status: "expired", metadata: { expiredReason: "target_completed" } }), + }); + expect(expired.textContent).toContain("target issues were completed"); + expect(expired.textContent).not.toContain("expiry deadline"); + }); }); diff --git a/ui/src/components/DecisionCard.tsx b/ui/src/components/DecisionCard.tsx index 647309889f..57efb647fc 100644 --- a/ui/src/components/DecisionCard.tsx +++ b/ui/src/components/DecisionCard.tsx @@ -11,7 +11,7 @@ import { ShieldAlert, XCircle, } from "lucide-react"; -import type { DecisionEffect, DecisionOption } from "@paperclipai/shared"; +import { decisionEffectTargetIssueIds, type DecisionEffect, type DecisionOption } from "@paperclipai/shared"; import type { Decision, DecisionEffectExecution, @@ -68,18 +68,6 @@ function humanStatus(status: string | null | undefined): string { return status.replaceAll("_", " "); } -function referencedTargetIds(effect: DecisionEffect): string[] { - const ids = new Set([effect.targetIssueId]); - if (effect.type === "create_issue") { - if (effect.draft.parentId) ids.add(effect.draft.parentId); - for (const id of effect.draft.blockedByIssueIds ?? []) ids.add(id); - } - if (effect.type === "resolve_blocker") { - for (const id of effect.removeBlockedByIssueIds) ids.add(id); - } - return [...ids]; -} - function issueLabel(ref: DecisionIssueRef | null, fallbackId: string): string { if (ref?.identifier) return ref.identifier; if (ref?.title) return ref.title; @@ -312,6 +300,22 @@ export function DecisionCard({ }; const dimmed = decision.status === "expired" || decision.status === "cancelled"; + const expiredReason = (decision.metadata as { expiredReason?: string } | null)?.expiredReason; + + // The issues this decision acts on. The origin issue is only where the agent + // was running when it proposed the decision — the queue links there, but the + // decision may target a different issue entirely, so name the targets + // explicitly or the card is undiscoverable from the issue it applies to. + const targetRefs = useMemo(() => { + const ids = new Set(); + for (const option of decision.options) { + for (const effect of option.effects) { + for (const id of decisionEffectTargetIssueIds(effect)) ids.add(id); + } + } + if (originIssue?.id) ids.delete(originIssue.id); + return [...ids].map((id) => ({ id, ref: resolveIssue(id) })); + }, [decision.options, originIssue?.id, resolveIssue]); return (
{" "}while running{" "} - + {issueLabel(originIssue, originIssue.id)} )} + {targetRefs.length > 0 && ( + <> + {" · applies to "} + {targetRefs.map(({ id, ref }, index) => ( + + {index > 0 && ", "} + {ref ? ( + + {issueLabel(ref, id)} + + ) : ( + {issueLabel(null, id)} + )} + + ))} + + )} {runHref && ( <> {" · "} @@ -418,7 +439,7 @@ export function DecisionCard({ {decision.options.map((option) => { const destructive = isDestructiveOption(option); const blockedStale = option.effects.some( - (effect) => effect.staleness === "strict" && referencedTargetIds(effect).some((id) => staleTargetIdSet.has(id)), + (effect) => effect.staleness === "strict" && decisionEffectTargetIssueIds(effect).some((id) => staleTargetIdSet.has(id)), ); const disabled = busy || requiredUnmet || blockedStale; const cancelTree = cancelTreeEffect(option); @@ -559,9 +580,11 @@ export function DecisionCard({ The decision window closed

- {((decision.metadata as { expiredReason?: string } | null)?.expiredReason === "target_gone") + {expiredReason === "target_gone" ? "A target issue was cancelled before this was decided." - : "No response before the expiry deadline."} + : expiredReason === "target_completed" + ? "All target issues were completed before this was decided." + : "No response before the expiry deadline."} {decision.continuationPolicy === "wake_origin_agent" && " The proposer was re-woken."}

diff --git a/ui/src/components/DecisionResolver.test.tsx b/ui/src/components/DecisionResolver.test.tsx index 0eeba3aa3e..2f2f793030 100644 --- a/ui/src/components/DecisionResolver.test.tsx +++ b/ui/src/components/DecisionResolver.test.tsx @@ -4,7 +4,7 @@ import { flushSync } from "react-dom"; import { createRoot } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -const state = vi.hoisted(() => ({ issueStatus: "todo" })); +const state = vi.hoisted(() => ({ issueStatus: "todo", queriedIssueIds: [] as string[] })); vi.mock("@tanstack/react-query", () => ({ useQueryClient: () => ({ invalidateQueries: vi.fn(), setQueryData: vi.fn() }), @@ -18,9 +18,20 @@ vi.mock("@tanstack/react-query", () => ({ originIssueId: "origin-1", status: "open", targetSnapshots: { "target-1": { updatedAt: "2026-07-31T00:00:00.000Z" } }, - options: [{ id: "yes", label: "Yes", effects: [{ - type: "comment_on_issue", targetIssueId: "target-1", staleness: "lenient", bodyMarkdown: "hello", - }] }], + options: [{ id: "yes", label: "Yes", effects: [ + { + type: "create_issue", + targetIssueId: "target-1", + staleness: "strict", + draft: { title: "Follow-up", parentId: "parent-1", blockedByIssueIds: ["blocker-1"] }, + }, + { + type: "resolve_blocker", + targetIssueId: "target-1", + staleness: "strict", + removeBlockedByIssueIds: ["removed-blocker-1"], + }, + ] }], executions: [], }, isLoading: false, @@ -34,8 +45,9 @@ vi.mock("@tanstack/react-query", () => ({ queries: Array<{ queryKey: readonly string[] }>; combine?: (results: Array<{ data: { id: string; identifier: string; title: string; status: string } }>) => unknown; }) => { - const results = queries.map(() => ({ - data: { id: "target-1", identifier: "PAP-1", title: "Target", status: state.issueStatus }, + state.queriedIssueIds = queries.map((query) => String(query.queryKey[2])); + const results = queries.map((query) => ({ + data: { id: String(query.queryKey[2]), identifier: "PAP-1", title: "Target", status: state.issueStatus }, })); return combine ? combine(results) : results; }, @@ -60,6 +72,7 @@ describe("DecisionResolver", () => { beforeEach(() => { state.issueStatus = "todo"; + state.queriedIssueIds = []; container = document.createElement("div"); document.body.appendChild(container); }); @@ -85,4 +98,18 @@ describe("DecisionResolver", () => { flushSync(() => root.unmount()); }); + + it("loads every primary and secondary effect target", () => { + const root = createRoot(container); + flushSync(() => root.render()); + + expect(state.queriedIssueIds).toEqual(expect.arrayContaining([ + "target-1", + "parent-1", + "blocker-1", + "removed-blocker-1", + ])); + + flushSync(() => root.unmount()); + }); }); diff --git a/ui/src/components/DecisionResolver.tsx b/ui/src/components/DecisionResolver.tsx index 2b49bb90e2..5946edda62 100644 --- a/ui/src/components/DecisionResolver.tsx +++ b/ui/src/components/DecisionResolver.tsx @@ -1,7 +1,7 @@ import { useCallback, useMemo } from "react"; import { useMutation, useQueries, useQuery, useQueryClient } from "@tanstack/react-query"; import { Loader2 } from "lucide-react"; -import type { Agent, AttentionSubject } from "@paperclipai/shared"; +import { decisionEffectTargetIssueIds, type Agent, type AttentionSubject } from "@paperclipai/shared"; import { decisionsApi, type DecisionOutcome } from "../api/decisions"; import { issuesApi } from "../api/issues"; import { queryKeys } from "../lib/queryKeys"; @@ -78,8 +78,7 @@ export function DecisionResolver({ companyId, decisionId, originIssue, agentMap, } for (const option of decision.options) { for (const effect of option.effects) { - ids.add(effect.targetIssueId); - if (effect.type === "create_issue" && effect.draft.parentId) ids.add(effect.draft.parentId); + for (const id of decisionEffectTargetIssueIds(effect)) ids.add(id); } } for (const execution of decision.executions ?? []) {