mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 01:50:13 +01:00
fix(onboarding): close the onboarding task when its interview moves to chat (#19643)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - New users start in an onboarding task. The first agent can run the onboarding interview there or in its chat. > - #19512 moves the interview card to the chat when the agent asks it there. The task's card expires. > - The onboarding task then stays `in_review` with no card, run, wake or reviewer. Review attention calls it "stalled". > - Needs you then shows "Choose how this task continues" with Send back to work / Request changes / Mark task done. A new user cannot know what to pick. The card stays after the interview, plan approval and handoff finish in chat. > - This pull request closes the onboarding task when its interview moves to chat, and links the chat in a system comment. > - The benefit is that the user never decides what happens to an internal task. ## Linked Issues or Issue Description Refs #19512 **What happened?** In a browser walk (Codex API path), the user chose "Interview me" in the onboarding task. Then the user asked in Chat: "Can we do my onboarding interview here in chat instead of in the task?". The chat card worked. The onboarding task stayed `in_review`, and Needs you showed "Choose how this task continues" for it until the end of onboarding. **What did you expect?** The onboarding task follows the conversation. It closes with a link to the chat. No decision about an internal task goes to the user. **Steps to reproduce** 1. Start onboarding. Choose "Interview me" on the onboarding task. 2. Open Chat with the agent. Ask to do the interview in chat. 3. Open Inbox. See "Choose how this task continues" for the onboarding task. ## What Changed - `server/src/services/issue-thread-interactions.ts`: after the chat question supersedes the onboarding task's interview cards, the onboarding task is closed (`in_review` → `done`) when no other card is pending on it. A system comment links to the chat. The change writes an `issue.updated` activity entry (`source: "onboarding_moved_to_chat"`). Activity publications and issue post-commit actions run after commit. - The task stays open if it is not `in_review` or if another card is still pending (for example a card for a teammate). - `first-task` skill: tells the agent that the task closes when onboarding moves to chat. The chat then replaces the task for the rest of the workflow (plan, proposal cards, child tasks, results). The agent skips the steps that keep the task `in_review` or make it wait for a child. - `onboarding-assets/first-task/README.md`: documents the behavior. - Tests in `server/src/__tests__/coordinator-handoff.test.ts`: the new red/green case, plus a case that keeps the task open when another card is pending. ## Verification Red on master (fix reverted, same test): ``` FAIL src/__tests__/coordinator-handoff.test.ts > Coordinator canonical handoffs > resolves the onboarding task when its interview moves to the agent chat AssertionError: expected [ 'stalled' ] to deeply equal [] - [] + [ + "stalled", + ] ❯ src/__tests__/coordinator-handoff.test.ts:257:40 256| // No "Choose how this task continues" stalled-review card for the… 257| expect(await stalledReviewCards()).toEqual([]); Tests 1 failed | 3 passed | 24 skipped (28) ``` The test reads the real Needs you feed (`attentionService.list`). On master it holds a `review` item with `reviewAttentionState: "stalled"` for the onboarding task. That is the "Choose how this task continues" card. Green with the fix: ``` cd server && npx vitest run src/__tests__/coordinator-handoff.test.ts Tests 28 passed (28) ``` Neighbour suites: ``` npx vitest run src/__tests__/onboarding-first-task-admission.test.ts src/__tests__/issue-onboarding-first-task-routes.test.ts \ src/__tests__/issue-thread-interaction-routes.test.ts src/__tests__/issue-thread-interactions-service.test.ts \ src/__tests__/issue-thread-interactions-telemetry.test.ts src/__tests__/coordinator-handoff.test.ts Test Files 6 passed (6) Tests 373 passed (373) ``` - `server` typecheck (`tsc --noEmit`): pass. - `pnpm verify` exits 2 (the README is a broad input). Required CI supplies broad coverage. - Not run: a live browser walk. The Vitest case drives the same service path the chat uses. ## Risks - Low risk. The change runs only when an agent asks a question in its own chat and that question supersedes a pending onboarding-task question (the #19512 path). - The onboarding task becomes `done` without a recorded first-task decision. Later onboarding happens in chat, so nothing waits on this task. A done onboarding parent still accepts child-completion report wakes. - The onboarding task rows are locked before their question rows, the same order question creation on the task uses (Greptile review follow-up). The chat row is locked first; no path locks an onboarding task and then a chat row. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
DottaandClaude Opus 5.5 authored and GitHub committed
1 parent
9feabfee48
commit
a3f80ea043
4 files changed
+129
-4
No files matched your search
@@ -235,6 +235,47 @@ const support = externalDatabase ? { supported: true } : await getEmbeddedPostgr
|
||||
} });
|
||||
});
|
||||
|
||||
// After the interview moves to chat, nothing drives the onboarding task.
|
||||
// It must follow the chat instead of asking the user how it continues.
|
||||
it("resolves the onboarding task when its interview moves to the agent chat", async () => {
|
||||
const f = await seed();
|
||||
const onboarding = await f.direct();
|
||||
await db.update(issues).set({ originKind: "onboarding_first_task", status: "in_review" }).where(eq(issues.id, onboarding.id));
|
||||
const payload = { version: 1 as const, questions: [
|
||||
{ id: "business", prompt: "What does your organization do?", selectionMode: "single" as const, options: [{ id: "shop", label: "A shop" }] },
|
||||
] };
|
||||
await f.service.create(onboarding, { kind: "ask_user_questions", continuationPolicy: "wake_assignee", payload }, { agentId: f.agentId });
|
||||
const stalledReviewCards = async () => (await attentionService(db).list(f.companyId, {
|
||||
actor: { type: "board", source: "local_implicit", userId: f.userId }, userId: f.userId,
|
||||
})).items.filter(item => item.sourceKind === "review" && item.subject.id === onboarding.id)
|
||||
.map(item => item.subject.metadata?.reviewAttentionState);
|
||||
expect(await stalledReviewCards()).toEqual([]);
|
||||
|
||||
await f.service.create(f.scope, { kind: "ask_user_questions", continuationPolicy: "wake_assignee", payload }, { agentId: f.agentId });
|
||||
|
||||
// No "Choose how this task continues" stalled-review card for the user.
|
||||
expect(await stalledReviewCards()).toEqual([]);
|
||||
const [task] = await db.select().from(issues).where(eq(issues.id, onboarding.id));
|
||||
expect(task.status).toBe("done");
|
||||
const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, onboarding.id));
|
||||
expect(comments.map(comment => comment.body).join("\n")).toContain(`/issues/${f.conversationId}`);
|
||||
});
|
||||
|
||||
it("leaves the onboarding task open when it still has a pending card for the user", async () => {
|
||||
const f = await seed();
|
||||
const onboarding = await f.direct();
|
||||
await db.update(issues).set({ originKind: "onboarding_first_task", status: "in_review" }).where(eq(issues.id, onboarding.id));
|
||||
const payload = { version: 1 as const, questions: [
|
||||
{ id: "business", prompt: "What does your organization do?", selectionMode: "single" as const, options: [{ id: "shop", label: "A shop" }] },
|
||||
] };
|
||||
await f.service.create(onboarding, { kind: "ask_user_questions", payload }, { agentId: f.agentId });
|
||||
await db.insert(issueThreadInteractions).values({ companyId: f.companyId, issueId: onboarding.id,
|
||||
kind: "ask_user_questions", createdByAgentId: f.agentId, addresseeUserId: "teammate-user", payload });
|
||||
await f.service.create(f.scope, { kind: "ask_user_questions", payload }, { agentId: f.agentId });
|
||||
const [task] = await db.select().from(issues).where(eq(issues.id, onboarding.id));
|
||||
expect(task.status).toBe("in_review");
|
||||
});
|
||||
|
||||
it("moves the onboarding interview for native provider questions, but keeps cards meant for someone else", async () => {
|
||||
const f = await seed();
|
||||
const onboarding = await f.direct();
|
||||
|
||||
@@ -36,7 +36,10 @@ and the message wakes the agent as before.
|
||||
With the coordinator chat enabled, the agent's chat with the user mirrors the
|
||||
onboarding task's pending questions. If the agent asks a question in that chat
|
||||
(for example, the user moved onboarding there), the onboarding task's pending
|
||||
question cards expire as superseded, so only one card stays answerable.
|
||||
question cards expire as superseded, so only one card stays answerable. When
|
||||
that leaves the onboarding task in review with no other pending card, it is
|
||||
marked done with a comment linking to the chat, so the user is never asked to
|
||||
decide how an internal task continues.
|
||||
|
||||
## Placeholders
|
||||
|
||||
|
||||
@@ -29,6 +29,8 @@ Work in this order.
|
||||
|
||||
- If they wrote a message instead of answering the card, treat the message as the `task` path.
|
||||
|
||||
- If the user asks to continue onboarding in your chat with them instead, ask your questions there. Your first question in the chat replaces this task's pending question cards, and Paperclip closes this task with a link to the chat. From then on the chat replaces this task for the rest of this workflow: save the plan and post proposal cards in the chat, create accepted child tasks from the chat, and report their results there. Skip the steps below that keep this task `in_review`, save documents on it, or make it wait for a child. Do not reopen this task or ask the user what should happen to it.
|
||||
|
||||
2. Propose, then wait for acceptance.
|
||||
|
||||
- Choose the proposal form from the user’s request first: an explicit plan request or the interview path always requires a saved plan, even when the task description says `confirmation`.
|
||||
|
||||
@@ -131,6 +131,8 @@ import {
|
||||
} from "./issue-review-policy.js";
|
||||
import {
|
||||
issueService,
|
||||
executeIssuePostCommitActions,
|
||||
type IssuePostCommitAction,
|
||||
ensureAssignmentIssueAccessGrant,
|
||||
readAcceptedPlanConfirmationTarget,
|
||||
runWorkspaceIsFinalized,
|
||||
@@ -1038,15 +1040,21 @@ async function supersedeOnboardingTaskQuestionsMovedToChat(
|
||||
conversationUserId: string;
|
||||
replacementInteractionId: string;
|
||||
actor: InteractionActor;
|
||||
conversationIssue: { id: string; identifier: string | null };
|
||||
postCommitActivityPublications: ActivityPublication[];
|
||||
postCommitIssueActions: IssuePostCommitAction[];
|
||||
},
|
||||
) {
|
||||
const onboardingTaskIds = tx.select({ id: issues.id }).from(issues).where(and(
|
||||
// Lock the onboarding tasks before their questions, matching the
|
||||
// task-then-question order of question creation on the task itself.
|
||||
const onboardingTaskIds = (await tx.select({ id: issues.id }).from(issues).where(and(
|
||||
eq(issues.companyId, input.companyId),
|
||||
eq(issues.originKind, "onboarding_first_task"),
|
||||
isNull(issues.conversationAgentId),
|
||||
eq(issues.assigneeAgentId, input.agentId),
|
||||
eq(issues.createdByUserId, input.conversationUserId),
|
||||
));
|
||||
)).orderBy(asc(issues.id)).for("update")).map(task => task.id);
|
||||
if (onboardingTaskIds.length === 0) return [];
|
||||
const now = new Date();
|
||||
const rows = await tx
|
||||
.update(issueThreadInteractions)
|
||||
@@ -1081,9 +1089,73 @@ async function supersedeOnboardingTaskQuestionsMovedToChat(
|
||||
for (const row of rows) {
|
||||
await enqueueTerminalIssueInteractionChatPublications(tx, hydrateInteraction(row));
|
||||
}
|
||||
for (const issueId of new Set(rows.map(row => row.issueId))) {
|
||||
await completeOnboardingTaskMovedToChat(tx, { ...input, issueId });
|
||||
}
|
||||
return rows;
|
||||
}
|
||||
|
||||
// The onboarding task was waiting in review on the interview that just moved
|
||||
// to chat. Nothing drives it any more, so it would surface as a stalled review
|
||||
// asking the user how an internal task continues. Close it with a link to the
|
||||
// chat instead. A task still waiting on another pending card stays open.
|
||||
async function completeOnboardingTaskMovedToChat(
|
||||
tx: Db,
|
||||
input: {
|
||||
companyId: string;
|
||||
agentId: string;
|
||||
issueId: string;
|
||||
conversationIssue: { id: string; identifier: string | null };
|
||||
postCommitActivityPublications: ActivityPublication[];
|
||||
postCommitIssueActions: IssuePostCommitAction[];
|
||||
},
|
||||
) {
|
||||
// The caller already holds this task's row lock.
|
||||
const [task] = await tx.select({ id: issues.id, status: issues.status, identifier: issues.identifier })
|
||||
.from(issues)
|
||||
.where(and(eq(issues.id, input.issueId), eq(issues.companyId, input.companyId)));
|
||||
if (task?.status !== "in_review") return;
|
||||
const [stillPending] = await tx.select({ id: issueThreadInteractions.id })
|
||||
.from(issueThreadInteractions)
|
||||
.where(and(
|
||||
eq(issueThreadInteractions.companyId, input.companyId),
|
||||
eq(issueThreadInteractions.issueId, task.id),
|
||||
eq(issueThreadInteractions.status, "pending"),
|
||||
))
|
||||
.limit(1);
|
||||
if (stillPending) return;
|
||||
const chatRef = input.conversationIssue.identifier ?? input.conversationIssue.id;
|
||||
const svc = issueService(tx);
|
||||
const comment = await svc.addComment(
|
||||
task.id,
|
||||
`Onboarding continues in [chat](/issues/${chatRef}). This task is closed so nothing is left waiting here.`,
|
||||
{},
|
||||
{ authorType: "system" },
|
||||
tx,
|
||||
);
|
||||
const updated = await svc.update(task.id, { status: "done", actorAgentId: input.agentId }, tx,
|
||||
input.postCommitActivityPublications, input.postCommitIssueActions);
|
||||
if (!updated) return;
|
||||
await logActivity(tx, {
|
||||
companyId: input.companyId,
|
||||
actorType: "agent",
|
||||
actorId: input.agentId,
|
||||
agentId: input.agentId,
|
||||
action: "issue.updated",
|
||||
entityType: "issue",
|
||||
entityId: task.id,
|
||||
issueId: task.id,
|
||||
details: {
|
||||
status: "done",
|
||||
identifier: task.identifier,
|
||||
source: "onboarding_moved_to_chat",
|
||||
conversationIssueId: input.conversationIssue.id,
|
||||
commentId: comment.id,
|
||||
_previous: { status: task.status },
|
||||
},
|
||||
}, input.postCommitActivityPublications);
|
||||
}
|
||||
|
||||
function buildAdministrativeOutcomeResult(
|
||||
row: IssueThreadInteractionRow,
|
||||
outcome: "withdrawn" | "issue_closed" | "addressee_deleted",
|
||||
@@ -3843,6 +3915,8 @@ export function issueThreadInteractionService(
|
||||
|
||||
let created: IssueThreadInteractionRow;
|
||||
let superseded: IssueThreadInteractionRow[] = [];
|
||||
const postCommitActivityPublications: ActivityPublication[] = [];
|
||||
const postCommitIssueActions: IssuePostCommitAction[] = [];
|
||||
try {
|
||||
// A terminal issue must not regain pending actionable cards. FOR UPDATE
|
||||
// on the issue row serializes this insert both against terminal status
|
||||
@@ -3852,7 +3926,7 @@ export function issueThreadInteractionService(
|
||||
const result = await db.transaction(async (tx) => {
|
||||
await assertInteractionRunWriteAllowed(tx as unknown as Db, issue, actor);
|
||||
const [issueRow] = await tx
|
||||
.select({ executionPolicy: issues.executionPolicy, status: issues.status, assigneeAgentId: issues.assigneeAgentId, assigneeUserId: issues.assigneeUserId, conversationAgentId: issues.conversationAgentId, conversationUserId: issues.conversationUserId })
|
||||
.select({ executionPolicy: issues.executionPolicy, status: issues.status, assigneeAgentId: issues.assigneeAgentId, assigneeUserId: issues.assigneeUserId, conversationAgentId: issues.conversationAgentId, conversationUserId: issues.conversationUserId, identifier: issues.identifier })
|
||||
.from(issues)
|
||||
.where(
|
||||
and(
|
||||
@@ -4083,6 +4157,9 @@ export function issueThreadInteractionService(
|
||||
conversationUserId: issueRow.conversationUserId,
|
||||
replacementInteractionId: row.id,
|
||||
actor,
|
||||
conversationIssue: { id: issue.id, identifier: issueRow.identifier },
|
||||
postCommitActivityPublications,
|
||||
postCommitIssueActions,
|
||||
})
|
||||
: [];
|
||||
await enqueueIssueInteractionChatPublications(
|
||||
@@ -4180,6 +4257,8 @@ export function issueThreadInteractionService(
|
||||
return interaction;
|
||||
}
|
||||
|
||||
for (const publication of postCommitActivityPublications) publishActivity(publication);
|
||||
await executeIssuePostCommitActions(db, postCommitIssueActions);
|
||||
await touchIssue(db, issue.id, issue.companyId);
|
||||
if (superseded.length > 0) {
|
||||
await emitResolvedInteractionsTelemetry(
|
||||
|
||||
Reference in new issue
Block a user