mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-06 14:02:36 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A task conversation must let a user guide a running agent and resume stopped work. > - The active run owns its input protocol, even when the user changes the next model or effort. > - Queue delivery waits for a provider receipt, which must be able to persist during the request. > - A stopped startup also needs a clear user action that passes normal task admission. > - This pull request fixes steering delivery, makes queue actions immediate, and restores explicit continuation. > - The benefit is a responsive conversation that can recover without losing saved input. ## Linked Issues or Issue Description **What happened?** A queued message could change from Steer to Interrupt while a native run prepared. A steer request could wait on its own database lock and fail to deliver. A stopped startup could then leave the conversation without a working Retry or message continuation. Interrupt also waited for the server and showed a toast. **Expected behavior** The active run keeps its input protocol. Steer delivers input to that run. Steer and Interrupt clear the submitted queue rows and show the input in the conversation immediately. Failed delivery restores the latest queue with an inline error. An eligible stopped run offers Retry, and authenticated user input can start a fresh turn through normal task admission. **Steps to reproduce** 1. Start a task with a native Paperclip Runner. 2. Change the selected model or effort while that run prepares. 3. Queue a message and press Steer. 4. Observe the provider receipt and queue state during the request. 5. Stop a startup before its provider process begins, then try Retry or send a new message. 6. Repeat queued delivery with a legacy runner and press Interrupt. **Paperclip version or commit** Reproduced on the parent of this branch, `59c07ede7`. **Deployment mode** Authenticated private deployment. The fixes also cover local task conversations. Related work: Refs #12834, Refs #13354, Refs #13275. The open refactor in #13160 moves the same queue route; it does not fix the receipt lock or stopped-run continuation addressed here. ## What Changed - Select queue behavior from the active run's immutable dispatch and runtime resolution. - Leave the run row unlocked during provider acknowledgement, then lock and read it before merging the receipt. - Retain queued input if the target run stops during that wait. Keep inline delivery errors visible after empty queue updates. - Permit exact Retry and authenticated continuation after verified native startup cancellation. Preserve pause, approval, budget, ownership, and process-stop gates. - Carry undelivered native queue input into a fresh turn once the old execution is confirmed stopped. - Show Steer and Interrupt input in the conversation and clear submitted composer rows immediately. Restore the latest queue inline on failure. Remove delivery toasts. - Keep optimistic delivery stable across stale polls, empty queues, and paginated history. Preserve classic Interrupt error handling. - Document recovery and optimistic delivery behavior. Add regression tests across server, shared queue projection, and UI boundaries. ## Verification - Red-green regression tests reproduced the queue protocol, receipt lock, stopped-startup continuation, and optimistic delivery failures. - The focused server route, continuation, queue, and runner boundary suites passed during implementation. - The queue-route suite passes with 78 tests. The three complete conversation UI suites pass with 347 tests. - UI typecheck, production build, and `pnpm check:token-gates` pass. - Workspace `pnpm -r typecheck` and `pnpm build` pass. The local monolithic `pnpm test:run` is still running; remote CI verifies the complete suite on the latest commit. - All CI gates pass on `bd9031ad56abfcde13d13a13488c1b9217c2fd3a`, including the full test shards, runner verification, browser E2E, typecheck, release registry, and canary dry run. - Greptile reports 5/5 for that commit. Both review threads are resolved. ## Risks This changes queue display and explicit continuation admission. The UI must restore rejected delivery without losing other-session edits. The server must preserve concurrent provider result updates and must not resume a process whose stop is uncertain. Focused tests cover these boundaries. This change has no database migration. ## Model Used OpenAI Codex, an agent based on GPT-6. The exact runtime model ID and context window are not exposed in this session. Used reasoning, repository tools, code execution, and browser inspection. ## 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 - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] 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: Paperclip <noreply@paperclip.ing>
182 lines
7.1 KiB
TypeScript
182 lines
7.1 KiB
TypeScript
// Ports for the three queued-comment queue mutations (edit, reorder,
|
|
// discard). Mirrors the release half's `IssueLockWriter` shape: the adapter
|
|
// owns the one transaction each mutation runs in, locks the issue and the
|
|
// wake row the caller named, classifies the locked state, and hands the
|
|
// caller a `LockedQueuedCommentState` plus a `QueuedCommentQueueTransaction`
|
|
// bound to that same transaction for every further read and write.
|
|
|
|
import type { IssueComment, IssueQueuedCommentEntry, IssueQueuedCommentQueue } from "@paperclipai/shared";
|
|
|
|
export type QueuedCommentActor = {
|
|
actorType: "agent" | "user";
|
|
/** The user id for a user actor, the agent id for an agent actor -- the same value `getActorInfo` names `actorId`. */
|
|
actorId: string;
|
|
/** Null for a user actor. */
|
|
agentId: string | null;
|
|
runId: string | null;
|
|
agentApiKeyId: string | null;
|
|
};
|
|
|
|
/** The fields a mutation needs to log its own activity row; entity type is always "issue". */
|
|
export type QueuedCommentActivityLogInput = {
|
|
actorType: "agent" | "user";
|
|
actorId: string;
|
|
agentId: string | null;
|
|
runId: string | null;
|
|
agentApiKeyId: string | null;
|
|
action: string;
|
|
entityId: string;
|
|
details: Record<string, unknown>;
|
|
};
|
|
|
|
/**
|
|
* Structurally mirrors the server's `ActivityPublication` (from
|
|
* `services/activity-log.ts`), which this layer cannot import by name --
|
|
* the module boundary check forbids the application layer from importing
|
|
* server services. The route casts this back to `ActivityPublication`
|
|
* before calling `publishActivity`.
|
|
*/
|
|
export type QueuedCommentActivityPublication = {
|
|
companyId: string;
|
|
payload: Record<string, unknown>;
|
|
pluginEvent: unknown;
|
|
};
|
|
|
|
export type QueuedCommentIssueContext = {
|
|
id: string;
|
|
companyId: string;
|
|
assigneeAgentId: string | null;
|
|
executionRunId: string | null;
|
|
};
|
|
|
|
export type QueuedCommentWakeRow = {
|
|
id: string;
|
|
agentId: string;
|
|
status: string;
|
|
runId: string | null;
|
|
payload: Record<string, unknown>;
|
|
};
|
|
|
|
export type QueuedCommentRunRow = {
|
|
id: string;
|
|
status: string;
|
|
runtimeMode: string | null;
|
|
runtimeModeResolvedAt?: Date | null;
|
|
runnerProfileJson?: Record<string, unknown> | null;
|
|
contextSnapshot: Record<string, unknown>;
|
|
};
|
|
|
|
/** The module's entry shape is the shared contract, so the compiler checks it directly; the route needs no cast. */
|
|
export type QueuedCommentEntrySnapshot = IssueQueuedCommentEntry;
|
|
|
|
/** The module's queue-snapshot shape is the shared contract, so the compiler checks it directly; the route needs no cast. */
|
|
export type QueuedCommentQueueSnapshot = IssueQueuedCommentQueue;
|
|
|
|
/** The locked, transaction-scoped state a mutation reads before it decides what to write. */
|
|
export type LockedQueuedCommentState = {
|
|
wake: QueuedCommentWakeRow;
|
|
state: "deferred" | "queued";
|
|
queueRun: QueuedCommentRunRow | null;
|
|
activeRun: QueuedCommentRunRow | null;
|
|
queue: QueuedCommentQueueSnapshot;
|
|
};
|
|
|
|
/**
|
|
* Every member is bound to the one transaction `withLockedQueue` owns, and
|
|
* to the one company that transaction is open for. Every read and every
|
|
* write names that bound `companyId` in its own predicate; a
|
|
* caller-supplied `issue`/`wake`/`queueRun` value is never trusted as an
|
|
* authorization boundary by itself.
|
|
*/
|
|
export interface QueuedCommentQueueTransaction {
|
|
updateCommentBody(input: {
|
|
issueId: string;
|
|
commentId: string;
|
|
body: string;
|
|
updatedAt: Date;
|
|
}): Promise<boolean>;
|
|
touchIssueUpdatedAt(input: { issueId: string; updatedAt: Date }): Promise<void>;
|
|
/** Compare-and-set on `id`; the wake's current status is not re-checked here because the row is already locked for the duration of this transaction. */
|
|
updateWakeQueuedCommentIds(input: {
|
|
wakeId: string;
|
|
payload: Record<string, unknown>;
|
|
ids: string[];
|
|
updatedAt: Date;
|
|
}): Promise<QueuedCommentWakeRow>;
|
|
/** Guarded on the run's current `queued` status. Returns `null` when a concurrent writer already moved the run off `queued`. */
|
|
updateQueueRunCommentIds(input: {
|
|
queueRunId: string;
|
|
/** The run's own context snapshot, as already read under lock; the rewrite is derived from this base. */
|
|
contextSnapshot: Record<string, unknown>;
|
|
ids: string[];
|
|
updatedAt: Date;
|
|
}): Promise<QueuedCommentRunRow | null>;
|
|
/** Returns the full deleted comment row so the caller can echo it back as the delete route's response body. */
|
|
deleteComment(input: {
|
|
issueId: string;
|
|
commentId: string;
|
|
}): Promise<IssueComment | null>;
|
|
cancelWake(input: { wakeId: string; reason: string; now: Date }): Promise<void>;
|
|
/**
|
|
* Guarded on the run's current `queued` status. Returns `null` when a
|
|
* concurrent writer already moved the run off `queued`; returns just the
|
|
* cancelled run's id, which is all a post-commit telemetry emission needs.
|
|
*/
|
|
cancelQueueRun(input: {
|
|
queueRunId: string;
|
|
reason: string;
|
|
now: Date;
|
|
}): Promise<{ id: string } | null>;
|
|
/**
|
|
* Clears the issue's execution-lock columns and performs the `updatedAt`
|
|
* touch, in the one update the original route issued for a discard that
|
|
* empties the queue with a live queue run. The write is guarded on the
|
|
* issue's current `executionRunId`; a lost guard silently skips the whole
|
|
* update, matching the pre-existing behavior of this best-effort touch.
|
|
*/
|
|
clearExecutionLockAndTouchIssue(input: {
|
|
issueId: string;
|
|
executionRunId: string;
|
|
updatedAt: Date;
|
|
}): Promise<void>;
|
|
buildQueueSnapshot(input: {
|
|
issue: QueuedCommentIssueContext;
|
|
actor: QueuedCommentActor;
|
|
wake: QueuedCommentWakeRow | null;
|
|
state: "deferred" | "queued" | null;
|
|
queueRun: QueuedCommentRunRow | null;
|
|
activeRun: QueuedCommentRunRow | null;
|
|
}): Promise<QueuedCommentQueueSnapshot>;
|
|
syncCommentReferences(commentId: string): Promise<void>;
|
|
deleteCommentReferenceSource(commentId: string): Promise<void>;
|
|
syncCommentExternalObjectsSafely(commentId: string): Promise<void>;
|
|
/**
|
|
* Persists the activity row on this same transaction, so a mutation and
|
|
* its audit record commit or roll back together. Returns the publication
|
|
* for the caller to publish once the transaction has committed; this
|
|
* write never publishes the live event itself.
|
|
*/
|
|
logActivity(input: QueuedCommentActivityLogInput): Promise<QueuedCommentActivityPublication>;
|
|
}
|
|
|
|
export interface QueuedCommentIssueLockWriter {
|
|
/**
|
|
* Opens the one transaction a mutation runs in: locks the issue row, locks
|
|
* the wake row named by `queueId`, classifies it (reading and locking the
|
|
* linked heartbeat run when the classification needs it), and builds the
|
|
* queue snapshot the caller's mutation target check compares against.
|
|
* Throws `QueuedCommentMutationError` with code `queued_comment_not_pending`
|
|
* or `queued_comment_already_dispatching` when the lock step itself cannot
|
|
* resolve a live queue; `fn` never runs in that case.
|
|
*/
|
|
withLockedQueue<T>(
|
|
input: {
|
|
/** Also carries the company id; every locked read and write binds its `companyId` predicate to `issue.companyId`. */
|
|
issue: QueuedCommentIssueContext;
|
|
actor: QueuedCommentActor;
|
|
queueId: string;
|
|
},
|
|
fn: (locked: LockedQueuedCommentState, transaction: QueuedCommentQueueTransaction) => Promise<T>,
|
|
): Promise<T>;
|
|
}
|