mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 21:49:18 +01:00
fix(server): wake chat milestones after review retirement and reset
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
3603b45c43
commit
41cc717572
7 files changed
+161
-6
No files matched your search
@@ -1524,3 +1524,15 @@ The audit retains the initial invalid tool-publication fixture assertion and
|
||||
corrected before-fix failures. Native external-review supersession is already
|
||||
covered by its status-decision outer transaction. Other external interaction and
|
||||
lineage producers remain under audit; the draft is not production-wired yet.
|
||||
|
||||
|
||||
Slice 28 covers native automatic/obsolete review retirement and conversation
|
||||
reset with seven additional governed cases, including retained-history and
|
||||
explicit-request controls. Three missing wakes were reproduced before repair;
|
||||
207 lifecycle/governed/interaction cases pass. The draft CI chat-shard failure
|
||||
was reproduced as a test subscribing to unrelated unresolved transaction intent;
|
||||
isolating that test's DB owner preserves the production protocol and passes three
|
||||
native review acceptance cases. Server production types pass. The slice audit
|
||||
retains the invalid fixture and missed test-file selection. Polling removal,
|
||||
remaining producers, fresh CI/review, older findings and the full inventory remain
|
||||
unfinished; no stack merge or whole-goal completion is claimed.
|
||||
@@ -943,3 +943,54 @@ Native automatic/obsolete review retirement, conversation reset, authorization
|
||||
prompt reactivation and tool-request execution lineage still need disposition,
|
||||
alongside remaining publication/configuration/recovery producers. Production
|
||||
registration, lifecycle polling removal and full-stack final checks remain pending.
|
||||
|
||||
|
||||
## Native review retirement, conversation reset, and CI isolation
|
||||
|
||||
Automatic fallback review retirement and obsolete policy review retirement now
|
||||
register milestone intent inside their existing transactions, after finding and
|
||||
locking the issue and before changing the interaction. Existing selection,
|
||||
explicit-review protections, lock order, and terminal publication handling remain
|
||||
with their owners. The real selectors are tested with retained prompt history,
|
||||
no prompt history, and explicit requests that must remain pending. Current
|
||||
system-authored cards are not newly externalized; the retained-prompt cases prove
|
||||
recovery of stored history rather than claiming a new provider send path.
|
||||
|
||||
Conversation reset registers intent before its existing issue/comment/question
|
||||
writes. The regression exercises the actual reset service with a valid saved
|
||||
conversation identity and chat binding. It establishes the service contract for
|
||||
stored bindings, not a claim that the current UI creates combined Agent Chat and
|
||||
external-chat conversations. Its failed prompt remains historical; expiring the
|
||||
question releases response ownership and reactivates the failed-run notice.
|
||||
|
||||
Evidence:
|
||||
- `/tmp/sleep28-native-retirement-before.log`: both retained-prompt retirement
|
||||
cases fail at the missing-notice assertion. Four control cases pass. The reset
|
||||
case initially fails fixture setup because conversationState was missing.
|
||||
- `/tmp/sleep28-conversation-reset-before.log`: after supplying the required
|
||||
active conversation state, reset expires the question but fails to produce
|
||||
the newly eligible notice before the signal is added.
|
||||
- `/tmp/sleep28-native-retirement-tests.log`: all 16 governed producer cases pass.
|
||||
- `/tmp/sleep28-retirement-combined-tests.log`: 101 tests pass (72 lifecycle,
|
||||
16 governed, 13 interaction unit). This command used the wrong integration
|
||||
filename, so it did not run that suite; its count is not reported as 207.
|
||||
- `/tmp/sleep28-retirement-interaction-service-tests.log`: the correctly selected
|
||||
existing interaction integration suite separately passes all 106 cases.
|
||||
- `/tmp/sleep28-native-retirement-typecheck.log`: server production types pass.
|
||||
|
||||
CI at 3603b45c43 failed chat shard 1 in the newly added native review metadata
|
||||
repair test: expected no signal, received an initial intent. The shared database
|
||||
owner retains failed transactions until XID reconciliation; subscribing correctly
|
||||
replays unresolved intent even when this test's operation is a no-op. A deliberate
|
||||
unrelated rolled-back producer reproduces the same failure in
|
||||
`/tmp/sleep28-ci-intent-before.log`. The repair test now uses a separate database
|
||||
signal owner, preserving the intentional rollback and all commit/no-op assertions.
|
||||
`/tmp/sleep28-ci-intent-tests.log` passes this test plus both current-authority
|
||||
revocation cases (3 total). Production uncertainty handling is unchanged. Remote
|
||||
acceptance still needs a fresh head; no blind CI retry was requested.
|
||||
|
||||
Fresh origin/master is aec86c794b. No changes landed in the three service owners
|
||||
above or chat-run-publications since the preceding 3536b3649f inspection. Full
|
||||
stack reconciliation remains outstanding. The lifecycle projector remains
|
||||
unregistered and its periodic production scan remains until the rest of the
|
||||
publication, configuration, and recovery producer audit is complete.
|
||||
@@ -1,4 +1,4 @@
|
||||
import { subscribeDatabaseWork } from "@paperclipai/db";
|
||||
import { signalDatabaseWork, subscribeDatabaseWork } from "@paperclipai/db";
|
||||
import { restoreNativeChatReviewPresentationInTransaction } from "../services/native-runtime/native-chat-review-presentation.js";
|
||||
import { notifyChatEndpointConfiguration, notifyChatCompanyConfiguration } from "../services/chat-channels/configuration-work-notifications.js";
|
||||
import { configureAgentLifecycle } from "../services/agent-lifecycle.js";
|
||||
@@ -70404,8 +70404,15 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
|
||||
it("restores changed native review metadata once and signals only its committed change", async () => {
|
||||
const context = await committedNativeReviewPublicationFixture("milestone-repair");
|
||||
// Earlier failure cases can leave intent awaiting XID reconciliation on the
|
||||
// suite's shared owner. Subscribing to that owner immediately replays intent.
|
||||
await expect(db.transaction(async tx => {
|
||||
await signalDatabaseWork(tx, "chat-run-milestones");
|
||||
throw new Error("unrelated rolled-back producer");
|
||||
})).rejects.toThrow("unrelated rolled-back producer");
|
||||
const repairDb = createDb(externalTestDatabaseUrl ?? tempDb!.connectionString);
|
||||
const events: string[] = [];
|
||||
const unsubscribe = subscribeDatabaseWork(db, "chat-run-milestones", event => events.push(event));
|
||||
const unsubscribe = subscribeDatabaseWork(repairDb, "chat-run-milestones", event => events.push(event));
|
||||
try {
|
||||
const [coordinator] = await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, context.runId));
|
||||
const input = { companyId: context.fixture.companyId, issueId: coordinator.issueId, runId: context.runId,
|
||||
@@ -70414,12 +70421,12 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
await db.update(heartbeatRuns).set({ updatedAt: savedAt }).where(eq(heartbeatRuns.id, context.runId));
|
||||
// Already-restored metadata still authorizes materialization by the caller,
|
||||
// but must neither rewrite the run nor wake projection again.
|
||||
await expect(db.transaction(tx => restoreNativeChatReviewPresentationInTransaction(tx as unknown as typeof db, input))).resolves.toBe(true);
|
||||
await expect(repairDb.transaction(tx => restoreNativeChatReviewPresentationInTransaction(tx as unknown as typeof db, input))).resolves.toBe(true);
|
||||
expect((await readHeartbeatRunsById(context.runId))[0].updatedAt).toEqual(savedAt);
|
||||
expect(events).toEqual([]);
|
||||
await db.update(heartbeatRuns).set({ resultJson: sql`${heartbeatRuns.resultJson} - 'externalChatReviewPresentation'` })
|
||||
.where(eq(heartbeatRuns.id, context.runId));
|
||||
await db.transaction(async tx => {
|
||||
await repairDb.transaction(async tx => {
|
||||
expect(await restoreNativeChatReviewPresentationInTransaction(tx as unknown as typeof db, input)).toBe(true);
|
||||
expect(events).toEqual(["intent"]);
|
||||
});
|
||||
@@ -70428,10 +70435,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
externalChatReviewPresentation: { decisionId: coordinator.decisionId, runId: context.runId },
|
||||
});
|
||||
events.length = 0;
|
||||
await db.transaction(tx => restoreNativeChatReviewPresentationInTransaction(tx as unknown as typeof db, input));
|
||||
await repairDb.transaction(tx => restoreNativeChatReviewPresentationInTransaction(tx as unknown as typeof db, input));
|
||||
expect(events).toEqual([]);
|
||||
} finally {
|
||||
unsubscribe();
|
||||
await repairDb.$client.end({ timeout: 1 });
|
||||
await context.service.shutdown();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -2,9 +2,10 @@ import { randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
completionContracts, nativeRunResults, nativeRunFinalizations, workAssessments, statusDecisions, statusDecisionEffects,
|
||||
agents, toolActionDeliveries, toolActionRequests, toolInvocations, toolGatewaySessions, toolCallEvents, toolAccessAuditEvents, toolPolicies,
|
||||
companySecretBindings, companySecretProposals, companySecrets, createDb,
|
||||
chatPublications, heartbeatRuns, issueThreadInteractions, issues,
|
||||
chatPublications, heartbeatRuns, issueComments, issueThreadInteractions, issues,
|
||||
} from "@paperclipai/db";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
||||
import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js";
|
||||
@@ -14,6 +15,10 @@ import { chatRunMilestoneWork } from "../services/chat-run-milestone-work.js";
|
||||
import { createSecretProposalsService } from "../services/secret-proposals.js";
|
||||
import { commitToolActionReview } from "../services/tool-action-review.js";
|
||||
import { createToolGatewayService } from "../services/tool-gateway.js";
|
||||
import { issueThreadInteractionService } from "../services/issue-thread-interactions.js";
|
||||
import { prepareConversationTurn } from "../services/agent-conversations.js";
|
||||
import { dismissAutomaticCompletionReviews } from "../services/native-runtime/automatic-completion-reviews.js";
|
||||
import { dismissObsoleteNativePolicyReviews } from "../services/native-runtime/obsolete-policy-reviews.js";
|
||||
import { idleWorkSnapshot } from "../services/task-admission.js";
|
||||
|
||||
const external = resolveExternalTestDatabaseUrl("PAPERCLIP_MILESTONE_GOVERNED_TEST_DATABASE_URL");
|
||||
@@ -149,4 +154,77 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu
|
||||
await quiet(); expect(remote).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["automatic", "prompt"], ["automatic", "no prompt"], ["automatic", "explicit request"],
|
||||
["obsolete", "prompt"], ["obsolete", "no prompt"], ["obsolete", "explicit request"],
|
||||
] as const)("handles %s review retirement with %s", async (kind, history) => {
|
||||
const runId = await run(), contractId = randomUUID(), resultId = randomUUID(), assessmentId = randomUUID(), decisionId = randomUUID();
|
||||
const prompt = kind === "automatic"
|
||||
? "Review the persisted native-run evidence and confirm whether this issue may be completed."
|
||||
: "Review the superseding native policy assessment.";
|
||||
await db.insert(completionContracts).values({ id: contractId, companyId: f.companyId, issueId: f.issueId, revision: 1,
|
||||
schemaVersion: "paperclip.completion-contract.v1", policyVersion: "phase6-v1", risk: kind === "automatic" ? "low" : "standard",
|
||||
completionAuthority: kind === "automatic" ? "agent_claim_policy" : "server_arbiter", incompleteCriteriaPolicy: "preserve_non_terminal",
|
||||
contractJson: { objective: "Respond to chat" }, canonicalSha256: contractId, createdByActorType: "system", createdByActorId: "test" });
|
||||
await db.update(heartbeatRuns).set({ status: "succeeded", runtimeMode: "native", nativeIssueId: f.issueId, completionContractId: contractId,
|
||||
resultJson: { presentationDecision: { source: "none" } } }).where(eq(heartbeatRuns.id, runId));
|
||||
await db.insert(nativeRunResults).values({ id: resultId, companyId: f.companyId, issueId: f.issueId, runId,
|
||||
completionContractId: contractId, serverFingerprint: resultId, schemaStatus: "accepted", canonicalSha256: resultId,
|
||||
resultJson: { terminal: { runTerminalState: "succeeded" } } });
|
||||
await db.insert(nativeRunFinalizations).values({ runId, companyId: f.companyId, issueId: f.issueId, phase: "committed", resultId });
|
||||
await db.insert(workAssessments).values({ id: assessmentId, companyId: f.companyId, issueId: f.issueId, runId,
|
||||
contractId, resultId, triggerKind: "run_result", triggerActorCompanyId: f.companyId, priorIssueStatus: "in_progress",
|
||||
priorStatusVersion: 0, policyVersion: "phase6-v1", assessmentJson: { attentionRequests: [] }, inputDigest: assessmentId });
|
||||
await db.insert(statusDecisions).values({ id: decisionId, companyId: f.companyId, issueId: f.issueId, runId, assessmentId,
|
||||
decisionVersion: 1, policyVersion: "phase6-v1", fromStatus: "in_progress", toStatus: "in_review", applicationState: "applied",
|
||||
reasonCode: kind === "automatic" ? "completion_claim_incomplete" : "completion_review_required", decisionDigest: decisionId,
|
||||
decisionJson: { projectedStatusVersion: 1, effects: [{ kind: "bind_reviewer", prompt, ownerUserId: null },
|
||||
...(kind === "obsolete" ? [{ kind: "append_superseding_assessment" }] : [])] } });
|
||||
await db.update(issues).set({ status: "in_review", statusVersion: 1, lastStatusDecisionId: decisionId }).where(eq(issues.id, f.issueId));
|
||||
const [interaction] = await db.insert(issueThreadInteractions).values({ companyId: f.companyId, issueId: f.issueId,
|
||||
kind: "request_confirmation", sourceRunId: runId, idempotencyKey: `native-review:${decisionId}`, status: "pending",
|
||||
payload: { version: 1, prompt: history === "explicit request" ? "Review the release before publishing." : prompt,
|
||||
target: { type: "custom", key: "native_completion_review", revisionId: decisionId } } }).returning();
|
||||
await db.insert(statusDecisionEffects).values({ companyId: f.companyId, issueId: f.issueId, decisionId,
|
||||
ordinal: 0, effectKind: "bind_reviewer", targetType: "issue_thread_interaction", targetId: interaction.id,
|
||||
idempotencyKey: decisionId, payload: {}, deliveryState: "delivered" });
|
||||
// System cards are no longer externalized, but stored prompt rows still own
|
||||
// the response slot. Exercise their supported retirement rather than insert
|
||||
// a synthetic new provider send through today's author-validation boundary.
|
||||
if (history !== "no prompt") await db.insert(chatPublications).values({ companyId: f.companyId, issueId: f.issueId,
|
||||
endpointId: f.endpointId, conversationId: f.conversationId, state: "pending", payload: { text: prompt, interactionId: interaction.id },
|
||||
idempotencyKey: `interaction:${interaction.id}:${f.endpointId}` });
|
||||
const notices = async () => (await publications()).filter(row => row.idempotencyKey === `run:${runId}:completed:${f.endpointId}`);
|
||||
const retire = () => kind === "automatic" ? dismissAutomaticCompletionReviews(db, f.issueId) : dismissObsoleteNativePolicyReviews(db, [runId]);
|
||||
await start(); expect(await notices()).toHaveLength(history === "no prompt" ? 1 : 0); await quiet();
|
||||
await retire();
|
||||
expect((await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, interaction.id)))[0].status)
|
||||
.toBe(history === "explicit request" ? "pending" : "cancelled");
|
||||
await flush(); expect(await notices()).toHaveLength(history === "explicit request" ? 0 : 1); await quiet();
|
||||
await retire(); await flush(); expect(await notices()).toHaveLength(history === "explicit request" ? 0 : 1); await quiet();
|
||||
});
|
||||
|
||||
it("rechecks a stored chat binding when a conversation reset expires its question", async () => {
|
||||
const runId = await run();
|
||||
await db.update(issues).set({ conversationAgentId: f.agentId, conversationUserId: f.userId, conversationState: "active" }).where(eq(issues.id, f.issueId));
|
||||
const interaction = await issueThreadInteractionService(db).create({ id: f.issueId, companyId: f.companyId }, {
|
||||
kind: "ask_user_questions", sourceRunId: runId, continuationPolicy: "wake_assignee",
|
||||
payload: { version: 1, title: "Choose", questions: [{ id: "choice", prompt: "Continue?", selectionMode: "single",
|
||||
options: [{ id: "yes", label: "Yes" }], required: true }] },
|
||||
}, { agentId: f.agentId, runId });
|
||||
// Failed delivery leaves the pending question itself as the response owner.
|
||||
await db.update(chatPublications).set({ state: "failed" }).where(eq(chatPublications.issueId, f.issueId));
|
||||
const [comment] = await db.insert(issueComments).values({ companyId: f.companyId, issueId: f.issueId,
|
||||
authorType: "user", authorUserId: f.userId, body: "/new" }).returning();
|
||||
const [resetRun] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId,
|
||||
status: "running", contextSnapshot: { issueId: f.issueId, source: "issue.comment", wakeCommentId: comment.id } }).returning();
|
||||
await start();
|
||||
expect((await publications()).some(row => row.idempotencyKey.startsWith(`run:${runId}:`))).toBe(false); await quiet();
|
||||
expect(await prepareConversationTurn(db, resetRun)).toMatchObject({ reset: true });
|
||||
expect((await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, interaction.id)))[0].status).toBe("expired");
|
||||
await flush();
|
||||
expect((await publications()).map(row => row.idempotencyKey)).toContain(`run:${runId}:failed:${f.endpointId}`);
|
||||
await quiet();
|
||||
});
|
||||
|
||||
});
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
type Db,
|
||||
} from "@paperclipai/db";
|
||||
|
||||
import { notifyChatRunMilestones } from "./chat-run-milestone-notifications.js";
|
||||
import { sanitizeQuarantinedCommentForHigherTrust } from "./source-trust.js";
|
||||
|
||||
export type ConversationIdentity = {
|
||||
@@ -164,6 +165,7 @@ export async function prepareConversationTurn(
|
||||
}
|
||||
// The boundary lives on the command comment. A crash/retry reuses it instead of resetting twice.
|
||||
if (reset && comment && comment.conversationSessionGeneration == null) {
|
||||
await notifyChatRunMilestones(tx, { companyId: issue.companyId, issueId: issue.id });
|
||||
generation += 1;
|
||||
await tx
|
||||
.update(issues)
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
type ActivityPublication,
|
||||
} from "../activity-log.js";
|
||||
import { enqueueTerminalIssueInteractionChatPublications } from "../chat-interaction-publications.js";
|
||||
import { notifyChatRunMilestones } from "../chat-run-milestone-notifications.js";
|
||||
import { issueThreadInteractionService } from "../issue-thread-interactions.js";
|
||||
import { logger } from "../../middleware/logger.js";
|
||||
|
||||
@@ -126,6 +127,7 @@ export async function dismissAutomaticCompletionReviews(
|
||||
)
|
||||
.for("update");
|
||||
if (!issue) return;
|
||||
await notifyChatRunMilestones(tx, interaction);
|
||||
const now = new Date();
|
||||
const [retired] = await tx
|
||||
.update(issueThreadInteractions)
|
||||
|
||||
@@ -8,6 +8,7 @@ import { logger } from "../../middleware/logger.js";
|
||||
import { issueService } from "../issues.js";
|
||||
import { issueThreadInteractionService } from "../issue-thread-interactions.js";
|
||||
import { enqueueTerminalIssueInteractionChatPublications } from "../chat-interaction-publications.js";
|
||||
import { notifyChatRunMilestones } from "../chat-run-milestone-notifications.js";
|
||||
import { persistActivity, publishActivity, type ActivityPublication } from "../activity-log.js";
|
||||
|
||||
const obsoletePrompt = "Review the superseding native policy assessment.";
|
||||
@@ -68,6 +69,7 @@ export async function dismissObsoleteNativePolicyReviews(db: Db, runIds?: string
|
||||
eq(issues.id, decision.issueId), eq(issues.companyId, decision.companyId),
|
||||
)).for("update").then((rows) => rows[0]);
|
||||
if (!issue) return;
|
||||
await notifyChatRunMilestones(tx, interaction);
|
||||
const now = new Date();
|
||||
const [cancelled] = await tx.update(issueThreadInteractions).set({
|
||||
status: "cancelled",
|
||||
|
||||
Reference in new issue
Block a user