mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 04:02:18 +01:00
Address watchdog greptile review feedback
This commit is contained in:
1 parent
80481f7fc9
commit
7e0d290929
5 files changed
+81
-13
No files matched your search
@@ -243,6 +243,25 @@ describe("task watchdog subtree classifier", () => {
|
||||
expect(resolved.proofObligationFingerprint).not.toBe(blocked.proofObligationFingerprint);
|
||||
});
|
||||
|
||||
it("includes the watchdog id in proof obligation fingerprints", () => {
|
||||
const first = classify({ issues: [issue({ status: "done" })] });
|
||||
const second = classify({
|
||||
watchdog: {
|
||||
id: "watchdog-config-2",
|
||||
companyId,
|
||||
issueId: sourceId,
|
||||
lastReviewedFingerprint: null,
|
||||
},
|
||||
issues: [issue({ status: "done" })],
|
||||
});
|
||||
|
||||
expect(first.state).toBe("stopped");
|
||||
expect(second.state).toBe("stopped");
|
||||
if (first.state !== "stopped" || second.state !== "stopped") return;
|
||||
expect(second.stopFingerprint).toBe(first.stopFingerprint);
|
||||
expect(second.proofObligationFingerprint).not.toBe(first.proofObligationFingerprint);
|
||||
});
|
||||
|
||||
it("suppresses a repeated proof obligation when a typed outcome already exists", () => {
|
||||
const stopped = classify({ issues: [issue({ status: "done" })] });
|
||||
expect(stopped.state).toBe("stopped");
|
||||
|
||||
@@ -439,6 +439,38 @@ describeEmbeddedPostgres("task watchdog scheduler", () => {
|
||||
expect(outcomes.map((outcome) => outcome.outcome).sort()).toEqual(["deferred", "failed"]);
|
||||
});
|
||||
|
||||
it("does not defer a blocked watchdog issue when its only blocker was cancelled", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const sourceId = await seedIssue(companyId, { identifier: "WDOG-CANCELLED-BLOCKER", status: "blocked" });
|
||||
const blockerId = await seedIssue(companyId, { identifier: "WDOG-CANCELLED", status: "cancelled" });
|
||||
const agentId = await seedAgent(companyId);
|
||||
await seedWatchdog(companyId, sourceId, agentId);
|
||||
const { service } = createService();
|
||||
|
||||
await service.reconcileTaskWatchdogs({ companyId });
|
||||
const [watchdog] = await db.select().from(issueWatchdogs).where(eq(issueWatchdogs.issueId, sourceId));
|
||||
const watchdogIssueId = watchdog!.watchdogIssueId!;
|
||||
await db.update(issues).set({ status: "blocked", updatedAt: new Date() }).where(eq(issues.id, watchdogIssueId));
|
||||
await db.insert(issueRelations).values({
|
||||
companyId,
|
||||
issueId: blockerId,
|
||||
relatedIssueId: watchdogIssueId,
|
||||
type: "blocks",
|
||||
});
|
||||
|
||||
await service.reconcileTaskWatchdogs({ companyId });
|
||||
|
||||
const outcomes = await db
|
||||
.select()
|
||||
.from(issueWatchdogProofOutcomes)
|
||||
.where(eq(issueWatchdogProofOutcomes.watchdogId, watchdog!.id));
|
||||
expect(outcomes).toHaveLength(1);
|
||||
expect(outcomes[0]).toMatchObject({
|
||||
outcome: "failed",
|
||||
redactedDetails: expect.objectContaining({ hasUnresolvedBlocker: false }),
|
||||
});
|
||||
});
|
||||
|
||||
it("does not let an old terminal watchdog review mark a newer observed fingerprint reviewed", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const sourceId = await seedIssue(companyId, { identifier: "WDOG-STALE", status: "done" });
|
||||
|
||||
@@ -329,6 +329,7 @@ function stableStopFingerprint(input: {
|
||||
|
||||
function stableProofObligationFingerprint(input: {
|
||||
companyId: string;
|
||||
watchdogId: string;
|
||||
watchedIssueId: string;
|
||||
stopFingerprint: string;
|
||||
leaves: TaskWatchdogStoppedLeaf[];
|
||||
@@ -336,6 +337,7 @@ function stableProofObligationFingerprint(input: {
|
||||
const payload = JSON.stringify({
|
||||
version: 1,
|
||||
companyId: input.companyId,
|
||||
watchdogId: input.watchdogId,
|
||||
watchedIssueId: input.watchedIssueId,
|
||||
stopFingerprint: input.stopFingerprint,
|
||||
targets: input.leaves.map((leaf) => ({
|
||||
@@ -492,6 +494,7 @@ export function classifyTaskWatchdogSubtree(input: TaskWatchdogClassifierInput):
|
||||
});
|
||||
const proofObligationFingerprint = stableProofObligationFingerprint({
|
||||
companyId: input.watchdog.companyId,
|
||||
watchdogId: input.watchdog.id,
|
||||
watchedIssueId: input.watchdog.issueId,
|
||||
stopFingerprint,
|
||||
leaves,
|
||||
@@ -1163,7 +1166,7 @@ export function taskWatchdogService(db: Db, deps: TaskWatchdogServiceDeps = {})
|
||||
eq(issueRelations.companyId, companyId),
|
||||
eq(issueRelations.type, "blocks"),
|
||||
eq(issueRelations.relatedIssueId, issueId),
|
||||
sql`${blocker.status} <> 'done'`,
|
||||
sql`${blocker.status} not in ('done', 'cancelled')`,
|
||||
isNull(blocker.hiddenAt),
|
||||
))
|
||||
.limit(1)
|
||||
|
||||
@@ -162,6 +162,15 @@ describe("WatchdogStateCallout", () => {
|
||||
expect(container.textContent).toContain("PAP-500");
|
||||
});
|
||||
|
||||
it("does not show a phantom outstanding blocker section for deferred review paths", async () => {
|
||||
getWatchdogMock.mockResolvedValue(watchdogWith("deferred"));
|
||||
renderCallout();
|
||||
await waitForText("Deferred");
|
||||
expect(container.textContent).toContain("waiting path must complete");
|
||||
expect(container.textContent).not.toContain("Outstanding");
|
||||
expect(container.textContent).not.toContain("next check on blocker resolve");
|
||||
});
|
||||
|
||||
it("shows the failed outcome with the recovery action owner and next step", async () => {
|
||||
getWatchdogMock.mockResolvedValue(watchdogWith("failed"));
|
||||
const recoveryAction = {
|
||||
|
||||
@@ -68,6 +68,7 @@ function issueLink(id: string, identifier: string | null | undefined): string {
|
||||
|
||||
function outcomeStatement(
|
||||
outcome: IssueWatchdogProofOutcomeSummary,
|
||||
context: { hasOpenBlockers?: boolean; hasMonitor?: boolean } = {},
|
||||
): string {
|
||||
const classification = outcome.resultClassification;
|
||||
switch (outcome.outcome) {
|
||||
@@ -76,7 +77,13 @@ function outcomeStatement(
|
||||
case "restored":
|
||||
return "Restored live work inside the watched subtree and woke the responsible owner.";
|
||||
case "deferred":
|
||||
return `Deferred verification — the proof obligation can't be checked yet (${classification}). It will re-check once, automatically, when the blocker resolves.`;
|
||||
if (context.hasOpenBlockers) {
|
||||
return `Deferred verification — the proof obligation can't be checked yet (${classification}). It will re-check once, automatically, when the blocker resolves.`;
|
||||
}
|
||||
if (context.hasMonitor) {
|
||||
return `Deferred verification — the proof obligation can't be checked yet (${classification}). A scheduled follow-up will re-check it.`;
|
||||
}
|
||||
return `Deferred verification — the proof obligation can't be checked yet (${classification}). The waiting path must complete before verification can resume.`;
|
||||
case "failed":
|
||||
return `The proof obligation failed (${classification}). A recovery action was opened with the owner and next step.`;
|
||||
case "dismissed":
|
||||
@@ -219,6 +226,8 @@ export function WatchdogStateCallout({
|
||||
const proof = data.latestProofOutcome ?? null;
|
||||
const wdAgent = agentName(data.watchdogAgentId, agentMap);
|
||||
const watchdogTaskHref = data.watchdogIssueId ? `/issues/${data.watchdogIssueId}` : null;
|
||||
const openBlockers = (blockers ?? []).filter((b) => b.status !== "done" && b.status !== "cancelled");
|
||||
const hasMonitor = !!monitorNextCheckAt;
|
||||
|
||||
// State 6 — armed but no review yet
|
||||
if (!proof) {
|
||||
@@ -272,7 +281,7 @@ export function WatchdogStateCallout({
|
||||
</>
|
||||
}
|
||||
badge={<WatchdogOutcomeBadge outcome={proof.outcome} />}
|
||||
statement={outcomeStatement(proof)}
|
||||
statement={outcomeStatement(proof, { hasOpenBlockers: openBlockers.length > 0, hasMonitor })}
|
||||
/>
|
||||
<dl
|
||||
className={cn(
|
||||
@@ -332,9 +341,8 @@ export function WatchdogStateCallout({
|
||||
|
||||
{proof.outcome === "deferred" ? (
|
||||
<OutstandingDeferred
|
||||
blockers={blockers}
|
||||
openBlockers={openBlockers}
|
||||
monitorNextCheckAt={monitorNextCheckAt}
|
||||
watchdogTaskHref={watchdogTaskHref}
|
||||
/>
|
||||
) : null}
|
||||
|
||||
@@ -355,24 +363,21 @@ export function WatchdogStateCallout({
|
||||
}
|
||||
|
||||
function OutstandingDeferred({
|
||||
blockers,
|
||||
openBlockers,
|
||||
monitorNextCheckAt,
|
||||
watchdogTaskHref,
|
||||
}: {
|
||||
blockers?: IssueRelationIssueSummary[];
|
||||
openBlockers: IssueRelationIssueSummary[];
|
||||
monitorNextCheckAt?: Date | string | null;
|
||||
watchdogTaskHref: string | null;
|
||||
}) {
|
||||
const open = (blockers ?? []).filter((b) => b.status !== "done" && b.status !== "cancelled");
|
||||
const hasMonitor = !!monitorNextCheckAt;
|
||||
if (open.length === 0 && !hasMonitor && !watchdogTaskHref) return null;
|
||||
if (openBlockers.length === 0 && !hasMonitor) return null;
|
||||
return (
|
||||
<MetaRow label="Outstanding">
|
||||
<div className="rounded-md border border-amber-300/60 bg-background/50 px-2 py-1.5 dark:border-amber-500/30">
|
||||
{open.length > 0 ? (
|
||||
{openBlockers.length > 0 ? (
|
||||
<div className="flex flex-wrap items-center gap-1.5">
|
||||
<span className="text-[11px] text-muted-foreground">Blocked by</span>
|
||||
{open.map((b) => (
|
||||
{openBlockers.map((b) => (
|
||||
<Link
|
||||
key={b.id}
|
||||
to={issueLink(b.id, b.identifier)}
|
||||
|
||||
Reference in new issue
Block a user